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=491497 --- Comment #15 from Christian Krause <chkr@xxxxxxxxxxx> 2009-12-03 17:37:10 EDT --- Sorry that it took a little bit longer than expected, but here is now the complete review: * rpmlint: TODO dmapd.i686: W: conffile-without-noreplace-flag /etc/sysconfig/dmapd - please consider to use "%config(noreplace)" dmapd.i686: W: non-standard-uid /var/run/dmapd dmapd dmapd.i686: E: non-standard-dir-perm /var/run/dmapd 0700 dmapd.i686: W: non-standard-uid /var/cache/dmapd dmapd dmapd.i686: E: non-standard-dir-perm /var/cache/dmapd 0700 These entries seem to be false positives. dmapd-devel.i686: W: no-documentation This is also a false positive. dmapd.i686: W: non-standard-uid /var/db/dmapd dmapd dmapd.i686: E: non-standard-dir-perm /var/db/dmapd 0700 dmapd.i686: W: non-standard-uid /var/db/dmapd/Pictures dmapd dmapd.i686: E: non-standard-dir-perm /var/db/dmapd/Pictures 0700 dmapd.i686: W: non-standard-uid /var/db/dmapd/Movies dmapd dmapd.i686: E: non-standard-dir-perm /var/db/dmapd/Movies 0700 dmapd.i686: W: non-standard-uid /var/db/dmapd/Music dmapd dmapd.i686: E: non-standard-dir-perm /var/db/dmapd/Music 0700 dmapd.i686: W: non-standard-dir-in-var db Regarding these directories I have some doubts. I've read your explanation in comment #14, but I think that the chosen directory /var/db/dmapd is quite arbitrary. I would leave it up completely to the user, where he puts its data and so I'd recommend: - don't create these directories and don't package them - put the "--user dmapd" argument into the init script - comment out the line DMAPDARGS in /etc/sysconfig/dmapd and make sure, that the daemon complains about a missing option when the user has not adjusted or activated this line * naming: OK - name matches upstream - spec file name matches package name * Source0: OK - md5sum: 4ac7dc73d4374a4735a5714ff23aaa26 dmapd-0.0.17.tar.gz - sources matches upstream - Source0 tag ok - spectool -g works * License: OK - GPLv2+ acceptable for Fedora - license matches the actual license: Although the sources uses LGPLv2+ and GPLv2+ it is OK to use soleley GPLv2+ as final license for the binary package. - license file packaged * spec file written in English and legible: TODO - please consider to use the standard order of the sections and the standard formatting of spec files * Compilation: OK - package builds in koji on Rawhide on all archs: OK - The package can not be built on F11 and F12 since libdmapsharing is not yet updated to 1.9.0.13 in these releases - parallel make ok * build requirements: TODO - IMHO avahi-devel and dbus-devel can be removed from the build requirements since all dbus/avahi handling will be done via libdmapsharing. A direct dependency for runtime or build time to avahi/dbus is not necessary. * locale handling: OK (n/a) * ldconfig in %post and %postun: OK * directory ownership: TODO Even if there is currently a proposal to change this, currently the official policy is still to "Require: pkgconfig" in the -devel subpackage if there are any pkgconfig files shipped. * files not listed twice: OK * file permissions: OK - %defattr used - actual permissions in packages ok * rm -rf $RPM_BUILD_ROOT in %install and %clean: OK * macro usage: TODO - please substitute "%{PACKAGE_VERSION}" with "%{version}" for consistency - please use also %{_initdir} * code vs. content: OK (no content) * large documentation in subpackage: OK (n/a) * header files in -devel subpackage: OK * static libraries in -static package: OK (n/a) * *.so link in -devel package: OK * -devel package requires base package using fully versioned dependency: OK * package must not contain *.la files: OK * package containing *.pc files must "Requires: pkgconfig": TODO - please require pkgconfig in -devel subpackage (see above) * desktop-file: OK (n/a, server application) * compiler flags: OK (rpmoptflags used) * debuginfo complete: OK * functional check: TODO - /etc/init.d/dmapd start works - service DAAP service is detected and serving music works - /etc/init.d/dmapd stop does not work: root@:~# /etc/init.d/dmapd start Starting DMAP server: [ OK ] root@:~# /etc/init.d/dmapd stop Shutting down DMAP server: [FAILED] * packages must not own files/directories already owned by other packages: OK * all file names UTF-8: OK * scriptlets: TODO ldconfig: OK chkconfig: TODO http://fedoraproject.org/wiki/Packaging:SysVInitScript: - "Requires(preun): initscripts" is missing - "Requires(postun): initscripts" is missing - scriptles in spec file are ok user creation/deletion: ok * sysvinit scripts: - stop() should return $RETVAL - script mentions "condrestart", but command won't be accepted - this command is required - starting the service twice will result twice the number of dmapd processes and strange error messages: root@:~# /etc/init.d/dmapd start Starting DMAP server: [ OK ] root@:~# ** (dmapd:12388): WARNING **: Unable to start music sharing server on port 8770, trying any open port ** (dmapd:12388): WARNING **: Unable to notify network of media sharing: Could not add service: Local name collision ** (dmapd:12386): WARNING **: Unable to start music sharing server on port 3689, trying any open port ** (dmapd:12386): WARNING **: Unable to notify network of media sharing: Could not add service: Local name collision - Please have a look at "http://fedoraproject.org/wiki/Packaging:SysVInitScript"; for the basic requirements for initscripts in Fedora (the LSB parts are not necessary). Probably it would be the best to use the example init scripts as a base for dmapd's init script. Most likely lots of the mentioned problems will be fixed then automatically. Summary: -------- - use "%config(noreplace)" for /etc/sysconfig/dmapd - consider not to package /var/db/dmapd at all - remove unneeded build requirements - minor macro usage fixes - consider to reformat the spec file to be more standard conform - add pkgconfig requirement for -devel subpackage - add the missing requires for the chkconfig scriptlets - rework the init scripts -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review