Fedora Merge Review: ksh http://cvs.fedora.redhat.com/viewcvs/devel/ksh/ Initial Owner: karsten
1.) Spec file legibility: No useless comments please. #ExclusiveArch: x86_64 #ExcludeArch: ia64 #Patch0: ksh-20041225-gcc4.patch #%patch0 -p1 -b .gcc4 ... 2.) If you temporarily disable a patch, please remove it only from %prep, not the tag itself, so that the patch is bundled with srpm: # for debugging only: #Patch100: ksh-20060124-iffedebug.patch 3.) Use macros. Replace /usr/bin with %{_bindir} mkdir -p $RPM_BUILD_ROOT{/bin,/usr/bin,%{_mandir}/man1} /usr//bin/ksh ln -sf /bin/ksh $RPM_BUILD_ROOT/usr/bin/ksh ... 4.) Use %find_lang for locales %files %{_datadir}/locale/*/LC_MESSAGES/* 5.) RPMlint: ksh.src:28: W: unversioned-explicit-provides ksh93 ksh.src:29: W: unversioned-explicit-obsoletes ksh93 Though it is advisable to version I think it's ok if you leave these as they are; this had been unversioned for some time already and ksh93 is probably won't coming back anyways. ksh.src: W: patch-not-applied Patch4: ksh-20080725-ttou.patch Either remove the file, or explain why is it disabled. 6.) Upstream the patches Please submit the patches to usptream & comment on status. https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment 7.) Latest upstream release Latest release seems to be 2008-11-04. Please update. 8.) Avoid absolute symlinks ln -sf /bin/ksh $RPM_BUILD_ROOT/usr/bin/ksh
(In reply to comment #1) > 1.) Spec file legibility: No useless comments please. > > #ExclusiveArch: x86_64 > #ExcludeArch: ia64 > #Patch0: ksh-20041225-gcc4.patch > #%patch0 -p1 -b .gcc4 > ... fixed: removed > 2.) If you temporarily disable a patch, please remove it only from %prep, not > the tag itself, so that the patch is bundled with srpm: > > # for debugging only: > #Patch100: ksh-20060124-iffedebug.patch fixed: removed > 3.) Use macros. Replace /usr/bin with %{_bindir} > > mkdir -p $RPM_BUILD_ROOT{/bin,/usr/bin,%{_mandir}/man1} > /usr//bin/ksh > ln -sf /bin/ksh $RPM_BUILD_ROOT/usr/bin/ksh > ... fixed: now uses %{_bindir} > 4.) Use %find_lang for locales > > %files > %{_datadir}/locale/*/LC_MESSAGES/* fiexed: locales are no longer supported by upstream - removed > 5.) RPMlint: > > ksh.src:28: W: unversioned-explicit-provides ksh93 > ksh.src:29: W: unversioned-explicit-obsoletes ksh93 > > Though it is advisable to version I think it's ok if you leave these as they > are; this had been unversioned for some time already and ksh93 is probably > won't coming back anyways. didn't decided what to do with these two yet > ksh.src: W: patch-not-applied Patch4: ksh-20080725-ttou.patch > > Either remove the file, or explain why is it disabled. fixed: removed, no longer needed > 6.) Upstream the patches > > Please submit the patches to usptream & comment on status. > https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment fixed: only one (Fedora specific) patch remains > 7.) Latest upstream release > > Latest release seems to be 2008-11-04. Please update. fixed: updated > 8.) Avoid absolute symlinks > > ln -sf /bin/ksh $RPM_BUILD_ROOT/usr/bin/ksh could you please point me to some packaging guidelines or something about why should I avoid them? ln -sf ../../bin/ksh seems to me pretty ugly to do it without knowing the reason. thanks
also this is fixed: # rpmlint x86_64/ksh-20081104-1.fc11.x86_64.rpm ksh.x86_64: W: dangerous-command-in-%postun mv
(In reply to comment #2) Thanks for quick reply and addressing the problems! /me is much impressed > > 8.) Avoid absolute symlinks > > > > ln -sf /bin/ksh $RPM_BUILD_ROOT/usr/bin/ksh > > could you please point me to some packaging guidelines or something about why > should I avoid them? ln -sf ../../bin/ksh seems to me pretty ugly to do it > without knowing the reason. thanks Apart from that I don't see what's ugly about "ln -sf ../../bin/ksh", you're probably right that the guidelines don't mandate this. At least I could not find anything. Only piece of information I was able to dig was a rpmlint warning, which is pretty non-specific: "Absolute symlinks are problematic eg. when working with chroot environments." I can't think of a real world-scenario where this would cause serious trouble. (Not considering "Imagine I have a complete tree somewhere and I do `something' to the linked file, such as write to it (there actually are tools that write to binaries, such as prelink or execstack)"). So, while I strongly urge you to get rid of that absolute symlink, it seems not to be mandatory. By the way -- how about removing it? What's ksh in /usr/bin good for? We're probably early enough in rawhide to have enough time to test if it breaks something.
> "Absolute symlinks are problematic eg. when working with chroot environments." yes, but because only common directory for /bin/ksh and /usr/bin/ksh is / directory, so chroot will never work here even with relative path I think there is no package that require 'ksh' at all, so yes... I'll remove this link.