Bug#291107: kernel-patch-debian-2.6.9: bashism in apply/debian file
reassign 291039 dash
reassign 291107 dash
thanks
There seems to be a problem with dash mistaking
mistaking $(( ... ) ... ) as $(( exp ))
--
Horms
On Wed, Jan 19, 2005 at 09:57:23AM +0100, Goswin von Brederlow wrote:
> Horms <horms@debian.org> writes:
>
> > 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
>
> $(...) works fine in dash and I see no difference in either line ($()
> or ``).
>
> >
> >> > @@ -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?
>
> I just tested it and dash does parse it as $(( ... )) and fails to
> find any )) token. $( ( fixes it.
>
> I guess it should be fixed in the kernel so it works now and cloned
> for dash unless you see a reason why $((...) | ... ) isn't legal
> POSIX.
>
> >> $( (cd $home/series/ && ls -d *) | sort -rnt- -k 2); should work too.
Reply to: