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

[Patrick DELAUNAY]

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 at chromium.org>
> Reviewed-by: Marek Behún <marek.behun at nic.cz>
> Tested-by: Marek Behún <marek.behun at 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