Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
Dear Simon, In message you wrote: > > > 1) This requires that the .env files are run through CPP, which is > >only added in a later patch. > > OK perhaps I should just merge the patches. It is a bit artificial > having two and it seems that people agree we need the += syntax. Yes, some way to append to the environment is useful, and using '+=' is an intuitive syntax for it. I'm just not happy about sneeking in a new, undocumented restriction on U-Boot variable names. Yes, there are probably not many systems around (if any) where a variable name ends in a plus sign, but we should be consistent. I have to admit that I don't have any nice alternative suggestion. If we stick with the rule that only NUL and '=' cannot be used in a variable name, we could write variable=+value instead. But this does not look as nice as '+=', and we need an escape mechanism for the case where we want a simple assignment of a value that starts with a '+'. BTW: If we would add such a feature to U-Boot (which seems to be no bad idea to me) we would probably implement an "env append" command? > > 2) Even if I add an "#include board//env/common.env" in my > >.env files, your logic would trigger on the existence of > >the common.env file and ignore the .env files. > > OK, so I if reverse that, are you happy? What do you think about my > explanation above? The longer I think about it the more I wonder if any hard coded file names are really a good way to handle this. For example there might be the case where we have 4 boards A, B, C and D, and boards A and B would use one env file, and C and D would use another. this does not match the ".env" scheme, and "common.env" does not fit either, as we have two "common" files. I wonder if there should rather be a Kconfig option so each board can select it's env file name; default would be ".env". what do you think? 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 Life would be so much easier if we could just look at the source code. -- Dave Olson
Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
Hi Tom, On Tue, 5 Oct 2021 at 09:52, Tom Rini wrote: > > On Tue, Oct 05, 2021 at 09:33:18AM -0600, Simon Glass wrote: > > Hi Wolfgang, > > > > On Tue, 5 Oct 2021 at 08:56, Wolfgang Denk wrote: > > > > > > Dear Simon, > > > > > > In message > > > you > > > wrote: > > > > > > > > > > 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//env > > > > > > directory called .env (or common.env if you want the same > > > > > > environment for all boards). > > > > > > > > > > This should be no exclusive "or" here. If a common.env exists, it > > > > > should be used for all boards, and if additionally one ore more > > > > > .env files exist, these should ALSO be applied to the > > > > > respective boards. > > > > > > > > Is it not enough to use #include in the main file? We have a similar > > > > feature with the u-boot.dtsi files and in that case we only choose the > > > > most specific. > > > > > > 1) This requires that the .env files are run through CPP, which is > > >only added in a later patch. > > > > OK perhaps I should just merge the patches. It is a bit artificial > > having two and it seems that people agree we need the += syntax. > > It's important to maintain bisectability, yes. But functionality > should be evaluated at the end of the series, not intermediate steps. I > don't have a strong opinion either way on if these two patches are > merged, or not. So on a similar note, all of the feedback about the > current env documentation is good and helpful, but I think a txt -> rST > then enhance the rST makes the most sense so that we don't "hide" > improvements within the migration. OK. I added Wolfgang's comments in the patch where he made them but will split them out. Regards, Simon
Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
On Tue, Oct 05, 2021 at 09:33:18AM -0600, Simon Glass wrote: > Hi Wolfgang, > > On Tue, 5 Oct 2021 at 08:56, Wolfgang Denk wrote: > > > > Dear Simon, > > > > In message > > you > > wrote: > > > > > > > > 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//env > > > > > directory called .env (or common.env if you want the same > > > > > environment for all boards). > > > > > > > > This should be no exclusive "or" here. If a common.env exists, it > > > > should be used for all boards, and if additionally one ore more > > > > .env files exist, these should ALSO be applied to the > > > > respective boards. > > > > > > Is it not enough to use #include in the main file? We have a similar > > > feature with the u-boot.dtsi files and in that case we only choose the > > > most specific. > > > > 1) This requires that the .env files are run through CPP, which is > >only added in a later patch. > > OK perhaps I should just merge the patches. It is a bit artificial > having two and it seems that people agree we need the += syntax. It's important to maintain bisectability, yes. But functionality should be evaluated at the end of the series, not intermediate steps. I don't have a strong opinion either way on if these two patches are merged, or not. So on a similar note, all of the feedback about the current env documentation is good and helpful, but I think a txt -> rST then enhance the rST makes the most sense so that we don't "hide" improvements within the migration. -- Tom signature.asc Description: PGP signature
Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
Hi Wolfgang, On Tue, 5 Oct 2021 at 08:56, Wolfgang Denk wrote: > > Dear Simon, > > In message > you > wrote: > > > > > > 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//env > > > > directory called .env (or common.env if you want the same > > > > environment for all boards). > > > > > > This should be no exclusive "or" here. If a common.env exists, it > > > should be used for all boards, and if additionally one ore more > > > .env files exist, these should ALSO be applied to the > > > respective boards. > > > > Is it not enough to use #include in the main file? We have a similar > > feature with the u-boot.dtsi files and in that case we only choose the > > most specific. > > 1) This requires that the .env files are run through CPP, which is >only added in a later patch. OK perhaps I should just merge the patches. It is a bit artificial having two and it seems that people agree we need the += syntax. > > 2) Even if I add an "#include board//env/common.env" in my >.env files, your logic would trigger on the existence of >the common.env file and ignore the .env files. OK, so I if reverse that, are you happy? What do you think about my explanation above? Regards, Simon
Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
Dear Simon, In message you wrote: > > > > 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//env > > > directory called .env (or common.env if you want the same > > > environment for all boards). > > > > This should be no exclusive "or" here. If a common.env exists, it > > should be used for all boards, and if additionally one ore more > > .env files exist, these should ALSO be applied to the > > respective boards. > > Is it not enough to use #include in the main file? We have a similar > feature with the u-boot.dtsi files and in that case we only choose the > most specific. 1) This requires that the .env files are run through CPP, which is only added in a later patch. 2) Even if I add an "#include board//env/common.env" in my .env files, your logic would trigger on the existence of the common.env file and ignore the .env files. 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 Work 8 hours, sleep 8 hours; but not the same 8 hours.
Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
Hi Rasmus, On Mon, 4 Oct 2021 at 01:28, Rasmus Villemoes wrote: > > On 02/10/2021 02.38, 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. > > Indeed, the pain of CONFIG_EXTRA_ENV_SETTINGS was part of the motivation > for introducing CONFIG_USE_DEFAULT_ENV_FILE. > > > 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//env > > directory called .env (or common.env if you want the same > > environment for all boards). > > > > 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. > > > > Comments are not permitted in the environment with this commit. > > Perhaps some remarks on how this compares/relates to > CONFIG_USE_DEFAULT_ENV_FILE and CONFIG_ENV_IMPORT_FDT would be in order? > In particular, the latter seems like it could already do the "amend the > environent per vendor/board" with appropriate settings in the > -u-boot.dtsi files? > > I don't think either of those currently support using CONFIG_ variables > in the definitions, but perhaps that could be fixed. > > I don't have anything against these patches as such, I'd just like to > understand precisely what they bring that cannot already be done with > existing mechanisms. Yes I forgot about that. I will take a look. Regards, Simon
Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
Hi Wolfgang, On Mon, 4 Oct 2021 at 06:08, Wolfgang Denk wrote: > > Dear Simon, > > In message > <20211001183842.v5.3.If789ba3e2667c46c03eda3386ca84a863baeda55@changeid> you > wrote: > > > > 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//env > > directory called .env (or common.env if you want the same > > environment for all boards). > > This should be no exclusive "or" here. If a common.env exists, it > should be used for all boards, and if additionally one ore more > .env files exist, these should ALSO be applied to the > respective boards. Is it not enough to use #include in the main file? We have a similar feature with the u-boot.dtsi files and in that case we only choose the most specific. Regards, Simon
Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
On Mon, Oct 04, 2021 at 09:28:43AM +0200, Rasmus Villemoes wrote: > On 02/10/2021 02.38, 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. > > Indeed, the pain of CONFIG_EXTRA_ENV_SETTINGS was part of the motivation > for introducing CONFIG_USE_DEFAULT_ENV_FILE. > > > 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//env > > directory called .env (or common.env if you want the same > > environment for all boards). > > > > 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. > > > > Comments are not permitted in the environment with this commit. > > Perhaps some remarks on how this compares/relates to > CONFIG_USE_DEFAULT_ENV_FILE and CONFIG_ENV_IMPORT_FDT would be in order? > In particular, the latter seems like it could already do the "amend the > environent per vendor/board" with appropriate settings in the > -u-boot.dtsi files? > > I don't think either of those currently support using CONFIG_ variables > in the definitions, but perhaps that could be fixed. > > I don't have anything against these patches as such, I'd just like to > understand precisely what they bring that cannot already be done with > existing mechanisms. So, the high level requirement is to move the environment out of the board.h file (or the nest of #includes that uses to cobble it together). That does mean that CONFIG_USE_DEFAULT_ENV_FILE at least would likely need some tweaking, but may also be just not as useful, if it's no longer such a pain to modify the default environment. -- Tom signature.asc Description: PGP signature
Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
Dear Simon, In message <20211001183842.v5.3.If789ba3e2667c46c03eda3386ca84a863baeda55@changeid> you wrote: > > 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//env > directory called .env (or common.env if you want the same > environment for all boards). This should be no exclusive "or" here. If a common.env exists, it should be used for all boards, and if additionally one ore more .env files exist, these should ALSO be applied to the respective boards. 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 Save yourself! Reboot in 5 seconds!
Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
On 02/10/2021 02.38, 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. Indeed, the pain of CONFIG_EXTRA_ENV_SETTINGS was part of the motivation for introducing CONFIG_USE_DEFAULT_ENV_FILE. > 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//env > directory called .env (or common.env if you want the same > environment for all boards). > > 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. > > Comments are not permitted in the environment with this commit. Perhaps some remarks on how this compares/relates to CONFIG_USE_DEFAULT_ENV_FILE and CONFIG_ENV_IMPORT_FDT would be in order? In particular, the latter seems like it could already do the "amend the environent per vendor/board" with appropriate settings in the -u-boot.dtsi files? I don't think either of those currently support using CONFIG_ variables in the definitions, but perhaps that could be fixed. I don't have anything against these patches as such, I'd just like to understand precisely what they bring that cannot already be done with existing mechanisms. Rasmus
[PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
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//env
directory called .env (or common.env if you want the same
environment for all boards).
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.
Comments are not permitted in the environment with this commit.
Signed-off-by: Simon Glass
---
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
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
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
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
MAINTAINERS | 7 ++
Makefile | 34 +-
config.mk | 2 ++
doc/usage/environment.rst | 42
env/embedded.c| 1 +
include/env_default.h | 8 +++
scripts/env2string.awk| 50 +++
7 files changed, 143 insertions(+), 1 deletion(-)
create mode 100644 scripts/env2string.awk
diff --git a/MAINTAINERS b/MAINTAINERS
index 31b49c0a95f..d79dac06cb5 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
+R: Wolfgang Denk
+S: Maintained
+F: doc/usage/environment.rst
+F: scripts/env2string.awk
+
FPGA
M: Michal Simek
S: Maintained
diff --git a/Makefile b/Makefile
index b23c2cea912..5c3ba37398f 100644
--- a/Makefile
+++ b/Makefile
@@ -513,6 +513,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 \
@@ -1785,6 +1786,37 @@ quiet_cmd_sym ?= SYM $@
u-boot.sym: u-boot FORCE
$(call if_changed,sym)
+# We expect '.env' but failing that will use 'common.env'
+ENV_DIR := $(if $(VENDOR),$(VENDOR)/env,$(BOARD)/env)
+ENV_FILE_BOARD := $(srctree)/board/${ENV_DIR}/$(BOARD).env
+ENV_FILE_COMMON := $(srctree)/board/${ENV_DIR}/common.env
+ENV_FILE := $(if $(wildcard
$(ENV_FILE_BOARD)),$(ENV_FILE_BOARD),$(ENV_FILE_COMMON))
+
+# Run the environment text file through the preprocessor
+quiet_cmd_gen_envp = ENVP$@
+ cmd_gen_envp = \
+ if [ -f "$(ENV_FILE)" ]; then \
+ cat $(ENV_FILE) >$@; \
+ else \
+ echo -n >$@ ; \
+ fi
+
+# 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
+include/generated/environment.in: \
+ $(if $(wildcard $(ENV_FILE)),$(wildcard $(ENV_FILE)),FORCE)
+ $(call cmd,gen_envp)
+
+quiet_cmd_gen_envt = ENVT$@
+ cmd_gen_envt = \
+ echo -n "\#define CONFIG_EXTRA_ENV_TEXT " >$@; \
+ awk -f $(srctree)/scripts/env2string.awk $< >>$@
+
+$(env_h): include/generated/environment.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) ;
@@ -1840,7 +1872,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)),)
