Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 225977 - Merge Review: ksh
Summary: Merge Review: ksh
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Lubomir Rintel
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 19:17 UTC by Nobody's working on this, feel free to take it
Modified: 2009-01-21 08:18 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-01-20 20:23:21 UTC
lkundrak: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 19:17:08 UTC
Fedora Merge Review: ksh

http://cvs.fedora.redhat.com/viewcvs/devel/ksh/
Initial Owner: karsten@redhat.com

Comment 1 Lubomir Rintel 2009-01-14 00:21:38 UTC
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

Comment 2 Michal Hlavinka 2009-01-20 12:07:55 UTC
(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

Comment 3 Michal Hlavinka 2009-01-20 12:32:20 UTC
also this is fixed:

# rpmlint x86_64/ksh-20081104-1.fc11.x86_64.rpm 

ksh.x86_64: W: dangerous-command-in-%postun mv

Comment 4 Lubomir Rintel 2009-01-20 20:23:21 UTC
(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.

Comment 5 Michal Hlavinka 2009-01-21 08:18:27 UTC
> "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.


Note You need to log in before you can comment on or make changes to this bug.