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: rpm https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226377 kevin@xxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@xxxxxxxxxxxxxxxxx |pnasrat@xxxxxxxxxx CC| |kevin@xxxxxxxxx Flag| |fedora-review- ------- Additional Comments From kevin@xxxxxxxxx 2007-02-22 02:20 EST ------- OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistant macro usage. OK - Meets Packaging Guidelines. OK - License (GPL) OK - License field in spec matches See below - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: e24ce468082479fe850c9d6563f56db5 rpm-4.4.2.tar.gz e24ce468082479fe850c9d6563f56db5 rpm-4.4.2.tar.gz.1 See below - BuildRequires correct See below - Spec handles locales/find_lang OK - Package is relocatable and has a reason to be. OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. See below - Package has correct buildroot OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Headers/static libs in -devel subpackage. OK - Spec has needed ldconfig in post and postun OK - .so files in -devel subpackage. OK - -devel package Requires: %{name} = %{version}-%{release} OK - .la files are removed. OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. OK - Package owns all the directories it creates. See below - No rpmlint output. See below - final provides and requires are sane: SHOULD Items: OK - Should build in mock. OK - Should build on all supported archs OK - Should function as described. OK - Should have sane scriptlets. OK - Should have subpackages require base package with fully versioned depend. OK - Should have dist tag OK - Should package latest version 147 outstanding open bugs - check for outstanding bugs on package. 1. There are a number of different liceses here: main part - GPL and some LGPL db - BSDish zlib - weird zlib license. I assume they can all handle being released GPL as a whole? Can you include a copy of the GPL COPYING file? 2. Is there any way that find_lang could be used? I guess it would need to be called from the local ./scripts/find-lang.sh ? 3. Please use the one true buildroot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) 4. Are all the "%ifos linux" conditionals needed? Is this spec used on non linux systems very often? 5. Per http://www.fedoraproject.org/wiki/PackagingDrafts/Conflicts (which is only a draft it's true), could you change the Conflicts: patch < 2.5 to Requires: patch > 2.5 or since this has been true since fc1, can we just remove that? Also in the -build subpackage there's a 'Requires: patch >= 2.5' 6. BuildRequires: sed isn't needed, thats in the base build exceptions. 7. What does the "#XXX: lua fix this" comment mean? 8. Can this conditional be removed now: # XXX Red Hat 5.2 has not bzip2 or python %if %{with_bzip2} 9. In both build and install there is: # XXX rpm needs functioning nptl for configure tests unset LD_ASSUME_KERNEL || : Perhaps instead it could bomb out of the build if someone is stupid enough to have that set these days? Right now if they do have it set and they build rpm, it will magically be unset after the build. 10. Since there's a cron.daily file, shouldn't there also be Requires: crontab 11. Likewise for logrotate, should there be a 'Requires: logrotate' 12. This block in install doesn't seem to be doing anything: %if %{with_apidocs} gzip -9n apidocs/man/man*/* || : %endif from build.log: + gzip -9n 'apidocs/man/man*/*' gzip: apidocs/man/man*/*: No such file or directory The directory in the devel subpackage is empty. 13. The python subpackage should not need to call ldconfig on post/postun. This block can be removed: %if %{with_python_subpackage} %post python -p /sbin/ldconfig %postun python -p /sbin/ldconfig %endif 14. rpmlint has a few things to say: First, there are 240 lines of 'non-standard-uid' or 'non-standard-gid' These can all be ignored in this case. a) W: popt no-url-tag W: rpm no-url-tag W: rpm-build no-url-tag W: rpm-libs no-url-tag W: rpm-python no-url-tag W: rpm-devel no-url-tag Suggest: add a "URL: http://www.rpm.org"; ? b) W: popt summary-ended-with-dot A C library for parsing command line parameters. W: rpm summary-ended-with-dot The RPM package management system. W: rpm summary-ended-with-dot The RPM package management system. W: rpm-build summary-ended-with-dot Scripts and executable programs used to build packages. W: rpm-devel summary-ended-with-dot Development files for manipulating RPM packages. W: rpm-libs summary-ended-with-dot Libraries for manipulating RPM packages. W: rpm-python summary-ended-with-dot Python bindings for apps which will manipulate RPM packages. Suggest: remove . at end of summary. c) W: popt devel-file-in-non-devel-package /usr/lib64/libpopt.a W: popt devel-file-in-non-devel-package /usr/include/popt.h W: popt devel-file-in-non-devel-package /usr/lib64/libpopt.so There's a comment that says: # XXX These may end up in popt-devel but it hardly seems worth the effort. %{__libdir}/libpopt.a %{__libdir}/libpopt.so %{__includedir}/popt.h Is it still not worth the effort? Or perhaps now is a good time? Or perhaps they can just be removed if nothing is using them? d) W: rpm prereq-use fileutils shadow-utils fileutils was replaced a while back with coreutils, which is in the min build root exceptions. For shadow-utils, you could do: Requires(pre): shadow-utils Requires(postun): shadow-utils e) W: rpm macro-in-%changelog ghost Suggest: Change "%ghost" in the changelog to "%%ghost" f) W: rpm mixed-use-of-spaces-and-tabs (spaces: line 267, tab: line 1) Suggest: pick tabs or spaces. g) W: rpm patch-not-applied Patch9: rpm-4.4.2-contextverify.patch W: rpm patch-not-applied Patch12: rpm-4.4.2-exclude.patch Suggest: should those patches just be removed? h) E: rpm obsolete-not-provided rpm-perl Suggest: Do we need to have this obsolete around anymore? i) W: rpm file-not-utf8 /usr/share/man/sk/man8/rpm.8.gz W: rpm file-not-utf8 /usr/share/man/pl/man8/rpm.8.gz W: rpm file-not-utf8 /usr/share/man/pl/man8/rpmbuild.8.gz W: rpm file-not-utf8 /usr/share/man/pl/man8/rpmdeps.8.gz W: rpm file-not-utf8 /usr/share/man/pl/man8/rpmgraph.8.gz W: rpm file-not-utf8 /usr/share/man/pl/man1/gendiff.1.gz W: rpm file-not-utf8 /usr/share/man/pl/man8/rpmcache.8.gz W: rpm file-not-utf8 /usr/share/man/pl/man8/rpm2cpio.8.gz Suggest: Perhaps run iconv on those? j) E: rpm script-without-shebang /usr/lib/rpm/rpm.xinetd Suggest: should this be shipped at all? or installed in /etc/xinetd.d/ ? Or at the very least it should be mode 644. k) E: rpm script-without-shebang /usr/lib/rpm/rpm.log Suggest: This is a duplicate of /etc/logrotate.d/rpm, and can be removed? l) E: rpm executable-marked-as-config-file /etc/cron.daily/rpm Suggest: Don't mark that a config file. m) E: rpm-build script-without-shebang /usr/lib/rpm/magic E: rpm-build script-without-shebang /usr/lib/rpm/magic.mime E: rpm-build script-without-shebang /usr/lib/rpm/config.site Suggest: all 3 of those should be mode 644? n) W: rpm-debuginfo spurious-executable-perm /usr/src/debug/rpm-4.4.2/rpmqv.c Suggest: source file should be 644? The following can also be ignored. Since rpm needs rpmmacros to use %configure or the like it can't do that when building itself. Or they otherwise appear to be false positives to me. E: rpm configure-without-libdir-spec E: rpm configure-without-libdir-spec E: rpm hardcoded-library-path in /usr/lib/rpmrc E: rpm standard-dir-owned-by-package /var/lib/rpm W: rpm-libs no-documentation W: rpm-python no-documentation W: rpm dangerous-command-in-%pre rpm W: rpm dangerous-command-in-%post chown W: rpm dangerous-command-in-%postun userdel E: rpm-build statically-linked-binary /usr/lib/rpm/debugedit E: rpm-python script-without-shebang /usr/lib64/python2.5/site-packages/rpm/__init__.py 15. Are all the static libs in -devel needed? If so perhaps a -static subpackage? 16. The 'Requires: python >= %{with_python_version}' isn't needed. rpm adds a 'python(abi) = 2.5' to requires without it. 17. There are currently 147 outstanding open bugs against rpm. You might consider going and doing some triage on them. If you like I would be willing to assist in doing so. Some of them do seem related to packaging concerns. Since there is a upstream at rpm.org, many could possibly be pushed up there (especially patches/enhancement requests). Perhaps we could even round up a QA team session and get them addressed. 18. Is there any chance of trying the sqlite backend? I know it's supposed to be slower than DB, but if it is more reliable it will be well worth it in my opinion. I would also be willing to assist in generating a patch to the spec addressing many of the above if you would like me to. Once you have addressed these items (either by making the suggested changes, or by explaining why they don't make sense), please reassign this review back to me, and change the 'fedora-review' flag back to ? for me to take action. -- 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