[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH] tools: Have flex and bison mandatory
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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |