Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
Hi, On Tue, 26 Oct 2021 at 04:16, Wolfgang Denk wrote: > > Dear Simon, > > In message > you > wrote: > > > > > > 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 .=3D operator I also suggested. > > > > Do you have a link to the docs for that? > > > > Perhaps we should get this initial thing in and we can take it from > > there. I expect that as we start to convert more environments we'll > > find more things we need. > > I think it is not a good idea to use two different operators for the > same appand operation, just to add a space in one case. > > So assume I want to start the appended part with a TAB character, > would we define another operator then? > > We have problems with escaping characters for the variable _name_ > part, but not for the value. We can for example use standard shell > escape rules, like: > > foo += bar > foo += \ bar > foo += ' bar' > I don't mind either way and there is precedent for .= maybe in perl? Can't remember. But I think that change would be for user-friendliness, rather than a strict requirement, so if we have agreement on the series as is now, I say let's go ahead with that and refine it later. Patches welcome, as they say. After that we can discuss: - this idea to relax the whitespace rules - the idea of restricting env-var names to A-Za-z_0-9- or similar - whether we can write a tool to convert all the envs automatically - if not, what to do to encourage people to migrate so we can drop env from the ad-hoc CONFIG thing Regards, Simon
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
Dear Simon, In message you wrote: > > > > 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 .=3D operator I also suggested. > > Do you have a link to the docs for that? > > Perhaps we should get this initial thing in and we can take it from > there. I expect that as we start to convert more environments we'll > find more things we need. I think it is not a good idea to use two different operators for the same appand operation, just to add a space in one case. So assume I want to start the appended part with a TAB character, would we define another operator then? We have problems with escaping characters for the variable _name_ part, but not for the value. We can for example use standard shell escape rules, like: foo += bar foo += \ bar foo += ' bar' Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de "There is no statute of limitations on stupidity." - Randomly produced by a computer program called Markov3.
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
On 25/10/2021 17.18, Simon Glass wrote: > Hi Rasmus, > > On Mon, 25 Oct 2021 at 01:06, Rasmus Villemoes > wrote: >> 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. > > Do you have a link to the docs for that? > https://docs.yoctoproject.org/bitbake/bitbake-user-manual/bitbake-user-manual-metadata.html#appending-and-prepending-with-spaces As I said, it will be familiar to anyone who has ever dealt with Yocto/bitbake, and the behaviour of += (of adding the RHS with a space prepended) is known from Make. I think there are uses for both, and it shouldn't be very hard to support both operators. 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. 0x1234#"foo", and while one can do >> de-quotification in U-Boot shell, it's rather cumbersome. > > Badly. I really don't like the stringify stuff so we don't have that > problem, but we have this one. > > I would much prefer it be automatic, if possible. This needs some > thought...e.g. I wonder what the default behaviour should be? I think it should be the value without quotes, if that is at all possible. But, it's not completely trivial, because cpp won't do macro expansion inside what it sees as a string literal, so if one wants a CONFIG string inside double quotes, one can't do foo = "CONFIG_FOO" Dunno, I think it needs some more thought. Rasmus
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
Hi Rasmus, On Mon, 25 Oct 2021 at 01:06, Rasmus Villemoes wrote: > > On 24/10/2021 21.54, Simon Glass wrote: > > Hi Rasmus, > > > > On Fri, 22 Oct 2021 at 00:41, Rasmus Villemoes > > 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 wrote: > > > > On Thu, 21 Oct 2021 15:25:37 +0200 > > Marek Behún wrote: > > > >> Hello, > >> > >> On Thu, 21 Oct 2021 15:06:51 +0200 > >> Wolfgang Denk 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. Do you have a link to the docs for that? Perhaps we should get
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
Dear Tom, In message <20211024164404.GQ3577824@bill-the-cat> you wrote: > > > It is a convenience tool, and it is OK if it has a few restrictions, > > like for the character set of supported variable names. > > > > But: > > > > 1) These restrictions must be clearly documented, both in the commit > >message and in the related documentation/readme. > > 2) There should be another, more primitive way to generate > >environment settings without these restrictions.. > > First, in that we don't have tests today for any of the "interesting" > possible variable options, I have no clue which ones even work as > intended. > > Second, yes, an end result here should be that yes, the default > environment should be more easily buildable and integrated with > arbitrary tools, so if something else can parse it (libubootenv?) it can > be done. Actually I have an even more low-level approach in mind, like the capability to include (or rather import) binary U-Boot environment data given in the usual =\0...=\0\0 form. This might come in handy if your data comes from exporting the environmentof a running system. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de An expert is a person who avoids the small errors while sweeping on to the grand fallacy.
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
On 24/10/2021 21.54, Simon Glass wrote: > Hi Rasmus, > > On Fri, 22 Oct 2021 at 00:41, Rasmus Villemoes > 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 wrote: > > On Thu, 21 Oct 2021 15:25:37 +0200 > Marek Behún wrote: > >> Hello, >> >> On Thu, 21 Oct 2021 15:06:51 +0200 >> Wolfgang Denk 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
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
Hi Rasmus, On Fri, 22 Oct 2021 at 00:41, Rasmus Villemoes 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 wrote: > >>> > >>> On Thu, 21 Oct 2021 15:25:37 +0200 > >>> Marek Behún wrote: > >>> > Hello, > > On Thu, 21 Oct 2021 15:06:51 +0200 > Wolfgang Denk 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! But how do I do this? bootargs=console=fred #ifdef SOMETHING bootargs+= dm-verity=... #endif We need the space between the bootargs. Regards, Simon
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
On Sun, Oct 24, 2021 at 05:46:00PM +0200, Wolfgang Denk wrote: > Dear Tom, > > In message <20211022144759.GG3577824@bill-the-cat> you wrote: > > > > > Any escape character is also a legal name character. > > > > I am struggling to have a non-meme reaction to this. Perhaps the best > > step is just earlier on in the series note that variable names need to > > fit within the broadly and commonly used set of characters and assorted > > funny business you can do historically needs to be migrated. > > Indeed I think this is the most reasonable approach. > > Like you cannot write any aritrary code in plain C and have to fall > back to assembler in a few places, this patch series should simply > not claim to be able to support all legal environment settings. > > It is a convenience tool, and it is OK if it has a few restrictions, > like for the character set of supported variable names. > > But: > > 1) These restrictions must be clearly documented, both in the commit >message and in the related documentation/readme. > 2) There should be another, more primitive way to generate >environment settings without these restrictions.. First, in that we don't have tests today for any of the "interesting" possible variable options, I have no clue which ones even work as intended. Second, yes, an end result here should be that yes, the default environment should be more easily buildable and integrated with arbitrary tools, so if something else can parse it (libubootenv?) it can be done. -- Tom signature.asc Description: PGP signature
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
Dear Tom, In message <20211022144759.GG3577824@bill-the-cat> you wrote: > > > Any escape character is also a legal name character. > > I am struggling to have a non-meme reaction to this. Perhaps the best > step is just earlier on in the series note that variable names need to > fit within the broadly and commonly used set of characters and assorted > funny business you can do historically needs to be migrated. Indeed I think this is the most reasonable approach. Like you cannot write any aritrary code in plain C and have to fall back to assembler in a few places, this patch series should simply not claim to be able to support all legal environment settings. It is a convenience tool, and it is OK if it has a few restrictions, like for the character set of supported variable names. But: 1) These restrictions must be clearly documented, both in the commit message and in the related documentation/readme. 2) There should be another, more primitive way to generate environment settings without these restrictions.. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Whom the gods would destroy, they first teach BASIC.
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
On Fri, Oct 22, 2021 at 10:06:55AM +0200, Wolfgang Denk wrote: > Dear Simon, > > In message > you > wrote: > > > > > > 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. > > This might seem to be a harmless change, but it is actually a > fundamental one. And it breaks backward compatiility. And all this > without need, as a list of alternatives have been suggested. > > > My distance second preference is what Marek has here, using a > > backslash to escape the + character. > > Actually this has the same problem, as the backslash is also a legal > character in a variable name: > > => setenv foo\\+ bar > => printenv foo\\+ > foo\+=bar > > > Yes, it was probably not a good idea not to restrict the allowed > character set when I implemented this stuff 21 years ago, but then > code size was critical - we had U-Boot running from 128 kB EPROM > (you remember these huge chips which were erased under UV light?). > > The fact is, '=' and NUL are the only characters that cannot be used > in a variable name. > > > > As for =+ ...while I can see how people might parse it (we are setting > > the var equal to what it has with an appending string) I think it is a > > terrible idea as it is just not what people expect. > > What do people expect? This is a totally new feature, so people will > use what they find in the documentation and in example code. > > > Also, putting the > > + after the = places (similarly unlikely) restrictions on the > > expression. > > There is a fundamental difference here. > > For the '+=' case, there is no way to escape the '+', as all > commonly used escapes are valid characters in the variable name, > too. > > With '=+', the '=' defines where the variable name ends, and from > here you can define your own rules as where the value part begins - > this is just a matter how you implement your parser. > > > The current format is basically the same as 'print'. So if I can't > > have the first preference, we could ensure that it prints a \ in the > > case that the var ends with + > > But '\' is a legal character in the variable name, too. Anything but > '=' and NUL is a legal char. And this makes escaping impossible: > > => setenv \'foo\\-\' foobar > => printenv \'foo\\-\' > 'foo\-'=foobar > > > > Also, I think that it would be better if spaces and tabs were allowed > > > to indent the .env file, i.e. > > > > > > var_a = 3 > > > var_bcd = 7 > > > > > > should set "var_a" to "3", "var_bcd" to "7". > > > > > > If special character are needed in either name or value, they could be > > > escaped and/or quoted. > > > > They are allowed in the value but are reduced to a single space in the > > front. We need this for multi-line strings (but I'm a bit worried > > about it). > > You mean this automatically insert a newline between parts? ugh... > I didn't realize this. Did I miss it in the documentation? > > > We could update it to skip any leading space after the = I think. > > So what if you need a leading space? > > > > I don't like spaces before the = though. It doesn't match the 'print' > > output (which has no space) and it is confusing: > > env print also does not add any spaces after the '='. > > > I think we need strict rules so it is easy for people to get exactly > > the env they want. > > Strict rules, proper documentation, and a set of examples. And sanity and restrictions introduced to our environment variables. The amount of "fun" things that were allowed by disallowing only NUL and = from names, and also allowing us to stay crazy tiny are just not relevant to where we are now. -- Tom signature.asc Description: PGP signature
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
On Fri, Oct 22, 2021 at 10:08:05AM +0200, Wolfgang Denk wrote: > Dear Tom, > > In message <20211021160311.GC3577824@bill-the-cat> you wrote: > > > > 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. > > Any escape character is also a legal name character. I am struggling to have a non-meme reaction to this. Perhaps the best step is just earlier on in the series note that variable names need to fit within the broadly and commonly used set of characters and assorted funny business you can do historically needs to be migrated. -- Tom signature.asc Description: PGP signature
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
Hello Wolfgang, On Fri, 22 Oct 2021 10:06:55 +0200 Wolfgang Denk wrote: > For the '+=' case, there is no way to escape the '+', as all > commonly used escapes are valid characters in the variable name, > too. We can define that backslash is to be also escaped if it is to be used as variable name: weird_var\\\+=abcd I still think it is far more intuitive than =+. Anyway, IMO '+' as the last character in varname is a extreme corner case; I think that no one sane actually uses it. But even if they did, Simon's patches do not break it. Simon's patches only disallow it in board environment definition during compilation. I think it is a completely reasonable thing to diallow board maintainers (i.e. only U-Boot developers, not users) from using such construction. Marek
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
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 wrote: >>> >>> On Thu, 21 Oct 2021 15:25:37 +0200 >>> Marek Behún wrote: >>> Hello, On Thu, 21 Oct 2021 15:06:51 +0200 Wolfgang Denk 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. Rasmus
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
Dear Tom, In message <20211021160311.GC3577824@bill-the-cat> you wrote: > > 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. Any escape character is also a legal name character. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Q: How many IBM CPU's does it take to execute a job? A: Four; three to hold it down, and one to rip its head off.
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
Dear Simon, In message you wrote: > > > > 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. This might seem to be a harmless change, but it is actually a fundamental one. And it breaks backward compatiility. And all this without need, as a list of alternatives have been suggested. > My distance second preference is what Marek has here, using a > backslash to escape the + character. Actually this has the same problem, as the backslash is also a legal character in a variable name: => setenv foo\\+ bar => printenv foo\\+ foo\+=bar Yes, it was probably not a good idea not to restrict the allowed character set when I implemented this stuff 21 years ago, but then code size was critical - we had U-Boot running from 128 kB EPROM (you remember these huge chips which were erased under UV light?). The fact is, '=' and NUL are the only characters that cannot be used in a variable name. > As for =+ ...while I can see how people might parse it (we are setting > the var equal to what it has with an appending string) I think it is a > terrible idea as it is just not what people expect. What do people expect? This is a totally new feature, so people will use what they find in the documentation and in example code. > Also, putting the > + after the = places (similarly unlikely) restrictions on the > expression. There is a fundamental difference here. For the '+=' case, there is no way to escape the '+', as all commonly used escapes are valid characters in the variable name, too. With '=+', the '=' defines where the variable name ends, and from here you can define your own rules as where the value part begins - this is just a matter how you implement your parser. > The current format is basically the same as 'print'. So if I can't > have the first preference, we could ensure that it prints a \ in the > case that the var ends with + But '\' is a legal character in the variable name, too. Anything but '=' and NUL is a legal char. And this makes escaping impossible: => setenv \'foo\\-\' foobar => printenv \'foo\\-\' 'foo\-'=foobar > > Also, I think that it would be better if spaces and tabs were allowed > > to indent the .env file, i.e. > > > > var_a = 3 > > var_bcd = 7 > > > > should set "var_a" to "3", "var_bcd" to "7". > > > > If special character are needed in either name or value, they could be > > escaped and/or quoted. > > They are allowed in the value but are reduced to a single space in the > front. We need this for multi-line strings (but I'm a bit worried > about it). You mean this automatically insert a newline between parts? ugh... I didn't realize this. Did I miss it in the documentation? > We could update it to skip any leading space after the = I think. So what if you need a leading space? > I don't like spaces before the = though. It doesn't match the 'print' > output (which has no space) and it is confusing: env print also does not add any spaces after the '='. > I think we need strict rules so it is easy for people to get exactly > the env they want. Strict rules, proper documentation, and a set of examples. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Any time things appear to be going better, you have overlooked some- thing.
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
Hi Tom, On Thu, 21 Oct 2021 at 10: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 wrote: > > > > > > On Thu, 21 Oct 2021 15:25:37 +0200 > > > Marek Behún wrote: > > > > > > > Hello, > > > > > > > > On Thu, 21 Oct 2021 15:06:51 +0200 > > > > Wolfgang Denk 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. I'm pretty sure we can do it easily enough. I'll take a look when I get back to this. Regards, Simon
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
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 wrote: > > > > On Thu, 21 Oct 2021 15:25:37 +0200 > > Marek Behún wrote: > > > > > Hello, > > > > > > On Thu, 21 Oct 2021 15:06:51 +0200 > > > Wolfgang Denk 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. -- Tom signature.asc Description: PGP signature
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
Hi Marek, On Thu, 21 Oct 2021 at 07:28, Marek Behún wrote: > > On Thu, 21 Oct 2021 15:25:37 +0200 > Marek Behún wrote: > > > Hello, > > > > On Thu, 21 Oct 2021 15:06:51 +0200 > > Wolfgang Denk 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. As for =+ ...while I can see how people might parse it (we are setting the var equal to what it has with an appending string) I think it is a terrible idea as it is just not what people expect. Also, putting the + after the = places (similarly unlikely) restrictions on the expression. The current format is basically the same as 'print'. So if I can't have the first preference, we could ensure that it prints a \ in the case that the var ends with + > > > > Marek > > Also, I think that it would be better if spaces and tabs were allowed > to indent the .env file, i.e. > > var_a = 3 > var_bcd = 7 > > should set "var_a" to "3", "var_bcd" to "7". > > If special character are needed in either name or value, they could be > escaped and/or quoted. They are allowed in the value but are reduced to a single space in the front. We need this for multi-line strings (but I'm a bit worried about it). We could update it to skip any leading space after the = I think. I don't like spaces before the = though. It doesn't match the 'print' output (which has no space) and it is confusing: fred=echo something; if [ a == b ]; then fi We may well have things like this if we try automatic conversion (I haven't looked though). I think we need strict rules so it is easy for people to get exactly the env they want. Regards, Simon
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
On Thu, Oct 21, 2021 at 05:12:26PM +0200, Wolfgang Denk wrote:
> Dear Marek,
>
> In message <20211021152831.15524883@thinkpad> you wrote:
> >
> >
> > > I think =+ will confuse far more people than + as last character of var
>
> I still fail to see why '=+' could be confusing if properly
> documented to be the append operator.
>
> I mean, it is not a new invention of mine.
>
> OpenEmbedded / Yocto uses '=+' a lot, like in
>
> meta/recipes-kernel/dtc/dtc.inc:
>
> PACKAGES =+ "${PN}-misc"
>
> Actually they use both '+=' and '=+', like
>
> RESULT+=${ERRORS}
The OE example is exactly why I want to avoid =+. You cannot
interchangeably use += and =+ as they evaluate differently. See
https://www.yoctoproject.org/docs/3.1/bitbake-user-manual/bitbake-user-manual.html#appending-and-prepending
and that =+ there is the opposite of what we want. So, more confusion.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
Dear Marek,
In message <20211021152831.15524883@thinkpad> you wrote:
>
>
> > I think =+ will confuse far more people than + as last character of var
I still fail to see why '=+' could be confusing if properly
documented to be the append operator.
I mean, it is not a new invention of mine.
OpenEmbedded / Yocto uses '=+' a lot, like in
meta/recipes-kernel/dtc/dtc.inc:
PACKAGES =+ "${PN}-misc"
Actually they use both '+=' and '=+', like
RESULT+=${ERRORS}
> > 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+"
Yes, this is yet another alternative for handling this problem.
> Also, I think that it would be better if spaces and tabs were allowed
> to indent the .env file, i.e.
>
> var_a = 3
> var_bcd = 7
>
> should set "var_a" to "3", "var_bcd" to "7".
Why not...
In the end it boils down that the file format should be properly
defined and have a clear syntax description. Apparently the
"=\0" format as used in U-Boot persistent storage
should not be taken literally here.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
If you're out of tree, you don't exist.
- David Woodhouse in <1304620350.2398.29.ca...@i7.infradead.org>
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
On Thu, 21 Oct 2021 15:25:37 +0200 Marek Behún wrote: > Hello, > > On Thu, 21 Oct 2021 15:06:51 +0200 > Wolfgang Denk 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+" > > Marek Also, I think that it would be better if spaces and tabs were allowed to indent the .env file, i.e. var_a = 3 var_bcd = 7 should set "var_a" to "3", "var_bcd" to "7". If special character are needed in either name or value, they could be escaped and/or quoted. Marek
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
Hello, On Thu, 21 Oct 2021 15:06:51 +0200 Wolfgang Denk 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+" Marek
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
Dear Tom, In message <20211021122325.GX7964@bill-the-cat> you wrote: > > Do you have any other feedback on the entire rest of the series? I already wrote that I support the concept, and the few nit I saw have been fixed, I think. Except this unneeded breaking of backward compatibility. > Because I'm not sure the benefit of "we can still support '+' at the end > of a variable name, if anyone uses that" outweighs "we can more easily > append variables in constructing our environment without relying on > uncommon operators". We introduce a new feature here. Defining an append operator is a convenience thing. It could probably also solved using the preprocessor, likely in a more ugly way. In any case, the feature is new, and the operator is new. For the implementation it does not matter if we define this operator as "+=" or '=+" or "=." or something else. the only difference is that any ioperator starting with an equal sign is inherently backward compatible without need for arbitrary new restrictions. > To me "=+" as the append syntax is worse than "no > + at the end of your variables". In which way is it worse? For esthetic reasons? I confirm that '+=' looks better. But '+=" is technically broken. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Brain: an apparatus with which we think we think.- Ambrose Bierce
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
On Thu, Oct 21, 2021 at 11:50:02AM +0200, Wolfgang Denk wrote: > Dear Simon, > > In message > <20211019164418.v9.3.Ie78bfbfca0d01d9cba501e127f446ec48e1f7afe@changeid> you > wrote: > > At present U-Boot environment variables, and thus scripts, are defined > > by CONFIG_EXTRA_ENV_SETTINGS. It is painful to add large amounts of text > > to this file and dealing with quoting and newlines is harder than it > > should be. It would be better if we could just type the script into a > > text file and have it included by U-Boot. > > > > Add a feature that brings in a .env file associated with the board > > config, if present. To use it, create a file in a board/ > > directory, typically called .env and controlled by the > > CONFIG_ENV_SOURCE_FILE option. > > > > The environment variables should be of the form "var=value". Values can > > extend to multiple lines. See the README under 'Environment Variables:' > > for more information and an example. Note that variables names may > > not end in + due to the += syntax below. > > I still object to placing new, arbitrary restrictions on what may or > may not be used in environment variable names. > > We have discussed alternative implementations, and trivial changes > like using "=+" instead of "+=" as append operator do not require > such restictions. > > Thus, and only for the restictions on variable names: > > Naked-by: Wolfgang Denk Do you have any other feedback on the entire rest of the series? Because I'm not sure the benefit of "we can still support '+' at the end of a variable name, if anyone uses that" outweighs "we can more easily append variables in constructing our environment without relying on uncommon operators". To me "=+" as the append syntax is worse than "no + at the end of your variables". -- Tom signature.asc Description: PGP signature
Re: [PATCH v9 3/7] env: Allow U-Boot scripts to be placed in a .env file
Dear Simon, In message <20211019164418.v9.3.Ie78bfbfca0d01d9cba501e127f446ec48e1f7afe@changeid> you wrote: > At present U-Boot environment variables, and thus scripts, are defined > by CONFIG_EXTRA_ENV_SETTINGS. It is painful to add large amounts of text > to this file and dealing with quoting and newlines is harder than it > should be. It would be better if we could just type the script into a > text file and have it included by U-Boot. > > Add a feature that brings in a .env file associated with the board > config, if present. To use it, create a file in a board/ > directory, typically called .env and controlled by the > CONFIG_ENV_SOURCE_FILE option. > > The environment variables should be of the form "var=value". Values can > extend to multiple lines. See the README under 'Environment Variables:' > for more information and an example. Note that variables names may > not end in + due to the += syntax below. I still object to placing new, arbitrary restrictions on what may or may not be used in environment variable names. We have discussed alternative implementations, and trivial changes like using "=+" instead of "+=" as append operator do not require such restictions. Thus, and only for the restictions on variable names: Naked-by: Wolfgang Denk Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de There is is no reason for any individual to have a computer in their home. -- Ken Olsen (President of Digital Equipment Corporation), Convention of the World Future Society, in Boston, 1977
