Patchwork [V9,2/8] Buildsystem clean tests directory clearly

login
register
mail settings
Submitter Paolo Bonzini
Date Nov. 16, 2012, 11:16 a.m.
Message ID <50A620A0.2020102@redhat.com>
Download mbox | patch
Permalink /patch/199544/
State New
Headers show

Comments

Paolo Bonzini - Nov. 16, 2012, 11:16 a.m.
Il 16/11/2012 12:01, Wenchao Xia ha scritto:
>>
>>
>> ifneq ($(wildcard config-host.mak),)
>> include $(SRC_PATH)/tests/Makefile
>> endif
>>
>> to
>>
>> -include $(SRC_PATH)/tests/Makefile
>>
>> ?
>>
>   yes, but original author seems not include tests/Makefile in
> this condition on purpose, so using MAKEFILES to keep it unchanged.

git blame tells you that the ifneq was added by commit e454e2e
(Makefile: Exclude tests/Makefile in unconfigured tree, 2012-01-17).

Instead of the ifneq, I proposed to use a minus in front of the include 
directive.  It will avoid failures if tests/Makefile does not exist.  
However, it is not very good because it will cause /tests/Makefile to 
be opened (see the commit message of e454e2e).

Perhaps an even better way to avoid the problem would be this: just move
the removal of makefile fragments to the _end_ of "make distclean":



With this change, no need for MAKEFILES anymore.

Paolo
Wayne Xia - Nov. 16, 2012, 12:40 p.m.
于 2012-11-16 19:16, Paolo Bonzini 写道:
> Il 16/11/2012 12:01, Wenchao Xia ha scritto:
>>>
>>>
>>> ifneq ($(wildcard config-host.mak),)
>>> include $(SRC_PATH)/tests/Makefile
>>> endif
>>>
>>> to
>>>
>>> -include $(SRC_PATH)/tests/Makefile
>>>
>>> ?
>>>
>>    yes, but original author seems not include tests/Makefile in
>> this condition on purpose, so using MAKEFILES to keep it unchanged.
>
> git blame tells you that the ifneq was added by commit e454e2e
> (Makefile: Exclude tests/Makefile in unconfigured tree, 2012-01-17).
>
> Instead of the ifneq, I proposed to use a minus in front of the include
> directive.  It will avoid failures if tests/Makefile does not exist.
> However, it is not very good because it will cause /tests/Makefile to
> be opened (see the commit message of e454e2e).
>
> Perhaps an even better way to avoid the problem would be this: just move
> the removal of makefile fragments to the _end_ of "make distclean":
>
> diff --git a/Makefile b/Makefile
> index 81c660f..b994109 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -203,9 +203,7 @@ qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated
>
>   gen-out-type = $(subst .,-,$(suffix $@))
>
> -ifneq ($(wildcard config-host.mak),)
>   include $(SRC_PATH)/tests/Makefile
> -endif
>
>   qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
>
> @@ -265,8 +263,7 @@ qemu-%.tar.bz2:
>   	$(SRC_PATH)/scripts/make-release "$(SRC_PATH)" "$(patsubst qemu-%.tar.bz2,%,$@)"
>
>   distclean: clean
> -	rm -f config-host.mak config-host.h* config-host.ld $(DOCS) qemu-options.texi qemu-img-cmds.texi qemu-monitor.texi
> -	rm -f config-all-devices.mak
> +	rm -f config-host.h* config-host.ld $(DOCS) qemu-options.texi qemu-img-cmds.texi qemu-monitor.texi
>   	rm -f roms/seabios/config.mak roms/vgabios/config.mak
>   	rm -f qemu-doc.info qemu-doc.aux qemu-doc.cp qemu-doc.cps qemu-doc.dvi
>   	rm -f qemu-doc.fn qemu-doc.fns qemu-doc.info qemu-doc.ky qemu-doc.kys
> @@ -279,6 +276,7 @@ distclean: clean
>   	rm -rf $$d || exit 1 ; \
>           done
>   	test -f pixman/config.log && make -C pixman distclean
> +	rm -f config-host.mak rm -f config-all-devices.mak
>
>   KEYMAPS=da     en-gb  et  fr     fr-ch  is  lt  modifiers  no  pt-br  sv \
>   ar      de     en-us  fi  fr-be  hr     it  lv  nl         pl  ru     th \
>
>
> With this change, no need for MAKEFILES anymore.
>
> Paolo
>

   I guess "make distclean" for second time would fail in this case,
because $(SRC_PATH) is not set. I found a better way to do it without
MAKEFILES:

-ifneq ($(wildcard config-host.mak),)
-   include $(SRC_PATH)/tests/Makefile
+   include tests/Makefile
-endif

   due to tests/Makefile is added in configure in this patch so it is
always soft linked then include always succeed.
Paolo Bonzini - Nov. 16, 2012, 12:49 p.m.
Il 16/11/2012 13:40, Wenchao Xia ha scritto:
>   I guess "make distclean" for second time would fail in this case,
> because $(SRC_PATH) is not set. I found a better way to do it without
> MAKEFILES:
> 
> -ifneq ($(wildcard config-host.mak),)
> -   include $(SRC_PATH)/tests/Makefile
> +   include tests/Makefile
> -endif
> 
>   due to tests/Makefile is added in configure in this patch so it is
> always soft linked then include always succeed.

Whatever, this seems to be ok.  Please post the complete patch so we can
review it.

Paolo
Wayne Xia - Nov. 18, 2012, 1:09 p.m.
于 2012-11-16 20:49, Paolo Bonzini 写道:
> Il 16/11/2012 13:40, Wenchao Xia ha scritto:
>>    I guess "make distclean" for second time would fail in this case,
>> because $(SRC_PATH) is not set. I found a better way to do it without
>> MAKEFILES:
>>
>> -ifneq ($(wildcard config-host.mak),)
>> -   include $(SRC_PATH)/tests/Makefile
>> +   include tests/Makefile
>> -endif
>>
>>    due to tests/Makefile is added in configure in this patch so it is
>> always soft linked then include always succeed.
>
> Whatever, this seems to be ok.  Please post the complete patch so we can
> review it.
>
> Paolo
>
   OK, reposted build system patches without libqblock. 4th patch of it
is exporting a function in block.c, which have been posted by
others but not upstream yet, pls ignore it.

Patch

diff --git a/Makefile b/Makefile
index 81c660f..b994109 100644
--- a/Makefile
+++ b/Makefile
@@ -203,9 +203,7 @@  qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated
 
 gen-out-type = $(subst .,-,$(suffix $@))
 
-ifneq ($(wildcard config-host.mak),)
 include $(SRC_PATH)/tests/Makefile
-endif
 
 qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
 
@@ -265,8 +263,7 @@  qemu-%.tar.bz2:
 	$(SRC_PATH)/scripts/make-release "$(SRC_PATH)" "$(patsubst qemu-%.tar.bz2,%,$@)"
 
 distclean: clean
-	rm -f config-host.mak config-host.h* config-host.ld $(DOCS) qemu-options.texi qemu-img-cmds.texi qemu-monitor.texi
-	rm -f config-all-devices.mak
+	rm -f config-host.h* config-host.ld $(DOCS) qemu-options.texi qemu-img-cmds.texi qemu-monitor.texi
 	rm -f roms/seabios/config.mak roms/vgabios/config.mak
 	rm -f qemu-doc.info qemu-doc.aux qemu-doc.cp qemu-doc.cps qemu-doc.dvi
 	rm -f qemu-doc.fn qemu-doc.fns qemu-doc.info qemu-doc.ky qemu-doc.kys
@@ -279,6 +276,7 @@  distclean: clean
 	rm -rf $$d || exit 1 ; \
         done
 	test -f pixman/config.log && make -C pixman distclean
+	rm -f config-host.mak rm -f config-all-devices.mak
 
 KEYMAPS=da     en-gb  et  fr     fr-ch  is  lt  modifiers  no  pt-br  sv \
 ar      de     en-us  fi  fr-be  hr     it  lv  nl         pl  ru     th \