[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: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v6 04/15] Makefile: Rules for docker testing |
Date: |
Tue, 31 May 2016 14:02:16 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 |
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.
>>
>> 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.
By the way, could you prepare a patch to rename tests/Makefile to
tests/Makefile.include? It's a good convention.
Thanks,
Paolo
[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