Re: [Xen-devel] [PATCH] x86emul: fix test harness and fuzzer build dependencies

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Jan Beulich writes ("Re: [Xen-devel] [PATCH] x86emul: fix test harness and 
fuzzer build dependencies"):
> On 20.12.18 at 17:05, <JBeulich@xxxxxxxx> wrote:
>> On 20.12.18 at 16:23, <ian.jackson@xxxxxxxxxx> wrote:
> >> Ie, it is there to satisfy the requirement I mention above, that the
> >> dependency directory is built first.
> > 
> > Which effectively means anything underneath tools/ (and whatever
> > else subdir which depend on one of said rules) is liable to fail to
> > build without having come through this (top level) rule.

Yes.

> > But this is not a property this patch introduces or changes. It just
> > re-arranges how things get done. That is, I'd like to ask for the
> > change to be acked (or a concrete proposal be made for what
> > needs to change) _without_ fixing breakage that might be there
> > and the introduction of which you may have missed, or else I'm
> > sure you would have commented on what is now eddf9559c9.

I'm afraid I don't follow this line of argument.  The uncommitted
patch we are now discussing introduce a new -j-unsafe call, which
could cause stochastic build failures.

And AFAICT the only thing the patch-under-discussion improves is to
relax (in some situations) the requirement to run
  make -C ..../tools/include && make
instead of just make, in x86emul.  That requirement is the one which I
am saying is entirely normal when using recursive make so it should
not be a surprise.

AFAICT eddf9559c9 doesn't seem to have this problem, although I could
be wrong.

> >> But in that case we need to make sure that either:
> >> 
> >>  A. 1. The top-level Makefiles ensure that *a* build of
> >>        tools/include completes *before* starting to enter
> >>        tools/fuzz/x86_instruction_emulator.  (Which I
> >>        think is the case.)
> > 
> > Yes, by said dependency on %-tools-public-headers.

Right.

> >>     AND
> >> 
> >>     2. make -C tools/include is definitely completely read-only if the
> >>        thing has already been built.  (This is somewhat hard to check
> >>        and maintain, and would need a comment in that Makefile to
> >>        ensure that this property is preserved.)
> > 
> > Isn't that a property that's supposed to hold for almost all (sub)trees,
> > i.e. it's rather the exception that an already built tree will see further
> > changes when re-built incrementally without the sources having
> > changed? That said, we have to consider such a case here, due to
> > the use of move-if-changed in xen/include/xen/lib/x86/Makefile. But
> > this still doesn't violate the fully-read-only requirement, as the rule's
> > commands won't be executed again when the dependencies are
> > older than the target (as is going to be the case after the invocation
> > of the build from the top level Makefile).
> 
> So the conclusion was wrong here. If the dependencies changed
> (as in are newer than the target) but the target file is the same as
> before, move-if-changed will hide the effect from consumers of the
> produced file, but the temporary file created gets in the way of -j
> here.

Indeed.

> There being just one invocation of the build in tools/include/ prior
> to the patch here (and hence no race afaict), would you consider
> it reasonable to make the two new invocations dependent upon
> $(MAKELEVEL), thus protecting things in the recursive case?

I don't think this is particularly nice, although I wouldn't block it.

Why is this particular inter-directory dependency unusual ?  Do we
plan to introduce similar MAKELEVEL-based invocation of dependency
directory makefiles everyhere ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel