On 04/10/2023 04.10, Simon Glass wrote: > On Tue, 3 Oct 2023 at 04:02, Rasmus Villemoes > <rasmus.villem...@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.villem...@prevas.dk> >> --- >> Makefile | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> > > Reviewed-by: Simon Glass <s...@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
