[PATCH] hush: Fix assignments being misinterpreted as commands

[Sean Anderson <seanga2@gmail.com>]

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@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'

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.

--Sean