[PATCH] Makefile: make u-boot-initial-env target depend explicitly on scripts_basic

[Rasmus Villemoes]

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