* [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore @ 2017-09-04 9:03 Marc-André Lureau 2017-09-04 10:20 ` Thomas Huth 2017-09-05 20:43 ` Eric Blake 0 siblings, 2 replies; 7+ messages in thread From: Marc-André Lureau @ 2017-09-04 9:03 UTC (permalink / raw) To: qemu-devel; +Cc: eblake, famz, 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore 2017-09-04 9:03 [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore Marc-André Lureau @ 2017-09-04 10:20 ` Thomas Huth 2017-09-05 10:42 ` Marc-André Lureau 2017-09-05 20:43 ` Eric Blake 1 sibling, 1 reply; 7+ messages in thread From: Thomas Huth @ 2017-09-04 10:20 UTC (permalink / raw) To: Marc-André Lureau, qemu-devel; +Cc: famz 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore 2017-09-04 10:20 ` Thomas Huth @ 2017-09-05 10:42 ` Marc-André Lureau 2017-09-05 20:49 ` Eric Blake 2017-09-06 5:05 ` Thomas Huth 0 siblings, 2 replies; 7+ messages in thread From: Marc-André Lureau @ 2017-09-05 10:42 UTC (permalink / raw) To: Thomas Huth, qemu-devel; +Cc: famz 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore 2017-09-05 10:42 ` Marc-André Lureau @ 2017-09-05 20:49 ` Eric Blake 2017-09-06 6:27 ` Markus Armbruster 2017-09-06 5:05 ` Thomas Huth 1 sibling, 1 reply; 7+ messages in thread From: Eric Blake @ 2017-09-05 20:49 UTC (permalink / raw) To: Marc-André Lureau, Thomas Huth, qemu-devel; +Cc: famz [-- 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 --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore 2017-09-05 20:49 ` Eric Blake @ 2017-09-06 6:27 ` Markus Armbruster 0 siblings, 0 replies; 7+ messages in thread From: Markus Armbruster @ 2017-09-06 6:27 UTC (permalink / raw) To: Eric Blake; +Cc: Marc-André Lureau, Thomas Huth, qemu-devel, famz 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore 2017-09-05 10:42 ` Marc-André Lureau 2017-09-05 20:49 ` Eric Blake @ 2017-09-06 5:05 ` Thomas Huth 1 sibling, 0 replies; 7+ messages in thread From: Thomas Huth @ 2017-09-06 5:05 UTC (permalink / raw) To: Marc-André Lureau, qemu-devel; +Cc: famz 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore 2017-09-04 9:03 [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore Marc-André Lureau 2017-09-04 10:20 ` Thomas Huth @ 2017-09-05 20:43 ` Eric Blake 1 sibling, 0 replies; 7+ messages in thread From: Eric Blake @ 2017-09-05 20:43 UTC (permalink / raw) To: Marc-André Lureau, qemu-devel; +Cc: famz [-- 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 --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-09-06 6:27 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-09-04 9:03 [Qemu-devel] [PATCH] build-sys: generate tests/.gitignore Marc-André Lureau 2017-09-04 10:20 ` Thomas Huth 2017-09-05 10:42 ` Marc-André Lureau 2017-09-05 20:49 ` Eric Blake 2017-09-06 6:27 ` Markus Armbruster 2017-09-06 5:05 ` Thomas Huth 2017-09-05 20:43 ` Eric Blake
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).