diff mbox series

[v10,3/9] env: Allow U-Boot scripts to be placed in a .env file

Message ID 20211021210847.v10.3.Ie78bfbfca0d01d9cba501e127f446ec48e1f7afe@changeid
State Accepted
Commit 86b9c3e4e48ba47ef28781d06b97846aca74bc8e
Delegated to: Tom Rini
Headers show
Series env: Allow environment in text files | expand

Commit Message

Simon Glass Oct. 22, 2021, 3:08 a.m. UTC
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/<vendor>
directory, typically called <board>.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.

In many cases environment variables need access to the U-Boot CONFIG
variables to select different options. Enable this so that the environment
scripts can be as useful as the ones currently in the board config files.
This uses the C preprocessor, means that comments can be included in the
environment using /* ... */

Also support += to allow variables to be appended to. This is needed when
using the preprocessor.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Marek Behún <marek.behun@nic.cz>
Tested-by: Marek Behún <marek.behun@nic.cz>
---

Changes in v10:
- Use backslash to allow assignment to a variable ending in +
- Add rST file into the index
- Minor tweaks to the script's pattern matching

Changes in v9:
- Drop mention of other strange characters
- Clarify that the + restriction is on the variable name not its value
- Add some tests for the script
- Deal with leading tabs
- Squash indentation down to one space
- Convert newlines within strings to spaces, which seems more consistent
- Handle appending an empty string to an empty var

Changes in v8:
- Update commit message to avoid mentioning the 'env' subdirectory
- Update commit message to mention the + restriction, etc.
- Overwrite the env file each time, to avoid incremental-build problems

Changes in v7:
- Use 'env' basename instead of 'environment' for intermediate output files
- Show a message indicating the source text file being used
- Give an error if CONFIG_EXTRA_ENV_SETTINGS is also defined
- Use CONFIG_ENV_SOURCE_FILE instead of rules to specify the text-file name
- Make board.env the default name if CONFIG_ENV_SOURCE_FILE is empty
- Rewrite the documentation
- Drop the use of common.env
- Update awk script to output the whole CONFIG string, or just a comment

Changes in v6:
- Combine the two env2string.awk patches into one

Changes in v5:
- Explain how to include the common.env file
- Explain why variables starting with _ , and / are not supported
- Expand the definition of how to declare an environment variable
- Explain what happens to empty variables
- Update maintainer
- Move use of += to this patch
- Explain that environment variables may not end in +

Changes in v4:
- Move this from being part of configuring U-Boot to part of building it
- Don't put the environment in autoconf.mk as it is not needed
- Add documentation in rST format instead of README
- Drop mention of import/export
- Update awk script to ignore blank lines, as generated by clang
- Add documentation in rST format instead of README

Changes in v3:
- Adjust Makefile to generate the .inc and .h files in separate fules
- Add more detail in the README about the format of .env files
- Improve the comment about " in the awk script
- Correctly terminate environment files with \n
- Define __UBOOT_CONFIG__ when collecting environment files

Changes in v2:
- Move .env file from include/configs to board/
- Use awk script to process environment since it is much easier on the brain
- Add information and updated example script to README
- Add dependency rule so that the environment is rebuilt when it changes
- Add separate patch to enable C preprocessor for environment files
- Enable var+=value form to simplify composing variables in multiple steps

 MAINTAINERS               |   7 +++
 Makefile                  |  66 ++++++++++++++++++++++-
 config.mk                 |   2 +
 doc/usage/environment.rst |  81 ++++++++++++++++++++++++++++-
 doc/usage/index.rst       |   1 +
 env/Kconfig               |  18 +++++++
 env/embedded.c            |   1 +
 include/env_default.h     |  11 ++++
 scripts/env2string.awk    |  80 ++++++++++++++++++++++++++++
 test/py/tests/test_env.py | 107 ++++++++++++++++++++++++++++++++++++++
 10 files changed, 372 insertions(+), 2 deletions(-)
 create mode 100644 scripts/env2string.awk

Comments

Wolfgang Denk Oct. 22, 2021, 8:29 a.m. UTC | #1
Dear Simon,

In message <20211021210847.v10.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/<vendor>
> directory, typically called <board>.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.

The README does not contain this information as it has been moved
into doc/usage/environment.rst

I think the documentation is lacking a hint that multiline
definitions will always be separated by spaces.

> Also support += to allow variables to be appended to. This is needed when
> using the preprocessor.

I cannot see what the preprocessor has to do with this feature.  It
would be useful in any case, even without the preporcessor.


The documentation reads:

"Variables can contain `+` characters but in the unlikely
event that you want to have a variable name ending in plus, put a backslash
before the `+` so that the script knows you are not adding to an existing
variable but assigning to a new one::
 
    maximum\+=value
"

However, '\' is also a legal character in a variable name (and
doubled backslashes or apostrophes etc. are legal, too), so
above line should actually set the environment variable "maximum\+"
to "value".

Best regards,

Wolfgang Denk
Tom Rini Oct. 22, 2021, 2:29 p.m. UTC | #2
On Fri, Oct 22, 2021 at 10:29:26AM +0200, Wolfgang Denk wrote:
> Dear Simon,
> 
> In message <20211021210847.v10.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/<vendor>
> > directory, typically called <board>.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.
> 
> The README does not contain this information as it has been moved
> into doc/usage/environment.rst
> 
> I think the documentation is lacking a hint that multiline
> definitions will always be separated by spaces.
> 
> > Also support += to allow variables to be appended to. This is needed when
> > using the preprocessor.
> 
> I cannot see what the preprocessor has to do with this feature.  It
> would be useful in any case, even without the preporcessor.
> 
> 
> The documentation reads:
> 
> "Variables can contain `+` characters but in the unlikely
> event that you want to have a variable name ending in plus, put a backslash
> before the `+` so that the script knows you are not adding to an existing
> variable but assigning to a new one::
>  
>     maximum\+=value
> "
> 
> However, '\' is also a legal character in a variable name (and
> doubled backslashes or apostrophes etc. are legal, too), so
> above line should actually set the environment variable "maximum\+"
> to "value".

I feel I should preface this with "I am cranky".  Now I see that I can
indeed do:
=> setenv foo\\bar baz
=> printenv foo\\bar
foo\bar=baz

on a system today.  To what value, I know not.  I can see how you write
some fancy state machine set of scripts where you end up with "foo+bar+"
needing to be set because you do a bunch of probing of things at run
time and concatenate and ... there, we've got a variable that ends in +
(and fwiw, sh says baz+ is a bad variable name, bash gets kinda funny
with it).  I don't see the value in allowing "\" in a variable name, nor
can I make bash nor sh handle that.   We can just disallow "\" in
variable names.
Tony Dinh Oct. 22, 2021, 11:29 p.m. UTC | #3
Hi all,

My 2 cents.

As a maintainer for some hobbyist downstream u-boots (tracking
mainline) and some out-of-tree u-boots, I do lots of envs scripting.
The most common scenario for me is to create an env by combining 2 or
more strings. Examples, init_usb, init_sata, init_sata_usb and so on,
i.e concatenating "init" with the device name(s). The device names
might be some other envs, and so on.

Note that I used _ (underscore) and could end up with an env name that
has trailing underscore. So I can see the trailing + in env names
might occur often in some existing scripts. It's just my preference to
use underscore, but someone else might use a + sign!

OTOH, It never occurred to me that I can use a backward slash \ in env
names. Since we use this escape character in
./include/configs/someboard.h CONFIG_EXTRA_ENV_SETTINGS as a
linebreak, I think perhaps nobody would have attempted to make use of
that in the env name.

So I agree with Tom, we could disallow \ in env name, and use it as a
separator during parsing. Such as in the following 2 assignments, the
1st is appending the string bar to foo, the 2nd is assigning the
string bar to foo+.

foo\+=bar
foo+\=bar

Thanks,
Tony
Wolfgang Denk Oct. 24, 2021, 3:41 p.m. UTC | #4
Dear Tom,

In message <20211022142912.GF3577824@bill-the-cat> you wrote:
> 
> > However, '\' is also a legal character in a variable name (and
> > doubled backslashes or apostrophes etc. are legal, too), so
> > above line should actually set the environment variable "maximum\+"
> > to "value".
>
> I feel I should preface this with "I am cranky".  Now I see that I can
> indeed do:
> => setenv foo\\bar baz
> => printenv foo\\bar
> foo\bar=baz
>
> on a system today.  To what value, I know not.

That's simple: historical reasons.

I already explained that: when I wrote the environment code, memory
was a very precious resource, so I implemented absolutely no
checking that could be avoided.  '=' was nevessary to separate name
from value, and NUL was necessary to terminate an entry. All other
characters where legal.

Yes, this can be misused to have all kinds of fun, like embedded
terminal control sequences or "invisible" variable names:

	=> setenv 'foo^H^H^H' bar
	=> printenv
	=bar
	arch=sandbox
	baudrate=115200
	...

You don't like it? Don't do it, then.

Yes, robust programming is something different, but at that time we
were fighting for 10 or 20 byte code size - there were so many
systems where U-Boot, Linux, and root file system had to fit into
4 MB flash or so.

Best regards,

Wolfgang Denk
Daniel Golle Nov. 12, 2021, 6:12 p.m. UTC | #5
On Thu, Oct 21, 2021 at 09:08:46PM -0600, Simon Glass 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/<vendor>
> directory, typically called <board>.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.
> 
> In many cases environment variables need access to the U-Boot CONFIG
> variables to select different options. Enable this so that the environment
> scripts can be as useful as the ones currently in the board config files.
> This uses the C preprocessor, means that comments can be included in the
> environment using /* ... */
> 
> Also support += to allow variables to be appended to. This is needed when
> using the preprocessor.

I hope to see this change moving forward!
It would be of great value for use in OpenWrt, as right now a per board
default environment is often included using CONFIG_ENV_SOURCE_FILE and
I was about to convert everything to C precompiler #defines to be more
flexible...
The suggested .env files made possible by this commit would provide an
ideal solution.


> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Marek Behún <marek.behun@nic.cz>
> Tested-by: Marek Behún <marek.behun@nic.cz>
> ---
> 
> Changes in v10:
> - Use backslash to allow assignment to a variable ending in +
> - Add rST file into the index
> - Minor tweaks to the script's pattern matching
> 
> Changes in v9:
> - Drop mention of other strange characters
> - Clarify that the + restriction is on the variable name not its value
> - Add some tests for the script
> - Deal with leading tabs
> - Squash indentation down to one space
> - Convert newlines within strings to spaces, which seems more consistent
> - Handle appending an empty string to an empty var
> 
> Changes in v8:
> - Update commit message to avoid mentioning the 'env' subdirectory
> - Update commit message to mention the + restriction, etc.
> - Overwrite the env file each time, to avoid incremental-build problems
> 
> Changes in v7:
> - Use 'env' basename instead of 'environment' for intermediate output files
> - Show a message indicating the source text file being used
> - Give an error if CONFIG_EXTRA_ENV_SETTINGS is also defined
> - Use CONFIG_ENV_SOURCE_FILE instead of rules to specify the text-file name
> - Make board.env the default name if CONFIG_ENV_SOURCE_FILE is empty
> - Rewrite the documentation
> - Drop the use of common.env
> - Update awk script to output the whole CONFIG string, or just a comment
> 
> Changes in v6:
> - Combine the two env2string.awk patches into one
> 
> Changes in v5:
> - Explain how to include the common.env file
> - Explain why variables starting with _ , and / are not supported
> - Expand the definition of how to declare an environment variable
> - Explain what happens to empty variables
> - Update maintainer
> - Move use of += to this patch
> - Explain that environment variables may not end in +
> 
> Changes in v4:
> - Move this from being part of configuring U-Boot to part of building it
> - Don't put the environment in autoconf.mk as it is not needed
> - Add documentation in rST format instead of README
> - Drop mention of import/export
> - Update awk script to ignore blank lines, as generated by clang
> - Add documentation in rST format instead of README
> 
> Changes in v3:
> - Adjust Makefile to generate the .inc and .h files in separate fules
> - Add more detail in the README about the format of .env files
> - Improve the comment about " in the awk script
> - Correctly terminate environment files with \n
> - Define __UBOOT_CONFIG__ when collecting environment files
> 
> Changes in v2:
> - Move .env file from include/configs to board/
> - Use awk script to process environment since it is much easier on the brain
> - Add information and updated example script to README
> - Add dependency rule so that the environment is rebuilt when it changes
> - Add separate patch to enable C preprocessor for environment files
> - Enable var+=value form to simplify composing variables in multiple steps
> 
>  MAINTAINERS               |   7 +++
>  Makefile                  |  66 ++++++++++++++++++++++-
>  config.mk                 |   2 +
>  doc/usage/environment.rst |  81 ++++++++++++++++++++++++++++-
>  doc/usage/index.rst       |   1 +
>  env/Kconfig               |  18 +++++++
>  env/embedded.c            |   1 +
>  include/env_default.h     |  11 ++++
>  scripts/env2string.awk    |  80 ++++++++++++++++++++++++++++
>  test/py/tests/test_env.py | 107 ++++++++++++++++++++++++++++++++++++++
>  10 files changed, 372 insertions(+), 2 deletions(-)
>  create mode 100644 scripts/env2string.awk
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8845c6fd750..8820a0f895e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -738,6 +738,13 @@ F:	test/env/
>  F:	tools/env*
>  F:	tools/mkenvimage.c
>  
> +ENVIRONMENT AS TEXT
> +M:	Simon Glass <sjg@chromium.org>
> +R:	Wolfgang Denk <wd@denx.de>
> +S:	Maintained
> +F:	doc/usage/environment.rst
> +F:	scripts/env2string.awk
> +
>  FPGA
>  M:	Michal Simek <michal.simek@xilinx.com>
>  S:	Maintained
> diff --git a/Makefile b/Makefile
> index 5194e4dc782..a80be71c78a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -517,6 +517,7 @@ version_h := include/generated/version_autogenerated.h
>  timestamp_h := include/generated/timestamp_autogenerated.h
>  defaultenv_h := include/generated/defaultenv_autogenerated.h
>  dt_h := include/generated/dt.h
> +env_h := include/generated/environment.h
>  
>  no-dot-config-targets := clean clobber mrproper distclean \
>  			 help %docs check% coccicheck \
> @@ -1786,6 +1787,69 @@ quiet_cmd_sym ?= SYM     $@
>  u-boot.sym: u-boot FORCE
>  	$(call if_changed,sym)
>  
> +# Environment processing
> +# ---------------------------------------------------------------------------
> +
> +# Directory where we expect the .env file, if it exists
> +ENV_DIR := $(srctree)/board/$(BOARDDIR)
> +
> +# Basename of .env file, stripping quotes
> +ENV_SOURCE_FILE := $(CONFIG_ENV_SOURCE_FILE:"%"=%)
> +
> +# Filename of .env file
> +ENV_FILE_CFG := $(ENV_DIR)/$(ENV_SOURCE_FILE).env
> +
> +# Default filename, if CONFIG_ENV_SOURCE_FILE is empty
> +ENV_FILE_BOARD := $(ENV_DIR)/$(CONFIG_SYS_BOARD:"%"=%).env
> +
> +# Select between the CONFIG_ENV_SOURCE_FILE and the default one
> +ENV_FILE := $(if $(ENV_SOURCE_FILE),$(ENV_FILE_CFG),$(wildcard $(ENV_FILE_BOARD)))
> +
> +# Run the environment text file through the preprocessor, but only if it is
> +# non-empty, to save time and possible build errors if something is wonky with
> +# the board
> +quiet_cmd_gen_envp = ENVP    $@
> +      cmd_gen_envp = \
> +	if [ -s "$(ENV_FILE)" ]; then \
> +		$(CPP) -P $(CFLAGS) -x assembler-with-cpp -D__ASSEMBLY__ \
> +			-D__UBOOT_CONFIG__ \
> +			-I . -I include -I $(srctree)/include \
> +			-include linux/kconfig.h -include include/config.h \
> +			-I$(srctree)/arch/$(ARCH)/include \
> +			$< -o $@; \
> +	else \
> +		echo -n >$@ ; \
> +	fi
> +include/generated/env.in: include/generated/env.txt FORCE
> +	$(call cmd,gen_envp)
> +
> +# Regenerate the environment if it changes
> +# We use 'wildcard' since the file is not required to exist (at present), in
> +# which case we don't want this dependency, but instead should create an empty
> +# file
> +# This rule is useful since it shows the source file for the environment
> +quiet_cmd_envc = ENVC    $@
> +      cmd_envc = \
> +	if [ -f "$<" ]; then \
> +		cat $< > $@; \
> +	elif [ -n "$(ENV_SOURCE_FILE)" ]; then \
> +		echo "Missing file $(ENV_FILE_CFG)"; \
> +	else \
> +		echo -n >$@ ; \
> +	fi
> +
> +include/generated/env.txt: $(wildcard $(ENV_FILE)) FORCE
> +	$(call cmd,envc)
> +
> +# Write out the resulting environment, converted to a C string
> +quiet_cmd_gen_envt = ENVT    $@
> +      cmd_gen_envt = \
> +	awk -f $(srctree)/scripts/env2string.awk $< >$@
> +$(env_h): include/generated/env.in
> +	$(call cmd,gen_envt)
> +
> +# ---------------------------------------------------------------------------
> +
>  # The actual objects are generated when descending,
>  # make sure no implicit rule kicks in
>  $(sort $(u-boot-init) $(u-boot-main)): $(u-boot-dirs) ;
> @@ -1841,7 +1905,7 @@ endif
>  # prepare2 creates a makefile if using a separate output directory
>  prepare2: prepare3 outputmakefile cfg
>  
> -prepare1: prepare2 $(version_h) $(timestamp_h) $(dt_h) \
> +prepare1: prepare2 $(version_h) $(timestamp_h) $(dt_h) $(env_h) \
>                     include/config/auto.conf
>  ifeq ($(wildcard $(LDSCRIPT)),)
>  	@echo >&2 "  Could not find linker script."
> diff --git a/config.mk b/config.mk
> index 7bb1fd4ed1b..2595aed218b 100644
> --- a/config.mk
> +++ b/config.mk
> @@ -50,8 +50,10 @@ endif
>  ifneq ($(BOARD),)
>  ifdef	VENDOR
>  BOARDDIR = $(VENDOR)/$(BOARD)
> +ENVDIR=${vendor}/env
>  else
>  BOARDDIR = $(BOARD)
> +ENVDIR=${board}/env
>  endif
>  endif
>  ifdef	BOARD
> diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
> index 7a733b44556..043c02d9a94 100644
> --- a/doc/usage/environment.rst
> +++ b/doc/usage/environment.rst
> @@ -15,7 +15,86 @@ environment is erased by accident, a default environment is provided.
>  
>  Some configuration options can be set using Environment Variables.
>  
> -List of environment variables (most likely not complete):
> +Text-based Environment
> +----------------------
> +
> +The default environment for a board is created using a `.env` environment file
> +using a simple text format. The base filename for this is defined by
> +`CONFIG_ENV_SOURCE_FILE`, or `CONFIG_SYS_BOARD` if that is empty.
> +
> +The file must be in the board directory and have a .env extension, so
> +assuming that there is a board vendor, the resulting filename is therefore::
> +
> +   board/<vendor>/<board>/<CONFIG_ENV_SOURCE_FILE>.env
> +
> +or::
> +
> +   board/<vendor>/<board>/<CONFIG_SYS_BOARD>.env
> +
> +This is a plain text file where you can type your environment variables in
> +the form `var=value`. Blank lines and multi-line variables are supported.
> +The conversion script looks for a line that starts in column 1 with a string
> +and has an equals sign immediately afterwards. Spaces before the = are not
> +permitted. It is a good idea to indent your scripts so that only the 'var='
> +appears at the start of a line.
> +
> +To add additional text to a variable you can use `var+=value`. This text is
> +merged into the variable during the make process and made available as a
> +single value to U-Boot. Variables can contain `+` characters but in the unlikely
> +event that you want to have a variable name ending in plus, put a backslash
> +before the `+` so that the script knows you are not adding to an existing
> +variable but assigning to a new one::
> +
> +    maximum\+=value
> +
> +This file can include C-style comments. Blank lines and multi-line
> +variables are supported, and you can use normal C preprocessor directives
> +and CONFIG defines from your board config also.
> +
> +For example, for snapper9260 you would create a text file called
> +`board/bluewater/snapper9260.env` containing the environment text.
> +
> +Example::
> +
> +    stdout=serial
> +    #ifdef CONFIG_LCD
> +    stdout+=,lcd
> +    #endif
> +    bootcmd=
> +        /* U-Boot script for booting */
> +
> +        if [ -z ${tftpserverip} ]; then
> +            echo "Use 'setenv tftpserverip a.b.c.d' to set IP address."
> +        fi
> +
> +        usb start; setenv autoload n; bootp;
> +        tftpboot ${tftpserverip}:
> +        bootm
> +    failed=
> +        /* Print a message when boot fails */
> +        echo CONFIG_SYS_BOARD boot failed - please check your image
> +        echo Load address is CONFIG_SYS_LOAD_ADDR
> +
> +If CONFIG_ENV_SOURCE_FILE is empty and the default filename is not present, then
> +the old-style C environment is used instead. See below.
> +
> +Old-style C environment
> +-----------------------
> +
> +Traditionally, the default environment is created in `include/env_default.h`,
> +and can be augmented by various `CONFIG` defines. See that file for details. In
> +particular you can define `CONFIG_EXTRA_ENV_SETTINGS` in your board file
> +to add environment variables.
> +
> +Board maintainers are encouraged to migrate to the text-based environment as it
> +is easier to maintain. The distro-board script still requires the old-style
> +environment but work is underway to address this.
> +
> +
> +List of environment variables
> +-----------------------------
> +
> +This is most-likely not complete:
>  
>  baudrate
>      see CONFIG_BAUDRATE
> diff --git a/doc/usage/index.rst b/doc/usage/index.rst
> index 356f2a56181..4314112ff34 100644
> --- a/doc/usage/index.rst
> +++ b/doc/usage/index.rst
> @@ -10,6 +10,7 @@ Use U-Boot
>     netconsole
>     partitions
>     cmdline
> +   environment
>  
>  Shell commands
>  --------------
> diff --git a/env/Kconfig b/env/Kconfig
> index f75f2b13536..b93ad5c8ee0 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -3,6 +3,24 @@ menu "Environment"
>  config ENV_SUPPORT
>  	def_bool y
>  
> +config ENV_SOURCE_FILE
> +	string "Environment file to use"
> +	default ""
> +	help
> +	  This sets the basename to use to generate the default environment.
> +	  This a text file as described in doc/usage/environment.rst
> +
> +	  The file must be in the board directory and have a .env extension, so
> +	  the resulting filename is typically
> +	  board/<vendor>/<board>/<CONFIG_ENV_SOURCE_FILE>.env
> +
> +	  If the file is not present, an error is produced.
> +
> +	  If this CONFIG is empty, U-Boot uses CONFIG SYS_BOARD as a default, if
> +	  the file board/<vendor>/<board>/<SYS_BOARD>.env exists. Otherwise the
> +	  environment is assumed to come from the ad-hoc
> +	  CONFIG_EXTRA_ENV_SETTINGS #define
> +
>  config SAVEENV
>  	def_bool y if CMD_SAVEENV
>  
> diff --git a/env/embedded.c b/env/embedded.c
> index 208553e6af1..9f26e6cad9c 100644
> --- a/env/embedded.c
> +++ b/env/embedded.c
> @@ -66,6 +66,7 @@
>  #endif
>  
>  #define DEFAULT_ENV_INSTANCE_EMBEDDED
> +#include <config.h>
>  #include <env_default.h>
>  
>  #ifdef CONFIG_ENV_ADDR_REDUND
> diff --git a/include/env_default.h b/include/env_default.h
> index 66e203eb6e4..c06506313e5 100644
> --- a/include/env_default.h
> +++ b/include/env_default.h
> @@ -10,6 +10,10 @@
>  #include <env_callback.h>
>  #include <linux/stringify.h>
>  
> +#ifndef USE_HOSTCC
> +#include <generated/environment.h>
> +#endif
> +
>  #ifdef DEFAULT_ENV_INSTANCE_EMBEDDED
>  env_t embedded_environment __UBOOT_ENV_SECTION__(environment) = {
>  	ENV_CRC,	/* CRC Sum */
> @@ -110,6 +114,13 @@ const uchar default_environment[] = {
>  #if defined(CONFIG_BOOTCOUNT_BOOTLIMIT) && (CONFIG_BOOTCOUNT_BOOTLIMIT > 0)
>  	"bootlimit="	__stringify(CONFIG_BOOTCOUNT_BOOTLIMIT)"\0"
>  #endif
> +#ifdef CONFIG_EXTRA_ENV_TEXT
> +# ifdef CONFIG_EXTRA_ENV_SETTINGS
> +# error "Your board uses a text-file environment, so must not define CONFIG_EXTRA_ENV_SETTINGS"
> +# endif
> +	/* This is created in the Makefile */
> +	CONFIG_EXTRA_ENV_TEXT
> +#endif
>  #ifdef	CONFIG_EXTRA_ENV_SETTINGS
>  	CONFIG_EXTRA_ENV_SETTINGS
>  #endif
> diff --git a/scripts/env2string.awk b/scripts/env2string.awk
> new file mode 100644
> index 00000000000..57d0fc8f3ba
> --- /dev/null
> +++ b/scripts/env2string.awk
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright 2021 Google, Inc
> +#
> +# SPDX-License-Identifier:	GPL-2.0+
> +#
> +# Awk script to parse a text file containing an environment and convert it
> +# to a C string which can be compiled into U-Boot.
> +
> +# The resulting output is:
> +#
> +#   #define CONFIG_EXTRA_ENV_TEXT "<environment here>"
> +#
> +# If the input is empty, this script outputs a comment instead.
> +
> +BEGIN {
> +	# env holds the env variable we are currently processing
> +	env = "";
> +	ORS = ""
> +}
> +
> +# Skip empty lines, as these are generated by the clang preprocessor
> +NF {
> +	# Quote quotes
> +	gsub("\"", "\\\"")
> +
> +	# Is this the start of a new environment variable?
> +	if (match($0, "^([^ \t=][^ =]*)=(.*)$", arr)) {
> +		if (length(env) != 0) {
> +			# Record the value of the variable now completed
> +			vars[var] = env
> +		}
> +		var = arr[1]
> +		env = arr[2]
> +
> +		# Deal with += which concatenates the new string to the existing
> +		# variable
> +		if (length(env) != 0 && match(var, "^(.*)[+]$", var_arr))
> +		{
> +			# Allow var\+=val to indicate that the variable name is
> +			# var+ and this is not actually a concatenation
> +			if (substr(var_arr[1], length(var_arr[1])) == "\\") {
> +				# Drop the backslash
> +				sub(/\\[+]$/, "+", var)
> +			} else {
> +				var = var_arr[1]
> +				env = vars[var] env
> +			}
> +		}
> +	} else {
> +		# Change newline to space
> +		gsub(/^[ \t]+/, "")
> +
> +		# Don't keep leading spaces generated by the previous blank line
> +		if (length(env) == 0) {
> +			env = $0
> +		} else {
> +			env = env " " $0
> +		}
> +	}
> +}
> +
> +END {
> +	# Record the value of the variable now completed. If the variable is
> +	# empty it is not set.
> +	if (length(env) != 0) {
> +		vars[var] = env
> +	}
> +
> +	if (length(vars) != 0) {
> +		printf("%s", "#define CONFIG_EXTRA_ENV_TEXT \"")
> +
> +		# Print out all the variables
> +		for (var in vars) {
> +			env = vars[var]
> +			print var "=" vars[var] "\\0"
> +		}
> +		print "\"\n"
> +	}
> +}
> diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
> index 9bed2f48d77..f85cb031382 100644
> --- a/test/py/tests/test_env.py
> +++ b/test/py/tests/test_env.py
> @@ -7,6 +7,7 @@
>  import os
>  import os.path
>  from subprocess import call, check_call, CalledProcessError
> +import tempfile
>  
>  import pytest
>  import u_boot_utils
> @@ -515,3 +516,109 @@ def test_env_ext4(state_test_env):
>      finally:
>          if fs_img:
>              call('rm -f %s' % fs_img, shell=True)
> +
> +def test_env_text(u_boot_console):
> +    """Test the script that converts the environment to a text file"""
> +
> +    def check_script(intext, expect_val):
> +        """Check a test case
> +
> +        Args:
> +            intext: Text to pass to the script
> +            expect_val: Expected value of the CONFIG_EXTRA_ENV_TEXT string, or
> +                None if we expect it not to be defined
> +        """
> +        with tempfile.TemporaryDirectory() as path:
> +            fname = os.path.join(path, 'infile')
> +            with open(fname, 'w') as inf:
> +                print(intext, file=inf)
> +            result = u_boot_utils.run_and_log(cons, ['awk', '-f', script, fname])
> +            if expect_val is not None:
> +                expect = '#define CONFIG_EXTRA_ENV_TEXT "%s"\n' % expect_val
> +                assert result == expect
> +            else:
> +                assert result == ''
> +
> +    cons = u_boot_console
> +    script = os.path.join(cons.config.source_dir, 'scripts', 'env2string.awk')
> +
> +    # simple script with a single var
> +    check_script('fred=123', 'fred=123\\0')
> +
> +    # no vars
> +    check_script('', None)
> +
> +    # two vars
> +    check_script('''fred=123
> +ernie=456''', 'fred=123\\0ernie=456\\0')
> +
> +    # blank lines
> +    check_script('''fred=123
> +
> +
> +ernie=456
> +
> +''', 'fred=123\\0ernie=456\\0')
> +
> +    # append
> +    check_script('''fred=123
> +ernie=456
> +fred+= 456''', 'fred=123 456\\0ernie=456\\0')
> +
> +    # append from empty
> +    check_script('''fred=
> +ernie=456
> +fred+= 456''', 'fred= 456\\0ernie=456\\0')
> +
> +    # variable with + in it
> +    check_script('fred+ernie=123', 'fred+ernie=123\\0')
> +
> +    # ignores variables that are empty
> +    check_script('''fred=
> +fred+=
> +ernie=456''', 'ernie=456\\0')
> +
> +    # single-character env name
> +    check_script('''f=123
> +e=456
> +f+= 456''', 'e=456\\0f=123 456\\0')
> +
> +    # contains quotes
> +    check_script('''fred="my var"
> +ernie=another"''', 'fred=\\"my var\\"\\0ernie=another\\"\\0')
> +
> +    # variable name ending in +
> +    check_script('''fred\\+=my var
> +fred++= again''', 'fred+=my var again\\0')
> +
> +    # variable name containing +
> +    check_script('''fred+jane=both
> +fred+jane+=again
> +ernie=456''', 'fred+jane=bothagain\\0ernie=456\\0')
> +
> +    # multi-line vars - new vars always start at column 1
> +    check_script('''fred=first
> + second
> +\tthird with tab
> +
> +   after blank
> + confusing=oops
> +ernie=another"''', 'fred=first second third with tab after blank confusing=oops\\0ernie=another\\"\\0')
> +
> +    # real-world example
> +    check_script('''ubifs_boot=
> +	env exists bootubipart ||
> +		env set bootubipart UBI;
> +	env exists bootubivol ||
> +		env set bootubivol boot;
> +	if ubi part ${bootubipart} &&
> +		ubifsmount ubi${devnum}:${bootubivol};
> +	then
> +		devtype=ubi;
> +		run scan_dev_for_boot;
> +	fi
> +''',
> +        'ubifs_boot=env exists bootubipart || env set bootubipart UBI; '
> +        'env exists bootubivol || env set bootubivol boot; '
> +        'if ubi part ${bootubipart} && ubifsmount ubi${devnum}:${bootubivol}; '
> +        'then devtype=ubi; run scan_dev_for_boot; fi\\0')
> -- 
> 2.33.0.1079.g6e70778dc9-goog
>
Simon Glass Nov. 13, 2021, 2:19 p.m. UTC | #6
Hi Daniel,

On Fri, 12 Nov 2021 at 11:13, Daniel Golle <daniel@makrotopia.org> wrote:
>
> On Thu, Oct 21, 2021 at 09:08:46PM -0600, Simon Glass 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/<vendor>
> > directory, typically called <board>.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.
> >
> > In many cases environment variables need access to the U-Boot CONFIG
> > variables to select different options. Enable this so that the environment
> > scripts can be as useful as the ones currently in the board config files.
> > This uses the C preprocessor, means that comments can be included in the
> > environment using /* ... */
> >
> > Also support += to allow variables to be appended to. This is needed when
> > using the preprocessor.
>
> I hope to see this change moving forward!
> It would be of great value for use in OpenWrt, as right now a per board
> default environment is often included using CONFIG_ENV_SOURCE_FILE and
> I was about to convert everything to C precompiler #defines to be more
> flexible...
> The suggested .env files made possible by this commit would provide an
> ideal solution.

OK, well let 's see what happens. It is ready to apply, I think. I am
sure some additional tweaks will be needed as we migrate things.

[..]

Regards,
Simon
Patrick Delaunay Feb. 10, 2022, 11:20 a.m. UTC | #7
Hi Simon,

On 10/22/21 05:08, Simon Glass 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/<vendor>
> directory, typically called <board>.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.
>
> In many cases environment variables need access to the U-Boot CONFIG
> variables to select different options. Enable this so that the environment
> scripts can be as useful as the ones currently in the board config files.
> This uses the C preprocessor, means that comments can be included in the
> environment using /* ... */
>
> Also support += to allow variables to be appended to. This is needed when
> using the preprocessor.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Marek Behún <marek.behun@nic.cz>
> Tested-by: Marek Behún <marek.behun@nic.cz>
> ---
...
>
>
>   MAINTAINERS               |   7 +++
>   Makefile                  |  66 ++++++++++++++++++++++-
>   config.mk                 |   2 +
>   doc/usage/environment.rst |  81 ++++++++++++++++++++++++++++-
>   doc/usage/index.rst       |   1 +
>   env/Kconfig               |  18 +++++++
>   env/embedded.c            |   1 +
>   include/env_default.h     |  11 ++++
>   scripts/env2string.awk    |  80 ++++++++++++++++++++++++++++
>   test/py/tests/test_env.py | 107 ++++++++++++++++++++++++++++++++++++++
>   10 files changed, 372 insertions(+), 2 deletions(-)
>   create mode 100644 scripts/env2string.awk
>

...


For information, it seems the new test "test_env_text" failed when the 
gawk is not installed on Ubuntu distribution,

or when /usr/bin/mawk is used as alternative (sudo update-alternatives 
--config awk)

The test result is

test/py/tests/test_env.py:556: in test_env_text
     check_script('''fred=123
test/py/tests/test_env.py:542: in check_script
     assert result == expect
E   assert '#define CONF...red=123\\0"\n' == '#define CONF...nie=456\\0"\n'
E     - #define CONFIG_EXTRA_ENV_TEXT "ernie=456\0fred=123\0"
E     ?                                           ----------
E     + #define CONFIG_EXTRA_ENV_TEXT "fred=123\0ernie=456\0"
E     ?                                ++++++++++
------------------------------------------------------------ Captured 
stdout call -------------------------------------------------------------
+awk -f <PATH>/u-boot/scripts/env2string.awk /tmp/tmp0zgiwrd9/infile
#define CONFIG_EXTRA_ENV_TEXT "fred=123\0"
+awk -f <PATH>/u-boot/scripts/env2string.awk /tmp/tmpq6xej0ct/infile
+awk -f <PATH>/u-boot/scripts/env2string.awk /tmp/tmpyrn10apn/infile
#define CONFIG_EXTRA_ENV_TEXT "ernie=456\0fred=123\0"


=> the env variables are sorted in alphabetic order in mawk output / in 
creation order in gawk ouput

I don't found solution to be POSIX compliant and to guarantee the output 
order


 >> By default, when a for loop traverses an array, the order is undefined,

 >> meaning that the awk implementation determines the order in which

 >> the array is traversed. This order is usually based on the internal

 >> implementation of arrays and will vary from one version of awk to 
the next.


References:

https://www.gnu.org/software/gawk/manual/html_node/Controlling-Scanning.html

https://www.gnu.org/software/gawk/manual/html_node/Controlling-Array-Traversal.html


> diff --git a/scripts/env2string.awk b/scripts/env2string.awk
> new file mode 100644
> index 00000000000..57d0fc8f3ba
> --- /dev/null
> +++ b/scripts/env2string.awk
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright 2021 Google, Inc
> +#
> +# SPDX-License-Identifier:	GPL-2.0+
> +#
> +# Awk script to parse a text file containing an environment and convert it
> +# to a C string which can be compiled into U-Boot.
> +
> +# The resulting output is:
> +#
> +#   #define CONFIG_EXTRA_ENV_TEXT "<environment here>"
> +#
> +# If the input is empty, this script outputs a comment instead.
> +
> +BEGIN {
> +	# env holds the env variable we are currently processing
> +	env = "";
> +	ORS = ""
> +}
> +


...


> +
> +END {
> +	# Record the value of the variable now completed. If the variable is
> +	# empty it is not set.
> +	if (length(env) != 0) {
> +		vars[var] = env
> +	}
> +
> +	if (length(vars) != 0) {
> +		printf("%s", "#define CONFIG_EXTRA_ENV_TEXT \"")
> +
> +		# Print out all the variables


ORDER is not guarantee here by awk for the loop on the "vars" array


> +		for (var in vars) {
> +			env = vars[var]
> +			print var "=" vars[var] "\\0"
> +		}
> +		print "\"\n"
> +	}
> +}
> diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
> index 9bed2f48d77..f85cb031382 100644
> --- a/test/py/tests/test_env.py
> +++ b/test/py/tests/test_env.py
> @@ -7,6 +7,7 @@
>   import os
>   import os.path
>   from subprocess import call, check_call, CalledProcessError
> +import tempfile
>   
>   import pytest
>   import u_boot_utils
> @@ -515,3 +516,109 @@ def test_env_ext4(state_test_env):
>       finally:
>           if fs_img:
>               call('rm -f %s' % fs_img, shell=True)
> +
> +def test_env_text(u_boot_console):
> +    """Test the script that converts the environment to a text file"""
> +
> +    def check_script(intext, expect_val):
> +        """Check a test case
> +
> +        Args:
> +            intext: Text to pass to the script
> +            expect_val: Expected value of the CONFIG_EXTRA_ENV_TEXT string, or
> +                None if we expect it not to be defined
> +        """
> +        with tempfile.TemporaryDirectory() as path:
> +            fname = os.path.join(path, 'infile')
> +            with open(fname, 'w') as inf:
> +                print(intext, file=inf)
> +            result = u_boot_utils.run_and_log(cons, ['awk', '-f', script, fname])
> +            if expect_val is not None:
> +                expect = '#define CONFIG_EXTRA_ENV_TEXT "%s"\n' % expect_val
> +                assert result == expect
> +            else:
> +                assert result == ''
> +
> +    cons = u_boot_console
> +    script = os.path.join(cons.config.source_dir, 'scripts', 'env2string.awk')
> +
> +    # simple script with a single var
> +    check_script('fred=123', 'fred=123\\0')
> +
> +    # no vars
> +    check_script('', None)
> +
> +    # two vars
> +    check_script('''fred=123
> +ernie=456''', 'fred=123\\0ernie=456\\0')

Here according the awk implementation  the test result can be

'fred=123\\0ernie=456\\0' => creation order

'ernie=456\\0fred=123\\0' => alphabetic order

and perhaps other order for other awk ?


do you think you can have a solution with POSIX awk ?
today I found only solution for gawk:


---------------------------- scripts/env2string.awk ----------------------------
index 1bfe9ed07a..f3215c369a 100644
@@ -81,7 +81,8 @@ END {
  	if (do_output) {
  		printf("%s", "#define CONFIG_EXTRA_ENV_TEXT \"")
  
-		# Print out all the variables
+		# Print out all the variables by alphabetic order
+		PROCINFO["sorted_in"] = "@ind_str_asc"
  		for (var in vars) {
  			env = vars[var]
  			print var "=" vars[var] "\\0"


But this GNU feature must be avoid,
see commit 7acb32256831 ("env: Avoid using GNU features in awk")

Regard

Patrick Delaunay
Simon Glass March 12, 2022, 6:14 p.m. UTC | #8
Hi Patrick,

On Thu, 10 Feb 2022 at 04:20, Patrick DELAUNAY
<patrick.delaunay@foss.st.com> wrote:
>
> Hi Simon,
>
> On 10/22/21 05:08, Simon Glass 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/<vendor>
> > directory, typically called <board>.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.
> >
> > In many cases environment variables need access to the U-Boot CONFIG
> > variables to select different options. Enable this so that the environment
> > scripts can be as useful as the ones currently in the board config files.
> > This uses the C preprocessor, means that comments can be included in the
> > environment using /* ... */
> >
> > Also support += to allow variables to be appended to. This is needed when
> > using the preprocessor.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > Reviewed-by: Marek Behún <marek.behun@nic.cz>
> > Tested-by: Marek Behún <marek.behun@nic.cz>
> > ---
> ...
> >
> >
> >   MAINTAINERS               |   7 +++
> >   Makefile                  |  66 ++++++++++++++++++++++-
> >   config.mk                 |   2 +
> >   doc/usage/environment.rst |  81 ++++++++++++++++++++++++++++-
> >   doc/usage/index.rst       |   1 +
> >   env/Kconfig               |  18 +++++++
> >   env/embedded.c            |   1 +
> >   include/env_default.h     |  11 ++++
> >   scripts/env2string.awk    |  80 ++++++++++++++++++++++++++++
> >   test/py/tests/test_env.py | 107 ++++++++++++++++++++++++++++++++++++++
> >   10 files changed, 372 insertions(+), 2 deletions(-)
> >   create mode 100644 scripts/env2string.awk
> >
>
> ...
>
>
> For information, it seems the new test "test_env_text" failed when the
> gawk is not installed on Ubuntu distribution,
>
> or when /usr/bin/mawk is used as alternative (sudo update-alternatives
> --config awk)
>
> The test result is
>
> test/py/tests/test_env.py:556: in test_env_text
>      check_script('''fred=123
> test/py/tests/test_env.py:542: in check_script
>      assert result == expect
> E   assert '#define CONF...red=123\\0"\n' == '#define CONF...nie=456\\0"\n'
> E     - #define CONFIG_EXTRA_ENV_TEXT "ernie=456\0fred=123\0"
> E     ?                                           ----------
> E     + #define CONFIG_EXTRA_ENV_TEXT "fred=123\0ernie=456\0"
> E     ?                                ++++++++++
> ------------------------------------------------------------ Captured
> stdout call -------------------------------------------------------------
> +awk -f <PATH>/u-boot/scripts/env2string.awk /tmp/tmp0zgiwrd9/infile
> #define CONFIG_EXTRA_ENV_TEXT "fred=123\0"
> +awk -f <PATH>/u-boot/scripts/env2string.awk /tmp/tmpq6xej0ct/infile
> +awk -f <PATH>/u-boot/scripts/env2string.awk /tmp/tmpyrn10apn/infile
> #define CONFIG_EXTRA_ENV_TEXT "ernie=456\0fred=123\0"
>
>
> => the env variables are sorted in alphabetic order in mawk output / in
> creation order in gawk ouput
>
> I don't found solution to be POSIX compliant and to guarantee the output
> order
>
>
>  >> By default, when a for loop traverses an array, the order is undefined,
>
>  >> meaning that the awk implementation determines the order in which
>
>  >> the array is traversed. This order is usually based on the internal
>
>  >> implementation of arrays and will vary from one version of awk to
> the next.
>
>
> References:
>
> https://www.gnu.org/software/gawk/manual/html_node/Controlling-Scanning.html
>
> https://www.gnu.org/software/gawk/manual/html_node/Controlling-Array-Traversal.html
>
>
> > diff --git a/scripts/env2string.awk b/scripts/env2string.awk
> > new file mode 100644
> > index 00000000000..57d0fc8f3ba
> > --- /dev/null
> > +++ b/scripts/env2string.awk
> > @@ -0,0 +1,80 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# Copyright 2021 Google, Inc
> > +#
> > +# SPDX-License-Identifier:   GPL-2.0+
> > +#
> > +# Awk script to parse a text file containing an environment and convert it
> > +# to a C string which can be compiled into U-Boot.
> > +
> > +# The resulting output is:
> > +#
> > +#   #define CONFIG_EXTRA_ENV_TEXT "<environment here>"
> > +#
> > +# If the input is empty, this script outputs a comment instead.
> > +
> > +BEGIN {
> > +     # env holds the env variable we are currently processing
> > +     env = "";
> > +     ORS = ""
> > +}
> > +
>
>
> ...
>
>
> > +
> > +END {
> > +     # Record the value of the variable now completed. If the variable is
> > +     # empty it is not set.
> > +     if (length(env) != 0) {
> > +             vars[var] = env
> > +     }
> > +
> > +     if (length(vars) != 0) {
> > +             printf("%s", "#define CONFIG_EXTRA_ENV_TEXT \"")
> > +
> > +             # Print out all the variables
>
>
> ORDER is not guarantee here by awk for the loop on the "vars" array
>
>
> > +             for (var in vars) {
> > +                     env = vars[var]
> > +                     print var "=" vars[var] "\\0"
> > +             }
> > +             print "\"\n"
> > +     }
> > +}
> > diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
> > index 9bed2f48d77..f85cb031382 100644
> > --- a/test/py/tests/test_env.py
> > +++ b/test/py/tests/test_env.py
> > @@ -7,6 +7,7 @@
> >   import os
> >   import os.path
> >   from subprocess import call, check_call, CalledProcessError
> > +import tempfile
> >
> >   import pytest
> >   import u_boot_utils
> > @@ -515,3 +516,109 @@ def test_env_ext4(state_test_env):
> >       finally:
> >           if fs_img:
> >               call('rm -f %s' % fs_img, shell=True)
> > +
> > +def test_env_text(u_boot_console):
> > +    """Test the script that converts the environment to a text file"""
> > +
> > +    def check_script(intext, expect_val):
> > +        """Check a test case
> > +
> > +        Args:
> > +            intext: Text to pass to the script
> > +            expect_val: Expected value of the CONFIG_EXTRA_ENV_TEXT string, or
> > +                None if we expect it not to be defined
> > +        """
> > +        with tempfile.TemporaryDirectory() as path:
> > +            fname = os.path.join(path, 'infile')
> > +            with open(fname, 'w') as inf:
> > +                print(intext, file=inf)
> > +            result = u_boot_utils.run_and_log(cons, ['awk', '-f', script, fname])
> > +            if expect_val is not None:
> > +                expect = '#define CONFIG_EXTRA_ENV_TEXT "%s"\n' % expect_val
> > +                assert result == expect
> > +            else:
> > +                assert result == ''
> > +
> > +    cons = u_boot_console
> > +    script = os.path.join(cons.config.source_dir, 'scripts', 'env2string.awk')
> > +
> > +    # simple script with a single var
> > +    check_script('fred=123', 'fred=123\\0')
> > +
> > +    # no vars
> > +    check_script('', None)
> > +
> > +    # two vars
> > +    check_script('''fred=123
> > +ernie=456''', 'fred=123\\0ernie=456\\0')
>
> Here according the awk implementation  the test result can be
>
> 'fred=123\\0ernie=456\\0' => creation order
>
> 'ernie=456\\0fred=123\\0' => alphabetic order
>
> and perhaps other order for other awk ?
>
>
> do you think you can have a solution with POSIX awk ?
> today I found only solution for gawk:
>
>
> ---------------------------- scripts/env2string.awk ----------------------------
> index 1bfe9ed07a..f3215c369a 100644
> @@ -81,7 +81,8 @@ END {
>         if (do_output) {
>                 printf("%s", "#define CONFIG_EXTRA_ENV_TEXT \"")
>
> -               # Print out all the variables
> +               # Print out all the variables by alphabetic order
> +               PROCINFO["sorted_in"] = "@ind_str_asc"
>                 for (var in vars) {
>                         env = vars[var]
>                         print var "=" vars[var] "\\0"
>
>
> But this GNU feature must be avoid,
> see commit 7acb32256831 ("env: Avoid using GNU features in awk")

Thanks for this. I filed:

https://source.denx.de/u-boot/u-boot/-/issues/10

Regards,
Simon
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 8845c6fd750..8820a0f895e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -738,6 +738,13 @@  F:	test/env/
 F:	tools/env*
 F:	tools/mkenvimage.c
 
+ENVIRONMENT AS TEXT
+M:	Simon Glass <sjg@chromium.org>
+R:	Wolfgang Denk <wd@denx.de>
+S:	Maintained
+F:	doc/usage/environment.rst
+F:	scripts/env2string.awk
+
 FPGA
 M:	Michal Simek <michal.simek@xilinx.com>
 S:	Maintained
diff --git a/Makefile b/Makefile
index 5194e4dc782..a80be71c78a 100644
--- a/Makefile
+++ b/Makefile
@@ -517,6 +517,7 @@  version_h := include/generated/version_autogenerated.h
 timestamp_h := include/generated/timestamp_autogenerated.h
 defaultenv_h := include/generated/defaultenv_autogenerated.h
 dt_h := include/generated/dt.h
+env_h := include/generated/environment.h
 
 no-dot-config-targets := clean clobber mrproper distclean \
 			 help %docs check% coccicheck \
@@ -1786,6 +1787,69 @@  quiet_cmd_sym ?= SYM     $@
 u-boot.sym: u-boot FORCE
 	$(call if_changed,sym)
 
+# Environment processing
+# ---------------------------------------------------------------------------
+
+# Directory where we expect the .env file, if it exists
+ENV_DIR := $(srctree)/board/$(BOARDDIR)
+
+# Basename of .env file, stripping quotes
+ENV_SOURCE_FILE := $(CONFIG_ENV_SOURCE_FILE:"%"=%)
+
+# Filename of .env file
+ENV_FILE_CFG := $(ENV_DIR)/$(ENV_SOURCE_FILE).env
+
+# Default filename, if CONFIG_ENV_SOURCE_FILE is empty
+ENV_FILE_BOARD := $(ENV_DIR)/$(CONFIG_SYS_BOARD:"%"=%).env
+
+# Select between the CONFIG_ENV_SOURCE_FILE and the default one
+ENV_FILE := $(if $(ENV_SOURCE_FILE),$(ENV_FILE_CFG),$(wildcard $(ENV_FILE_BOARD)))
+
+# Run the environment text file through the preprocessor, but only if it is
+# non-empty, to save time and possible build errors if something is wonky with
+# the board
+quiet_cmd_gen_envp = ENVP    $@
+      cmd_gen_envp = \
+	if [ -s "$(ENV_FILE)" ]; then \
+		$(CPP) -P $(CFLAGS) -x assembler-with-cpp -D__ASSEMBLY__ \
+			-D__UBOOT_CONFIG__ \
+			-I . -I include -I $(srctree)/include \
+			-include linux/kconfig.h -include include/config.h \
+			-I$(srctree)/arch/$(ARCH)/include \
+			$< -o $@; \
+	else \
+		echo -n >$@ ; \
+	fi
+include/generated/env.in: include/generated/env.txt FORCE
+	$(call cmd,gen_envp)
+
+# Regenerate the environment if it changes
+# We use 'wildcard' since the file is not required to exist (at present), in
+# which case we don't want this dependency, but instead should create an empty
+# file
+# This rule is useful since it shows the source file for the environment
+quiet_cmd_envc = ENVC    $@
+      cmd_envc = \
+	if [ -f "$<" ]; then \
+		cat $< > $@; \
+	elif [ -n "$(ENV_SOURCE_FILE)" ]; then \
+		echo "Missing file $(ENV_FILE_CFG)"; \
+	else \
+		echo -n >$@ ; \
+	fi
+
+include/generated/env.txt: $(wildcard $(ENV_FILE)) FORCE
+	$(call cmd,envc)
+
+# Write out the resulting environment, converted to a C string
+quiet_cmd_gen_envt = ENVT    $@
+      cmd_gen_envt = \
+	awk -f $(srctree)/scripts/env2string.awk $< >$@
+$(env_h): include/generated/env.in
+	$(call cmd,gen_envt)
+
+# ---------------------------------------------------------------------------
+
 # The actual objects are generated when descending,
 # make sure no implicit rule kicks in
 $(sort $(u-boot-init) $(u-boot-main)): $(u-boot-dirs) ;
@@ -1841,7 +1905,7 @@  endif
 # prepare2 creates a makefile if using a separate output directory
 prepare2: prepare3 outputmakefile cfg
 
-prepare1: prepare2 $(version_h) $(timestamp_h) $(dt_h) \
+prepare1: prepare2 $(version_h) $(timestamp_h) $(dt_h) $(env_h) \
                    include/config/auto.conf
 ifeq ($(wildcard $(LDSCRIPT)),)
 	@echo >&2 "  Could not find linker script."
diff --git a/config.mk b/config.mk
index 7bb1fd4ed1b..2595aed218b 100644
--- a/config.mk
+++ b/config.mk
@@ -50,8 +50,10 @@  endif
 ifneq ($(BOARD),)
 ifdef	VENDOR
 BOARDDIR = $(VENDOR)/$(BOARD)
+ENVDIR=${vendor}/env
 else
 BOARDDIR = $(BOARD)
+ENVDIR=${board}/env
 endif
 endif
 ifdef	BOARD
diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
index 7a733b44556..043c02d9a94 100644
--- a/doc/usage/environment.rst
+++ b/doc/usage/environment.rst
@@ -15,7 +15,86 @@  environment is erased by accident, a default environment is provided.
 
 Some configuration options can be set using Environment Variables.
 
-List of environment variables (most likely not complete):
+Text-based Environment
+----------------------
+
+The default environment for a board is created using a `.env` environment file
+using a simple text format. The base filename for this is defined by
+`CONFIG_ENV_SOURCE_FILE`, or `CONFIG_SYS_BOARD` if that is empty.
+
+The file must be in the board directory and have a .env extension, so
+assuming that there is a board vendor, the resulting filename is therefore::
+
+   board/<vendor>/<board>/<CONFIG_ENV_SOURCE_FILE>.env
+
+or::
+
+   board/<vendor>/<board>/<CONFIG_SYS_BOARD>.env
+
+This is a plain text file where you can type your environment variables in
+the form `var=value`. Blank lines and multi-line variables are supported.
+The conversion script looks for a line that starts in column 1 with a string
+and has an equals sign immediately afterwards. Spaces before the = are not
+permitted. It is a good idea to indent your scripts so that only the 'var='
+appears at the start of a line.
+
+To add additional text to a variable you can use `var+=value`. This text is
+merged into the variable during the make process and made available as a
+single value to U-Boot. Variables can contain `+` characters but in the unlikely
+event that you want to have a variable name ending in plus, put a backslash
+before the `+` so that the script knows you are not adding to an existing
+variable but assigning to a new one::
+
+    maximum\+=value
+
+This file can include C-style comments. Blank lines and multi-line
+variables are supported, and you can use normal C preprocessor directives
+and CONFIG defines from your board config also.
+
+For example, for snapper9260 you would create a text file called
+`board/bluewater/snapper9260.env` containing the environment text.
+
+Example::
+
+    stdout=serial
+    #ifdef CONFIG_LCD
+    stdout+=,lcd
+    #endif
+    bootcmd=
+        /* U-Boot script for booting */
+
+        if [ -z ${tftpserverip} ]; then
+            echo "Use 'setenv tftpserverip a.b.c.d' to set IP address."
+        fi
+
+        usb start; setenv autoload n; bootp;
+        tftpboot ${tftpserverip}:
+        bootm
+    failed=
+        /* Print a message when boot fails */
+        echo CONFIG_SYS_BOARD boot failed - please check your image
+        echo Load address is CONFIG_SYS_LOAD_ADDR
+
+If CONFIG_ENV_SOURCE_FILE is empty and the default filename is not present, then
+the old-style C environment is used instead. See below.
+
+Old-style C environment
+-----------------------
+
+Traditionally, the default environment is created in `include/env_default.h`,
+and can be augmented by various `CONFIG` defines. See that file for details. In
+particular you can define `CONFIG_EXTRA_ENV_SETTINGS` in your board file
+to add environment variables.
+
+Board maintainers are encouraged to migrate to the text-based environment as it
+is easier to maintain. The distro-board script still requires the old-style
+environment but work is underway to address this.
+
+
+List of environment variables
+-----------------------------
+
+This is most-likely not complete:
 
 baudrate
     see CONFIG_BAUDRATE
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index 356f2a56181..4314112ff34 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -10,6 +10,7 @@  Use U-Boot
    netconsole
    partitions
    cmdline
+   environment
 
 Shell commands
 --------------
diff --git a/env/Kconfig b/env/Kconfig
index f75f2b13536..b93ad5c8ee0 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -3,6 +3,24 @@  menu "Environment"
 config ENV_SUPPORT
 	def_bool y
 
+config ENV_SOURCE_FILE
+	string "Environment file to use"
+	default ""
+	help
+	  This sets the basename to use to generate the default environment.
+	  This a text file as described in doc/usage/environment.rst
+
+	  The file must be in the board directory and have a .env extension, so
+	  the resulting filename is typically
+	  board/<vendor>/<board>/<CONFIG_ENV_SOURCE_FILE>.env
+
+	  If the file is not present, an error is produced.
+
+	  If this CONFIG is empty, U-Boot uses CONFIG SYS_BOARD as a default, if
+	  the file board/<vendor>/<board>/<SYS_BOARD>.env exists. Otherwise the
+	  environment is assumed to come from the ad-hoc
+	  CONFIG_EXTRA_ENV_SETTINGS #define
+
 config SAVEENV
 	def_bool y if CMD_SAVEENV
 
diff --git a/env/embedded.c b/env/embedded.c
index 208553e6af1..9f26e6cad9c 100644
--- a/env/embedded.c
+++ b/env/embedded.c
@@ -66,6 +66,7 @@ 
 #endif
 
 #define DEFAULT_ENV_INSTANCE_EMBEDDED
+#include <config.h>
 #include <env_default.h>
 
 #ifdef CONFIG_ENV_ADDR_REDUND
diff --git a/include/env_default.h b/include/env_default.h
index 66e203eb6e4..c06506313e5 100644
--- a/include/env_default.h
+++ b/include/env_default.h
@@ -10,6 +10,10 @@ 
 #include <env_callback.h>
 #include <linux/stringify.h>
 
+#ifndef USE_HOSTCC
+#include <generated/environment.h>
+#endif
+
 #ifdef DEFAULT_ENV_INSTANCE_EMBEDDED
 env_t embedded_environment __UBOOT_ENV_SECTION__(environment) = {
 	ENV_CRC,	/* CRC Sum */
@@ -110,6 +114,13 @@  const uchar default_environment[] = {
 #if defined(CONFIG_BOOTCOUNT_BOOTLIMIT) && (CONFIG_BOOTCOUNT_BOOTLIMIT > 0)
 	"bootlimit="	__stringify(CONFIG_BOOTCOUNT_BOOTLIMIT)"\0"
 #endif
+#ifdef CONFIG_EXTRA_ENV_TEXT
+# ifdef CONFIG_EXTRA_ENV_SETTINGS
+# error "Your board uses a text-file environment, so must not define CONFIG_EXTRA_ENV_SETTINGS"
+# endif
+	/* This is created in the Makefile */
+	CONFIG_EXTRA_ENV_TEXT
+#endif
 #ifdef	CONFIG_EXTRA_ENV_SETTINGS
 	CONFIG_EXTRA_ENV_SETTINGS
 #endif
diff --git a/scripts/env2string.awk b/scripts/env2string.awk
new file mode 100644
index 00000000000..57d0fc8f3ba
--- /dev/null
+++ b/scripts/env2string.awk
@@ -0,0 +1,80 @@ 
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright 2021 Google, Inc
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+# Awk script to parse a text file containing an environment and convert it
+# to a C string which can be compiled into U-Boot.
+
+# The resulting output is:
+#
+#   #define CONFIG_EXTRA_ENV_TEXT "<environment here>"
+#
+# If the input is empty, this script outputs a comment instead.
+
+BEGIN {
+	# env holds the env variable we are currently processing
+	env = "";
+	ORS = ""
+}
+
+# Skip empty lines, as these are generated by the clang preprocessor
+NF {
+	# Quote quotes
+	gsub("\"", "\\\"")
+
+	# Is this the start of a new environment variable?
+	if (match($0, "^([^ \t=][^ =]*)=(.*)$", arr)) {
+		if (length(env) != 0) {
+			# Record the value of the variable now completed
+			vars[var] = env
+		}
+		var = arr[1]
+		env = arr[2]
+
+		# Deal with += which concatenates the new string to the existing
+		# variable
+		if (length(env) != 0 && match(var, "^(.*)[+]$", var_arr))
+		{
+			# Allow var\+=val to indicate that the variable name is
+			# var+ and this is not actually a concatenation
+			if (substr(var_arr[1], length(var_arr[1])) == "\\") {
+				# Drop the backslash
+				sub(/\\[+]$/, "+", var)
+			} else {
+				var = var_arr[1]
+				env = vars[var] env
+			}
+		}
+	} else {
+		# Change newline to space
+		gsub(/^[ \t]+/, "")
+
+		# Don't keep leading spaces generated by the previous blank line
+		if (length(env) == 0) {
+			env = $0
+		} else {
+			env = env " " $0
+		}
+	}
+}
+
+END {
+	# Record the value of the variable now completed. If the variable is
+	# empty it is not set.
+	if (length(env) != 0) {
+		vars[var] = env
+	}
+
+	if (length(vars) != 0) {
+		printf("%s", "#define CONFIG_EXTRA_ENV_TEXT \"")
+
+		# Print out all the variables
+		for (var in vars) {
+			env = vars[var]
+			print var "=" vars[var] "\\0"
+		}
+		print "\"\n"
+	}
+}
diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
index 9bed2f48d77..f85cb031382 100644
--- a/test/py/tests/test_env.py
+++ b/test/py/tests/test_env.py
@@ -7,6 +7,7 @@ 
 import os
 import os.path
 from subprocess import call, check_call, CalledProcessError
+import tempfile
 
 import pytest
 import u_boot_utils
@@ -515,3 +516,109 @@  def test_env_ext4(state_test_env):
     finally:
         if fs_img:
             call('rm -f %s' % fs_img, shell=True)
+
+def test_env_text(u_boot_console):
+    """Test the script that converts the environment to a text file"""
+
+    def check_script(intext, expect_val):
+        """Check a test case
+
+        Args:
+            intext: Text to pass to the script
+            expect_val: Expected value of the CONFIG_EXTRA_ENV_TEXT string, or
+                None if we expect it not to be defined
+        """
+        with tempfile.TemporaryDirectory() as path:
+            fname = os.path.join(path, 'infile')
+            with open(fname, 'w') as inf:
+                print(intext, file=inf)
+            result = u_boot_utils.run_and_log(cons, ['awk', '-f', script, fname])
+            if expect_val is not None:
+                expect = '#define CONFIG_EXTRA_ENV_TEXT "%s"\n' % expect_val
+                assert result == expect
+            else:
+                assert result == ''
+
+    cons = u_boot_console
+    script = os.path.join(cons.config.source_dir, 'scripts', 'env2string.awk')
+
+    # simple script with a single var
+    check_script('fred=123', 'fred=123\\0')
+
+    # no vars
+    check_script('', None)
+
+    # two vars
+    check_script('''fred=123
+ernie=456''', 'fred=123\\0ernie=456\\0')
+
+    # blank lines
+    check_script('''fred=123
+
+
+ernie=456
+
+''', 'fred=123\\0ernie=456\\0')
+
+    # append
+    check_script('''fred=123
+ernie=456
+fred+= 456''', 'fred=123 456\\0ernie=456\\0')
+
+    # append from empty
+    check_script('''fred=
+ernie=456
+fred+= 456''', 'fred= 456\\0ernie=456\\0')
+
+    # variable with + in it
+    check_script('fred+ernie=123', 'fred+ernie=123\\0')
+
+    # ignores variables that are empty
+    check_script('''fred=
+fred+=
+ernie=456''', 'ernie=456\\0')
+
+    # single-character env name
+    check_script('''f=123
+e=456
+f+= 456''', 'e=456\\0f=123 456\\0')
+
+    # contains quotes
+    check_script('''fred="my var"
+ernie=another"''', 'fred=\\"my var\\"\\0ernie=another\\"\\0')
+
+    # variable name ending in +
+    check_script('''fred\\+=my var
+fred++= again''', 'fred+=my var again\\0')
+
+    # variable name containing +
+    check_script('''fred+jane=both
+fred+jane+=again
+ernie=456''', 'fred+jane=bothagain\\0ernie=456\\0')
+
+    # multi-line vars - new vars always start at column 1
+    check_script('''fred=first
+ second
+\tthird with tab
+
+   after blank
+ confusing=oops
+ernie=another"''', 'fred=first second third with tab after blank confusing=oops\\0ernie=another\\"\\0')
+
+    # real-world example
+    check_script('''ubifs_boot=
+	env exists bootubipart ||
+		env set bootubipart UBI;
+	env exists bootubivol ||
+		env set bootubivol boot;
+	if ubi part ${bootubipart} &&
+		ubifsmount ubi${devnum}:${bootubivol};
+	then
+		devtype=ubi;
+		run scan_dev_for_boot;
+	fi
+''',
+        'ubifs_boot=env exists bootubipart || env set bootubipart UBI; '
+        'env exists bootubivol || env set bootubivol boot; '
+        'if ubi part ${bootubipart} && ubifsmount ubi${devnum}:${bootubivol}; '
+        'then devtype=ubi; run scan_dev_for_boot; fi\\0')