[U-Boot] [RFC PATCH] Allow providing default environment from file

[U-Boot] [RFC PATCH] Allow providing default environment from file

[Rasmus Villemoes]

It is sometimes useful to be able to define the entire default
environment in an external file. This implements a Kconfig option for
allowing that.

It is somewhat annoying to have two visible Kconfig options; it would
probably be more user-friendly to just have the string option (with
empty string obviously meaning not to use this feature). But then we'd
also need a hidden CONFIG that we can use in the #ifdef in
env_default.h, and I don't think one can set a def_bool based on
whether a string-valued config is empty or not.

I've tried to make the accepted format the same as the one the
mkenvimage tool accepts. I have no idea how portable the sed script
implementing the "allow embedded newlines in values" is. Nor do I know
if one can expect xxd to be available.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 Makefile              | 16 ++++++++++++++++
 env/Kconfig           | 18 ++++++++++++++++++
 include/env_default.h |  4 ++++
 3 files changed, 38 insertions(+)

diff --git a/Makefile b/Makefile
index 4981a2ed6f..e5ba5213fd 100644
--- a/Makefile
+++ b/Makefile
@@ -423,6 +423,7 @@ endif
 
 version_h := include/generated/version_autogenerated.h
 timestamp_h := include/generated/timestamp_autogenerated.h
+defaultenv_h := include/generated/defaultenv_autogenerated.h
 
 no-dot-config-targets := clean clobber mrproper distclean \
 			 help %docs check% coccicheck \
@@ -1366,6 +1367,10 @@ ifeq ($(wildcard $(LDSCRIPT)),)
 	@/bin/false
 endif
 
+ifeq ($(CONFIG_DEFAULT_ENV_FROM_FILE),y)
+prepare1: $(defaultenv_h)
+endif
+
 archprepare: prepare1 scripts_basic
 
 prepare0: archprepare FORCE
@@ -1413,12 +1418,23 @@ define filechk_timestamp.h
 	fi)
 endef
 
+define filechk_defaultenv.h
+	(grep -v '^#' | \
+	 grep -v '^$$' | \
+	 tr '\n' '\0' | \
+	 sed -e 's/\\\x0/\n/' | \
+	 xxd -i ; echo ", 0x00" ; )
+endef
+
 $(version_h): include/config/uboot.release FORCE
 	$(call filechk,version.h)
 
 $(timestamp_h): $(srctree)/Makefile FORCE
 	$(call filechk,timestamp.h)
 
+$(defaultenv_h): $(CONFIG_DEFAULT_ENV_FILE:"%"=%) FORCE
+	$(call filechk,defaultenv.h)
+
 # ---------------------------------------------------------------------------
 quiet_cmd_cpp_lds = LDS     $@
 cmd_cpp_lds = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) \
diff --git a/env/Kconfig b/env/Kconfig
index a24370786b..1baebd743b 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -482,4 +482,22 @@ config ENV_SIZE
 
 endif
 
+config DEFAULT_ENV_FROM_FILE
+	bool "Create default environment from file"
+	help
+	  Normally, the default environment is automatically generated
+	  based on the settings of various CONFIG_* options, as well
+	  as the CONFIG_EXTRA_ENV_SETTINGS. By selecting this option,
+	  you can instead define the entire default environment in an
+	  external file.
+
+config DEFAULT_ENV_FILE
+	string "Path to default environment file"
+	depends on DEFAULT_ENV_FROM_FILE
+	help
+	  The path containing the default environment. The format is
+	  the same as accepted by the mkenvimage tool: lines
+	  containing key=value pairs, blank lines and lines beginning
+	  with # are ignored.
+
 endmenu
diff --git a/include/env_default.h b/include/env_default.h
index b574345af2..656d202cc7 100644
--- a/include/env_default.h
+++ b/include/env_default.h
@@ -22,6 +22,7 @@ static char default_environment[] = {
 #else
 const uchar default_environment[] = {
 #endif
+#ifndef CONFIG_DEFAULT_ENV_FROM_FILE
 #ifdef	CONFIG_ENV_CALLBACK_LIST_DEFAULT
 	ENV_CALLBACK_VAR "=" CONFIG_ENV_CALLBACK_LIST_DEFAULT "\0"
 #endif
@@ -108,6 +109,9 @@ const uchar default_environment[] = {
 	CONFIG_EXTRA_ENV_SETTINGS
 #endif
 	"\0"
+#else /* CONFIG_DEFAULT_ENV_FROM_FILE */
+#include "generated/defaultenv_autogenerated.h"
+#endif
 #ifdef DEFAULT_ENV_INSTANCE_EMBEDDED
 	}
 #endif
-- 
2.15.1

[U-Boot] [RFC PATCH] Allow providing default environment from file

[Lukasz Majewski]

Hi Rasmus,

> It is sometimes useful to be able to define the entire default
> environment in an external file.

There is already available script for extracting the environment.

Please look into:
./scripts/get_default_envs.sh

Maybe you can reuse it in this patch?

> This implements a Kconfig option for
> allowing that.
> 
> It is somewhat annoying to have two visible Kconfig options; it would
> probably be more user-friendly to just have the string option (with
> empty string obviously meaning not to use this feature). But then we'd
> also need a hidden CONFIG that we can use in the #ifdef in
> env_default.h, and I don't think one can set a def_bool based on
> whether a string-valued config is empty or not.
> 
> I've tried to make the accepted format the same as the one the
> mkenvimage tool accepts. I have no idea how portable the sed script
> implementing the "allow embedded newlines in values" is. Nor do I know
> if one can expect xxd to be available.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
>  Makefile              | 16 ++++++++++++++++
>  env/Kconfig           | 18 ++++++++++++++++++
>  include/env_default.h |  4 ++++
>  3 files changed, 38 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 4981a2ed6f..e5ba5213fd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -423,6 +423,7 @@ endif
>  
>  version_h := include/generated/version_autogenerated.h
>  timestamp_h := include/generated/timestamp_autogenerated.h
> +defaultenv_h := include/generated/defaultenv_autogenerated.h
>  
>  no-dot-config-targets := clean clobber mrproper distclean \
>  			 help %docs check% coccicheck \
> @@ -1366,6 +1367,10 @@ ifeq ($(wildcard $(LDSCRIPT)),)
>  	@/bin/false
>  endif
>  
> +ifeq ($(CONFIG_DEFAULT_ENV_FROM_FILE),y)
> +prepare1: $(defaultenv_h)
> +endif
> +
>  archprepare: prepare1 scripts_basic
>  
>  prepare0: archprepare FORCE
> @@ -1413,12 +1418,23 @@ define filechk_timestamp.h
>  	fi)
>  endef
>  
> +define filechk_defaultenv.h
> +	(grep -v '^#' | \
> +	 grep -v '^$$' | \
> +	 tr '\n' '\0' | \
> +	 sed -e 's/\\\x0/\n/' | \
> +	 xxd -i ; echo ", 0x00" ; )
> +endef
> +
>  $(version_h): include/config/uboot.release FORCE
>  	$(call filechk,version.h)
>  
>  $(timestamp_h): $(srctree)/Makefile FORCE
>  	$(call filechk,timestamp.h)
>  
> +$(defaultenv_h): $(CONFIG_DEFAULT_ENV_FILE:"%"=%) FORCE
> +	$(call filechk,defaultenv.h)
> +
>  #
> ---------------------------------------------------------------------------
> quiet_cmd_cpp_lds = LDS     $@ cmd_cpp_lds = $(CPP)
> -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) \ diff --git
> a/env/Kconfig b/env/Kconfig index a24370786b..1baebd743b 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -482,4 +482,22 @@ config ENV_SIZE
>  
>  endif
>  
> +config DEFAULT_ENV_FROM_FILE
> +	bool "Create default environment from file"
> +	help
> +	  Normally, the default environment is automatically
> generated
> +	  based on the settings of various CONFIG_* options, as well
> +	  as the CONFIG_EXTRA_ENV_SETTINGS. By selecting this option,
> +	  you can instead define the entire default environment in an
> +	  external file.
> +
> +config DEFAULT_ENV_FILE
> +	string "Path to default environment file"
> +	depends on DEFAULT_ENV_FROM_FILE
> +	help
> +	  The path containing the default environment. The format is
> +	  the same as accepted by the mkenvimage tool: lines
> +	  containing key=value pairs, blank lines and lines beginning
> +	  with # are ignored.
> +
>  endmenu
> diff --git a/include/env_default.h b/include/env_default.h
> index b574345af2..656d202cc7 100644
> --- a/include/env_default.h
> +++ b/include/env_default.h
> @@ -22,6 +22,7 @@ static char default_environment[] = {
>  #else
>  const uchar default_environment[] = {
>  #endif
> +#ifndef CONFIG_DEFAULT_ENV_FROM_FILE
>  #ifdef	CONFIG_ENV_CALLBACK_LIST_DEFAULT
>  	ENV_CALLBACK_VAR "=" CONFIG_ENV_CALLBACK_LIST_DEFAULT "\0"
>  #endif
> @@ -108,6 +109,9 @@ const uchar default_environment[] = {
>  	CONFIG_EXTRA_ENV_SETTINGS
>  #endif
>  	"\0"
> +#else /* CONFIG_DEFAULT_ENV_FROM_FILE */
> +#include "generated/defaultenv_autogenerated.h"
> +#endif
>  #ifdef DEFAULT_ENV_INSTANCE_EMBEDDED
>  	}
>  #endif



Best regards,

Lukasz Majewski

--

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: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180125/eeedb078/attachment.sig>

[U-Boot] [RFC PATCH] Allow providing default environment from file

[Rasmus Villemoes]

On 2018-01-25 10:30, Lukasz Majewski wrote:
> Hi Rasmus,
> 
>> It is sometimes useful to be able to define the entire default
>> environment in an external file.
> 
> There is already available script for extracting the environment.
> 
> Please look into:
> ./scripts/get_default_envs.sh
> 
> Maybe you can reuse it in this patch?

I'm sorry, but I don't see what I could use that for. It seems to do the
opposite of what I want, namely extract the default environment and
store it in a plain-text file. I want to provide a plain-text file to
define the default environment.

It's quite likely that that script can be useful for generating a sketch
for the external file (i.e., build a U-boot as "usual" with default
environment built from various config options etc. etc., then hand-edit
that file to remove redundant stuff and add the things one needs). The
thing is, having the default environment in an external file makes it
much easier to put it under version control than having to maintain a
branch inside the U-boot repo just to tweak CONFIG_EXTRA_ENV_SETTINGS.

Rasmus

[U-Boot] [RFC PATCH] Allow providing default environment from file

[Lukasz Majewski]

Hi Rasmus,

> On 2018-01-25 10:30, Lukasz Majewski wrote:
> > Hi Rasmus,
> >   
> >> It is sometimes useful to be able to define the entire default
> >> environment in an external file.  
> > 
> > There is already available script for extracting the environment.
> > 
> > Please look into:
> > ./scripts/get_default_envs.sh
> > 
> > Maybe you can reuse it in this patch?  
> 
> I'm sorry, but I don't see what I could use that for. It seems to do
> the opposite of what I want, 

Sorry. I have had misunderstood the patch description.

> namely extract the default environment
> and store it in a plain-text file. I want to provide a plain-text
> file to define the default environment.

Ok. Then your patch seems perfectly valid.

> 
> It's quite likely that that script can be useful for generating a
> sketch for the external file (i.e., build a U-boot as "usual" with
> default environment built from various config options etc. etc., then
> hand-edit that file to remove redundant stuff and add the things one
> needs). The thing is, having the default environment in an external
> file makes it much easier to put it under version control than having
> to maintain a branch inside the U-boot repo just to tweak
> CONFIG_EXTRA_ENV_SETTINGS.
> 
> Rasmus



Best regards,

Lukasz Majewski

--

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: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180125/1f279520/attachment.sig>

[U-Boot] [RFC PATCH] Allow providing default environment from file

[Sean Nyekjaer]

On 24-01-2018 10:55, Rasmus Villemoes wrote:
> It is sometimes useful to be able to define the entire default
> environment in an external file. This implements a Kconfig option for
> allowing that.
>
> It is somewhat annoying to have two visible Kconfig options; it would
> probably be more user-friendly to just have the string option (with
> empty string obviously meaning not to use this feature). But then we'd
> also need a hidden CONFIG that we can use in the #ifdef in
> env_default.h, and I don't think one can set a def_bool based on
> whether a string-valued config is empty or not.
>
> I've tried to make the accepted format the same as the one the
> mkenvimage tool accepts. I have no idea how portable the sed script
> implementing the "allow embedded newlines in values" is. Nor do I know
> if one can expect xxd to be available.
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Tested-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
> ---
>   Makefile              | 16 ++++++++++++++++
>   env/Kconfig           | 18 ++++++++++++++++++
>   include/env_default.h |  4 ++++
>   3 files changed, 38 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 4981a2ed6f..e5ba5213fd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -423,6 +423,7 @@ endif
>   
>   version_h := include/generated/version_autogenerated.h
>   timestamp_h := include/generated/timestamp_autogenerated.h
> +defaultenv_h := include/generated/defaultenv_autogenerated.h
>   
>   no-dot-config-targets := clean clobber mrproper distclean \
>   			 help %docs check% coccicheck \
> @@ -1366,6 +1367,10 @@ ifeq ($(wildcard $(LDSCRIPT)),)
>   	@/bin/false
>   endif
>   
> +ifeq ($(CONFIG_DEFAULT_ENV_FROM_FILE),y)
> +prepare1: $(defaultenv_h)
> +endif
> +
>   archprepare: prepare1 scripts_basic
>   
>   prepare0: archprepare FORCE
> @@ -1413,12 +1418,23 @@ define filechk_timestamp.h
>   	fi)
>   endef
>   
> +define filechk_defaultenv.h
> +	(grep -v '^#' | \
> +	 grep -v '^$$' | \
> +	 tr '\n' '\0' | \
> +	 sed -e 's/\\\x0/\n/' | \
> +	 xxd -i ; echo ", 0x00" ; )
> +endef
> +
>   $(version_h): include/config/uboot.release FORCE
>   	$(call filechk,version.h)
>   
>   $(timestamp_h): $(srctree)/Makefile FORCE
>   	$(call filechk,timestamp.h)
>   
> +$(defaultenv_h): $(CONFIG_DEFAULT_ENV_FILE:"%"=%) FORCE
> +	$(call filechk,defaultenv.h)
> +
>   # ---------------------------------------------------------------------------
>   quiet_cmd_cpp_lds = LDS     $@
>   cmd_cpp_lds = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) \
> diff --git a/env/Kconfig b/env/Kconfig
> index a24370786b..1baebd743b 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -482,4 +482,22 @@ config ENV_SIZE
>   
>   endif
>   
> +config DEFAULT_ENV_FROM_FILE
> +	bool "Create default environment from file"
> +	help
> +	  Normally, the default environment is automatically generated
> +	  based on the settings of various CONFIG_* options, as well
> +	  as the CONFIG_EXTRA_ENV_SETTINGS. By selecting this option,
> +	  you can instead define the entire default environment in an
> +	  external file.
> +
> +config DEFAULT_ENV_FILE
> +	string "Path to default environment file"
> +	depends on DEFAULT_ENV_FROM_FILE
> +	help
> +	  The path containing the default environment. The format is
> +	  the same as accepted by the mkenvimage tool: lines
> +	  containing key=value pairs, blank lines and lines beginning
> +	  with # are ignored.
> +
>   endmenu
> diff --git a/include/env_default.h b/include/env_default.h
> index b574345af2..656d202cc7 100644
> --- a/include/env_default.h
> +++ b/include/env_default.h
> @@ -22,6 +22,7 @@ static char default_environment[] = {
>   #else
>   const uchar default_environment[] = {
>   #endif
> +#ifndef CONFIG_DEFAULT_ENV_FROM_FILE
>   #ifdef	CONFIG_ENV_CALLBACK_LIST_DEFAULT
>   	ENV_CALLBACK_VAR "=" CONFIG_ENV_CALLBACK_LIST_DEFAULT "\0"
>   #endif
> @@ -108,6 +109,9 @@ const uchar default_environment[] = {
>   	CONFIG_EXTRA_ENV_SETTINGS
>   #endif
>   	"\0"
> +#else /* CONFIG_DEFAULT_ENV_FROM_FILE */
> +#include "generated/defaultenv_autogenerated.h"
> +#endif
>   #ifdef DEFAULT_ENV_INSTANCE_EMBEDDED
>   	}
>   #endif

[U-Boot] [RFC PATCH] Allow providing default environment from file

[Rasmus Villemoes]

On 2018-01-24 10:55, Rasmus Villemoes wrote:
> It is sometimes useful to be able to define the entire default
> environment in an external file. This implements a Kconfig option for
> allowing that.

ping

[U-Boot] [RFC PATCH] Allow providing default environment from file

[Sean Nyekjær]

On 2018-02-02 21:15, Rasmus Villemoes wrote:
> On 2018-01-24 10:55, Rasmus Villemoes wrote:
>> It is sometimes useful to be able to define the entire default
>> environment in an external file. This implements a Kconfig option for
>> allowing that.
> ping
Hi Tom
Will you please take a look a this?
/Sean

[U-Boot] [RFC PATCH] Allow providing default environment from file

[Tom Rini]

On Wed, Jan 24, 2018 at 10:55:59AM +0100, Rasmus Villemoes wrote:

> It is sometimes useful to be able to define the entire default
> environment in an external file. This implements a Kconfig option for
> allowing that.
> 
> It is somewhat annoying to have two visible Kconfig options; it would
> probably be more user-friendly to just have the string option (with
> empty string obviously meaning not to use this feature). But then we'd
> also need a hidden CONFIG that we can use in the #ifdef in
> env_default.h, and I don't think one can set a def_bool based on
> whether a string-valued config is empty or not.
> 
> I've tried to make the accepted format the same as the one the
> mkenvimage tool accepts. I have no idea how portable the sed script
> implementing the "allow embedded newlines in values" is. Nor do I know
> if one can expect xxd to be available.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
>  Makefile              | 16 ++++++++++++++++
>  env/Kconfig           | 18 ++++++++++++++++++
>  include/env_default.h |  4 ++++
>  3 files changed, 38 insertions(+)

Conceptually, this is fine.  But can you please re-word the commit
message and put some of the commentary below the --- ?  Also, in general
we do a pair of CONFIG_USE_xxx and CONFIG_xxx, so you might need to
re-word the rest of the option name a bit too to match that.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180319/5e42360a/attachment.sig>

[U-Boot] [PATCH v2] Allow providing default environment from file

[Rasmus Villemoes]

Modifying the default environment via CONFIG_EXTRA_ENV_SETTINGS is
somewhat inflexible, partly because the cpp language does not allow
appending to an existing macro. This prevents reuse of "environment
fragments" for different boards, which in turn makes maintaining that
environment consistently tedious and error-prone.

This implements a Kconfig option for allowing one to define the entire
default environment in an external file, which can then, for example, be
generated programmatically as part of a Yocto recipe, or simply be kept
in version control separately from the U-boot repository.

Tested-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
v2:
* rename CONFIG_DEFAULT_ENV_FROM_FILE -> CONFIG_USE_DEFAULT_ENV_FILE
* add Tested-by
* provide a little more rationale (example use case instead of just
  "sometimes be useful")
* rebase to current master (v2018.03-189-gda773532cd)

This adds xxd as a build-time requirement. Not sure whether that needs
mentioning in the Kconfig help text. On my Ubuntu 16.04, it is
provided by the vim-common package, while more recent Ubuntu and
Debian seem to have split it to a separate package.

 Makefile              | 16 ++++++++++++++++
 env/Kconfig           | 18 ++++++++++++++++++
 include/env_default.h |  4 ++++
 3 files changed, 38 insertions(+)

diff --git a/Makefile b/Makefile
index 5fa14789d9..867b189c41 100644
--- a/Makefile
+++ b/Makefile
@@ -423,6 +423,7 @@ endif
 
 version_h := include/generated/version_autogenerated.h
 timestamp_h := include/generated/timestamp_autogenerated.h
+defaultenv_h := include/generated/defaultenv_autogenerated.h
 
 no-dot-config-targets := clean clobber mrproper distclean \
 			 help %docs check% coccicheck \
@@ -1387,6 +1388,10 @@ ifeq ($(wildcard $(LDSCRIPT)),)
 	@/bin/false
 endif
 
+ifeq ($(CONFIG_USE_DEFAULT_ENV_FILE),y)
+prepare1: $(defaultenv_h)
+endif
+
 archprepare: prepare1 scripts_basic
 
 prepare0: archprepare FORCE
@@ -1434,12 +1439,23 @@ define filechk_timestamp.h
 	fi)
 endef
 
+define filechk_defaultenv.h
+	(grep -v '^#' | \
+	 grep -v '^$$' | \
+	 tr '\n' '\0' | \
+	 sed -e 's/\\\x0/\n/' | \
+	 xxd -i ; echo ", 0x00" ; )
+endef
+
 $(version_h): include/config/uboot.release FORCE
 	$(call filechk,version.h)
 
 $(timestamp_h): $(srctree)/Makefile FORCE
 	$(call filechk,timestamp.h)
 
+$(defaultenv_h): $(CONFIG_DEFAULT_ENV_FILE:"%"=%) FORCE
+	$(call filechk,defaultenv.h)
+
 # ---------------------------------------------------------------------------
 quiet_cmd_cpp_lds = LDS     $@
 cmd_cpp_lds = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) \
diff --git a/env/Kconfig b/env/Kconfig
index a3c6298273..e8e21dcfc6 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -486,4 +486,22 @@ config ENV_SIZE
 
 endif
 
+config USE_DEFAULT_ENV_FILE
+	bool "Create default environment from file"
+	help
+	  Normally, the default environment is automatically generated
+	  based on the settings of various CONFIG_* options, as well
+	  as the CONFIG_EXTRA_ENV_SETTINGS. By selecting this option,
+	  you can instead define the entire default environment in an
+	  external file.
+
+config DEFAULT_ENV_FILE
+	string "Path to default environment file"
+	depends on USE_DEFAULT_ENV_FILE
+	help
+	  The path containing the default environment. The format is
+	  the same as accepted by the mkenvimage tool: lines
+	  containing key=value pairs, blank lines and lines beginning
+	  with # are ignored.
+
 endmenu
diff --git a/include/env_default.h b/include/env_default.h
index b574345af2..1fbeb92f38 100644
--- a/include/env_default.h
+++ b/include/env_default.h
@@ -22,6 +22,7 @@ static char default_environment[] = {
 #else
 const uchar default_environment[] = {
 #endif
+#ifndef CONFIG_USE_DEFAULT_ENV_FILE
 #ifdef	CONFIG_ENV_CALLBACK_LIST_DEFAULT
 	ENV_CALLBACK_VAR "=" CONFIG_ENV_CALLBACK_LIST_DEFAULT "\0"
 #endif
@@ -108,6 +109,9 @@ const uchar default_environment[] = {
 	CONFIG_EXTRA_ENV_SETTINGS
 #endif
 	"\0"
+#else /* CONFIG_USE_DEFAULT_ENV_FILE */
+#include "generated/defaultenv_autogenerated.h"
+#endif
 #ifdef DEFAULT_ENV_INSTANCE_EMBEDDED
 	}
 #endif
-- 
2.15.1

[U-Boot] [PATCH v2] Allow providing default environment from file

[Lukasz Majewski]

Hi Rasmus,

> Modifying the default environment via CONFIG_EXTRA_ENV_SETTINGS is
> somewhat inflexible, partly because the cpp language does not allow
> appending to an existing macro. This prevents reuse of "environment
> fragments" for different boards, which in turn makes maintaining that
> environment consistently tedious and error-prone.

It is also possible to build boot.scr image, which is afterwards read
from e.g. vfat, from text file.

As a reference and example please look
into ./boards/samsung/common/bootscripts/*.cmd

> 
> This implements a Kconfig option for allowing one to define the entire
> default environment in an external file, which can then, for example,
> be generated programmatically as part of a Yocto recipe, 

Is this yocto generation upstreamed? Or this is some kind of internal
patch?

> or simply be
> kept in version control separately from the U-boot repository.
> 
> Tested-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
> v2:
> * rename CONFIG_DEFAULT_ENV_FROM_FILE -> CONFIG_USE_DEFAULT_ENV_FILE
> * add Tested-by
> * provide a little more rationale (example use case instead of just
>   "sometimes be useful")
> * rebase to current master (v2018.03-189-gda773532cd)
> 
> This adds xxd as a build-time requirement. Not sure whether that needs
> mentioning in the Kconfig help text. On my Ubuntu 16.04, it is
> provided by the vim-common package, while more recent Ubuntu and
> Debian seem to have split it to a separate package.
> 
>  Makefile              | 16 ++++++++++++++++
>  env/Kconfig           | 18 ++++++++++++++++++
>  include/env_default.h |  4 ++++
>  3 files changed, 38 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 5fa14789d9..867b189c41 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -423,6 +423,7 @@ endif
>  
>  version_h := include/generated/version_autogenerated.h
>  timestamp_h := include/generated/timestamp_autogenerated.h
> +defaultenv_h := include/generated/defaultenv_autogenerated.h
>  
>  no-dot-config-targets := clean clobber mrproper distclean \
>  			 help %docs check% coccicheck \
> @@ -1387,6 +1388,10 @@ ifeq ($(wildcard $(LDSCRIPT)),)
>  	@/bin/false
>  endif
>  
> +ifeq ($(CONFIG_USE_DEFAULT_ENV_FILE),y)
> +prepare1: $(defaultenv_h)
> +endif
> +
>  archprepare: prepare1 scripts_basic
>  
>  prepare0: archprepare FORCE
> @@ -1434,12 +1439,23 @@ define filechk_timestamp.h
>  	fi)
>  endef
>  
> +define filechk_defaultenv.h
> +	(grep -v '^#' | \
> +	 grep -v '^$$' | \
> +	 tr '\n' '\0' | \
> +	 sed -e 's/\\\x0/\n/' | \
> +	 xxd -i ; echo ", 0x00" ; )
> +endef
> +
>  $(version_h): include/config/uboot.release FORCE
>  	$(call filechk,version.h)
>  
>  $(timestamp_h): $(srctree)/Makefile FORCE
>  	$(call filechk,timestamp.h)
>  
> +$(defaultenv_h): $(CONFIG_DEFAULT_ENV_FILE:"%"=%) FORCE
> +	$(call filechk,defaultenv.h)
> +
>  #
> ---------------------------------------------------------------------------
> quiet_cmd_cpp_lds = LDS     $@ cmd_cpp_lds = $(CPP)
> -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) \ diff --git
> a/env/Kconfig b/env/Kconfig index a3c6298273..e8e21dcfc6 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -486,4 +486,22 @@ config ENV_SIZE
>  
>  endif
>  
> +config USE_DEFAULT_ENV_FILE
> +	bool "Create default environment from file"
> +	help
> +	  Normally, the default environment is automatically
> generated
> +	  based on the settings of various CONFIG_* options, as well
> +	  as the CONFIG_EXTRA_ENV_SETTINGS. By selecting this option,
> +	  you can instead define the entire default environment in an
> +	  external file.
> +
> +config DEFAULT_ENV_FILE
> +	string "Path to default environment file"
> +	depends on USE_DEFAULT_ENV_FILE
> +	help
> +	  The path containing the default environment. The format is
> +	  the same as accepted by the mkenvimage tool: lines
> +	  containing key=value pairs, blank lines and lines beginning
> +	  with # are ignored.
> +
>  endmenu
> diff --git a/include/env_default.h b/include/env_default.h
> index b574345af2..1fbeb92f38 100644
> --- a/include/env_default.h
> +++ b/include/env_default.h
> @@ -22,6 +22,7 @@ static char default_environment[] = {
>  #else
>  const uchar default_environment[] = {
>  #endif
> +#ifndef CONFIG_USE_DEFAULT_ENV_FILE
>  #ifdef	CONFIG_ENV_CALLBACK_LIST_DEFAULT
>  	ENV_CALLBACK_VAR "=" CONFIG_ENV_CALLBACK_LIST_DEFAULT "\0"
>  #endif
> @@ -108,6 +109,9 @@ const uchar default_environment[] = {
>  	CONFIG_EXTRA_ENV_SETTINGS
>  #endif
>  	"\0"
> +#else /* CONFIG_USE_DEFAULT_ENV_FILE */
> +#include "generated/defaultenv_autogenerated.h"
> +#endif
>  #ifdef DEFAULT_ENV_INSTANCE_EMBEDDED
>  	}
>  #endif

The patch looks ok.

Reviewed-by: Lukasz Majewski <lukma@denx.de>


Best regards,

Lukasz Majewski

--

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: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180320/50d34f4e/attachment.sig>

[U-Boot] [PATCH v2] Allow providing default environment from file

[Rasmus Villemoes]

On 2018-03-20 15:20, Lukasz Majewski wrote:
> Hi Rasmus,
> 
>> Modifying the default environment via CONFIG_EXTRA_ENV_SETTINGS is
>> somewhat inflexible, partly because the cpp language does not allow
>> appending to an existing macro. This prevents reuse of "environment
>> fragments" for different boards, which in turn makes maintaining that
>> environment consistently tedious and error-prone.
> 
> It is also possible to build boot.scr image, which is afterwards read
> from e.g. vfat, from text file.
> 
> As a reference and example please look
> into ./boards/samsung/common/bootscripts/*.cmd
> 
>>
>> This implements a Kconfig option for allowing one to define the entire
>> default environment in an external file, which can then, for example,
>> be generated programmatically as part of a Yocto recipe, 
> 
> Is this yocto generation upstreamed? Or this is some kind of internal
> patch?

Neither, for now, we're just using the "entire environment in file
maintained elsewhere" model. But I can easily see us using the ability
to amend the environment with a simple "echo ... >> foo.env" in some
pre/postfunc, or having foo.env contain some __PLACEHOLDER__ which we
fix up with sed using values from .conf files before the U-boot build.
Stuff like tftp server ip address is nice to be able to override easily
to point at one's own laptop while doing development.

> 
> The patch looks ok.
> 
> Reviewed-by: Lukasz Majewski <lukma@denx.de>

Thanks.

Rasmus

[U-Boot] [PATCH v2] Allow providing default environment from file

[Rasmus Villemoes]

On 2018-03-20 15:47, Rasmus Villemoes wrote:
> On 2018-03-20 15:20, Lukasz Majewski wrote:
>> Hi Rasmus,
>>
>>> Modifying the default environment via CONFIG_EXTRA_ENV_SETTINGS is
>>> somewhat inflexible, partly because the cpp language does not allow
>>> appending to an existing macro. This prevents reuse of "environment
>>> fragments" for different boards, which in turn makes maintaining that
>>> environment consistently tedious and error-prone.
[...]
>>
>> The patch looks ok.
>>
>> Reviewed-by: Lukasz Majewski <lukma@denx.de>

Any chance this could get picked up, or is there anything more needed on
my end?

Thanks,
Rasmus

[U-Boot] [U-Boot, v2] Allow providing default environment from file

[Tom Rini]

On Tue, Mar 20, 2018 at 11:38:45AM +0100, Rasmus Villemoes wrote:

> Modifying the default environment via CONFIG_EXTRA_ENV_SETTINGS is
> somewhat inflexible, partly because the cpp language does not allow
> appending to an existing macro. This prevents reuse of "environment
> fragments" for different boards, which in turn makes maintaining that
> environment consistently tedious and error-prone.
> 
> This implements a Kconfig option for allowing one to define the entire
> default environment in an external file, which can then, for example, be
> generated programmatically as part of a Yocto recipe, or simply be kept
> in version control separately from the U-boot repository.
> 
> Tested-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> Reviewed-by: Lukasz Majewski <lukma@denx.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180407/e094974c/attachment.sig>