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: coreutils https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225655 ------- Additional Comments From pertusus@xxxxxxx 2007-02-16 13:16 EST ------- Missing Requires(pre): /sbin/install-info Requires(preun): /sbin/install-info Requires(post): grep, /sbin/install-info, coreutils I am not sure that coreutils in Requires(post) makes sense... Is Requires: grep, findutils really right? The versions are certainly wrong in Obsoletes. The latest packaged version (and versions below) should be obsoleted. The Provides also seems wrong to me. First of all I guess it should be %{version}-%{release}. But I am not sure that the current package version should be used for the Provides since it is a merge of packages that certainly had their own version. I think it is a bit strange to depend on itself and other packages that also depends on coreutils and are not really required, that is, anything else than the shell, gcc and make since there is a bootstrapping issue otherwise. But I guess that it is impossible to avoid that bootstraping issue (except by doing complicated things like using busybox...). I also guess that this issue is also there for gcc, make and bash, so... There is a huge amount of patches. Many could be submitted (even the pam one), or have they been submitted and rejected? /etc, /var and /usr/bin are hardcoded in the spec file, maybe they could be changed to macros. It may be relevant to to use -p for files installed, like DIR_COLORS, colorls.* ... pam files, since the files are certainly not changed often and the timestamp carry some information. sed could be used instead of perl -pi -e 's/basic-1//g' tests/stty/Makefile* and perl -pi -e 's,/etc/utmp,/var/run/utmp,g;s,/etc/wtmp,/var/run/wtmp,g' doc/coreutils.texi Suggestions: Add a comment above bzip2 -f9 old/*/C* || : to explain what it does Don't use -f for some rm invocations (like rm -f $RPM_BUILD_ROOT{%_bindir/$i,%_mandir/man1/$i.1}), don't add || : after bzip2 -f9 old/*/C* || :, and also do bzip2 -9f ChangeLog unconditionally such that they fail if something changes upstream. At the same time short-circuit should still work, so care should be taken. Maybe [[ -f ChangeLog && -f ChangeLog.bz2 ]] || bzip2 -9f ChangeLog should be in %install In the info scriptlets, I suggest removing the .gz from preun and post, and replace .bz2 by something appropriate. Maybe *, but care should be taken that it doesn't expands to more than one file. replace %defattr(-,root,root) by %defattr(-,root,root,-) -- 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