All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: Ian Jackson <ian.jackson@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Wei Liu <wei.liu2@citrix.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH] x86emul: fix test harness and fuzzer build dependencies
Date: Mon, 14 Jan 2019 08:44:47 -0700	[thread overview]
Message-ID: <5C3CAE6F020000780020D663@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <23612.42558.631150.227465@mariner.uk.xensource.com>

>>> On 14.01.19 at 16:09, <ian.jackson@citrix.com> wrote:
> Jan Beulich writes ("Re: [Xen-devel] [PATCH] x86emul: fix test harness and 
> fuzzer build dependencies"):
>> On 21.12.18 at 15:16, <ian.jackson@citrix.com> wrote:
>> > Why is this particular inter-directory dependency unusual ?  Do we
>> > plan to introduce similar MAKELEVEL-based invocation of dependency
>> > directory makefiles everyhere ?
>> 
>> If need be, anywhere where independent building of the sub-tree
>> is specifically meant to work as a standalone operation. That
>> invocation of "make -C ..../tools/include && ..." is in particular not
>> meant to be necessary for the test harness'es "run" target. It is
>> also not intended to be used for the fuzzer builds (as per
>> tools/fuzz/README.afl), albeit there one might as well call it a doc
>> omission.
> 
> This interface intent is inconsistent with the design principles for
> recursive make, and can not, in general, be realised.
> 
> It can in some special cases, including I think this one, be realised
> using MAKELEVEL, but only at the cost of significant extra complexity.
> I see you've sent another patch based on MAKELEVEL.  I don't have a
> strong enough objection to this change in this one place to block it.

But from the rest of your reply it sounds as if you don't want to
see it go in either. I'm feeling left in the dark as to how to make
progress here.

> However, I worry is that this is setting a precedent.
> What is wrong is the idea that in general it is OK in xen.git to
> start with a clean tree, or modify an existing build tree,, run
>    make -C some/direct/ory
> and expect everything to be (re)built as needed.  The result of
> trying to implement that everywhere via MAKELEVEL would be awful.
> 
> Can you promise me not to send any more patches based on using
> MAKELEVEL this way ?

I don't intend to, but promise? No. For the fuzzer target I could
accept a doc change making the re-build of the include directory
a necessary prereq step. For the test harness'es "run" target,
though, I don't view this as a viable alternative. The name of the
goal is very clearly (to me at least) indicating that this is
(supposed to be) a standalone one. Any other harnesses
under tools/test/ may want to have something similar (some
already have a "run" target).

>  If not, where does it end ?  Noting that even
> your updated patch does not work for (eg):
>    make -C tools/fuzz

Of course it doesn't; it's not meant to be. I don't see how this
relates here though - it wouldn't work without the change here
either. I'm not even convinced "make -C tools" works, as
opposed to "make tools", due to the very prereq step to (re-)
make tools/include/.

So how do we make progress here? For the two changes that
you dislike I don't formally need your ack, and I have Andrew's.
I would (have to) respect an active NAK of yours, of course.

For the one change that I need your (or Wei's) ack on, I didn't
see any strong objection so far, and this fixes an actual issue
with the overall tools build, i.e. _outside_ of the area you're
concerned about. The $(MAKE) invocation there is not overly
nice, but I thought I did convince you that - with the way
tools/include/ gets dealt with from the top level - this should
not be an issue. Plus I'm just moving it.

Jan



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

  reply	other threads:[~2019-01-14 15:46 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-14  8:49 [PATCH] x86emul: fix test harness and fuzzer build dependencies Jan Beulich
2018-12-17 15:57 ` Andrew Cooper
2018-12-20 14:46 ` Ian Jackson
2018-12-20 14:56   ` Jan Beulich
2018-12-20 15:23     ` Ian Jackson
2018-12-20 16:05       ` Jan Beulich
2018-12-21  7:39         ` Jan Beulich
2018-12-21 14:16           ` Ian Jackson
2019-01-04  9:32             ` Jan Beulich
2019-01-14 15:09               ` Ian Jackson
2019-01-14 15:44                 ` Jan Beulich [this message]
2019-01-14 16:59                   ` Ian Jackson
2019-01-15  8:29                     ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5C3CAE6F020000780020D663@prv1-mh.provo.novell.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.jackson@citrix.com \
    --cc=roger.pau@citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.