Re: [Buildroot] [PATCH] uboot: allow taking the entire default environment from a text file

[Luca Ceresoli via buildroot <buildroot@buildroot.org>]

Hi Arnout,

On Fri, 12 Jul 2024 11:23:12 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

>   Hi Luca,
> 
> On 17/06/2024 18:00, Luca Ceresoli via buildroot wrote:
> > By default U-Boot builds a default environment from its own configuration
> > and a board-specific set of variables. However it also allows to bypass
> > this entirely and define the entire default environment from a text file.
> > 
> > Expose this feature to Buildroot. This allows to have a file e.g. in
> > board/.../uboot.env which contains an easy to maintain text file with the
> > wanted default environment, without patching the U-Boot source code.  
> 
>   Earlier, Heiko sent a similar patch but using a different U-Boot config option 
> [1]. We're going to merge only one of them :-)

Wrong URL (below). i think you refer to this, dating almost a year ago
so congratulations for your memory:

  https://lore.kernel.org/all/20230804085118.315117-1-heiko.thiery@gmail.com/

Thanks for letting me know about this.


>   This one is a bit more elegant I think because it doesn't require discovering 
> the vendor directory.

Ok, good to know, so let me refine it and send a v2.

> > --- a/boot/uboot/Config.in
> > +++ b/boot/uboot/Config.in
> > @@ -136,6 +136,35 @@ config BR2_TARGET_UBOOT_CONFIG_FRAGMENT_FILES
> >   	  that will be merged to the main U-Boot configuration file.
> >   endif
> >   
> > +config BR2_TARGET_UBOOT_USE_DEFAULT_ENV_FILE
> > +	bool "Generate default environment from text file"  
> 
>   I think that rather than two options, we want just one string option. If it's 
> empty, it is disabled.

OK.

> > +	select BR2_TARGET_UBOOT_NEEDS_XXD
> > +	help
> > +	  Define the entire U-Boot default environment from a file,
> > +	  disabling all the logic to generate it from the U-Boot source
> > +	  code and other configuration values.
> > +
> > +	  Based on the USE_DEFAULT_ENV_FILE and DEFAULT_ENV_FILE U-Boot
> > +	  configuration variables.
> > +
> > +	  Requires U-Boot >= v2018.05.
> > +
> > +config BR2_TARGET_UBOOT_DEFAULT_ENV_FILE
> > +	string "Text file with default environment"
> > +	depends on BR2_TARGET_UBOOT_USE_DEFAULT_ENV_FILE
> > +	help
> > +	  Text file containing the variables to be used as the default
> > +	  environment in U-Boot.
> > +
> > +	  From the U-Boot documentation:
> > +
> > +	    The format is the same as accepted by the mkenvimage tool, with
> > +	    lines containing key=value pairs. Blank lines and lines
> > +	    beginning with '#' are ignored.
> > +
> > +	  For more info see:
> > +	  https://docs.u-boot.org/en/latest/usage/environment.html#external-environment-file  
> 
>   Excellent help text!

Thanks!

[that's because I'm lazy: finding the info was painful, I don't want to
do that again so let's write it down, upstream it and let the Internet
make it available to the future myself :) ]

> > --- a/boot/uboot/uboot.mk
> > +++ b/boot/uboot/uboot.mk
> > @@ -390,6 +390,14 @@ UBOOT_KCONFIG_EDITORS = menuconfig xconfig gconfig nconfig
> >   # override again. In addition, host-ccache is not ready at kconfig
> >   # time, so use HOSTCC_NOCCACHE.
> >   UBOOT_KCONFIG_OPTS = $(UBOOT_MAKE_OPTS) HOSTCC="$(HOSTCC_NOCCACHE)" HOSTLDFLAGS=""
> > +
> > +ifeq ($(BR2_TARGET_UBOOT_USE_DEFAULT_ENV_FILE),y)
> > +UBOOT_DEFAULT_ENV_FILE = $(call qstrip,$(BR2_TARGET_UBOOT_DEFAULT_ENV_FILE))
> > +define UBOOT_KCONFIG_DEFAULT_ENV_FILE
> > +	$(call KCONFIG_SET_OPT,CONFIG_USE_DEFAULT_ENV_FILE,y)
> > +	$(call KCONFIG_SET_OPT,CONFIG_DEFAULT_ENV_FILE,"$(shell readlink -f $(UBOOT_DEFAULT_ENV_FILE))")  
> 
>   I think we mostly use `` rather than $(shell ...) - with the backticks, it's 
> much easier to understand when it's going to be evaluated.

I did some quick tests with the backticks and wasn't able to make it
work, but if you have a suggestion to make it work I'll be glad to try
that.

So I'm sending v2 still using $(shell ...) for the time being.

About which is the "common habit", I found very few cases with both, so
no obvious winner apparently. However in this same file
(boot/uboot/uboot.mk) there are two readlink usages already, both using
the $(shell ...) pattern. Disclaimer: both have been added by me :)

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot