diff mbox series

envtools lack extra settings since commit 86b9c3e4e4 ("env: Allow U-Boot scripts to be placed in

Message ID 35cf6d244f1207b640c46caa43010b2164deab6b.1683785212.git.christophe.leroy@csgroup.eu
State Accepted
Commit 40b77f2a3ac13a7547c1b7c9c51a4090869de8f4
Delegated to: Tom Rini
Headers show
Series envtools lack extra settings since commit 86b9c3e4e4 ("env: Allow U-Boot scripts to be placed in | expand

Commit Message

Christophe Leroy May 11, 2023, 6:16 a.m. UTC 
After converting my targets from CFG_EXTRA_ENV_SETTINGS to
CONFIG_EXTRA_ENV_TEXT as suggested by Tom, I discovered that
fw_setenv doesn't set the entire defaut environment anymore.

I tried to fix it with the below patch, but it fails qemu-x86 CI test,
see https://source.denx.de/u-boot/custodians/u-boot-mpc8xx/-/pipelines/16326
That's the only CI test that fails AFAICS.

Could you help with a solution ? This needs to be fixed.

Thanks
Christophe

---- >8 ----
From: Christophe Leroy <christophe.leroy@csgroup.eu>
Subject: [RFC PATCH] envtools: Fix default environment

After converting some targets from CFG_EXTRA_ENV_SETTINGS to
CONFIG_EXTRA_ENV_TEXT, default environment embedded in
fw_env tool missed all extra settings.

Commit 86b9c3e4e4 ("env: Allow U-Boot scripts to be placed in
a .env file") restricted the inclusion of the content of that
file to builds without USE_HOSTCC.

But as mentionned in commit 79fc0c5f49 ("tools/env: cross-compile
fw_printenv without setting HOSTCC"), HOSTCC and USE_HOSTCC are
kept for code re-use.

Remove the restricting so that settings included in a .env
file are also added to fw_env tool.

Fixes: 86b9c3e4e4 ("env: Allow U-Boot scripts to be placed in a .env file")
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 Makefile              | 2 +-
 include/env_default.h | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

Comments

Tom Rini May 11, 2023, 12:50 p.m. UTC | #1 
On Thu, May 11, 2023 at 08:16:49AM +0200, Christophe Leroy wrote:

> After converting my targets from CFG_EXTRA_ENV_SETTINGS to
> CONFIG_EXTRA_ENV_TEXT as suggested by Tom, I discovered that
> fw_setenv doesn't set the entire defaut environment anymore.
> 
> I tried to fix it with the below patch, but it fails qemu-x86 CI test,
> see https://source.denx.de/u-boot/custodians/u-boot-mpc8xx/-/pipelines/16326
> That's the only CI test that fails AFAICS.
> 
> Could you help with a solution ? This needs to be fixed.

Sometimes qemu fails in random ways, and the job just needs to be
re-run.  I hit re-run on that one now.
Christophe Leroy May 11, 2023, 1:32 p.m. UTC | #2 
Le 11/05/2023 à 14:50, Tom Rini a écrit :
> On Thu, May 11, 2023 at 08:16:49AM +0200, Christophe Leroy wrote:
> 
>> After converting my targets from CFG_EXTRA_ENV_SETTINGS to
>> CONFIG_EXTRA_ENV_TEXT as suggested by Tom, I discovered that
>> fw_setenv doesn't set the entire defaut environment anymore.
>>
>> I tried to fix it with the below patch, but it fails qemu-x86 CI test,
>> see https://source.denx.de/u-boot/custodians/u-boot-mpc8xx/-/pipelines/16326
>> That's the only CI test that fails AFAICS.
>>
>> Could you help with a solution ? This needs to be fixed.
> 
> Sometimes qemu fails in random ways, and the job just needs to be
> re-run.  I hit re-run on that one now.
> 

Gosh. It passed now.

So you can take the patch as is under the scisor line. Or you prefer I 
resend it ?

Thanks
Christophe
Tom Rini May 11, 2023, 1:57 p.m. UTC | #3 
On Thu, May 11, 2023 at 01:32:23PM +0000, Christophe Leroy wrote:
> 
> 
> Le 11/05/2023 à 14:50, Tom Rini a écrit :
> > On Thu, May 11, 2023 at 08:16:49AM +0200, Christophe Leroy wrote:
> > 
> >> After converting my targets from CFG_EXTRA_ENV_SETTINGS to
> >> CONFIG_EXTRA_ENV_TEXT as suggested by Tom, I discovered that
> >> fw_setenv doesn't set the entire defaut environment anymore.
> >>
> >> I tried to fix it with the below patch, but it fails qemu-x86 CI test,
> >> see https://source.denx.de/u-boot/custodians/u-boot-mpc8xx/-/pipelines/16326
> >> That's the only CI test that fails AFAICS.
> >>
> >> Could you help with a solution ? This needs to be fixed.
> > 
> > Sometimes qemu fails in random ways, and the job just needs to be
> > re-run.  I hit re-run on that one now.
> > 
> 
> Gosh. It passed now.
> 
> So you can take the patch as is under the scisor line. Or you prefer I 
> resend it ?

I'll pick it up, thanks.
Tom Rini May 16, 2023, 3:21 p.m. UTC | #4 
On Thu, May 11, 2023 at 08:16:49AM +0200, Christophe Leroy wrote:

> After converting my targets from CFG_EXTRA_ENV_SETTINGS to
> CONFIG_EXTRA_ENV_TEXT as suggested by Tom, I discovered that
> fw_setenv doesn't set the entire defaut environment anymore.
> 
> I tried to fix it with the below patch, but it fails qemu-x86 CI test,
> see https://source.denx.de/u-boot/custodians/u-boot-mpc8xx/-/pipelines/16326
> That's the only CI test that fails AFAICS.
> 
> Could you help with a solution ? This needs to be fixed.
> 
> Thanks
> Christophe
> 

Resending with corrected addresses, applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index fb02bba08f..a5ab5e3da9 100644
--- a/Makefile
+++ b/Makefile
@@ -2119,7 +2119,7 @@  tools/version.h: include/version.h
 	$(Q)mkdir -p $(dir $@)
 	$(call if_changed,copy)
 
-envtools: scripts_basic $(version_h) $(timestamp_h) tools/version.h
+envtools: u-boot-initial-env scripts_basic $(version_h) $(timestamp_h) tools/version.h
 	$(Q)$(MAKE) $(build)=tools/env
 
 tools-only: export TOOLS_ONLY=y
diff --git a/include/env_default.h b/include/env_default.h
index c0df39d62f..b16c22d5a2 100644
--- a/include/env_default.h
+++ b/include/env_default.h
@@ -10,9 +10,7 @@ 
 #include <env_callback.h>
 #include <linux/stringify.h>
 
-#ifndef USE_HOSTCC
 #include <generated/environment.h>
-#endif
 
 #ifdef DEFAULT_ENV_INSTANCE_EMBEDDED
 env_t embedded_environment __UBOOT_ENV_SECTION__(environment) = {