Red Hat Bugzilla – Bug 215659
Review Request: python-daap - A DAAP client implemented in Python
Last modified: 2007-11-30 17:11:49 EST
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.
+ 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.
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
+ 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
(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
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.
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.
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 10.0.0.1
Connecting to '10.0.0.1':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
(10.0.0.1:3689): search elvis
1 tracks found.
45: u'Elvis Presley' - u'The Time-Life Treasury of Christmas (disc 1)' -
u'Here Comes Santa Claus'
(10.0.0.1:3689): 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
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
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.
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 :-)
(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
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.
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 <firstname.lastname@example.org> - 0.5-2
- Drop "A " from the summary
- Use "install -m 0644" instead of "cp".
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.
Sure, I'll take over. Expect (hopefully) a first review later today.
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.
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.
Looks like 0.7 will be OK license-wise once it's been released:
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.
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
- 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 ;-)
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
New Package CVS Request
Package Name: python-daap
Short Description: DAAP client implemented in Python
Branches: devel FC-6