Horms <[EMAIL PROTECTED]> writes:

> On Wed, Jan 19, 2005 at 09:23:27AM +0100, Goswin von Brederlow wrote:
>> Horms <[EMAIL PROTECTED]> 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


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]

Reply via email to