[Bug 459855] Review Request: ncid - Caller ID distributed over a network to a variety of devices

Mon Oct 20 17:50:43 UTC 2008

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=459855

--- Comment #1 from Andreas Thienemann <andreas at bawue.net>  2008-10-20 13:50:42 EDT ---
The package will need a bit more work before I can approve it.

OK: source files match upstream:
aefe6c6ddb46fb5eb5927974f0080518f63cf61a
OK: dist tag is present.
OK: build root is correct.
OK: license field matches the actual license.
OK: license is open source-compatible.
 GPLv2+
OK: license text included in package.
OK: latest version is being packaged.
OK: BuildRequires are proper.
OK: compiler flags are appropriate.
OK: %clean is present.
OK: package builds in mock.
OK: debuginfo package looks complete.
OK: no shared libraries are added to the regular linker search paths.
OK: doesn't own any directories it shouldn't.
OK: no duplicates in %files.
OK: file permissions are appropriate.
OK: code, not content.
OK: documentation is small, so no -docs subpackage is necessary.
OK: %docs are not necessary for the proper functioning of the package.
OK: no headers.
OK: no pkgconfig files.
OK: no libtool .la droppings.
OK: desktop files valid and installed properly not necessary.

PASS: package meets naming and versioning guidelines.
 Did you consider naming ncidd differently? Maybe ncid-server to fit in with 
 the ncid-client?

NOK: specfile is properly named, is cleanly written and uses macros
consistently.

The .spec/srpm file is a bit messy:

 * Please do use full URLs for Source0.
   Instead of "Source0:        %{name}-%{version}-src.tar.gz" please use 
   "Source0:        http://downloads.sourceforge.net/%{name}/%{name}-%
   {version}-src.tar.gz"
 * Please name your patches accordingly:
   ncid-0.71-no-default-runlevels.patch would be a good convention to follow.
 * Please start your patches with the id 0.
 * Trim the descriptions. They are way too long. More then 10 lines should 
   never be necessary.
 * I'd prefer you to move all the %post scripts to the bottom of the .spec 
   between %clean and %file. It's again a convention most packagers seem to 
   follow and it makes reading .spec files so much easier.
 * Swap the /usr hardcode in the %build and %install part for %{_prefix}

NOK: rpmlint is silent.
 ncid-client.i386: W: spurious-executable-perm
/usr/share/doc/ncid-client-0.71/ncid-mythtv
 ncidd.i386: E: non-ghost-file /var/log/cidcall.log
 ncidd.i386: E: zero-length /var/log/cidcall.log
 ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/ncidd $prog
 ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/ncidd $prog
 ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/ncidd $prog
 ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/ncidsip $prog
 ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/ncidsip $prog
 ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/ncidsip $prog
 ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/sip2ncid $prog
 ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/sip2ncid $prog
 ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/sip2ncid $prog
 ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/yac2ncid $prog
 ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/yac2ncid $prog
 ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/yac2ncid $prog
 ncid-page.i386: W: no-documentation
 ncid-popup.i386: W: no-documentation
 ncid-samba.i386: W: no-documentation
 ncid-speak.i386: W: no-documentation
 8 packages and 0 specfiles checked; 2 errors, 17 warnings.

Please remove the executable bit from the mythtv initscript. You might wanna
drop it completely though.
Please consider dropping the cidcall.log file and %ghost it instead if it will
be created by the daemon if non-existant.
The incoherent-subsys warnings should be ignoreable, possible the same for the
plugin subpackages.

NOK: package installs properly.
 The ncid-page tool will not install, the Require for mail is invalid.
 [root at workstation ncid]# repoquery --whatprovides mail
 [root at workstation ncid]# 
 Either use mailx or /bin/mail as a dep.

PASS: scriptlets present and work okay.
 It would be great though if you could rework the initscripts to support 
 the newer upstart syntax.

NOK: final provides and requires are sane:
Requires:
 /bin/bash  
 /bin/sh  
 config(ncid-client) = 0.71-1.fc9
 config(ncidd) = 0.71-1.fc9
 festival  
 kdebase  
 kdelibs  
 kdemultimedia  
 libc.so.6  
 libc.so.6(GLIBC_2.0)  
 libc.so.6(GLIBC_2.1)  
 libc.so.6(GLIBC_2.3)  
 libc.so.6(GLIBC_2.3.4)  
 libc.so.6(GLIBC_2.4)  
 libpcap.so.0.9  
 mail  
 ncid-client = 0.71-1.fc9
 ncid-speak = 0.71-1.fc9
 perl(Data::Dumper)  
 perl(Getopt::Long)  
 perl(Getopt::Std)  
 perl(IO::Socket::INET)  
 perl(Net::Pcap)  
 perl(Pod::Usage)  
 perl(POSIX)  
 perl(strict)  
 perl(warnings)  
 rpmlib(CompressedFileNames) <= 3.0.4-1
 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
 rtld(GNU_HASH)  
 smbclient  
 /usr/bin/perl  
Provides:
 config(ncid-client) = 0.71-1.fc9
 ncid-client = 0.71-1.fc9
 config(ncidd) = 0.71-1.fc9
 ncidd = 0.71-1.fc9
 ncid-debuginfo = 0.71-1.fc9
 ncid-page = 0.71-1.fc9
 ncid-popup = 0.71-1.fc9
 ncid-samba = 0.71-1.fc9
 ncid-speak = 0.71-1.fc9

Problem is the mail dependency again.

NOK: owns the directories it creates.
 /etc/ncid
 /usr/share/ncid
 are shared by the packages but no package owns these directories.

Another observation:
 Why did you remove the shebang line from the ncidrotate-script? If it's 
 supposed to be executable, drop it in /usr/libexec/%{name}/.

A general suggestions: I found upstream .spec files often too much work to
adapt to the fedora packaging guidelines which is why I'm rolling my own
usually. Might be wort considering the next time. :)

-- 
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.