Re: [Qemu-devel] [PATCH v6 04/15] Makefile: Rules for docker testing

[Paolo Bonzini]

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