[Bug 226483] Merge Review: tcsh

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=226483





--- Comment #7 from Vitezslav Crhonek <vcrhonek at redhat.com>  2009-04-28 11:39:18 EDT ---
Hi Jussi,

Thanks for adding me to the CC.

(In reply to comment #4)
> - The patches are not commented. Comments should be added why any specific
> patch is needed.

I think it's good idea with new patches, but I don't want comment them
retrospectively.

> 
> - A newer version 6.16 has been released in September.

Rebased.

> 
> - Requires(post): grep and Requires(postun): coreutils, grep are a bit silly,
> aren't these already required by some minimal system rpm?

Probably not (e. g. when you have busybox, then you don't need coreutils). We
should rewrite these scripts to RPM Lua maybe...:)

> 
> - Is the autoreconf really necessary?

Yes, it is. Because of using ltinfo instead of ltermcap. We need to refresh
configure.

> 
> - Drop the buildroot checks
> [ -n "$RPM_BUILD_ROOT" -a "$RPM_BUILD_ROOT" != / ] 
> in install and clean phase.

Fixed.

> 
> - Consider safening the %post and %postun phases with
> 
> %post
> if [ ! -f /etc/shells ]; then
>  echo "%{_bindir}/tcsh" >> /etc/shells
>  echo "%{_bindir}/csh"  >> /etc/shells
> else
>  grep -q '^%{_bindir}/tcsh$' /etc/shells || \
>  echo "%{_bindir}/tcsh" >> /etc/shells
>  grep -q '^%{_bindir}/csh$'  /etc/shells || \
>  echo "%{_bindir}/csh"  >> /etc/shells
> fi
> 
> %postun
> if [ ! -x %{_bindir}/tcsh ]; then
>  grep -v '^%{_bindir}/tcsh$'  /etc/shells | \
>  grep -v '^%{_bindir}/csh$' > /etc/shells.rpm &&
>  mv /etc/shells.rpm /etc/shells
> fi

Done.

> 
> 
> - Package does not handle locales in the right manner. Installing manually is
> OK, but after that use %{find_lang} to build the file list.  

See comment #2 from Miroslav, %{find_lang} doesn't work here. The file list
(tcsh.lang) is assembled in %install, locales manually installed here too and
finally packed in %files (taken from the file list).

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.