[PATCH] hush: Fix assignments being misinterpreted as commands

[Sean Anderson]

On 3/2/21 8:34 AM, Tom Rini wrote:
> On Tue, Mar 02, 2021 at 08:24:20AM -0500, Sean Anderson wrote:
>> 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.
> 
> OK.  But to what end?  Historically we have a buggy but mostly
> compatible "hush" that acts like "sh" does.  A more flexible shell could
> solve a lot of different use cases including making boot scripts that
> people end up writing being clearer and easier to write/debug/maintain.
> What I worry about here is making our shell not act like any regular
> shell people use.
> 

Good news: this patch brings hush *more* in-line with other shells.

dash:
	$ foo=bar echo qux
	qux

sh:
	sh-5.1$ foo=bar echo qux
	qux

bash:
	$ foo=bar echo qux
	qux

U-Boot without this patch:
	=> foo=bar echo qux
	Unknown command 'foo=bar' - try 'help'

U-Boot with this patch:
	=> foo=bar echo qux
	qux

--Sean