[Bug 226377] Merge Review: rpm
bugzilla at redhat.com
bugzilla at redhat.com
Thu Feb 22 07:21:02 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: rpm
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226377
kevin at tummy.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
AssignedTo|nobody at fedoraproject.org |pnasrat at redhat.com
CC| |kevin at tummy.com
Flag| |fedora-review-
------- Additional Comments From kevin at tummy.com 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.
More information about the Fedora-package-review
mailing list