[PATCH] Makefile: make u-boot-initial-env target depend explicitly on scripts_basic
We're seeing sporadic errors like
ENVC include/generated/env.txt HOSTCC scripts/basic/fixdep ENVP include/generated/env.in ENVT include/generated/environment.h HOSTCC tools/printinitialenv /bin/sh: 1: scripts/basic/fixdep: not found make[1]: *** [scripts/Makefile.host:95: tools/printinitialenv] Error 127 make[1]: *** Deleting file 'tools/printinitialenv' make: *** [Makefile:2446: u-boot-initial-env] Error 2 make: *** Waiting for unfinished jobs....
where sometimes the "fixdep: not found" is instead "fixdep: Permission denied" and the Error 127 becomes 126.
This smells like a race condition, and indeed it is: Currently, u-boot-initial-env is a prerequisite of the envtools target, which also lists scripts_basic as a prerequisite:
envtools: u-boot-initial-env scripts_basic $(version_h) $(timestamp_h) tools/version.h $(Q)$(MAKE) $(build)=tools/env
However, the u-boot-initial-env rule involves building the printinitialenv helper, which in turn is built using an if_changed_dep rule. That means we must ensure scripts/basic/fixdep is built and ready before trying to build printinitialenv, i.e. the u-boot-initial-env rule itself must depend on the phony scripts_basic target.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index 8af1fa9468..069c03696a 100644 --- a/Makefile +++ b/Makefile @@ -2447,7 +2447,7 @@ cmd_genenv = \ sed -e '/^\s*$$/d' | \ sort -t '=' -k 1,1 -s -o $@
-u-boot-initial-env: $(env_h) FORCE +u-boot-initial-env: scripts_basic $(env_h) FORCE $(Q)$(MAKE) $(build)=tools $(objtree)/tools/printinitialenv $(call if_changed,genenv)
On Tue, 3 Oct 2023 at 04:02, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
We're seeing sporadic errors like
ENVC include/generated/env.txt HOSTCC scripts/basic/fixdep ENVP include/generated/env.in ENVT include/generated/environment.h HOSTCC tools/printinitialenv /bin/sh: 1: scripts/basic/fixdep: not found make[1]: *** [scripts/Makefile.host:95: tools/printinitialenv] Error 127 make[1]: *** Deleting file 'tools/printinitialenv' make: *** [Makefile:2446: u-boot-initial-env] Error 2 make: *** Waiting for unfinished jobs....
where sometimes the "fixdep: not found" is instead "fixdep: Permission denied" and the Error 127 becomes 126.
This smells like a race condition, and indeed it is: Currently, u-boot-initial-env is a prerequisite of the envtools target, which also lists scripts_basic as a prerequisite:
envtools: u-boot-initial-env scripts_basic $(version_h) $(timestamp_h) tools/version.h $(Q)$(MAKE) $(build)=tools/env
However, the u-boot-initial-env rule involves building the printinitialenv helper, which in turn is built using an if_changed_dep rule. That means we must ensure scripts/basic/fixdep is built and ready before trying to build printinitialenv, i.e. the u-boot-initial-env rule itself must depend on the phony scripts_basic target.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
I have wondered for a while if we could have a few tests of the form:
- build sandbox - delete an output file - build again - check that the build succeeds and the file is there
Regards, Simon
On 04/10/2023 04.10, Simon Glass wrote:
On Tue, 3 Oct 2023 at 04:02, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
We're seeing sporadic errors like
ENVC include/generated/env.txt HOSTCC scripts/basic/fixdep ENVP include/generated/env.in ENVT include/generated/environment.h HOSTCC tools/printinitialenv /bin/sh: 1: scripts/basic/fixdep: not found make[1]: *** [scripts/Makefile.host:95: tools/printinitialenv] Error 127 make[1]: *** Deleting file 'tools/printinitialenv' make: *** [Makefile:2446: u-boot-initial-env] Error 2 make: *** Waiting for unfinished jobs....
where sometimes the "fixdep: not found" is instead "fixdep: Permission denied" and the Error 127 becomes 126.
This smells like a race condition, and indeed it is: Currently, u-boot-initial-env is a prerequisite of the envtools target, which also lists scripts_basic as a prerequisite:
envtools: u-boot-initial-env scripts_basic $(version_h) $(timestamp_h) tools/version.h $(Q)$(MAKE) $(build)=tools/env
However, the u-boot-initial-env rule involves building the printinitialenv helper, which in turn is built using an if_changed_dep rule. That means we must ensure scripts/basic/fixdep is built and ready before trying to build printinitialenv, i.e. the u-boot-initial-env rule itself must depend on the phony scripts_basic target.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
Thanks.
Tom, can you pick this up, it's a pretty annoying race to hit in our CI, and I'd like an upstream sha1 to cherry-pick from.
I have wondered for a while if we could have a few tests of the form:
- build sandbox
- delete an output file
- build again
- check that the build succeeds and the file is there
Well, in this case one would have to delete two files I think, both the printinitialenv and the fixdep binaries.
I dug a little further, and while I can reproduce with sandbox_defconfig by just building, deleting those two files, and building again, I think one reason we hit it much more easily in our actual setup (i.e. without such manual intervention) is that we set TOOLS_DEBUG=y, and because of the way that option ends up affecting the make rules (in particular, the .config is not always read in by make), fixdep ends up getting rebuilt several times due to command line change (the -g coming and going). I don't know if there's a way to fix that, or why we don't just always build the host tools with debug info (I added that option back when I had to debug some weird mkimage failure). But regardless, this patch is needed for correctness.
Rasmus
On Tue, Oct 03, 2023 at 12:02:17PM +0200, Rasmus Villemoes wrote:
We're seeing sporadic errors like
ENVC include/generated/env.txt HOSTCC scripts/basic/fixdep ENVP include/generated/env.in ENVT include/generated/environment.h HOSTCC tools/printinitialenv /bin/sh: 1: scripts/basic/fixdep: not found make[1]: *** [scripts/Makefile.host:95: tools/printinitialenv] Error 127 make[1]: *** Deleting file 'tools/printinitialenv' make: *** [Makefile:2446: u-boot-initial-env] Error 2 make: *** Waiting for unfinished jobs....
where sometimes the "fixdep: not found" is instead "fixdep: Permission denied" and the Error 127 becomes 126.
This smells like a race condition, and indeed it is: Currently, u-boot-initial-env is a prerequisite of the envtools target, which also lists scripts_basic as a prerequisite:
envtools: u-boot-initial-env scripts_basic $(version_h) $(timestamp_h) tools/version.h $(Q)$(MAKE) $(build)=tools/env
However, the u-boot-initial-env rule involves building the printinitialenv helper, which in turn is built using an if_changed_dep rule. That means we must ensure scripts/basic/fixdep is built and ready before trying to build printinitialenv, i.e. the u-boot-initial-env rule itself must depend on the phony scripts_basic target.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (3)
-
Rasmus Villemoes -
Simon Glass -
Tom Rini