Bug#291107: kernel-patch-debian-2.6.9: bashism in apply/debian file
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.
>>
>> MfG
>> Goswin
>
> --
> Horms
MfG
Goswin
Reply to: