Re: [XEN PATCH] tools: Have flex and bison mandatory

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

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