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@xxxxxxxxxxx 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review