[Bug 585607] Review Request: gnump3d - The GNU Streaming MP3 / OGG Vorbis Server

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=585607

Jason Tibbitts <tibbs at math.uh.edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|nobody at fedoraproject.org    |tibbs at math.uh.edu
               Flag|                            |fedora-review?

--- Comment #3 from Jason Tibbitts <tibbs at math.uh.edu> 2010-11-22 22:33:39 EST ---
OK, here's a review.  Unfortunately there is significant work to be done before
this package can be acceptable.

Note that if you're not targeting RHEL4 or 5, you can remove BuildRoot:, %clean
and the first line of %install.

Note that your manual "Requires: perl" is unnecessary; rpmlint already finds
the perl dependency.  (Actually it finds four of them, but that's not really
your problem.)

I was going to comment on the lack of indentation in your %pre script making it
tough to read, but it looks like the example from the UsersAndGroups page
somehow lost its indentation; I fixed it.

Your license tag seems to be incorrect.  All of the source files which have the
proper GPL license header indicate GPLv2+.  Unpack the tarball and see:
  grep -rC1 'of the License' .
(This doesn't count the improperly bundled libraries, which are a separate
issue.)

/etc/logrotate.d/gnump3d needs to be marked %config(noreplace) as admins do
edit these files and will expect them to be preserved.  rpmlint complains about
this:
  gnump3d.noarch: W: non-conffile-in-etc /etc/logrotate.d/gnump3d
It is indeed in the right place; rpmlint is properly warning that it's not
marked properly as a config file.

I've no way to properly test this as all my music is FLAC.  I guess I could
convert some of it, but it's taken long enough to review this as it is.

The file lib/gnump3d/WMA.pm seems to be a bundled copy of
http://search.cpan.org/~daniel/Audio-WMA-1.3/.  This is a blocker.  It also
makes me wonder what else they're bundling; a quick check shows IP.pm, MD5.pm
and mp3info.pm are ripped off from some other packages as well.  It's nice that
they say the software is self-contained, but achieving that through bundling is
not proper and must be fixed.

* source files match upstream.  sha256sum:
  1ac5bd0e850b0e18ccd9d19219f5108fa84b50d8ae3825a361e8b907eab7f19c
   gnump3d-3.0.tar.bz2
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
X license field should be GPLv2+
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper (none).
* package builds in mock (rawhide, x86_64).
* package installs properly.
X rpmlint has a valid complaint.
* final provides and requires are sane:
   config(gnump3d) = 3.0-4.fc15
   perl(gnump3d::IP) = 3.14
   perl(gnump3d::MD5) = 1.6
   perl(gnump3d::Request)  
   perl(gnump3d::WMA) = 0.7
   perl(gnump3d::base64)  
   perl(gnump3d::config) = 0.01
   perl(gnump3d::files)  
   perl(gnump3d::filetypes)  
   perl(gnump3d::lang::lookup)  
   perl(gnump3d::mp3info) = 1.13
   perl(gnump3d::mp4info) = 1.03
   perl(gnump3d::ogginfo) = 0.01
   perl(gnump3d::readtags)  
   perl(gnump3d::sorting) = 0.01
   perl(gnump3d::tagcache)  
   perl(gnump3d::url) = 0.01
   perl(plugins::copying) = 1..2
   perl(plugins::info) = 1..2
   perl(plugins::now) = 1..2
   perl(plugins::playlist) = 1..2
   perl(plugins::prefs) = 1..2
   perl(plugins::random) = 1..2
   perl(plugins::recent) = 1..2
   perl(plugins::search) = 1..2
   perl(plugins::size) = 1..2
   perl(plugins::stats) = 1..2
   perl(plugins::tagbrowse) = 1..2
   gnump3d = 3.0-4.fc15
  =
   /bin/bash  
   /bin/sh  
   /sbin/chkconfig  
   /sbin/service  
   /usr/bin/perl  
   config(gnump3d) = 3.0-4.fc15
?  perl  <------- unneeded manual dependency
   perl >= 0:5.004
   perl >= 0:5.005
   perl >= 0:5.005_62
   perl >= 0:5.006
   perl(AutoLoader)  
   perl(Carp)  
   perl(Encode)  
   perl(Encode::Guess)  
   perl(English)  
   perl(Env)  
   perl(Exporter)  
   perl(File::Find)  
   perl(Getopt::Long)  
   perl(IO::File)  
   perl(IO::Socket)  
   perl(POSIX)  
   perl(Pod::Usage)  
   perl(Socket)  
   perl(Symbol)  
   perl(gnump3d::IP)  
   perl(gnump3d::MD5)  
   perl(gnump3d::Request)  
   perl(gnump3d::WMA)  
   perl(gnump3d::config)  
   perl(gnump3d::files)  
   perl(gnump3d::filetypes)  
   perl(gnump3d::lang::lookup)  
   perl(gnump3d::mp3info)  
   perl(gnump3d::mp4info)  
   perl(gnump3d::ogginfo)  
   perl(gnump3d::readtags)  
   perl(gnump3d::sorting)  
   perl(gnump3d::tagcache)  
   perl(gnump3d::url)  
   perl(integer)  
   perl(overload)  
   perl(sigtrap)  
   perl(strict)  
   perl(vars)  
   perl(warnings)  
   shadow-utils  

X contains many bundled libraries.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files.
* scriptlets are OK (user/group creation).
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.