Fedora Merge Review: yum http://cvs.fedora.redhat.com/viewcvs/devel/yum/ Initial Owner: katzj
This is my first review so maybe somebody should take a extra look at my review.
Must : OK - spec filename is %{name}.spec OK - source match upstream md5sum CVS: 6e6953f92531aa0f9074199f2925d22a yum-3.1.1.tar.gz Upstream : 6e6953f92531aa0f9074199f2925d22a yum-3.1.1.tar.gz OK - Package naming OK - Spec in American English and legible OK - License : GPL OK - BuildRequires correct OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - License file (COPYING) is included in %doc - Package compiles and builds on at least one arch. OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Package owns all the directories it creates. FAIL - Buildroot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) Comments: * Source0 should end with %{name}-%{version}, not yum-%{version} * Requires: python (Upstream spec has Requires: python >= 2.4) * Requires: rpm >= 0:4.1.1 ( Upstream spec has Requires: rpm >= 0:4.4.2)
rpmlinst output: [tim@naboo devel]$ rpmlint yum-3.1.1-1.src.rpm W: yum prereq-use /sbin/chkconfig W: yum prereq-use /sbin/service E: yum hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/yum-plugins E: yum hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/yum-plugins/installonlyn.py E: yum hardcoded-library-path in /usr/lib/yum-plugins E: yum hardcoded-library-path in /usr/lib/yum-plugins/* I am sure what the prereq warning means. the hardcoded library error should be ignored i think, because the plugins has to go into /usr/lib/yum-plugins/ always, even on 64 bit systems.
[tim@naboo devel]$ rpmlint ~/rpmbuild/RPMS/noarch/yum-3.1.1-1.noarch.rpm E: yum only-non-binary-in-usr-lib W: yum conffile-without-noreplace-flag /etc/logrotate.d/yum E: yum non-executable-script /usr/lib/python2.5/site-packages/yum/packages.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/yum/depsolve.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/yum/Errors.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/yum/sqlitesack.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/rpmUtils/arch.py 0644 E: yum non-executable-script /usr/share/yum-cli/progress_meter.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/yum/storagefactory.py 0644 E: yum non-executable-script /usr/share/yum-cli/yumupd.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/yum/__init__.py 0644 E: yum non-executable-script /usr/share/yum-cli/yummain.py 0644 E: yum non-executable-script /usr/share/yum-cli/utils.py 0644 E: yum non-executable-script /usr/share/yum-cli/output.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/yum/config.py 0644 E: yum non-executable-script /usr/share/yum-cli/callback.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/rpmUtils/transaction.py 0644 E: yum non-executable-script /usr/share/yum-cli/i18n.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/rpmUtils/__init__.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/rpmUtils/updates.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/rpmUtils/oldUtils.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/yum/packageSack.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/rpmUtils/miscutils.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/yum/sqlitecache.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/yum/rpmsack.py 0644 E: yum non-executable-script /usr/share/yum-cli/yumcommands.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/yum/repoMDObject.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/yum/sqlutils.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/yum/update_md.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/yum/repos.py 0644 E: yum non-executable-script /usr/share/yum-cli/cli.py 0644 E: yum non-executable-script /usr/lib/python2.5/site-packages/yum/failover.py 0644 only-non-binary-in-usr-lib is ok, because the yum-plugins has to go into /usr/lib/yum-plugins. yum non-executable-script is ok too, the .py files is not called directly, so they dont need to be executables, could be fixed in the upstream Makefile's to use 755 insted of 644 for the .py files. conffile-without-noreplace-flag /etc/logrotate.d/yum has to be checked, should it be overwriten every time.
Summery: * Buildroot should be fixed. * The rpm and version should match the ones in upstream spec. * The 'prereq' warning should be checked out. * the 'conffile-without-noreplace-flag' warning should be investigated.
(In reply to comment #5) > Summery: > * Buildroot should be fixed. > * The rpm and version should match the ones in upstream spec. > * The 'prereq' warning should be checked out. > * the 'conffile-without-noreplace-flag' warning should be investigated. I fixed up the buildroot, rpm and python versions, and the noreplace config file. I also used the name macro in source0. prereq should be ok, since we do want to have yum-updatesd running by default. The new version is yum-3.1.1-2
(In reply to comment #4) > only-non-binary-in-usr-lib is ok, because the yum-plugins has to go into > /usr/lib/yum-plugins. Wouldn't /usr/share/yum-plugins be a more appropriate location for them?
(In reply to comment #7) > Wouldn't /usr/share/yum-plugins be a more appropriate location for them? Probably. I'll bring it up on yum-devel.
The updated spec look fine to me, so i will approve it APPROVED
Issues: * Why is it a BuildRequires: python and not Buildrequires: python-devel? * it seems to me that the python-devel BuildRequires should be versionned then Requires: python >= 2.4 would be autodetected * Prereq should be replaced with Requires(preun) and so on... * Is the conflict really needed? pirut >= 1.1.4 should requires a recent enough yum version. suggestions: * use %defattr(-, root, root, -) instead of %defattr(-, root, root) * prefix plugin.conf source file by a disambiguating prefix like yum-plugin.conf * remove the -f to be notified when the file doesn't exist/change name... rm -f $RPM_BUILD_ROOT/%{_sysconfdir}/yum/yum.conf * there is an occurence of /etc hardcoded in the specfile, it could be %_sysconfdir. However the upstream Makefiles and programs have many paths hardcoded so this is not really an issue. (There is also a /var and some /usr, but I guess they are also hardcoded in the package). * It seems to me that BuildArch is more used that BuildArchitectures * There is no need of / after $RPM_BUILD_ROOT
(In reply to comment #10) > Issues: > > * Why is it a BuildRequires: python and not Buildrequires: python-devel? > > * it seems to me that the python-devel BuildRequires should be versionned > then Requires: python >= 2.4 would be autodetected Yum uses a Makefile + python to byte compile the code rather than setup.py, so it wouldn't actually BR: python-devel > * Prereq should be replaced with Requires(preun) and so on... These are fixed up already. > * Is the conflict really needed? pirut >= 1.1.4 should requires a > recent enough yum version. pirut < 1.1.4 would blow up with new yum, so the conflict is needed. Now, whether or not yum in F7 and rawhide should care about older pirut is a different issue... > suggestions: > > * use %defattr(-, root, root, -) instead of %defattr(-, root, root) > > * prefix plugin.conf source file by a disambiguating prefix like > yum-plugin.conf > > * remove the -f to be notified when the file doesn't exist/change name... > rm -f $RPM_BUILD_ROOT/%{_sysconfdir}/yum/yum.conf > > * there is an occurence of /etc hardcoded in the specfile, it could be > %_sysconfdir. However the upstream Makefiles and programs have many > paths hardcoded so this is not really an issue. (There is also a /var > and some /usr, but I guess they are also hardcoded in the package). > > * It seems to me that BuildArch is more used that BuildArchitectures > > * There is no need of / after $RPM_BUILD_ROOT defattr, buildarch, and macro suggestions incorporated in devel. Everyone ok with closing this one?
Ok, with me
You should keep timestamps of installed files, for example you could use install -m 0644 -p %{SOURCE2} $RPM_BUILD_ROOT/%{_sysconfdir}/yum/yum-updatesd.conf For the file are installed by make install nothing can be done, maybe it should be suggested to upstream to use $(INSTALL) instead of install, and set INSTALL = install In any case, this is not blocking.