Bug#291107: kernel-patch-debian-2.6.9: bashism in apply/debian file
On Wed, Jan 19, 2005 at 09:23:27AM +0100, Goswin von Brederlow wrote:
> Horms <horms@debian.org> writes:
>
> > On Tue, Jan 18, 2005 at 08:37:21PM +0100, Roberto Suarez Soto wrote:
> >> Package: kernel-patch-debian-2.6.9
> >> Version: 2.6.9-5
> >> Severity: normal
> >>
> >> The file apply/debian (/usr/src/kernel-patches/all/2.6.9/apply/debian
> >> in my system) has a bashism in line 160:
> >>
> >> for base in $((cd $home/series/ && ls -d *) | sort -rnt- -k 2); do
> >>
> >> I have dash as /bin/sh. So, when I try to apply the patch with
> >> "make-kpkg --added-patches debian", it goes like this:
> >>
> >> /usr/src/kernel-patches/all/2.6.9/apply/debian: 160: Syntax error: Missing '))'
> >>
> >> I think the solution would be to change the "$(...)" stuff for a
> >> backquote block (i.e., "`...`") or to specify /bin/bash as the shell
> >> to use with this script. I've opted for the latter, but the former
> >> looks prettier :-)
> >
> > Wow, nobody notices this for months then two in one day.
> > I just made a fix for this and sent it to #291039. Could you
> > please test out the attached patch and see if it works for you.
> > I agree that this is not a good state for things to be in.
> >
> > --
> > Horms
> >
> > Index: apply
> > ===================================================================
> > --- apply (revision 2324)
> > +++ apply (working copy)
> > @@ -38,7 +38,7 @@
> > }
> >
> > apply_patch() {
> > - patch=$(find_patch $home/$1)
> > + patch=`find_patch $home/$1`
> > base=$1
> > if uncompress_patch "$patch" | patch -p1 -f -s -t --no-backup-if-mismatch; then
> > printf "%-${length}s\tOK (+)\n" "$base"
>
> Nothing wrong with $(). In fact many people prefer $().
I am one of those people. I just assumed dash didn't like it ias it is
the only thing suspicous I could see on line 160
> > @@ -139,8 +139,7 @@
> > die "Upstream $target_up doesn't match $upstream!"
> > # We don't have that version out yet!
> > elif [ ! -n "$target_rev" ] || ( [ "$target_rev" != "$target" ] && [ $target_rev -gt $revision ] ); then
> > - year=$(($(date +%Y) + 1))
> > - die "Can't patch to nonexistent revision $target_rev (wait until $year)"
> > + die "Can't patch to nonexistent revision $target_rev"
> > fi
> >
> > # At this point, we must handle three cases.
>
> $(( ... )) is a math expression and $() a subshell. Both look fine too
> me.
Yes, I understand that. But the code is bogus and I took
the chance to axe it.
> Use $((`date +%Y` + 1)) if you must.
Says he who just complained about using `` instead of $()
> > @@ -157,7 +156,7 @@
> > exit 0
> > fi
> >
> > - for base in $((cd $home/series/ && ls -d *) | sort -rnt- -k 2); do
> > + for base in `(cd $home/series/ && ls -d *) | sort -rnt- -k 2` do
> > srev=${base#*-}
> > if [ -n "$srev" ]; then
> > if [ $srev -le $current_rev ]; then
>
> Could that be a bug in dash for mistaking $(( ... ) ... ) as $(( exp
> )) construct?
That is a possibility to. If so its a dash bug and I guess we
don't need to change anything after all, just reassign the bug
to dash. Can someone confirm this?
> $( (cd $home/series/ && ls -d *) | sort -rnt- -k 2); should work too.
>
> MfG
> Goswin
--
Horms
Reply to: