[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 04/15] Makefile: Rules for docker testing
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH v6 04/15] Makefile: Rules for docker testing |
Date: |
Tue, 31 May 2016 20:40:02 +0800 |
User-agent: |
Mutt/1.6.1 (2016-04-27) |
On Tue, 05/31 14:02, Paolo Bonzini wrote:
>
>
> On 31/05/2016 13:00, Fam Zheng wrote:
> > On Tue, 05/31 10:51, Paolo Bonzini wrote:
> >>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> >>> new file mode 100644
> >>> index 0000000..372733d
> >>> --- /dev/null
> >>> +++ b/tests/docker/Makefile.include
> >>> @@ -0,0 +1,121 @@
> >>> +# Makefile for Docker tests
> >>> +
> >>> +include $(SRC_PATH)/rules.mak
> >>
> >> Why include this _and_ include tests/docker/Makefile.include from the
> >> top Makefile?
> >
> > This is for quiet-command, which is only conditionally included by top
> > Makefile.
>
> Ah, it's for the case when configure has not been executed yet and the
> toplevel Makefile "assumes we are in the search tree".
>
> >>
> >> I think you should do one of this:
> >>
> >> a) drop this inclusion; nice, but it pollutes the toplevel makefile a bit
> >>
> >> b) do the following:
> >>
> >> - link this file into the build tree in configure
> >>
> >> - include ../../config-host.mak
> >
> > I prefer we support running from the src tree without running configure, but
> > $(MAKE) invocations doesn't propagate make variables such as SRC_PATH...
> >
> >>
> >> - add to the toplevel Makefile a rule like
> >>
> >> docker docker-%:
> >> $(MAKE) -C tests/docker $@
> >
> > ... and explicitly passing it (and $(V), etc.) here seems very ad-hocery.
>
> V and others would be passed down to the recursive make. Only SRC_PATH
> would not be passed down.
Yes, you are right. So it certainly will work, I just preferred recursive
include over resursive make for consistency (with tests/Makefile).
>
> >>
> >> I prefer the latter. Either would make patch 3 unnecessary.
> >
> > Maybe I should make patch 3 a patch to make top Makefile include rules.mak
> > unconditionally?
>
> Yeah, that would be good.
>
> I'm still a bit undecided about the pollution introduced by
> tests/docker/Makefile.include, but I guess that's okay.
I think it's also okay to switch to "make -C tests/docker" for docker targets
(so "make docker" becomes "make -C tests/docker help"), this way the top
Makefile is not touched.
>
> By the way, could you prepare a patch to rename tests/Makefile to
> tests/Makefile.include? It's a good convention.
Sounds good, will do.
Fam
[Qemu-devel] [PATCH v6 05/15] docker: Add images, Fam Zheng, 2016/05/27
[Qemu-devel] [PATCH v6 08/15] docker: Add quick test, Fam Zheng, 2016/05/27
[Qemu-devel] [PATCH v6 09/15] docker: Add full test, Fam Zheng, 2016/05/27
[Qemu-devel] [PATCH v6 07/15] docker: Add common.rc, Fam Zheng, 2016/05/27
[Qemu-devel] [PATCH v6 06/15] docker: Add test runner, Fam Zheng, 2016/05/27
[Qemu-devel] [PATCH v6 10/15] docker: Add clang test, Fam Zheng, 2016/05/27
[Qemu-devel] [PATCH v6 11/15] docker: Add mingw test, Fam Zheng, 2016/05/27
[Qemu-devel] [PATCH v6 14/15] docker: Add EXTRA_CONFIGURE_OPTS, Fam Zheng, 2016/05/27