[PATCH] hush: Fix assignments being misinterpreted as commands

[Sean Anderson]

On 3/2/21 8:20 AM, Tom Rini wrote:
> On Mon, Mar 01, 2021 at 06:07:36PM -0500, Sean Anderson wrote:
>> On 3/1/21 1:26 PM, Heinrich Schuchardt wrote:
>>> On 3/1/21 3:17 PM, Tom Rini wrote:
>>>> On Sun, Feb 28, 2021 at 06:51:53PM -0500, Sean Anderson wrote:
>>>>> On 2/28/21 6:40 PM, Heinrich Schuchardt wrote:
>>>>>> Am 28. Februar 2021 22:29:51 MEZ schrieb Sean Anderson <seanga2 at gmail.com>:
>>>>>>> If there were no variable substitutions in a command, then initial
>>>>>>> assignments would be misinterpreted as commands, instead of being
>>>>>>> skipped
>>>>>>> over. This is demonstrated by the following example:
>>>>>>>
>>>>>>>      => foo=bar echo baz
>>>>>>
>>>>>> The commit message does not explain why this patch is needed.
>>>>>
>>>>> This is a bug I noticed while writing some tests of hush.
>>>>>
>>>>>> What shall be the value off foo after this line?
>>>>>
>>>>> It should be bar. This is an existing difference when compared with
>>>>> bash. For example, without this patch, we have
>>>>>
>>>>>      => foo=bar echo $foo
>>>>>      bar
>>>>>      => echo $foo
>>>>>      bar
>>>
>>> This seems really awkward. In bash I get:
>>>
>>> $ foo=bar ./test.sh
>>> bar
>>> $ echo $foo
>>>
>>> $
>>>
>>> Where test.sh
>>>
>>> #!/bin/sh
>>> echo $foo
>>>
>>> I did not expect an assignment made before a command to stick.
>>
>> Yeah, this is because hush does not have the concept of per-command
>> assignments (scope). So everything happens in the global scope.
>>
>>>>>
>>>>>>
>>>>>> What will be the output of
>>>>>>
>>>>>> foo=bar echo ${foo}
>>>>>>
>>>>>> with and without your patch?
>>>>>
>>>>> It is the same.
>>>
>>> Please, provide an example where the patch makes a difference.
>>>
>>> Best regards
>>>
>>> Heinrich
>>>
>>>>
>>>> bash works as you describe.  dash and busybox-sh both function like
>>>> this:
>>>> $ foo=bar echo $foo
>>>>
>>>> $ echo $foo
>>>>
>>>> $
>>>>
>>>> That we error out entirely is different from everyone.  Is that a good
>>>> thing?  Maybe.  I know I've caught myself making thinkos due to that
>>>> logic.  It does also violate the principal of least surprise, that we
>>>> don't act like anything else.  But I would suggest the behavior of
>>>> busybox-sh (what we forked long long ago) is what we should model here
>>>> rather than be more bash-like.  I'm not all that firm on this opinion
>>>> frankly, especially given the one-line nature of the change to bring us
>>>> that behavior and I assume dash/busybox are acting like pure sh would in
>>>> this case, which we aren't anyhow.
>>
>> Ok, I'd like to clear things up. Here is the current behavior of U-Boot:
>>
>>
>> 	=> foo=bar echo $foo
>> 	bar
>> 	=> echo $foo
>> 	bar
>> 	=> baz=bar echo qux
>> 	Unknown command 'baz=bar' - try 'help'
> 
> I see this as well (which isn't what I said in another part of this
> thread, so I re-checked just now).
> 
>> with this patch, this changes to
>>
>> 	=> foo=bar echo $foo
>> 	bar
>> 	=> echo $foo
>> 	bar
>> 	=> baz=bar echo qux
>> 	qux
>>
>> This patch *only* affects cases where there is an assignment at the
>> beginning of the line, but there is *no* variable reference in the
>> command. I know this is an edge case, but the current logic is clearly
>> wrong here.
> 
> OK.  But I don't see that behavior in bash 4.4.20 (Ubuntu 18.04).  Where
> does one get a shell that works like you're changing our hush to?
> 

I'm not changing hush to work like the first two examples, it's already like this.

Only the third example is affected by this patch.

--Sean