Re: [PATCH v1] readd build dependency for startup_raw.S

[Glenn Washburn]

On Wed, 16 Aug 2023 21:43:25 -0400
Mike Gilbert <floppym@gentoo.org> wrote:

> On Wed, Aug 16, 2023 at 3:33 PM Glenn Washburn
> <development@efficientek.com> wrote:
> >
> > Hi Olaf,
> >
> > There's an extra 'd' in read in the subject line.
> >
> > On Wed, 16 Aug 2023 11:55:01 +0200
> > Olaf Hering <olaf@aepfle.de> wrote:
> >
> > > startup_raw.S includes a generated file, and used to have a dependency
> > > in the Makefile to make sure the generated file exists prior
> > > compilation. This dependency was removed, and as a result building
> > > startup_raw.S randomly fails:
> > >
> > > ../../20230814T181249.4fdcb339b/grub-core/boot/i386/pc/startup_raw.S:129:10:
> > >  fatal error: rs_decoder.h: No such file or directory
> > >   129 | #include <rs_decoder.h>
> > >       |          ^~~~~~~~~~~~~~
> > > compilation terminated.
> > > make: *** [Makefile:34654:
> > > boot/i386/pc/lzma_decompress_image-startup_raw.o] Error 1
> >
> > I don't see this error when building any of the targets that I test.
> > This includes i386-pc, which I'm assuming this error comes from (if not
> > which?). Are you running make if -jN where N > 1? Does rs_decoder.h
> > exist in the fail build directory? (its a generated file) What compiler
> > and version are you using?
> 
> This is almost certainly a race condition with parallel make jobs. It
> might also be possible to trigger it with make --shuffle so that the
> build order of targets is randomized.

Looks like I'm on v4.3 and this was introduced in v4.4. Perhaps you can
test this? It certainly did sound like a race condition, which is why
I was curious about the level of parallelism used. However, it appears
as though the default -j1 was used. The two machines I test on use -j9
and -j25 and I've not seen this yet in the hundreds of runs I've done.
So I'm skeptical of this. All my testing is done one Debian 11.

> > When I look at the contents of
> > ./grub-core/boot/i386/pc/.deps-core/lzma_decompress_image-startup_raw.Po
> > in a successful build directory, I see that GCC figured out that
> > rs_decoder.h is a dependency. So I'm not seeing yet why your build is
> > failing, but the below change *should* be unnecessary.
> 
> I think the .Po files are relevant only for incremental builds -- they
> let the build system detect which files need to be updated when
> headers are modified.
> 
> For an initial clean build, the .Po files will not exist. Any
> dependency on generated source files would need to be declared
> explicitly.

Yes, I now believe you're right, though its not 100% clear why to me.
The .Po files are included via the include make directive, and so I
would think make should fail if they do not exist. But it does seem
true that the .Po files are only generated after compilation. So
perhaps these rules are being included after compilation, which is why
a clean build wouldn't see them but also why make doesn't fail[1].

What I'm seeing is that the generated grub-core/Makefile has
rs_decoder.h in BUILT_SOURCES (via several levels of variable
expansions). This should mean that rs_decoder.h is generated before any
sources are compiled[2]. So it looks good to me. I wonder if Olaf has
an old version of automake that has a bug in it?

[1] https://www.gnu.org/software/make/manual/html_node/Include.html
[2] https://www.gnu.org/software/automake/manual/html_node/Sources.html

> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel