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