libubootenv: fw_setenv: how to build to support same script syntax used by fw_setenv built from a uboot?

[Richard Allen]

I like to know how to compile the libubootenv to use the same script syntax as is used by a fw_setenv built from uboot sources?

A fw_setenv from uboot has this syntax

Script Syntax:
  key [space] value
  lines starting with '#' are treated as comment

  A variable without value will be deleted. Any number of spaces are
  allowed between key and value. Space inside of the value is treated
  as part of the value itself.

Script Example:
  netdev         eth0
  kernel_addr    400000
  foo            empty empty empty    empty empty empty
  bar

but the libubootenv (fw_setenv) apparently wants an '=' 

Script Example:
  netdev = eth0
  kernel_addr =   400000
  foo  =         empty empty empty    empty empty empty
  bar =

This means a env-var script file can not be  used across machine which have different sources of fw_setenv.

Like to use the generic libubootenv, but this is a killer for me .

Thanks

Richard Allen

[Stefano Babic]

Hi Richard,

On 17/01/20 19:38, 'Richard Allen' via swupdate wrote:
> I like to know how to compile the libubootenv to use the same script
> syntax as is used by a fw_setenv built from uboot sources?
>

There is no option.

> A fw_setenv from uboot has this syntax
>
> Script Syntax:
>   key [space] value
>   lines starting with '#' are treated as comment
>   A variable without value will be deleted. Any number of spaces are
>   allowed between key and value. Space inside of the value is treated
>   as part of the value itself.
> Script Example:
>   netdev         eth0
>   kernel_addr    400000
>   foo            empty empty empty    empty empty empty
>   bar
>
> but the libubootenv (fw_setenv) apparently wants an '=' 

Right, this is the same for U-Boot handler in SWUpdate, too. The
difference is the delimiter (white space against '='). In U-Boot is
quite undefined, and it makes difficulties to assign leading spaces to a
variable.

>
> Script Example:
>   netdev = eth0
>   kernel_addr =   400000
>   foo  =         empty empty empty    empty empty empty
>   bar =

> **//___^

> This means a env-var script file can not be  used across machine which
> have different sources of fw_setenv.

Well, a conversion looks to me straightforward. It is easy to find which
fw_setenv is running, and a simple sed script converts from one to the
other one. Or a delimiter can be added to libubootenv, but it does not
change a lot, because fw_setenv should be called with an additional
parameter.

>
> Like to use the generic libubootenv, but this is a killer for me .
>

Best regards,
Stefano Babic



--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=====================================================================

[Richard Allen]

Thanks for the prompt reply
I reciently tested an fw_set/print from a 2018.03
If appears the fw_setenv currently supports using ether space or '=' after the first word (variable name )

-in script for fw_setenv -s
VAR=Value to set
VAR Value to set

That makes it more 'flexible'
Any reason the swupdate libubootenv can not /should not do the same when it is parsing a script file?

Richard Allen


-----Original Message-----
From: Stefano Babic <sba...@denx.de>
Sent: Sunday, January 19, 2020 6:36 AM
To: Richard Allen <richar...@keysight.com>; swupdate <swup...@googlegroups.com>
Subject: Re: [swupdate] libubootenv: fw_setenv: how to build to support same script syntax used by fw_setenv built from a uboot?

[EXTERNAL]

[Stefano Babic]

Hi Richard,

On 19/01/20 18:48, 'Richard Allen' via swupdate wrote:
> Thanks for the prompt reply
> I reciently tested an fw_set/print from a 2018.03
> If appears the fw_setenv currently supports using ether space or '=' after the first word (variable name )
>

It is not what I see from code, "=" should become part of the value.

> -in script for fw_setenv -s
> VAR=Value to set
> VAR Value to set
>
> That makes it more 'flexible'

Anyway, if you tested and you do not find any issue, it is already solved...

> Any reason the swupdate libubootenv can not /should not do the same when it is parsing a script file?

Current behavior is not wrong and more reliable (see example with value
with leading blanks). Behavior in U-Boot is weaker.

When you update the environment ? Is this not done in SWUpdate ?

Best regards,
Stefano Babic

[Richard Allen]

Swupdate is good for updating in field

But some cases, (especially during development ) swupdate is just providing the env-vars.script and a shell script
The shell is using fw_setenv to load the env-vars.scripts.

case occurs where the contents of the env-vars.script is changing , and editing sw-decription is an extra step
- with env-var.script , can test the results before having the extra steps of preparing a swu.

Once happy with env-var.script, can generate the swu (no editing/testing of a swdescription file)

I did find the uboot patch that added the '='
( so older u-boots ( which we are also supporting) will not support ether ' ' or '='

commit cd655514aa044c77fe3b895a51677ba47b26ba89
Author: Stefan Agner <stefan...@toradex.com>
Date: Thu Mar 1 14:06:32 2018 +0100

tools/env: allow equal sign as key value separation

Treat the first equal sign as a key/value separation too. This makes
the script files compatible with mkenvimage input file format. It
won't support variables with equal signs anymore, but this seems not
really like a loss.

Signed-off-by: Stefan Agner <stefan...@toradex.com>

Thanks

[Stefano Babic]

Hi Richard,

On 19/01/20 19:53, Richard Allen wrote:
> Swupdate is good for updating in field
>
> But some cases, (especially during development ) swupdate is just providing the env-vars.script and a shell script
> The shell is using fw_setenv to load the env-vars.scripts.
>
> case occurs where the contents of the env-vars.script is changing , and editing sw-decription is an extra step
> - with env-var.script , can test the results before having the extra steps of preparing a swu.
>
> Once happy with env-var.script, can generate the swu (no editing/testing of a swdescription file)
>
> I did find the uboot patch that added the '='
> ( so older u-boots ( which we are also supporting) will not support ether ' ' or '='
>
> commit cd655514aa044c77fe3b895a51677ba47b26ba89
> Author: Stefan Agner <stefan...@toradex.com>
> Date: Thu Mar 1 14:06:32 2018 +0100
>
> tools/env: allow equal sign as key value separation
>
> Treat the first equal sign as a key/value separation too. This makes
> the script files compatible with mkenvimage input file format. It
> won't support variables with equal signs anymore, but this seems not
> really like a loss.
>
> Signed-off-by: Stefan Agner <stefan...@toradex.com>
>

That's correct - this remember also the reason to use '=' in
libubootenv. It is then aligned to mkenvimage - the tools in U-Boot were
then the exception, Stefan fixed this.

Regards,
Stefano Babic

[Richard Allen]

Thanks , appreciate your time on this.

[Domenico Andreoli]

Hi Stefano,

On Sunday, January 19, 2020 at 2:35:52 PM UTC Stefano Babic wrote:

Hi Richard,

On 17/01/20 19:38, 'Richard Allen' via swupdate wrote:
> I like to know how to compile the libubootenv to use the same script
> syntax as is used by a fw_setenv built from uboot sources?
>

There is no option.

> A fw_setenv from uboot has this syntax
>
> Script Syntax:
>   key [space] value
>   lines starting with '#' are treated as comment
>   A variable without value will be deleted. Any number of spaces are
>   allowed between key and value. Space inside of the value is treated
>   as part of the value itself.
> Script Example:
>   netdev         eth0
>   kernel_addr    400000
>   foo            empty empty empty    empty empty empty
>   bar
>
> but the libubootenv (fw_setenv) apparently wants an '=' 

Right, this is the same for U-Boot handler in SWUpdate, too. The
difference is the delimiter (white space against '='). In U-Boot is
quite undefined, and it makes difficulties to assign leading spaces to a
variable.

Is it a priority to support spaces also in the variable names? If not then it would be

possible to detect the usage of the original u-boot syntax and at least warn the user.

The idea is that "name [space] value" in the original syntax would result in an empty

variable named "name [space] value" in the new one, something that we could consider

not a best practice.

Would you consider a patch for this?

[Stefano Babic]

Hi Domenico,

Spaces in variables is high discouraged and should not be done, but it
is working in U-Boot:

setenv alfa\ pluto
=> pri
alfa =pluto

> If not
> then it would be
> possible to detect the usage of the original u-boot syntax and at least
> warn the user.

libubootenv is a library, not a tool. It should then raise an error if
'=' as separator is not found.

>
> The idea is that "name [space] value" in the original syntax would
> result in an empty
> variable named "name [space] value" in the new one, something that we
> could consider
> not a best practice.
>
> Would you consider a patch for this?

I think that a rule is enough, having both is still more confusing.

Best regards,
Stefano Babic

>
>
> >
> > Script Example:
> >   netdev = eth0
> >   kernel_addr =   400000
> >   foo  =         empty empty empty    empty empty empty
> >   bar =
> > **//___^
> > This means a env-var script file can not be  used across machine
> which
> > have different sources of fw_setenv.
>
> Well, a conversion looks to me straightforward. It is easy to find
> which
> fw_setenv is running, and a simple sed script converts from one to the
> other one. Or a delimiter can be added to libubootenv, but it does not
> change a lot, because fw_setenv should be called with an additional
> parameter.
>
> >
> > Like to use the generic libubootenv, but this is a killer for me .
> >
>
> Best regards,
> Stefano Babic
>
>
>
> --
> =====================================================================
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

> Phone: +49-8142-66989-53 <tel:+49%208142%206698953> Fax:
> +49-8142-66989-80 <tel:+49%208142%206698980> Email: sba...@denx.de
> =====================================================================
>
> --
> You received this message because you are subscribed to the Google
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to swupdate+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/swupdate/219fb468-5a1d-4be5-a08c-30a250e669afn%40googlegroups.com
> <https://groups.google.com/d/msgid/swupdate/219fb468-5a1d-4be5-a08c-30a250e669afn%40googlegroups.com?utm_medium=email&utm_source=footer>.

[Domenico Andreoli]

On Mon, Nov 01, 2021 at 12:02:54PM +0100, Stefano Babic wrote:
> Hi Domenico,
>
> On 31.10.21 11:17, Domenico Andreoli wrote:
> > Hi Stefano,
> >

> > Is it a priority to support spaces also in the variable names?
>
> Spaces in variables is high discouraged and should not be done, but it is
> working in U-Boot:
>
> setenv alfa\ pluto
> => pri
> alfa =pluto
>
> > If not then it would be
> > possible to detect the usage of the original u-boot syntax and at least
> > warn the user.
>
> libubootenv is a library, not a tool. It should then raise an error if '='
> as separator is not found.

I'm not proposing to change anything in the library which, by the way,
does not seem to enforce the presence of the '=' (that also would be more
than enough to reject the legacy syntax).

My concern is purely fw_setenv tool eating the legacy syntax without a eye
blink and doing something different from what the user of such syntax
expects. Consider that U-Boot has a long history, fw_setenv has not
supported the '=' syntax for a very long time and many users would be
caught by surprise. I think they would be well served by an head-up sign.

So, given that spaces in the variable names are discouraged, it's maybe an
idea to take advantage of that and be able to detect the legacy syntax.

If you don't want to enforce a general "no spaces in the var names" (it
would catch both the discouraged behavior and the legacy syntax), the tool
could complain only if both the var name has spaces _and_ no '=' is found
on the line.

OTOH, if you think it's better to just check for the presence of '=',
that's absolutely fine with me!

> >
> > The idea is that "name [space] value" in the original syntax would
> > result in an empty
> > variable named "name [space] value" in the new one, something that we
> > could consider
> > not a best practice.
> >
> > Would you consider a patch for this?
>
> I think that a rule is enough, having both is still more confusing.

I value simplicity and linear behaviors, avoiding surprises and unexpected
side effects, reducing confusion. Flexibility it's not selling itself and
not what I'm asking to add.

>
> Best regards,
> Stefano Babic

Cheers,
Domenico

--
rsa4096: 3B10 0CA1 8674 ACBA B4FE FCD2 CE5B CF17 9960 DE13
ed25519: FFB4 0CC3 7F2E 091D F7DA 356E CC79 2832 ED38 CB05

[Stefano Babic]

On 01.11.21 13:40, 'Domenico Andreoli' via swupdate wrote:
> On Mon, Nov 01, 2021 at 12:02:54PM +0100, Stefano Babic wrote:
>> Hi Domenico,
>>
>> On 31.10.21 11:17, Domenico Andreoli wrote:
>>> Hi Stefano,
>>>
>>> Is it a priority to support spaces also in the variable names?
>>
>> Spaces in variables is high discouraged and should not be done, but it is
>> working in U-Boot:
>>
>> setenv alfa\ pluto
>> => pri
>> alfa =pluto
>>
>>> If not then it would be
>>> possible to detect the usage of the original u-boot syntax and at least
>>> warn the user.
>>
>> libubootenv is a library, not a tool. It should then raise an error if '='
>> as separator is not found.
>
> I'm not proposing to change anything in the library which, by the way,
> does not seem to enforce the presence of the '=' (that also would be more
> than enough to reject the legacy syntax).

It just skips lines when separator is not found.

>
> My concern is purely fw_setenv tool eating

The interface for a script is:

libuboot_load_file(ctx, scriptfile);

Parsing is part of the library and should not be moved outside it.

>the legacy syntax without a eye
> blink and doing something different from what the user of such syntax
> expects. Consider that U-Boot has a long history,

That is correct, but there is also a lot of big changes in ölast U-Boot
versions, such as switching to DM and DT. It is

> fw_setenv has not
> supported the '=' syntax for a very long time and many users would be
> caught by surprise.

Introducing "=" in U-Boot was done quite at the same time when
libubootenv was introduced.

> I think they would be well served by an head-up sign.
>
> So, given that spaces in the variable names are discouraged,

Discouraged but rather not forbidden, and if this should be, it should
be first done in U-Boot (but then, this becomes valid just for newer
U-Boot versions).

> it's maybe an
> idea to take advantage of that and be able to detect the legacy syntax.
>
> If you don't want to enforce a general "no spaces in the var names" (it
> would catch both the discouraged behavior and the legacy syntax), the tool
> could complain only if both the var name has spaces _and_ no '=' is found
> on the line.

It is not enough because '=' can be part of the script itself.

>
> OTOH, if you think it's better to just check for the presence of '=',
> that's absolutely fine with me!

At the moment, I think it is enough to document this without canging the
behavior.

>
>>>
>>> The idea is that "name [space] value" in the original syntax would
>>> result in an empty
>>> variable named "name [space] value" in the new one, something that we
>>> could consider
>>> not a best practice.
>>>
>>> Would you consider a patch for this?
>>
>> I think that a rule is enough, having both is still more confusing.
>
> I value simplicity and linear behaviors, avoiding surprises and unexpected
> side effects, reducing confusion. Flexibility it's not selling itself and
> not what I'm asking to add.
>

Best regards,
Stefano Babic

[Domenico Andreoli]

> That is correct, but there is also a lot of big changes in ??last U-Boot

> versions, such as switching to DM and DT. It is
>
> > fw_setenv has not
> > supported the '=' syntax for a very long time and many users would be
> > caught by surprise.
>
> Introducing "=" in U-Boot was done quite at the same time when libubootenv
> was introduced.
>
> > I think they would be well served by an head-up sign.
> >
> > So, given that spaces in the variable names are discouraged,
>
> Discouraged but rather not forbidden, and if this should be, it should be
> first done in U-Boot (but then, this becomes valid just for newer U-Boot
> versions).
>
> > it's maybe an
> > idea to take advantage of that and be able to detect the legacy syntax.
> >
> > If you don't want to enforce a general "no spaces in the var names" (it
> > would catch both the discouraged behavior and the legacy syntax), the tool
> > could complain only if both the var name has spaces _and_ no '=' is found
> > on the line.
>
> It is not enough because '=' can be part of the script itself.
>
> >
> > OTOH, if you think it's better to just check for the presence of '=',
> > that's absolutely fine with me!
>
> At the moment, I think it is enough to document this without canging the
> behavior.
>

I think that simply fixing the documentation is not enough, your (valued)
users deserve a proactive hint.

That said, I did my part. Hope you'll reconsider your position.

Regards,