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

["Jan Beulich" <JBeulich@suse.com>]

>>> On 20.12.18 at 16:23, <ian.jackson@citrix.com> wrote:
> Jan Beulich writes ("Re: [PATCH] x86emul: fix test harness and fuzzer build 
> dependencies"):
>> On 20.12.18 at 15:46, <ian.jackson@citrix.com> wrote:
>> > I think this introduces a `make -j' hazard ?  The problem is that this
>> > branch of the Makefiles tree might enter tools/include while
>> > another branch is also doing so, resulting in two simultaneous
>> > executions in the same directory.
>> 
>> What is "another branch" here? So far I was under the impression
>> that the ability of building x86 emulator fuzzer and test harness
>> independently is an exception, and that all other parts of the
>> tools/ subtree are supposed to be built by going through the top
>> level. Otherwise further dependency issues might arise, due to
>> top level Makefile's %-tools-public-headers rule.
>> 
>> Hence whether there's a "make -j" hazard here depends on what
>> that top level rule's purpose is.
> 
> I don't follow.
> 
> The top-level %-tools-public-headers rule is there to be something
> that you can write in the dependencies of other subdirs, to arrange
> that $(MAKE) -C tools/include is run before that other subdir.
> 
> 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.

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 wrote:
> 
>    (It is possible to violate this rule without creating races but it
>   is tricky and inadvisable.)
> 
> If we are determined that it must be possible to run make in the x86
> emulator fuzzer directory *without having previously built the rest of
> the tree normally*, then perhaps it is necessary to do this
> $(MAKE) -C thing.
> 
> 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.

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

Bottom line - can you please be concrete as to what I need to change
in the patch (and in particular in the change to tools/include/Makefile,
as the other pieces are under x86 maintainership anyway) to get your
ack, without requiring me to address issues I don't (currently) care
about, and that have been there before?

Jan



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