Spec URL: http://roma.fedorapeople.org/specs/gnump3d.spec SRPM URL: http://roma.fedorapeople.org/srpms/gnump3d-3.0-3.fc12.src.rpm Description: GNUMP3d is a streaming server for MP3's, OGG vorbis, and other streamable audio files. It is designed to be: * Small, stable, self-contained, and secure. * Simple to install, configure, and use. * Portable across different varieties of Unix. Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2135876 Rpmlint output: [cascavel:~/RPMS124/lcgrpms] rpmlint gnump3d-3.0-3.fc12.noarch.rpm gnump3d.noarch: W: spelling-error %description -l en_US vorbis -> Orbison, orbits, forbids gnump3d.noarch: W: spelling-error %description -l en_US streamable -> stream able, stream-able, streamline gnump3d.noarch: W: non-standard-uid /var/cache/gnump3d gnump3d gnump3d.noarch: W: non-standard-gid /var/cache/gnump3d gnump3d gnump3d.noarch: W: non-standard-uid /var/log/gnump3d/error.log gnump3d gnump3d.noarch: W: non-standard-gid /var/log/gnump3d/error.log gnump3d gnump3d.noarch: W: non-conffile-in-etc /etc/logrotate.d/gnump3d gnump3d.noarch: W: non-standard-uid /var/cache/gnump3d/song.tags gnump3d gnump3d.noarch: W: non-standard-gid /var/cache/gnump3d/song.tags gnump3d gnump3d.noarch: W: non-standard-uid /var/log/gnump3d gnump3d gnump3d.noarch: W: non-standard-gid /var/log/gnump3d gnump3d gnump3d.noarch: W: non-standard-uid /var/log/gnump3d/access.log gnump3d gnump3d.noarch: W: non-standard-gid /var/log/gnump3d/access.log gnump3d gnump3d.noarch: W: non-standard-uid /var/cache/gnump3d/serving gnump3d gnump3d.noarch: W: non-standard-gid /var/cache/gnump3d/serving gnump3d 1 packages and 0 specfiles checked; 0 errors, 15 warnings.
It's nice that you posted rpmlint output. However, it's expected that you'll also address that output. Also, you missed this one: gnump3d.src:14: W: unversioned-explicit-provides perl(gnump3d::readtags) because it looks like you didn't run rpmlint on the src.rpm. You should always do that. I think all of the rpmlint complaints are justifiable, but it's expected that you will justify them, not the reviewer. Does this really have no external dependencies on any kind of mp3 or vorbis decoder? Why would you bzip the init and logrotate files? It doesn't save any space and only makes them more difficult to inspect. This does not seem to follow the packaging guidelines for adding the user and group: https://fedoraproject.org/wiki/Packaging:UsersAndGroups
(In reply to comment #1) > It's nice that you posted rpmlint output. However, it's expected that you'll > also address that output. > > Also, you missed this one: > gnump3d.src:14: W: unversioned-explicit-provides perl(gnump3d::readtags) > because it looks like you didn't run rpmlint on the src.rpm. You should always > do that. gnump3d does not work without this Provide statement, and I do not think it needs to be versioned. > > I think all of the rpmlint complaints are justifiable, but it's expected that > you will justify them, not the reviewer. > The others rpmlint complaints are due to the new group and user created, and the logrotate file is in its appropriate place. > Does this really have no external dependencies on any kind of mp3 or vorbis > decoder? I think not: http://www.linux.com/archive/feature/61154 > > Why would you bzip the init and logrotate files? It doesn't save any space and > only makes them more difficult to inspect. I unpacked them in the new version. > > This does not seem to follow the packaging guidelines for adding the user and > group: https://fedoraproject.org/wiki/Packaging:UsersAndGroups Well, I just adopted the recommended template now. Spec URL: http://roma.fedorapeople.org/specs/gnump3d.spec SRPM URL: http://roma.fedorapeople.org/srpms/gnump3d-3.0-4.fc12.src.rpm Thnaks.
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.
No response in a month; I guess I'll just close this out if nothing happens soon.