Bug 402051 - Review Request: libopendaap - Library for connection to iTunes music shares
Review Request: libopendaap - Library for connection to iTunes music shares
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-11-27 20:00 EST by Nicolas Chauvet (kwizart)
Modified: 2007-12-15 07:20 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-12-15 07:20:19 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Nicolas Chauvet (kwizart) 2007-11-27 20:00:43 EST
Spec URL:
http://kwizart.fedorapeople.org/SPECS/libopendaap.spec
SRPM URL:
http://kwizart.fedorapeople.org/SRPMS/libopendaap-0.4.0-3.kwizart.fc8.src.rpm
Description: Library for connection to iTunes music shares
Comment 1 Mamoru TASAKA 2007-12-08 11:14:27 EST
I would appreciate it if you would review any of my review requests
(bug 413561 and bug 414211)
Comment 2 Mamoru TASAKA 2007-12-08 13:00:49 EST
For 0.4.0-3:

* License
  - This is MIT.

* pkgconfig .pc file / header files.
  - First, opendaap.pc says:
-----------------------------------------------------
includedir=/usr/include
Cflags: -I${includedir}
-----------------------------------------------------
    However all the header files are under %_includedir/daap
    so $ pkg-config --cflags opendaap cannot detect opendaap
    header file.

  - And /usr/include/daap/portability.h contains
-----------------------------------------------------
    36  #include <sys/types.h>
    37  #include "config.h"
-----------------------------------------------------
    But config.h is not installed
    ! Note
      config.h should not be installed. What is the problem
      is that installed header files should not include
      (or require) config.h.

* CPP macro
  - I guess this usually means "cpp" and this does not relate
    to "cp" command.
Comment 3 Nicolas Chauvet (kwizart) 2007-12-14 10:32:11 EST
It seems that the only application I knwo (which is vlc) is using <daap/client.h>
(and wasn't using daap/portability.h)
So i wonder if it would be better to stay with the default include scheme.
Because the client.h name is quite generic.

daap.c from vlc uses for now:
#include <stdlib.h>                                      /* malloc(), free() */
#include <vlc/vlc.h>
#include <vlc/intf.h>
#include "vlc_url.h"
#include <vlc/input.h>
#include <daap/client.h>
Comment 4 Mamoru TASAKA 2007-12-14 11:23:17 EST
Well, in that case opendaap.pc can be as it is, however,

(In reply to comment #3)
> (and wasn't using daap/portability.h)

daap/portability.h is currently needed by client.h as
-------------------------------------------------------
    33  #include <stdio.h>
    34  #include "portability.h"
    35  
--------------------------------------------------------
and current portability.h is problematic as I wrote in
comment 2.
Comment 5 Nicolas Chauvet (kwizart) 2007-12-14 11:32:16 EST
yes, indeed... so fixed with this release:

Spec URL:
http://kwizart.fedorapeople.org/SPECS/libopendaap.spec
SRPM URL:
http://kwizart.fedorapeople.org/SRPMS/libopendaap-0.4.0-4.fc7.kwizart.src.rpm
Description: Library for connection to iTunes music shares
Comment 6 Mamoru TASAKA 2007-12-14 12:08:00 EST
Okay.

--------------------------------------------------------
   This package (libopendaap) is APPROVED by me
--------------------------------------------------------
Comment 7 Nicolas Chauvet (kwizart) 2007-12-14 12:18:54 EST
New Package CVS Request
=======================
Package Name: libopendaap
Short Description: Library for connection to iTunes music shares
Owners: kwizart
Branches: F-7 F-8 EL-4 EL-5
Cvsextras Commits: yes
Comment 8 Nicolas Chauvet (kwizart) 2007-12-14 12:19:36 EST
thx for the review
Comment 9 Kevin Fenzi 2007-12-14 14:53:57 EST
cvs done.

Note You need to log in before you can comment on or make changes to this bug.