Bug 215659 - Review Request: python-daap - A DAAP client implemented in Python
Summary: Review Request: python-daap - A DAAP client implemented in Python
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Matthias Saou
QA Contact: Fedora Package Reviews List
Depends On:
TreeView+ depends on / blocked
Reported: 2006-11-15 02:03 UTC by Jeffrey C. Ollie
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Clone Of:
Last Closed: 2007-03-17 16:54:52 UTC
matthias: fedora-review+
wtogami: fedora-cvs+

Attachments (Terms of Use)

Description Jeffrey C. Ollie 2006-11-15 02:03:12 UTC
Spec URL: http://repo.ocjtech.us/misc/fedora/6/SRPMS/python-daap-0.4-1.fc6.spec
SRPM URL: http://repo.ocjtech.us/misc/fedora/6/SRPMS/python-daap-0.4-1.fc6.src.rpm

A DAAP client implemented in Python.

Comment 1 Jochen Schmitt 2006-11-16 19:00:10 UTC
+ Rpmlint doesn't complaints the source package.
+ Local build works fine.
+ Rpmlint doesn't complaints the binary package.
+ Tar Ball matches with upstream.
+ Local Install/Uninstall works fine.

- Strange License tag which refer to a non existing License file.

Comment 2 Jeffrey C. Ollie 2006-11-16 23:12:50 UTC
Spec URL: http://repo.ocjtech.us/misc/fedora/6/SRPMS/python-daap-0.4-2.fc6.spec
SRPM URL: http://repo.ocjtech.us/misc/fedora/6/SRPMS/python-daap-0.4-2.fc6.src.rpm

I've added a copy of the license text and clarified the comment regarding the

Comment 3 Jochen Schmitt 2006-11-19 20:52:41 UTC
+ License ok, package contains verbatin copy of the license text.
+ Naming confirm with name convention.

On the project page was a posting about a severe problem. It will be
nice to give me a method to test the library, becouse it seems to be
very unstable.

Comment 4 Jeffrey C. Ollie 2006-11-20 02:15:46 UTC
(In reply to comment #3)
> On the project page was a posting about a severe problem. It will be
> nice to give me a method to test the library, becouse it seems to be
> very unstable.

I think that the comment that you refer to was posted before the 0.4 version was

Comment 5 Jochen Schmitt 2006-11-20 15:55:51 UTC
That my be, but unfortunately I can't get this itshell programm talk on the
homepage of the project. do you know how I can get it to test your package.

Comment 6 Jeffrey C. Ollie 2006-11-20 17:34:41 UTC
I know that the latest versions of Elisa use PythonDaap, unfortunately Elisa has
a number of dependencies that aren't in Fedora yet.  The main one is
python-twisted-core - it's been reviewed and accepted but it hasn't been built
and pushed out yet.


Comment 7 Jeffrey C. Ollie 2006-11-20 18:10:47 UTC
I was able to get itshell.py working, it's a little non-intuitive...  First, you
need to set up a DAAP server.  I used Rhythmbox running on another PC, but
iTunes should work... There are probably other music players out there that can
be DAAP servers...

  $ python itshell.py 
      The python/daap interactive shell.
      Type 'help' for help.
  (no server): connect
  Connecting to '':3689
  2006-11-20 12:08:30,358 DEBUG getting /content-codes
  2006-11-20 12:08:30,408 DEBUG getting /server-info
  2006-11-20 12:08:30,411 DEBUG getting /login
  2006-11-20 12:08:30,412 DEBUG Logged in as session -1546218061
  2006-11-20 12:08:30,413 DEBUG getting /databases?session-id=-1546218061
  2006-11-20 12:08:30,415 DEBUG getting /databases?session-id=-1546218061
  using database 'u"Jeff's Music"'
  2006-11-20 12:08:30,416 DEBUG getting
  Got 52 tracks
  ( search elvis
  1 tracks found.
  45: u'Elvis Presley' - u'The Time-Life Treasury of Christmas (disc 1)' -
u'Here Comes Santa Claus'
  ( download 45 claus.flac
  2006-11-20 12:09:09,385 DEBUG saving to 'claus.flac'
  2006-11-20 12:09:09,386 DEBUG getting
  2006-11-20 12:09:10,422 DEBUG Done
  ( exit
  2006-11-20 12:09:27,502 DEBUG getting /logout?session-id=-1546218061
  2006-11-20 12:09:27,504 DEBUG DAAPSession: expired session id -1546218061

Comment 8 Jeffrey C. Ollie 2007-02-12 17:46:57 UTC
Jochen, ping?

Comment 9 Matthias Saou 2007-02-12 17:59:32 UTC
Just looked at the last spec file and it looks good to me, apart from the
comments that seems to be leftover from a template, thus could be removed.

Regarding Elisa, it requires a more recent version of PythonDAAP now. For the
package I had made, I had created this patch to bring 0.4 up to what's in SVN :


Note that it also contains the LICENSE file you've already added too ;-)
If Jochen doesn't finish reviewing this package, I wouldn't mind doing it.

Comment 10 Matthias Saou 2007-02-12 18:33:18 UTC
I just asked the author if he'd be willing to make a new release ASAP in order
to not have to package an svn snapshot... and he did! So 0.5 is now available,
and will probably even include the license file :-)

Comment 11 Jeffrey C. Ollie 2007-02-12 19:18:17 UTC
(In reply to comment #10)
> I just asked the author if he'd be willing to make a new release ASAP in order
> to not have to package an svn snapshot... and he did! So 0.5 is now available,

Yes, I saw that...

> and will probably even include the license file :-)

Unfortunately, he didn't.

Spec URL: http://repo.ocjtech.us/misc/fedora/6/SRPMS/python-daap-0.5-1.fc6.spec
SRPM URL: http://repo.ocjtech.us/misc/fedora/6/SRPMS/python-daap-0.5-1.fc6.src.rpm

Comment 12 Matthias Saou 2007-02-12 19:24:40 UTC
Too bad for the license, then...

Anyway, two more quickies :
- The consensus would seem to be to remove the leading "A " from the summary
- The "cp" for SOURCE1 is wrong, as a bad umask can lead to wrong file modes,
which is known to have happened in the build system, use "install -m 0644" instead

The rest looks quite alright to me. Jochen : Please review ASAP or let me know
if you want me to pick this one up.

Comment 13 Jeffrey C. Ollie 2007-02-12 19:36:04 UTC
Spec URL: http://repo.ocjtech.us/misc/fedora/6/SRPMS/python-daap-0.5-2.fc6.spec
SRPM URL: http://repo.ocjtech.us/misc/fedora/6/SRPMS/python-daap-0.5-2.fc6.src.rpm

* Mon Feb 12 2007 Jeffrey C. Ollie <jeff@ocjtech.us> - 0.5-2
- Drop "A " from the summary
- Use "install -m 0644" instead of "cp".

Comment 14 Jochen Schmitt 2007-02-15 19:28:27 UTC

unfortunately, I'm buissy with some multilib issues, so I don't have any time
for this review now. It will be nice, if anyone can over my part.

Comment 15 Matthias Saou 2007-02-16 10:03:16 UTC
Sure, I'll take over. Expect (hopefully) a first review later today.

Comment 16 Matthias Saou 2007-02-19 10:34:45 UTC
rpmlint output on the source rpm :
W: python-daap mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 15)
rpmlint output on the binary rpm is ok.

The spec file and resulting packages seem to comply with all MUST review items.

The only part which is quite vague is the LICENSE. The author has made a
statement about relicensing under the LGPL, but :
1) The source tarball for 0.5 still doesn't contain the LICENSE file
2) The PKG-INFO file still contains "License: UNKNOWN"
3) The sources contain two files (md5c.c and md5.h) which were taken from the
Python sources and have their own "RSA Data Security, Inc." license.

Points 1) and 2) could be easily dealt with by sending a quick patch to the
author. The most surprizing part is that the LICENSE file is actually in the svn
trunk, and has been for a while, but was skipped when making the 0.5 release it
Point 3) is a bit trickier, as I have no idea if that license is actually LGPL
compatible or not (I would think it is, though). If it isn't, then we have a
problem. If it is, then simply adding a note about "Parts of the software are
derived from the RSA Data Security, Inc. MD5 Message-Digest Algorithm" +
possibly a copy of the RSA terms should be enough.

Comment 17 Matthias Saou 2007-03-02 11:38:34 UTC
I just came across an interesting thread here :

Looks like the current content of the source might not be acceptable and might
need to be modified. Some possibilities are discussed further in the thread.

Comment 18 Jeffrey C. Ollie 2007-03-12 19:34:22 UTC
Looks like 0.7 will be OK license-wise once it's been released:


Comment 19 Jeffrey C. Ollie 2007-03-13 03:34:55 UTC
Spec URL: http://repo.ocjtech.us/misc/fedora/6/SRPMS/python-daap-0.7-1.fc6.spec
SRPM URL: http://repo.ocjtech.us/misc/fedora/6/SRPMS/python-daap-0.7-1.fc6.src.rpm

OK, I think that I have all the issues fixed in this one.  The LICENSE file is
included in the upstream tarball and the problematic MD5 code has been replaced
with a public domain version.

Comment 20 Matthias Saou 2007-03-13 10:07:25 UTC
Great! Tom Insam the author) has been really receptive to the licensing
concerns, which should indeed be fixed now. Now that things are clear, you could
remove the "See http://jerakeen.org/code/pythondaap/#comment_643" lines above
the "License:".

Nitpicks :
- Remove the comments above the License line of the spec.
- Add a %changelog entry for 0.7-1.
- I'd put "examples/" instead of "examples" for %doc to show it's a directory.
- You might want to include the PKG-INFO in %doc now that it's correct.

Formal review :
This is a quite trivial python package, which contains all what a python package
should. It's not noarch because of the included md5 module, the LGPL license is
correct and rpmlint is silent on the resulting packages... APPROVED. Feel free
to "fix" any of the nitpicks from above, though ;-)

Comment 21 Jeffrey C. Ollie 2007-03-13 15:52:42 UTC
Spec URL: http://repo.ocjtech.us/misc/fedora/6/SRPMS/python-daap-0.7-2.fc6.spec
SRPM URL: http://repo.ocjtech.us/misc/fedora/6/SRPMS/python-daap-0.7-2.fc6.src.rpm

For reference, here are the spec & SRPM with the recommended changes.
I had made some of them before but I must have forgotten to upload the
correct files.

Comment 22 Jeffrey C. Ollie 2007-03-13 15:54:55 UTC
New Package CVS Request
Package Name: python-daap
Short Description: DAAP client implemented in Python
Owners: jeff@ocjtech.us
Branches: devel FC-6

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