Spec URL: http://www.auroralinux.org/people/spot/review/new/xmms2.spec SRPM URL: http://www.auroralinux.org/people/spot/review/new/xmms2-0.5-1.fc11.src.rpm Description: XMMS2 is an audio framework, but it is not a general multimedia player - it will not play videos. It has a modular framework and plugin architecture for audio processing, visualisation and output, but this framework has not been designed to support video. Also the client-server design of XMMS2 (and the daemon being independent of any graphics output) practically prevents direct video output being implemented. It has support for a wide range of audio formats, which is expandable via plugins. It includes a basic CLI interface to the XMMS2 framework, but most users will want to install a graphical XMMS2 client (such as gxmms2 or esperanza).
OK, I finished the review. There are a few minor issues (*) and I also have some questions (?): - rpmlint says xmms2-devel.x86_64: W: no-documentation xmms2-perl.x86_64: W: no-documentation xmms2-python.x86_64: W: no-documentation xmms2-ruby.x86_64: W: no-documentation but I couldn't find anything relevant in the available docs to put into these packages (except maybe the license files?), so these warnings can be ignored. ? I didn't find any occurrences of the clauses "GPL" or "Artistic" or "licensed under the same terms as perl itself." among the perl files. Is there a rule that any perl program has to be released under "GPL+ or Artistic"? I would also like to remind you that there is a .so file in the perl package that links against the libraries in the main package. - TODO file can be included in %doc ? Is the doxygen documentation useless? ? What package(s) own the %{perl_archlib}/Audio/ %{perl_archlib}/auto/Audio/ directories? Are they among the dependencies? ? Some files in the wafadmin directory have /usr/local/ /usr/lib/ directories hardcoded. Will these have any effect on the application? ? There are no .desktop files but why are there pixmaps? * The devel package has .pc files, hence we must require pkgconfig. * Please add the -v flag to the waf script so we can see what it is actually doing. When I did this, I found that the fedora specific compiler flag -O2 is being overwritten. This needs fixed. ? The following provides seemed weird to me: $ rpm -qp --provides xmms2-devel-0.5-1.fc10.x86_64.rpm (git DrLecter b63ec5a270cfde0ae3d59c9b89d860b8650e430f-dirty) commit: pkgconfig(xmms2-client) = 0.5 pkgconfig(xmms2-client-cpp) = 0.5 pkgconfig(xmms2-client-cpp-glib) = 0.5 pkgconfig(xmms2-client-ecore) = 0.5 pkgconfig(xmms2-client-glib) = 0.5 pkgconfig(xmms2-plugin) = 0.5 xmms2-devel = 0.5-1.fc10 xmms2-devel(x86-64) = 0.5-1.fc10 What is that git parenthesis about? Is that normal? * The devel package must require glib2-devel, qt-devel, boost-devel at the least (but I think this is all). * Double BR: libmodplug-devel * Unnecessary BRs: libcurl-devel (picked up by ecore-devel), glib2-devel (picked up by avahi-glib-devel, pulseaudio-libs-devel...), libogg-devel, libvorbis-devel (both picked up by libshout-devel), python-devel (picked up by Pyrex)
(In reply to comment #1) > ? I didn't find any occurrences of the clauses "GPL" or "Artistic" or "licensed > under the same terms as perl itself." among the perl files. Is there a rule > that any perl program has to be released under "GPL+ or Artistic"? I would also > like to remind you that there is a .so file in the perl package that links > against the libraries in the main package. From COPYING: src/clients/lib/perl/: Copyright (C) 2006-2007 Florian Ragwitz <rafl> Licensed under the same terms as Perl itself. License attribution also shows up in the .xs files in src/clients/lib/perl/ > - TODO file can be included in %doc Good point. > ? Is the doxygen documentation useless? Nope. I've added a -docs subpackage for all 6.4 MB of it. > ? What package(s) own the > %{perl_archlib}/Audio/ > %{perl_archlib}/auto/Audio/ > directories? Are they among the dependencies? Multiple ownership for perl directories is acceptable. See: http://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership Since nothing in this depends on any other perl module which may own %{perl_archlib}/Audio/ or %{perl_archlib}/auto/Audio/, it is acceptable for this package to own it. > ? Some files in the wafadmin directory have /usr/local/ /usr/lib/ directories > hardcoded. Will these have any effect on the application? Not in my testing on x86_64, no. The /usr/local instances are being overridden, but it can't hurt to sed replace libdir, so I've done that. > ? There are no .desktop files but why are there pixmaps? They're provided in case third party clients want to use "approved" images. > * The devel package has .pc files, hence we must require pkgconfig. Fixed. > * Please add the -v flag to the waf script so we can see what it is actually > doing. When I did this, I found that the fedora specific compiler flag -O2 is > being overwritten. This needs fixed. Good point. Added -v to build, added a patch to disable the extra -O0 that was being appended to CFLAGS/CPPFLAGS. > ? The following provides seemed weird to me: > $ rpm -qp --provides xmms2-devel-0.5-1.fc10.x86_64.rpm > (git > DrLecter > b63ec5a270cfde0ae3d59c9b89d860b8650e430f-dirty) > commit: > pkgconfig(xmms2-client) = 0.5 > pkgconfig(xmms2-client-cpp) = 0.5 > pkgconfig(xmms2-client-cpp-glib) = 0.5 > pkgconfig(xmms2-client-ecore) = 0.5 > pkgconfig(xmms2-client-glib) = 0.5 > pkgconfig(xmms2-plugin) = 0.5 > xmms2-devel = 0.5-1.fc10 > xmms2-devel(x86-64) = 0.5-1.fc10 > What is that git parenthesis about? Is that normal? Hmm. It looks like it is getting implanted into one of the .pc files, then rpm is scraping it out as a Provides for some reason. Easy enough to fix the wscript to have a little versioning sanity. > * The devel package must require glib2-devel, qt-devel, boost-devel at the > least (but I think this is all). Added. > * Double BR: libmodplug-devel Fixed. > * Unnecessary BRs: libcurl-devel (picked up by ecore-devel), glib2-devel > (picked up by avahi-glib-devel, pulseaudio-libs-devel...), libogg-devel, > libvorbis-devel (both picked up by libshout-devel), python-devel (picked up by > Pyrex) Fixed. Okay, here is the new SPRM and SPEC: New SRPM: http://www.auroralinux.org/people/spot/review/new/xmms2-0.5-2.fc11.src.rpm New SPEC: http://www.auroralinux.org/people/spot/review/new/xmms2.spec
Thanks for the update. The package is good to go now. Btw, the following issues may need to be re-addressed by your friend for the freeworld package ;) > ? Some files in the wafadmin directory have /usr/local/ /usr/lib/ directories > hardcoded. Will these have any effect on the application? > * Please add the -v flag to the waf script so we can see what it is actually > doing. When I did this, I found that the fedora specific compiler flag -O2 is > being overwritten. This needs fixed. > * Double BR: libmodplug-devel > * Unnecessary BRs: libcurl-devel (picked up by ecore-devel), glib2-devel > (picked up by avahi-glib-devel, pulseaudio-libs-devel...), libogg-devel, > libvorbis-devel (both picked up by libshout-devel), python-devel (picked up by > Pyrex) ---------------------------------------- This package (xmms2) is APPROVED by oget ----------------------------------------
New Package CVS Request ======================= Package Name: xmms2 Short Description: A modular audio framework and plugin architecture Owners: spot Branches: F-9 F-10 devel InitialCC: ... and it's done.
xmms2-0.5-2.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/xmms2-0.5-2.fc9
xmms2-0.5-2.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/xmms2-0.5-2.fc10
xmms2-0.5-2.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report.
xmms2-0.5-2.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.