[Bug 226190] Merge Review: netatalk

Thu Feb 22 02:01:42 UTC 2007

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

Summary: Merge Review: netatalk

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

------- Additional Comments From tibbs at math.uh.edu  2007-02-21 21:01 EST -------
I have to admit to being a bit apprehensive about reviewing this package
because I know how ancient it is, but it's really not too bad.

Let's see what rpmlint has to say:

W: netatalk prereq-use /sbin/chkconfig, /sbin/service
   PreReq doesn't work as intended in RPM; use 
    Requires(post): /sbin/chkconfig
    Requires(preun): /sbin/chkconfig
    Requires(preun): /sbin/service
    Requires(postun): /sbin/service
   instead.

W: netatalk macro-in-%changelog config
W: netatalk macro-in-%changelog defattr
W: netatalk macro-in-%changelog exclusiveos
   '%' symbols in %changelog need to be escaped by doubling them.  Otherwise
   they can actually be expanded under some circumstances.

W: netatalk mixed-use-of-spaces-and-tabs (spaces: line 4, tab: line 9)
   rpmlint is needlessly picky, although line 9 is a bit odd, containing
   "space tab space".

W: netatalk conffile-without-noreplace-flag /etc/rc.d/init.d/atalk
   This is OK; there's ongoing discussion about the proper thing to do
   here but no concensus.

W: netatalk devel-file-in-non-devel-package /usr/bin/netatalk-config
   Any reason why this isn't in the -devel package?

E: netatalk wrong-script-interpreter /usr/share/doc/netatalk-2.0.3/ICDumpSuffixMap "perl"
   This file has nonstandard line endings.  It should be either converted or
   just deleted.  If you choose to keep it, you might as well patch the #!
   line to include a path.

E: netatalk executable-marked-as-config-file /etc/rc.d/init.d/atalk
   Generally init.d files are executable, so this is OK

E: netatalk setuid-binary /usr/bin/afppasswd root 04755
E: netatalk non-standard-executable-perm /usr/bin/afppasswd 04755
   This is just the way it has to be, as I understand things.

W: netatalk incoherent-init-script-name atalk
   rpmlint wants to see the init script named after the package, but after
   all this time I can't imagine changing this.

W: netatalk-devel no-documentation
   Not a problem.

Other issues found during review:

You can use %{_initrddir} instead of defining "initdir" if you like.

BuildRoot should be:
   %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

Any reason for not allowing parallel make?  It seems to work fine on my 8-way
machine.

There are a couple of odd dependencies.  Given that even the very old
RH9 shipped with pam 0.75, it's certainly safe to drop the versioned
requirement on pam, although it's not really problematic to keep it.  You
don't need the explicit openssl or cracklib dependencies as RPM picks up
the libcrypto and libcrack dependencies by itself.

Also, you have a runtime dependency on tcp_wrappers, but no build-time
dependency on it, so the resulting package is built without tcp_wrappers
support.  I think you should add a BR: on tcp_wrappers-devel and remove the
manual runtime tcp_wrappers dependency; RPM will pick up the libwrap
dependency which will pull in tcp_wrappers-libs automatically.

Also, I'm not sure why there's an explicit requirement for
/etc/pam.d/system-auth.  This file has been part of pam for as long as I can
recall, and having an explicit file dependency causes extra pain for end users
in dependency resolution because yum has to pull in filelists.xml.  (See
http://fedoraproject.org/wiki/PackagingDrafts/FileDeps for more info.)

Several libraries are installed into %{_libdir}, but there are no scriptlets
which call ldconfig.  You should add /sbin/ldconfig calls to %post and %postun
and the necessary dependencies.

There are several static libraries and .la files in the -devel package.
Generally static libraries aren't shipped in Fedora without a really good
reason, and if they are shipped, they need to be in a -static subpackage.
Libtool .la files shouldn't be shipped at all unless the package breaks
without them.

Review:
* source files match upstream:
   25e004732f471de0dd9a21ab129ee799da018fce3b313d4ab5e6f52e6e9e3998
   netatalk-2.0.3.tar.bz2
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
X build root is not correct.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper (you shouldn't need to specify pam, as pam-devel
   should pull it in, and you don't need to specify libtool explicitly.)
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo package looks complete.
X rpmlint has valid complaints.
? final provides and requires are sane:
  netatalk-2.0.3-9.fc7.x86_64.rpm
   config(netatalk) = 4:2.0.3-9.fc7
   uams_dhx_pam.so()(64bit)
   uams_dhx_passwd.so()(64bit)
   uams_gss.so()(64bit)
   uams_guest.so()(64bit)
   uams_pam.so()(64bit)
   uams_passwd.so()(64bit)
   uams_randnum.so()(64bit)
   netatalk = 4:2.0.3-9.fc7
  =
   /bin/sh
?  /etc/pam.d/system-auth
   /sbin/chkconfig
   /sbin/service
   /usr/bin/perl
   config(netatalk) = 4:2.0.3-9.fc7
   cracklib
   libcom_err.so.2()(64bit)
   libcrack.so.2()(64bit)
   libcrypto.so.6()(64bit)
   libdb-4.5.so()(64bit)
   libgssapi_krb5.so.2()(64bit)
   libgssapi_krb5.so.2(gssapi_krb5_2_MIT)(64bit)
   libk5crypto.so.3()(64bit)
   libkrb5.so.3()(64bit)
   libpam.so.0()(64bit)
   libpam.so.0(LIBPAM_1.0)(64bit)
?  openssl
?  pam >= 0.56
   perl >= 1:5
   perl(Getopt::Std)
   perl(IO::Socket)
   perl(Socket)
   perl(strict)
   perl(vars)
?  tcp_wrappers
   uams_dhx_pam.so()(64bit)
   uams_pam.so()(64bit)

* %check is not present; no test suite upstream.
X shared libraries are added to the regular linker search paths, but ldconfig
  is not called.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present (but there should be)
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
  Actually the package is about half documentation, but the whole thing isn't
  very large.
* %docs are not necessary for the proper functioning of the package.
* headers are in the -devel package.
* no pkgconfig files.
X static libraries.
X libtool .la droppings.

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.