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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.02.2023 19:02, Anthony PERARD wrote:
> 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.

Right, as said - imo this should be (have been) an error. In that sense
your change of course helps because it makes the issue more apparent.

>>> 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`.

Interesting. In my somewhat similar experiments I found that an issue
exists only when one of the .c files is missing. Things looked to work
properly when both were there, but one was outdated. Clearly that's
yet more surprising behavior than, as you appear to observe, the issue
simply being tied too "needs updating".

> 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.

Well, in my tests while 4.4 has fixed one issue with intermediate files
affecting the behavior here, the issue we're possibly running into is
still there. I've sent an inquiry to bug-make@xxxxxxx.

Jan