On Tue, Feb 07, 2023 at 06:03:12PM +0100, Jan Beulich wrote:
> On 07.02.2023 17:09, Anthony PERARD wrote:
> > Both are already mandatory to build the hypervisor.
>
> I'm not sure this is sufficient as a justification. From all we can
> tell even pretty old tool versions are okay for kconfig's purposes.
> However, pretty recently I've learned that some linker script language
> construct used for GNU ld runs into a bug in certain (old) versions of
> flex. Use of that construct will then cause an (almost?) infinite loop
> in ld. Therefore the question is whether libxlu's uses are immune to
> such issues (along the lines of kconfig apparently being).
>
> That said - I'm happy about the change in principle; if so desired we
> could simply see if anyone ever runs into an issue, and revert if need
> be. Nevertheless I'm not convinced it'll address the problem Andrew
> had noticed in CI (and where the consideration to remove the generated
Indeed, it won't fix the issue.
> files originated). It is likely to mask the issue in CI (simply
> because, aiui, there are no incremental builds done there), but that
> won't prevent people running into it on other occasions.
> > This will help avoid cases where the *.y or *.l files are been updated
> > but flex and bison aren't available.
>
> This is odd: How will this "help"? Right now the build ought to fail
> (it doesn't, there's merely a warning, which might be easily missed).
> With your change configure will fail when the tools aren't there.
The scenario I was thinking about is when someone edit the *.l source
file, and try to rebuild without flex been installed. It might take
sometime to find-out that the reason change aren't taken into account is
because flex is missing. At least there's a warning, but it is buried
within the rest of the build log.
> > This also remove the way the missing binaries are been handled, with
> > double-column-rules, which might be an issue sometime.
>
> These double-colon rules should never have been introduced. Double
> colons have a different meaning ("terminal rule") for pattern rules.
> In fact they were my initial suspects when looking into that odd build
> failure in CI.
After some more investigation, I don't think anymore the double-colon
rules here is an issue.
I think the issue that Andrew saw in the CI with "libxlu_cfg_y.o"
failing to build while "libxlu_cfg_l.[ch]" are been regenerated is
probably because make doesn't consider "libxlu_cfg_l.[ch]" as a group of
targets.
If for some reason we have:
libxlu_cfg_l.h newer than libxlu_cfg_l.l newer than libxlu_cfg_l.c
Then make seems to take two parallel decision:
When building libxlu_cfg_y.o:
libxlu_cfg_l.h is newer than libxlu_cfg_l.l
-> libxlu_cfg_l.h is up-to-date, start building libxlu_cfg_y.o
When building libxlu_cfg_l.o:
libxlu_cfg_l.c is older than libxlu_cfg_l.l
-> we need to generate libxlu_cfg_l.c first
Then, libxlu_cfg_y.o fails to build because libxlu_cfg_l.h is been
updated do to the parallel build of libxlu_cfg_l.o.
I can easily reproduce the issue with:
touch libxlu_cfg_l.c; sleep .1; touch libxlu_cfg_l.l; sleep .1;
touch libxlu_cfg_l.h; sleep .1; make -j
And having "sleep 1" in "%.c %.h: %.l" recipe, between `rm` and `flex`.
I don't know how to properly fix this yet.
Event writing "%.c %.h &: %.l" doesn't work, with make 4.3, which is
supposed to be grouped targets. But that's is maybe fixed in 4.4.
> > Adding ".SECONDARY:" to avoid "libxlu_cfg_y.c" been deleted by make
> > when building the library, and regenerating the file on the first
> > incremental build.
>
> While probably okay here, I'd still like to ask: Are you sure you
> don't want to specify the files we care about, rather than applying it
> to everything (by leaving blank the right side of the colon)?
I guess it would be better to specify each targets.
Thanks,
--
Anthony PERARD
