[Qemu-devel] [PATCH] build-sys: generate tests/.gitignore

[Qemu-devel] [PATCH] build-sys: generate tests/.gitignore

[Marc-André Lureau]

tests/.gitignore is often out of date. Let's generate it based on the
files and directories to clean.

Note: I didn't succeed yet at generalizing this approach for the rest
of qemu .gitignore files, but I hope it may eventually happen.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 Makefile                   |  7 +++-
 tests/.gitignore           | 97 ----------------------------------------------
 tests/Makefile.include     | 39 +++++++++++++++++--
 tests/migration/.gitignore |  2 -
 4 files changed, 41 insertions(+), 104 deletions(-)
 delete mode 100644 tests/.gitignore
 delete mode 100644 tests/migration/.gitignore

diff --git a/Makefile b/Makefile
index 81447b1f08..89d5edf531 100644
--- a/Makefile
+++ b/Makefile
@@ -6,7 +6,7 @@ BUILD_DIR=$(CURDIR)
 # Before including a proper config-host.mak, assume we are in the source tree
 SRC_PATH=.
 
-UNCHECKED_GOALS := %clean TAGS cscope ctags docker docker-%
+UNCHECKED_GOALS := %clean TAGS cscope ctags docker docker-% gitignore
 
 # All following code might depend on configuration variables
 ifneq ($(wildcard config-host.mak),)
@@ -14,6 +14,11 @@ ifneq ($(wildcard config-host.mak),)
 all:
 include config-host.mak
 
+.PHONY: gitignore
+ifneq ($(filter-out $(UNCHECKED_GOALS),$(MAKECMDGOALS)),$(if $(MAKECMDGOALS),,fail))
+all $(MAKECMDGOALS): gitignore
+endif
+
 # Check that we're not trying to do an out-of-tree build from
 # a tree that's been used for an in-tree build.
 ifneq ($(realpath $(SRC_PATH)),$(realpath .))
diff --git a/tests/.gitignore b/tests/.gitignore
deleted file mode 100644
index fed0189a5a..0000000000
--- a/tests/.gitignore
+++ /dev/null
@@ -1,97 +0,0 @@
-atomic_add-bench
-benchmark-crypto-cipher
-benchmark-crypto-hash
-benchmark-crypto-hmac
-check-qdict
-check-qnum
-check-qjson
-check-qlist
-check-qnull
-check-qstring
-check-qom-interface
-check-qom-proplist
-qht-bench
-rcutorture
-test-aio
-test-aio-multithread
-test-arm-mptimer
-test-base64
-test-bitops
-test-bitcnt
-test-blockjob
-test-blockjob-txn
-test-bufferiszero
-test-char
-test-clone-visitor
-test-coroutine
-test-crypto-afsplit
-test-crypto-block
-test-crypto-cipher
-test-crypto-hash
-test-crypto-hmac
-test-crypto-ivgen
-test-crypto-pbkdf
-test-crypto-secret
-test-crypto-tlscredsx509
-test-crypto-tlscredsx509-work/
-test-crypto-tlscredsx509-certs/
-test-crypto-tlssession
-test-crypto-tlssession-work/
-test-crypto-tlssession-client/
-test-crypto-tlssession-server/
-test-crypto-xts
-test-cutils
-test-hbitmap
-test-hmp
-test-int128
-test-iov
-test-io-channel-buffer
-test-io-channel-command
-test-io-channel-command.fifo
-test-io-channel-file
-test-io-channel-file.txt
-test-io-channel-socket
-test-io-channel-tls
-test-io-task
-test-keyval
-test-logging
-test-mul64
-test-opts-visitor
-test-qapi-event.[ch]
-test-qapi-types.[ch]
-test-qapi-util
-test-qapi-visit.[ch]
-test-qdev-global-props
-test-qemu-opts
-test-qdist
-test-qga
-test-qht
-test-qht-par
-test-qmp-commands
-test-qmp-commands.h
-test-qmp-event
-test-qobject-input-strict
-test-qobject-input-visitor
-test-qmp-introspect.[ch]
-test-qmp-marshal.c
-test-qobject-output-visitor
-test-rcu-list
-test-replication
-test-shift128
-test-string-input-visitor
-test-string-output-visitor
-test-thread-pool
-test-throttle
-test-timed-average
-test-uuid
-test-visitor-serialization
-test-vmstate
-test-write-threshold
-test-x86-cpuid
-test-x86-cpuid-compat
-test-xbzrle
-test-netfilter
-test-filter-mirror
-test-filter-redirector
-*-test
-qapi-schema/*.test.*
diff --git a/tests/Makefile.include b/tests/Makefile.include
index f08b7418f0..e94671e879 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -901,8 +901,34 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json
 check-tests/qapi-schema/doc-good.texi: tests/qapi-schema/doc-good.test.texi
 	@diff -q $(SRC_PATH)/tests/qapi-schema/doc-good.texi $<
 
-# Consolidated targets
+tests-cleanfiles = *.o
+tests-cleanfiles += .gitignore
+tests-cleanfiles += qht-bench$(EXESUF)
+tests-cleanfiles += qapi-schema/*.test.*
+tests-cleanfiles += test-qapi-event.[ch]
+tests-cleanfiles += test-qapi-types.[ch]
+tests-cleanfiles += test-qapi-visit.[ch]
+tests-cleanfiles += test-qmp-introspect.[ch]
+tests-cleanfiles += test-qmp-commands.h
+tests-cleanfiles += test-qmp-marshal.c
+tests-cleanfiles += $(subst tests/,,$(check-unit-y))
+tests-cleanfiles += $(subst tests/,,$(check-speed-y))
+tests-cleanfiles += $(subst tests/,,$(check-block-y))
+tests-cleanfiles += $(subst tests/,,$(check-qtest-y))
+tests-cleanfiles += $(subst tests/,,$(QEMU_IOTESTS_HELPERS-y))
+tests-cleanfiles += migration/initrd-stress.img
+tests-cleanfiles += migration/stress$(EXESUF)
+tests-cleanfiles += atomic_add-bench$(EXESUF)
+tests-cleanfiles += test-io-channel-file.txt
+tests-cleanfiles += test-io-channel-command.fifo
+
+tests-cleandirs += test-crypto-tlscredsx509-certs/
+tests-cleandirs += test-crypto-tlscredsx509-work/
+tests-cleandirs += test-crypto-tlssession-client/
+tests-cleandirs += test-crypto-tlssession-server/
+tests-cleandirs += test-crypto-tlssession-work/
 
+# Consolidated targets
 .PHONY: check-qapi-schema check-qtest check-unit check check-clean
 check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) check-tests/qapi-schema/doc-good.texi
 check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
@@ -912,15 +938,20 @@ check-block: $(patsubst %,check-%, $(check-block-y))
 check: check-qapi-schema check-unit check-qtest
 check-clean:
 	$(MAKE) -C tests/tcg clean
-	rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
-	rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST), $(check-qtest-$(target)-y)) $(check-qtest-generic-y))
-
+	rm -f $(addprefix tests/, $(tests-cleanfiles))
+	rm -rf $(addprefix tests/, $(tests-cleandirs))
 clean: check-clean
 
 # Build the help program automatically
 
 all: $(QEMU_IOTESTS_HELPERS-y)
 
+$(SRC_PATH)/tests/.gitignore: $(MAKEFILE_LIST)
+	$(call quiet-command, echo "$(tests-cleanfiles)" "$(tests-cleandirs)" | \
+		xargs -n1 | sort | uniq | sed -e s:^:/: > $@,"GEN","$(@F)")
+
+gitignore: $(SRC_PATH)/tests/.gitignore
+
 -include $(wildcard tests/*.d)
 -include $(wildcard tests/libqos/*.d)
 
diff --git a/tests/migration/.gitignore b/tests/migration/.gitignore
deleted file mode 100644
index 84f37552e4..0000000000
--- a/tests/migration/.gitignore
+++ /dev/null
@@ -1,2 +0,0 @@
-initrd-stress.img
-stress
-- 
2.14.1.146.gd35faa819

Re: [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore

[Thomas Huth]

On 04.09.2017 11:03, Marc-André Lureau wrote:
> tests/.gitignore is often out of date. Let's generate it based on the
> files and directories to clean.
> 
> Note: I didn't succeed yet at generalizing this approach for the rest
> of qemu .gitignore files, but I hope it may eventually happen.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  Makefile                   |  7 +++-
>  tests/.gitignore           | 97 ----------------------------------------------
>  tests/Makefile.include     | 39 +++++++++++++++++--
>  tests/migration/.gitignore |  2 -
>  4 files changed, 41 insertions(+), 104 deletions(-)
>  delete mode 100644 tests/.gitignore
>  delete mode 100644 tests/migration/.gitignore
> 
> diff --git a/Makefile b/Makefile
> index 81447b1f08..89d5edf531 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -6,7 +6,7 @@ BUILD_DIR=$(CURDIR)
>  # Before including a proper config-host.mak, assume we are in the source tree
>  SRC_PATH=.
>  
> -UNCHECKED_GOALS := %clean TAGS cscope ctags docker docker-%
> +UNCHECKED_GOALS := %clean TAGS cscope ctags docker docker-% gitignore
>  
>  # All following code might depend on configuration variables
>  ifneq ($(wildcard config-host.mak),)
> @@ -14,6 +14,11 @@ ifneq ($(wildcard config-host.mak),)
>  all:
>  include config-host.mak
>  
> +.PHONY: gitignore
> +ifneq ($(filter-out $(UNCHECKED_GOALS),$(MAKECMDGOALS)),$(if $(MAKECMDGOALS),,fail))
> +all $(MAKECMDGOALS): gitignore
> +endif
> +
>  # Check that we're not trying to do an out-of-tree build from
>  # a tree that's been used for an in-tree build.
>  ifneq ($(realpath $(SRC_PATH)),$(realpath .))
> diff --git a/tests/.gitignore b/tests/.gitignore
> deleted file mode 100644
> index fed0189a5a..0000000000
> --- a/tests/.gitignore
> +++ /dev/null
> @@ -1,97 +0,0 @@
> -atomic_add-bench
> -benchmark-crypto-cipher
> -benchmark-crypto-hash
> -benchmark-crypto-hmac
> -check-qdict
> -check-qnum
> -check-qjson
> -check-qlist
> -check-qnull
> -check-qstring
> -check-qom-interface
> -check-qom-proplist
> -qht-bench
> -rcutorture
> -test-aio
> -test-aio-multithread
> -test-arm-mptimer
> -test-base64
> -test-bitops
> -test-bitcnt
> -test-blockjob
> -test-blockjob-txn
> -test-bufferiszero
> -test-char
> -test-clone-visitor
> -test-coroutine
> -test-crypto-afsplit
> -test-crypto-block
> -test-crypto-cipher
> -test-crypto-hash
> -test-crypto-hmac
> -test-crypto-ivgen
> -test-crypto-pbkdf
> -test-crypto-secret
> -test-crypto-tlscredsx509
> -test-crypto-tlscredsx509-work/
> -test-crypto-tlscredsx509-certs/
> -test-crypto-tlssession
> -test-crypto-tlssession-work/
> -test-crypto-tlssession-client/
> -test-crypto-tlssession-server/
> -test-crypto-xts
> -test-cutils
> -test-hbitmap
> -test-hmp
> -test-int128
> -test-iov
> -test-io-channel-buffer
> -test-io-channel-command
> -test-io-channel-command.fifo
> -test-io-channel-file
> -test-io-channel-file.txt
> -test-io-channel-socket
> -test-io-channel-tls
> -test-io-task
> -test-keyval
> -test-logging
> -test-mul64
> -test-opts-visitor
> -test-qapi-event.[ch]
> -test-qapi-types.[ch]
> -test-qapi-util
> -test-qapi-visit.[ch]
> -test-qdev-global-props
> -test-qemu-opts
> -test-qdist
> -test-qga
> -test-qht
> -test-qht-par
> -test-qmp-commands
> -test-qmp-commands.h
> -test-qmp-event
> -test-qobject-input-strict
> -test-qobject-input-visitor
> -test-qmp-introspect.[ch]
> -test-qmp-marshal.c
> -test-qobject-output-visitor
> -test-rcu-list
> -test-replication
> -test-shift128
> -test-string-input-visitor
> -test-string-output-visitor
> -test-thread-pool
> -test-throttle
> -test-timed-average
> -test-uuid
> -test-visitor-serialization
> -test-vmstate
> -test-write-threshold
> -test-x86-cpuid
> -test-x86-cpuid-compat
> -test-xbzrle
> -test-netfilter
> -test-filter-mirror
> -test-filter-redirector
> -*-test
> -qapi-schema/*.test.*
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index f08b7418f0..e94671e879 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -901,8 +901,34 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json
>  check-tests/qapi-schema/doc-good.texi: tests/qapi-schema/doc-good.test.texi
>  	@diff -q $(SRC_PATH)/tests/qapi-schema/doc-good.texi $<
>  
> -# Consolidated targets
> +tests-cleanfiles = *.o
> +tests-cleanfiles += .gitignore
> +tests-cleanfiles += qht-bench$(EXESUF)
> +tests-cleanfiles += qapi-schema/*.test.*
> +tests-cleanfiles += test-qapi-event.[ch]
> +tests-cleanfiles += test-qapi-types.[ch]
> +tests-cleanfiles += test-qapi-visit.[ch]
> +tests-cleanfiles += test-qmp-introspect.[ch]
> +tests-cleanfiles += test-qmp-commands.h
> +tests-cleanfiles += test-qmp-marshal.c
> +tests-cleanfiles += $(subst tests/,,$(check-unit-y))
> +tests-cleanfiles += $(subst tests/,,$(check-speed-y))
> +tests-cleanfiles += $(subst tests/,,$(check-block-y))
> +tests-cleanfiles += $(subst tests/,,$(check-qtest-y))
> +tests-cleanfiles += $(subst tests/,,$(QEMU_IOTESTS_HELPERS-y))
> +tests-cleanfiles += migration/initrd-stress.img
> +tests-cleanfiles += migration/stress$(EXESUF)
> +tests-cleanfiles += atomic_add-bench$(EXESUF)
> +tests-cleanfiles += test-io-channel-file.txt
> +tests-cleanfiles += test-io-channel-command.fifo
> +
> +tests-cleandirs += test-crypto-tlscredsx509-certs/
> +tests-cleandirs += test-crypto-tlscredsx509-work/
> +tests-cleandirs += test-crypto-tlssession-client/
> +tests-cleandirs += test-crypto-tlssession-server/
> +tests-cleandirs += test-crypto-tlssession-work/
>  
> +# Consolidated targets
>  .PHONY: check-qapi-schema check-qtest check-unit check check-clean
>  check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) check-tests/qapi-schema/doc-good.texi
>  check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
> @@ -912,15 +938,20 @@ check-block: $(patsubst %,check-%, $(check-block-y))
>  check: check-qapi-schema check-unit check-qtest
>  check-clean:
>  	$(MAKE) -C tests/tcg clean
> -	rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
> -	rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST), $(check-qtest-$(target)-y)) $(check-qtest-generic-y))
> -
> +	rm -f $(addprefix tests/, $(tests-cleanfiles))
> +	rm -rf $(addprefix tests/, $(tests-cleandirs))

I think you should mention this in the patch description, too, that
you're touching the "clean" target here.

>  clean: check-clean
>  
>  # Build the help program automatically
>  
>  all: $(QEMU_IOTESTS_HELPERS-y)
>  
> +$(SRC_PATH)/tests/.gitignore: $(MAKEFILE_LIST)
> +	$(call quiet-command, echo "$(tests-cleanfiles)" "$(tests-cleandirs)" | \
> +		xargs -n1 | sort | uniq | sed -e s:^:/: > $@,"GEN","$(@F)")

Please do not use SRC_PATH here. I'm doing out of tree builds, and I
don't want that these are touching my source folder!

 Thomas

Re: [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore

[Marc-André Lureau]

Hi

On Mon, Sep 4, 2017 at 12:21 PM Thomas Huth <thuth@redhat.com> wrote:

> On 04.09.2017 11 <04%2009%2020%2017%2011>:03, Marc-André Lureau wrote:
> > tests/.gitignore is often out of date. Let's generate it based on the
> > files and directories to clean.
> >
> > Note: I didn't succeed yet at generalizing this approach for the rest
> > of qemu .gitignore files, but I hope it may eventually happen.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  Makefile                   |  7 +++-
> >  tests/.gitignore           | 97
> ----------------------------------------------
> >  tests/Makefile.include     | 39 +++++++++++++++++--
> >  tests/migration/.gitignore |  2 -
> >  4 files changed, 41 insertions(+), 104 deletions(-)
> >  delete mode 100644 tests/.gitignore
> >  delete mode 100644 tests/migration/.gitignore
> >
> > diff --git a/Makefile b/Makefile
> > index 81447b1f08..89d5edf531 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -6,7 +6,7 @@ BUILD_DIR=$(CURDIR)
> >  # Before including a proper config-host.mak, assume we are in the
> source tree
> >  SRC_PATH=.
> >
> > -UNCHECKED_GOALS := %clean TAGS cscope ctags docker docker-%
> > +UNCHECKED_GOALS := %clean TAGS cscope ctags docker docker-% gitignore
> >
> >  # All following code might depend on configuration variables
> >  ifneq ($(wildcard config-host.mak),)
> > @@ -14,6 +14,11 @@ ifneq ($(wildcard config-host.mak),)
> >  all:
> >  include config-host.mak
> >
> > +.PHONY: gitignore
> > +ifneq ($(filter-out $(UNCHECKED_GOALS),$(MAKECMDGOALS)),$(if
> $(MAKECMDGOALS),,fail))
> > +all $(MAKECMDGOALS): gitignore
> > +endif
> > +
> >  # Check that we're not trying to do an out-of-tree build from
> >  # a tree that's been used for an in-tree build.
> >  ifneq ($(realpath $(SRC_PATH)),$(realpath .))
> > diff --git a/tests/.gitignore b/tests/.gitignore
> > deleted file mode 100644
> > index fed0189a5a..0000000000
> > --- a/tests/.gitignore
> > +++ /dev/null
> > @@ -1,97 +0,0 @@
> > -atomic_add-bench
> > -benchmark-crypto-cipher
> > -benchmark-crypto-hash
> > -benchmark-crypto-hmac
> > -check-qdict
> > -check-qnum
> > -check-qjson
> > -check-qlist
> > -check-qnull
> > -check-qstring
> > -check-qom-interface
> > -check-qom-proplist
> > -qht-bench
> > -rcutorture
> > -test-aio
> > -test-aio-multithread
> > -test-arm-mptimer
> > -test-base64
> > -test-bitops
> > -test-bitcnt
> > -test-blockjob
> > -test-blockjob-txn
> > -test-bufferiszero
> > -test-char
> > -test-clone-visitor
> > -test-coroutine
> > -test-crypto-afsplit
> > -test-crypto-block
> > -test-crypto-cipher
> > -test-crypto-hash
> > -test-crypto-hmac
> > -test-crypto-ivgen
> > -test-crypto-pbkdf
> > -test-crypto-secret
> > -test-crypto-tlscredsx509
> > -test-crypto-tlscredsx509-work/
> > -test-crypto-tlscredsx509-certs/
> > -test-crypto-tlssession
> > -test-crypto-tlssession-work/
> > -test-crypto-tlssession-client/
> > -test-crypto-tlssession-server/
> > -test-crypto-xts
> > -test-cutils
> > -test-hbitmap
> > -test-hmp
> > -test-int128
> > -test-iov
> > -test-io-channel-buffer
> > -test-io-channel-command
> > -test-io-channel-command.fifo
> > -test-io-channel-file
> > -test-io-channel-file.txt
> > -test-io-channel-socket
> > -test-io-channel-tls
> > -test-io-task
> > -test-keyval
> > -test-logging
> > -test-mul64
> > -test-opts-visitor
> > -test-qapi-event.[ch]
> > -test-qapi-types.[ch]
> > -test-qapi-util
> > -test-qapi-visit.[ch]
> > -test-qdev-global-props
> > -test-qemu-opts
> > -test-qdist
> > -test-qga
> > -test-qht
> > -test-qht-par
> > -test-qmp-commands
> > -test-qmp-commands.h
> > -test-qmp-event
> > -test-qobject-input-strict
> > -test-qobject-input-visitor
> > -test-qmp-introspect.[ch]
> > -test-qmp-marshal.c
> > -test-qobject-output-visitor
> > -test-rcu-list
> > -test-replication
> > -test-shift128
> > -test-string-input-visitor
> > -test-string-output-visitor
> > -test-thread-pool
> > -test-throttle
> > -test-timed-average
> > -test-uuid
> > -test-visitor-serialization
> > -test-vmstate
> > -test-write-threshold
> > -test-x86-cpuid
> > -test-x86-cpuid-compat
> > -test-xbzrle
> > -test-netfilter
> > -test-filter-mirror
> > -test-filter-redirector
> > -*-test
> > -qapi-schema/*.test.*
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index f08b7418f0..e94671e879 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -901,8 +901,34 @@ $(patsubst %, check-%, $(check-qapi-schema-y)):
> check-%.json: $(SRC_PATH)/%.json
> >  check-tests/qapi-schema/doc-good.texi:
> tests/qapi-schema/doc-good.test.texi
> >       @diff -q $(SRC_PATH)/tests/qapi-schema/doc-good.texi $<
> >
> > -# Consolidated targets
> > +tests-cleanfiles = *.o
> > +tests-cleanfiles += .gitignore
> > +tests-cleanfiles += qht-bench$(EXESUF)
> > +tests-cleanfiles += qapi-schema/*.test.*
> > +tests-cleanfiles += test-qapi-event.[ch]
> > +tests-cleanfiles += test-qapi-types.[ch]
> > +tests-cleanfiles += test-qapi-visit.[ch]
> > +tests-cleanfiles += test-qmp-introspect.[ch]
> > +tests-cleanfiles += test-qmp-commands.h
> > +tests-cleanfiles += test-qmp-marshal.c
> > +tests-cleanfiles += $(subst tests/,,$(check-unit-y))
> > +tests-cleanfiles += $(subst tests/,,$(check-speed-y))
> > +tests-cleanfiles += $(subst tests/,,$(check-block-y))
> > +tests-cleanfiles += $(subst tests/,,$(check-qtest-y))
> > +tests-cleanfiles += $(subst tests/,,$(QEMU_IOTESTS_HELPERS-y))
> > +tests-cleanfiles += migration/initrd-stress.img
> > +tests-cleanfiles += migration/stress$(EXESUF)
> > +tests-cleanfiles += atomic_add-bench$(EXESUF)
> > +tests-cleanfiles += test-io-channel-file.txt
> > +tests-cleanfiles += test-io-channel-command.fifo
> > +
> > +tests-cleandirs += test-crypto-tlscredsx509-certs/
> > +tests-cleandirs += test-crypto-tlscredsx509-work/
> > +tests-cleandirs += test-crypto-tlssession-client/
> > +tests-cleandirs += test-crypto-tlssession-server/
> > +tests-cleandirs += test-crypto-tlssession-work/
> >
> > +# Consolidated targets
> >  .PHONY: check-qapi-schema check-qtest check-unit check check-clean
> >  check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y))
> check-tests/qapi-schema/doc-good.texi
> >  check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
> > @@ -912,15 +938,20 @@ check-block: $(patsubst %,check-%,
> $(check-block-y))
> >  check: check-qapi-schema check-unit check-qtest
> >  check-clean:
> >       $(MAKE) -C tests/tcg clean
> > -     rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
> > -     rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST),
> $(check-qtest-$(target)-y)) $(check-qtest-generic-y))
> > -
> > +     rm -f $(addprefix tests/, $(tests-cleanfiles))
> > +     rm -rf $(addprefix tests/, $(tests-cleandirs))
>
> I think you should mention this in the patch description, too, that
> you're touching the "clean" target here.
>
> >  clean: check-clean
> >
> >  # Build the help program automatically
> >
> >  all: $(QEMU_IOTESTS_HELPERS-y)
> >
> > +$(SRC_PATH)/tests/.gitignore: $(MAKEFILE_LIST)
> > +     $(call quiet-command, echo "$(tests-cleanfiles)"
> "$(tests-cleandirs)" | \
> > +             xargs -n1 | sort | uniq | sed -e s:^:/: > $@,"GEN","$(@F)")
>
> Please do not use SRC_PATH here. I'm doing out of tree builds, and I
> don't want that these are touching my source folder!
>

I understand the feeling, I do also mostly out of tree build. However, I
don't think it makes much sense to generate .gitignore in the build dir.
It's git related, and in the git source dir, you have .git that is writable
already: it'd be odd that the git wortree/srcdir is read-only, and the
point is to generate .gitignore. Not so true with tarballs though.

I wrote this patch inspired by how the
https://github.com/behdad/git.mk/blob/master/git.mk rules does it (it is
uses by many GNOME projects and others with autoconf). The way it gets away
with not touching the srcdir in non-git tree, is that the git.mk file is
not shipped in distributed tarball. We could have a similar approach if
necessary.

Another alternative, which I find much less appealing, is that qemu tree
keeps shipping .gitignore, and we just add a manual rule to maintain it.

You suggested using more test* *test etc wildcards. I think this is bad, I
would rather have a precise .gitignore, and not ignore extra files that are
not part of the build-system. I couldn't find a simple way to generate
precise rules for intermediary files (like .o etc), but for end targets,
this patch demonstrates it can be done quite easily.

Finally, there is this trend these days with meson to not even allow
in-tree build, and thus no need for .gitignore..
-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1403 bytes --]

On 09/04/2017 04:03 AM, Marc-André Lureau wrote:
> tests/.gitignore is often out of date. Let's generate it based on the
> files and directories to clean.

In fact, it got out-of-date again with the recent cbb6540.

> 
> Note: I didn't succeed yet at generalizing this approach for the rest
> of qemu .gitignore files, but I hope it may eventually happen.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  Makefile                   |  7 +++-
>  tests/.gitignore           | 97 ----------------------------------------------
>  tests/Makefile.include     | 39 +++++++++++++++++--
>  tests/migration/.gitignore |  2 -
>  4 files changed, 41 insertions(+), 104 deletions(-)
>  delete mode 100644 tests/.gitignore
>  delete mode 100644 tests/migration/.gitignore
> 

> @@ -14,6 +14,11 @@ ifneq ($(wildcard config-host.mak),)
>  all:
>  include config-host.mak
>  
> +.PHONY: gitignore
> +ifneq ($(filter-out $(UNCHECKED_GOALS),$(MAKECMDGOALS)),$(if $(MAKECMDGOALS),,fail))
> +all $(MAKECMDGOALS): gitignore
> +endif

As others have mentioned, can we make this conditional on being an
in-tree build, so that a VPATH build isn't modifying non-local files
(after all, .gitignore only matters for an in-tree build)?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2496 bytes --]

On 09/05/2017 05:42 AM, Marc-André Lureau wrote:
> 
> I understand the feeling, I do also mostly out of tree build. However, I
> don't think it makes much sense to generate .gitignore in the build dir.
> It's git related, and in the git source dir, you have .git that is writable
> already: it'd be odd that the git wortree/srcdir is read-only, and the
> point is to generate .gitignore. Not so true with tarballs though.
> 
> I wrote this patch inspired by how the
> https://github.com/behdad/git.mk/blob/master/git.mk rules does it (it is
> uses by many GNOME projects and others with autoconf). The way it gets away
> with not touching the srcdir in non-git tree, is that the git.mk file is
> not shipped in distributed tarball. We could have a similar approach if
> necessary.

Shipping .gitignore in the tarball is not necessary; it's main benefit
is for in-tree development.

> 
> Another alternative, which I find much less appealing, is that qemu tree
> keeps shipping .gitignore, and we just add a manual rule to maintain it.

Keeping .gitignore in git, if it is generated, is a pain, because then
we have to manually resync it every time it gets out of date (and we're
already demonstrating that we forget to update .gitignore with new
tests).  This patch is only worthwhile if it reduces maintainer
overhead, but not if it trades one task for another.

> 
> You suggested using more test* *test etc wildcards. I think this is bad, I
> would rather have a precise .gitignore, and not ignore extra files that are
> not part of the build-system. I couldn't find a simple way to generate
> precise rules for intermediary files (like .o etc), but for end targets,
> this patch demonstrates it can be done quite easily.
> 
> Finally, there is this trend these days with meson to not even allow
> in-tree build, and thus no need for .gitignore..

Please don't go the direction of forbidding in-tree builds.  While I am
a BIG fan of VPATH builds, I'm also a big fan of in-tree builds (it's
slightly faster to do a one-off in-tree build of a fresh git checkout
than it is to set up yet another VPATH build directory).  Projects that
force one way or the other are annoying.  I see no reason why we should
not keep both approaches working, particularly if we have patchew or
other automation that covers both styles.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore

[Thomas Huth]

On 05.09.2017 12:42, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Sep 4, 2017 at 12:21 PM Thomas Huth <thuth@redhat.com
> <mailto:thuth@redhat.com>> wrote:
> 
>     On 04.09.2017 11 <tel:04%2009%2020%2017%2011>:03, Marc-André Lureau
>     wrote:
[...]
>     >  # Build the help program automatically
>     >
>     >  all: $(QEMU_IOTESTS_HELPERS-y)
>     >
>     > +$(SRC_PATH)/tests/.gitignore: $(MAKEFILE_LIST)
>     > +     $(call quiet-command, echo "$(tests-cleanfiles)"
>     "$(tests-cleandirs)" | \
>     > +             xargs -n1 | sort | uniq | sed -e s:^:/: >
>     $@,"GEN","$(@F)")
> 
>     Please do not use SRC_PATH here. I'm doing out of tree builds, and I
>     don't want that these are touching my source folder!
> 
> I understand the feeling, I do also mostly out of tree build. However, I
> don't think it makes much sense to generate .gitignore in the build dir.

Why not? If you're doing out-of-tree builds, you don't need the
.gitignore in the source directory anyway, so it certainly does not make
sense to generate there such a file in the source directory.

> It's git related, and in the git source dir, you have .git that is
> writable already

It's unlikely, but I think it is perfectly legal to have your git source
directory e.g. mounted temporarily as a read-only file system. I'd then
still expect to be able to use my read-only sources to build QEMU out of
tree.

 Thomas


PS: Looks like you're sending HTML mails ... please check the setup of
your mail program.

Re: [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 09/05/2017 05:42 AM, Marc-André Lureau wrote:
>> On Mon, Sep 4, 2017 at 12:21 PM Thomas Huth <thuth@redhat.com> wrote:
>>
>>> On 04.09.2017 11 <04%2009%2020%2017%2011>:03, Marc-André Lureau wrote:
>>> > tests/.gitignore is often out of date. Let's generate it based on the
>>> > files and directories to clean.
>>> >
>>> > Note: I didn't succeed yet at generalizing this approach for the rest
>>> > of qemu .gitignore files, but I hope it may eventually happen.
>>> >
>>> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[...]
>>> > diff --git a/tests/Makefile.include b/tests/Makefile.include
>>> > index f08b7418f0..e94671e879 100644
>>> > --- a/tests/Makefile.include
>>> > +++ b/tests/Makefile.include
>>> > @@ -901,8 +901,34 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json
>>> >  check-tests/qapi-schema/doc-good.texi: tests/qapi-schema/doc-good.test.texi
>>> >       @diff -q $(SRC_PATH)/tests/qapi-schema/doc-good.texi $<
>>> >
>>> > -# Consolidated targets
>>> > +tests-cleanfiles = *.o
>>> > +tests-cleanfiles += .gitignore
>>> > +tests-cleanfiles += qht-bench$(EXESUF)
>>> > +tests-cleanfiles += qapi-schema/*.test.*
>>> > +tests-cleanfiles += test-qapi-event.[ch]
>>> > +tests-cleanfiles += test-qapi-types.[ch]
>>> > +tests-cleanfiles += test-qapi-visit.[ch]
>>> > +tests-cleanfiles += test-qmp-introspect.[ch]
>>> > +tests-cleanfiles += test-qmp-commands.h
>>> > +tests-cleanfiles += test-qmp-marshal.c
>>> > +tests-cleanfiles += $(subst tests/,,$(check-unit-y))
>>> > +tests-cleanfiles += $(subst tests/,,$(check-speed-y))
>>> > +tests-cleanfiles += $(subst tests/,,$(check-block-y))
>>> > +tests-cleanfiles += $(subst tests/,,$(check-qtest-y))
>>> > +tests-cleanfiles += $(subst tests/,,$(QEMU_IOTESTS_HELPERS-y))
>>> > +tests-cleanfiles += migration/initrd-stress.img
>>> > +tests-cleanfiles += migration/stress$(EXESUF)
>>> > +tests-cleanfiles += atomic_add-bench$(EXESUF)
>>> > +tests-cleanfiles += test-io-channel-file.txt
>>> > +tests-cleanfiles += test-io-channel-command.fifo
>>> > +
>>> > +tests-cleandirs += test-crypto-tlscredsx509-certs/
>>> > +tests-cleandirs += test-crypto-tlscredsx509-work/
>>> > +tests-cleandirs += test-crypto-tlssession-client/
>>> > +tests-cleandirs += test-crypto-tlssession-server/
>>> > +tests-cleandirs += test-crypto-tlssession-work/
>>> >
>>> > +# Consolidated targets
>>> >  .PHONY: check-qapi-schema check-qtest check-unit check check-clean
>>> >  check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) check-tests/qapi-schema/doc-good.texi
>>> >  check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
>>> > @@ -912,15 +938,20 @@ check-block: $(patsubst %,check-%, $(check-block-y))
>>> >  check: check-qapi-schema check-unit check-qtest
>>> >  check-clean:
>>> >       $(MAKE) -C tests/tcg clean
>>> > -     rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
>>> > -     rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST), $(check-qtest-$(target)-y)) $(check-qtest-generic-y))
>>> > -
>>> > +     rm -f $(addprefix tests/, $(tests-cleanfiles))
>>> > +     rm -rf $(addprefix tests/, $(tests-cleandirs))
>>>
>>> I think you should mention this in the patch description, too, that
>>> you're touching the "clean" target here.
>>>
>>> >  clean: check-clean
>>> >
>>> >  # Build the help program automatically
>>> >
>>> >  all: $(QEMU_IOTESTS_HELPERS-y)
>>> >
>>> > +$(SRC_PATH)/tests/.gitignore: $(MAKEFILE_LIST)
>>> > +     $(call quiet-command, echo "$(tests-cleanfiles)" "$(tests-cleandirs)" | \
>>> > +             xargs -n1 | sort | uniq | sed -e s:^:/: > $@,"GEN","$(@F)")
>>>
>>> Please do not use SRC_PATH here. I'm doing out of tree builds, and I
>>> don't want that these are touching my source folder!

I dislike generating .gitignore almost as much as I dislike maintaining
it, and I'm adamantly opposed to generating it into the source tree.

Let's step back and consider what the problem is.

The problem is that we choose to litter the source tree with haphazardly
named build artifacts.  "Doctor, it hurts when I do that!"

"Don't do that then" is the simplest and sanest solution.  Either name
the build artifacts so that a few simple patterns can safely catch them.
Or stop doing in-tree builds.  As a convenience for people who have
in-tree builds in muscle memory, have ./configure create and configure a
suitable build directory, and have a trivial Makefile in the source tree
that redirects there.  Name the default build directory bld, add bld* to
.gitignore, done.

I've meant to propose such a patch since forever, but I somehow never
get around to actually doing it.

>> I understand the feeling, I do also mostly out of tree build. However, I
>> don't think it makes much sense to generate .gitignore in the build dir.
>> It's git related, and in the git source dir, you have .git that is writable
>> already: it'd be odd that the git wortree/srcdir is read-only, and the
>> point is to generate .gitignore. Not so true with tarballs though.
>> 
>> I wrote this patch inspired by how the
>> https://github.com/behdad/git.mk/blob/master/git.mk rules does it (it is
>> uses by many GNOME projects and others with autoconf). The way it gets away
>> with not touching the srcdir in non-git tree, is that the git.mk file is
>> not shipped in distributed tarball. We could have a similar approach if
>> necessary.
>
> Shipping .gitignore in the tarball is not necessary; it's main benefit
> is for in-tree development.
>
>> 
>> Another alternative, which I find much less appealing, is that qemu tree
>> keeps shipping .gitignore, and we just add a manual rule to maintain it.
>
> Keeping .gitignore in git, if it is generated, is a pain, because then
> we have to manually resync it every time it gets out of date (and we're
> already demonstrating that we forget to update .gitignore with new
> tests).  This patch is only worthwhile if it reduces maintainer
> overhead, but not if it trades one task for another.

Keeping generated files in Git is almost always a bad idea.

>> You suggested using more test* *test etc wildcards. I think this is bad, I
>> would rather have a precise .gitignore, and not ignore extra files that are
>> not part of the build-system. I couldn't find a simple way to generate
>> precise rules for intermediary files (like .o etc), but for end targets,
>> this patch demonstrates it can be done quite easily.
>> 
>> Finally, there is this trend these days with meson to not even allow
>> in-tree build, and thus no need for .gitignore..
>
> Please don't go the direction of forbidding in-tree builds.  While I am
> a BIG fan of VPATH builds, I'm also a big fan of in-tree builds (it's
> slightly faster to do a one-off in-tree build of a fresh git checkout
> than it is to set up yet another VPATH build directory).  Projects that
> force one way or the other are annoying.  I see no reason why we should
> not keep both approaches working, particularly if we have patchew or
> other automation that covers both styles.

I do: people regularly break the one they're not using right now.
Even when Patchew catches these mistake, it's still a waste of developer
time.