[PATCH v2 1/1] Makefile: rework u-boot-initial-env target

[Pali Rohár]

Hello! This is really much better solution! Few comments are below.

On Friday 28 October 2022 18:18:49 Max Krummenacher wrote:
> From: Max Krummenacher <max.krummenacher at toradex.com>
> 
> With LTO enabled the U-Boot initial environment is no longer stored
> in an easy accessible section in env/common.o. I.e. the section name
> changes from build to build, its content maybe compressed and it is
> annotated with additional data.
> 
> Drop trying to read the initial env with elf tools from the compiler
> specific object file in favour of adding and using a host tool with
> the only functionality of printing the initial env to stdout.
> 
> See also:
> https://lore.kernel.org/all/927b122e-1f62-e790-f5ca-30bae4332c77@foss.st.com/
> 
> Signed-off-by: Max Krummenacher <max.krummenacher at toradex.com>
> 
> ---
> 
> Changes in v2:
> - reworked to build a host tool which prints the configured
>   environment as proposed by Pali Rohár
>   https://lore.kernel.org/u-boot/20221018174827.1393211-1-max.oss.09@gmail.com/
> - renamed patch, v1 used "Makefile: fix u-boot-initial-env target if lto is enabled"
> 
>  Makefile                  |  7 ++++---
>  scripts/.gitignore        |  1 +
>  scripts/Makefile          |  5 +++++
>  scripts/printinitialenv.c | 44 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 54 insertions(+), 3 deletions(-)
>  create mode 100644 scripts/printinitialenv.c
> 
> diff --git a/Makefile b/Makefile
> index 0f1174718f7..2f97d63c398 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2442,9 +2442,10 @@ endif
>  	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
>  
>  quiet_cmd_genenv = GENENV  $@
> -cmd_genenv = $(OBJCOPY) --dump-section .rodata.default_environment=$@ env/common.o; \
> -	sed --in-place -e 's/\x00/\x0A/g' $@; sed --in-place -e '/^\s*$$/d' $@; \
> -	sort --field-separator== -k1,1 --stable $@ -o $@
> +cmd_genenv = \
> +	scripts/printinitialenv | \
> +	sed -e 's/\x00/\x0A/g' -e '/^\s*$$/d' | \

I think that you do not need this sed anymore as you print newline in
host tool.

> +	sort --field-separator== -k1,1 --stable -o $@
>  
>  u-boot-initial-env: u-boot.bin

It is needed to update dependencies for u-boot-initial-env target. Now
it does not depend on u-boot.bin but rather on printinitialenv tool.

>  	$(call if_changed,genenv)
> diff --git a/scripts/.gitignore b/scripts/.gitignore
> index 08cc824bac3..6068724a0d4 100644
> --- a/scripts/.gitignore
> +++ b/scripts/.gitignore
> @@ -2,3 +2,4 @@
>  # Generated files
>  #
>  bin2c
> +printinitialenv
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 8731e6cecd7..ba993820571 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -5,8 +5,13 @@
>  # ---------------------------------------------------------------------------
>  
>  hostprogs-$(CONFIG_BUILD_BIN2C)		+= bin2c
> +hostprogs-y				+= printinitialenv

I'm not sure if the scripts/ is the correct directory for the this tool.
Maybe it should be in tools? Lets wait what maintainers or Tom say.

>  
>  always		:= $(hostprogs-y)
>  
> +HOSTCFLAGS_printinitialenv.o += \
> +		$(patsubst -I%,-idirafter%, $(filter -I%, $(UBOOTINCLUDE))) \
> +		-DUSE_HOSTCC
> +
>  # Let clean descend into subdirs
>  subdir-	+= basic kconfig dtc
> diff --git a/scripts/printinitialenv.c b/scripts/printinitialenv.c
> new file mode 100644
> index 00000000000..c58b234d679
> --- /dev/null
> +++ b/scripts/printinitialenv.c
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2022
> + * Max Krummenacher, Toradex
> + *
> + * Snippets taken from tools/env/fw_env.c
> + *
> + * This prints the list of default environment variables as currently
> + * configured.
> + *
> + */
> +
> +#include <stdio.h>
> +
> +/* Pull in the current config to define the default environment */
> +#include <linux/kconfig.h>
> +
> +#ifndef __ASSEMBLY__
> +#define __ASSEMBLY__ /* get only #defines from config.h */
> +#include <config.h>
> +#undef	__ASSEMBLY__
> +#else
> +#include <config.h>
> +#endif
> +
> +#define DEFAULT_ENV_INSTANCE_STATIC
> +#include <generated/environment.h>
> +#include <env_default.h>
> +
> +int main(void)
> +{
> +	char *env, *nxt;
> +
> +	for (env = default_environment; *env; env = nxt + 1) {
> +		for (nxt = env; *nxt; ++nxt) {
> +			if (nxt >= &default_environment[sizeof(default_environment)]) {
> +				fprintf(stderr, "## Error: environment not terminated\n");
> +				return -1;
> +			}
> +		}
> +		printf("%s\n", env);
> +	}
> +	return 0;
> +}
> -- 
> 2.35.3
>