[PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file

[Rasmus Villemoes]

On 24/10/2021 21.54, Simon Glass wrote:
> Hi Rasmus,
> 
> On Fri, 22 Oct 2021 at 00:41, Rasmus Villemoes
> <rasmus.villemoes at prevas.dk> wrote:
>>
>> On 21/10/2021 18.03, Tom Rini wrote:
>>> On Thu, Oct 21, 2021 at 09:59:38AM -0600, Simon Glass wrote:
>>>> Hi Marek,
>>>>
>>>> On Thu, 21 Oct 2021 at 07:28, Marek Behún <marek.behun at nic.cz> wrote:
>>>>>
>>>>> On Thu, 21 Oct 2021 15:25:37 +0200
>>>>> Marek Behún <marek.behun at nic.cz> wrote:
>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> On Thu, 21 Oct 2021 15:06:51 +0200
>>>>>> Wolfgang Denk <wd at denx.de> wrote:
>>>>>>
>>>>>>> I confirm that '+=' looks better.  But '+=" is technically broken.
>>>>>>
>>>>>> a bit of my opinion:
>>>>>> I think =+ will confuse far more people than + as last character of var
>>>>>> name working weirdly. But I also think that + should be supported as
>>>>>> last character. Therefore I propose backslash escaping in variable name,
>>>>>> i.e.
>>>>>>   var+=value
>>>>>> appends value to var, while
>>>>>>   var\+=value
>>>>>> sets variable with name "var+"
>>>>
>>>> My first preference is to disallow + at the end of an end var. Perhaps
>>>> we can start printing a warning if people do it, for a few releases.
>>>>
>>>> My distance second preference is what Marek has here, using a
>>>> backslash to escape the + character.
>>>
>>> How bad does it make the parser look if we allow trailing + in variable
>>> names, by escaping them?  It's seemingly the substantive objection at
>>> this point.
>>>
>>
>> So I don't understand all the bruhahaha around a + at the end of the
>> varname. Nobody suggests (that I have seen) changing anything about how
>> U-Boot at runtime interprets and handles variables, so all variable
>> names that used to be valid continue to be so.
>>
>> It's just that this _new_ method of generating the environment places
>> certain restrictions on what can be done. The old-fashioned ways
>> (mkenvimage, good'ol CONFIG_ENV_EXTRA_SETTINGS with all its warts, and
>> run-time 'env set') continue to allow people to define whatever env vars
>> they want. This new method is meant for transitioning in-tree boards'
>> default environment, and no in-tree boards need anything exotic.
>>
>> Also, completely independent of whether the subsequent parser is
>> implemented in awk or python or rust, or what syntax it accepts and the
>> semantics of that syntax, the fact that we first pass the input through
>> cpp already means some things just won't work the same way as when given
>> to mkenvimage, and that applies to both sides of the =:
>>
>> printf 'x/* huh */y=/* where did this go ? */\nmsg=I like unix\nfive
>>  spaces=5spaces\n' | gcc -E -P -x assembler-with-cpp -
>>
>> x y=
>> msg=I like 1
>> five spaces=5spaces
>>
>> [In case its broken by the email, there are actually five spaces in the
>> printf string between the words "five" and "spaces", the above should
>> illustrate that cpp collapses those to a single space].
>>
>> So, I'd much rather we do a cleaner break, and accept (and ignore)
>> whitespace on either side of the assignment operator - that's how Make
>> variable assignments work IIRC. And since a lot of people making use of
>> this will already be familiar with Yocto, I think we should have two
>> compound assignment operators, .= and +=. That still allows one to use
>> any non-whitespace, non-= characters (modulo the restrictions imposed by
>> the whole thing passing through cpp) in variable names, so
>>
>> foo+=abc
>>
>> means
>>
>> foo+ = abc
>>
>> while could append to foo by saying
>>
>> foo += abc
>>
>> That whitespace around the assignment operators would also make the
>> input files somewhat more readable - there really is no point in having
>> human-readable, human-editable text files having a format
>> almost-but-not-quite be that of the on-disk format.
> 
> I am OK with this on the name front, as I assume we don't want to
> allow space in a var name!

Exactly, there's really never any case where that would be sensible. But
I would probably go a bit further and simply restrict varnames to the
usual alphanumerics plus [_.+-] - in particular, exclude single and
double quotes and backslash. That leaves the door open for somebody to
later support "arbitrary" variable names by defining what it would mean
to say e.g.

"abc \n'\"\tfoo" = hahaha

or whatever syntax they'd propose, but there's absolutely no point in
implementing anything like that initially.

> But how do I do this?
> 
> bootargs=console=fred
> #ifdef SOMETHING
> bootargs+= dm-verity=...
> #endif
> 
> We need the space between the bootargs.

Exactly like that, and for the case where you want to append something
_without_ an extra space there's the .= operator I also suggested.

FWIW, I like the idea that an indented line represents a continuation of
the value of the current assignment.

I don't think I've seen it addressed, but how do you deal with CONFIG_
string items? In the config.h file, the macro definition includes the
double quotes, but I don't think one wants that in the env values.

conf = CONFIG_DEFAULT_CONF
bootcmd = bootm $loadaddr#$conf

would attempt to boot e.g. 0x12340000#"foo", and while one can do
de-quotification in U-Boot shell, it's rather cumbersome.

Rasmus