[Qemu-devel] [PATCH v2 0/2] trace: fix Makefile dependencies

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [Qemu-devel] [PATCH v2 0/2] trace: fix Makefile dependencies
@ 2015-10-30  9:53 Stefan Hajnoczi
  2015-10-30  9:53 ` [Qemu-devel] [PATCH v2 1/2] trace: fix make foo-timestamp rules Stefan Hajnoczi
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2015-10-30  9:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Lluís Vilanova, Stefan Hajnoczi, Markus Armbruster

v2:
 * Use find(1) to catch all tracetool *.py files [Lluís]

   Note: I used find -name instead of find -path for better POSIX compliance

Issues with trace/Makefile.objs:

1. Generated code is not recreated when patches modify scripts/tracetool/*.py.
   Typically such patches also modify trace/*.[ch] and the result is build
   failures when new C code compiles against stale generated code.

2. The timestamp mechanism used to avoid unnecessary rebuilding is broken, it
   currently requires two make invocations for a full build.

Stefan Hajnoczi (2):
  trace: fix make foo-timestamp rules
  trace: add make dependencies on tracetool source

 trace/Makefile.objs | 48 ++++++++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 20 deletions(-)

-- 
2.4.3

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Qemu-devel] [PATCH v2 1/2] trace: fix make foo-timestamp rules
  2015-10-30  9:53 [Qemu-devel] [PATCH v2 0/2] trace: fix Makefile dependencies Stefan Hajnoczi
@ 2015-10-30  9:53 ` Stefan Hajnoczi
  2015-10-30  9:53 ` [Qemu-devel] [PATCH v2 2/2] trace: add make dependencies on tracetool source Stefan Hajnoczi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2015-10-30  9:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Lluís Vilanova, Stefan Hajnoczi, Markus Armbruster

The Makefile uses intermediate timestamp files to avoid rebuilding if
tracetool output is unchanged.

Timestamps are implemented incorrectly.  This was fixed for rules.mak in
commit 4b25966ab976f3a7fd9008193b2defcc82f8f04d ("rules.mak: cleanup
config generation rules") but never fixed in trace/Makefile.objs.

The problem with the old timestamp implementation was that make doesn't
notice the updated file modification time until the next time it is run.
It was necessary to run make twice in a row to achieve a full rebuild.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 trace/Makefile.objs | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 32f7a32..73bec38 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -5,20 +5,20 @@
 
 ifeq ($(findstring ust,$(TRACE_BACKENDS)),ust)
 $(obj)/generated-ust-provider.h: $(obj)/generated-ust-provider.h-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-ust-provider.h-timestamp: $(SRC_PATH)/trace-events
 	$(call quiet-command,$(TRACETOOL) \
 		--format=ust-events-h \
 		--backends=$(TRACE_BACKENDS) \
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
 
 $(obj)/generated-ust.c: $(obj)/generated-ust.c-timestamp $(BUILD_DIR)/config-host.mak
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-ust.c-timestamp: $(SRC_PATH)/trace-events
 	$(call quiet-command,$(TRACETOOL) \
 		--format=ust-events-c \
 		--backends=$(TRACE_BACKENDS) \
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
 
 $(obj)/generated-events.h: $(obj)/generated-ust-provider.h
 $(obj)/generated-events.c: $(obj)/generated-ust.c
@@ -28,20 +28,20 @@ endif
 # Auto-generated event descriptions
 
 $(obj)/generated-events.h: $(obj)/generated-events.h-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-events.h-timestamp: $(SRC_PATH)/trace-events
 	$(call quiet-command,$(TRACETOOL) \
 		--format=events-h \
 		--backends=$(TRACE_BACKENDS) \
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
 
 $(obj)/generated-events.c: $(obj)/generated-events.c-timestamp $(BUILD_DIR)/config-host.mak
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-events.c-timestamp: $(SRC_PATH)/trace-events
 	$(call quiet-command,$(TRACETOOL) \
 		--format=events-c \
 		--backends=$(TRACE_BACKENDS) \
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
 
 util-obj-y += generated-events.o
 
@@ -81,12 +81,12 @@ $(obj)/generated-tracers.o: $(obj)/generated-tracers.c $(obj)/generated-tracers.
 # rule file. So we use '.dtrace' instead
 ifeq ($(findstring dtrace,$(TRACE_BACKENDS)),dtrace)
 $(obj)/generated-tracers-dtrace.dtrace: $(obj)/generated-tracers-dtrace.dtrace-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-tracers-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
 	$(call quiet-command,$(TRACETOOL) \
 		--format=d \
 		--backends=$(TRACE_BACKENDS) \
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
 
 $(obj)/generated-tracers-dtrace.h: $(obj)/generated-tracers-dtrace.dtrace
 	$(call quiet-command,dtrace -o $@ -h -s $<, "  GEN   $@")
@@ -100,28 +100,28 @@ endif
 # Translation level
 
 $(obj)/generated-helpers-wrappers.h: $(obj)/generated-helpers-wrappers.h-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-helpers-wrappers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
 	$(call quiet-command,$(TRACETOOL) \
 		--format=tcg-helper-wrapper-h \
 		--backend=$(TRACE_BACKENDS) \
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
 
 $(obj)/generated-helpers.h: $(obj)/generated-helpers.h-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-helpers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
 	$(call quiet-command,$(TRACETOOL) \
 		--format=tcg-helper-h \
 		--backend=$(TRACE_BACKENDS) \
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
 
 $(obj)/generated-helpers.c: $(obj)/generated-helpers.c-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-helpers.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
 	$(call quiet-command,$(TRACETOOL) \
 		--format=tcg-helper-c \
 		--backend=$(TRACE_BACKENDS) \
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
 
 $(obj)/generated-helpers.o: $(obj)/generated-helpers.c
 
@@ -129,12 +129,12 @@ target-obj-y += generated-helpers.o
 
 
 $(obj)/generated-tcg-tracers.h: $(obj)/generated-tcg-tracers.h-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-tcg-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
 	$(call quiet-command,$(TRACETOOL) \
 		--format=tcg-h \
 		--backend=$(TRACE_BACKENDS) \
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
 
 
 ######################################################################
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [Qemu-devel] [PATCH v2 2/2] trace: add make dependencies on tracetool source
  2015-10-30  9:53 [Qemu-devel] [PATCH v2 0/2] trace: fix Makefile dependencies Stefan Hajnoczi
  2015-10-30  9:53 ` [Qemu-devel] [PATCH v2 1/2] trace: fix make foo-timestamp rules Stefan Hajnoczi
@ 2015-10-30  9:53 ` Stefan Hajnoczi
  2015-10-30 19:31 ` [Qemu-devel] [PATCH v2 0/2] trace: fix Makefile dependencies Lluís Vilanova
  2015-11-02 13:24 ` Stefan Hajnoczi
  3 siblings, 0 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2015-10-30  9:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Lluís Vilanova, Stefan Hajnoczi, Markus Armbruster

Patches that change tracetool can break the build if old build output
files are lying around.

This happens because the Makefile does not specify dependencies on
tracetool.  The build will use old object files that do not match the
current source code.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 trace/Makefile.objs | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 73bec38..5145b34 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -1,12 +1,20 @@
 # -*- mode: makefile -*-
 
 ######################################################################
+# tracetool source files
+# Every rule that invokes tracetool must depend on this so code is regenerated
+# if tracetool itself changes.
+
+tracetool-y = $(SRC_PATH)/scripts/tracetool.py
+tracetool-y += $(shell find $(SRC_PATH)/scripts/tracetool -name "*.py")
+
+######################################################################
 # Auto-generated event descriptions for LTTng ust code
 
 ifeq ($(findstring ust,$(TRACE_BACKENDS)),ust)
 $(obj)/generated-ust-provider.h: $(obj)/generated-ust-provider.h-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-ust-provider.h-timestamp: $(SRC_PATH)/trace-events
+$(obj)/generated-ust-provider.h-timestamp: $(SRC_PATH)/trace-events $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=ust-events-h \
 		--backends=$(TRACE_BACKENDS) \
@@ -14,7 +22,7 @@ $(obj)/generated-ust-provider.h-timestamp: $(SRC_PATH)/trace-events
 
 $(obj)/generated-ust.c: $(obj)/generated-ust.c-timestamp $(BUILD_DIR)/config-host.mak
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-ust.c-timestamp: $(SRC_PATH)/trace-events
+$(obj)/generated-ust.c-timestamp: $(SRC_PATH)/trace-events $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=ust-events-c \
 		--backends=$(TRACE_BACKENDS) \
@@ -29,7 +37,7 @@ endif
 
 $(obj)/generated-events.h: $(obj)/generated-events.h-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-events.h-timestamp: $(SRC_PATH)/trace-events
+$(obj)/generated-events.h-timestamp: $(SRC_PATH)/trace-events $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=events-h \
 		--backends=$(TRACE_BACKENDS) \
@@ -37,7 +45,7 @@ $(obj)/generated-events.h-timestamp: $(SRC_PATH)/trace-events
 
 $(obj)/generated-events.c: $(obj)/generated-events.c-timestamp $(BUILD_DIR)/config-host.mak
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-events.c-timestamp: $(SRC_PATH)/trace-events
+$(obj)/generated-events.c-timestamp: $(SRC_PATH)/trace-events $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=events-c \
 		--backends=$(TRACE_BACKENDS) \
@@ -54,7 +62,7 @@ util-obj-y += generated-events.o
 
 $(obj)/generated-tracers.h: $(obj)/generated-tracers.h-timestamp
 	@cmp -s $< $@ || cp $< $@
-$(obj)/generated-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
+$(obj)/generated-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=h \
 		--backends=$(TRACE_BACKENDS) \
@@ -65,7 +73,7 @@ $(obj)/generated-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/conf
 
 $(obj)/generated-tracers.c: $(obj)/generated-tracers.c-timestamp
 	@cmp -s $< $@ || cp $< $@
-$(obj)/generated-tracers.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
+$(obj)/generated-tracers.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=c \
 		--backends=$(TRACE_BACKENDS) \
@@ -82,7 +90,7 @@ $(obj)/generated-tracers.o: $(obj)/generated-tracers.c $(obj)/generated-tracers.
 ifeq ($(findstring dtrace,$(TRACE_BACKENDS)),dtrace)
 $(obj)/generated-tracers-dtrace.dtrace: $(obj)/generated-tracers-dtrace.dtrace-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-tracers-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
+$(obj)/generated-tracers-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=d \
 		--backends=$(TRACE_BACKENDS) \
@@ -101,7 +109,7 @@ endif
 
 $(obj)/generated-helpers-wrappers.h: $(obj)/generated-helpers-wrappers.h-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-helpers-wrappers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
+$(obj)/generated-helpers-wrappers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=tcg-helper-wrapper-h \
 		--backend=$(TRACE_BACKENDS) \
@@ -109,7 +117,7 @@ $(obj)/generated-helpers-wrappers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_
 
 $(obj)/generated-helpers.h: $(obj)/generated-helpers.h-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-helpers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
+$(obj)/generated-helpers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=tcg-helper-h \
 		--backend=$(TRACE_BACKENDS) \
@@ -117,7 +125,7 @@ $(obj)/generated-helpers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/conf
 
 $(obj)/generated-helpers.c: $(obj)/generated-helpers.c-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-helpers.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
+$(obj)/generated-helpers.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=tcg-helper-c \
 		--backend=$(TRACE_BACKENDS) \
@@ -130,7 +138,7 @@ target-obj-y += generated-helpers.o
 
 $(obj)/generated-tcg-tracers.h: $(obj)/generated-tcg-tracers.h-timestamp
 	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-tcg-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
+$(obj)/generated-tcg-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
 	$(call quiet-command,$(TRACETOOL) \
 		--format=tcg-h \
 		--backend=$(TRACE_BACKENDS) \
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/2] trace: fix Makefile dependencies
  2015-10-30  9:53 [Qemu-devel] [PATCH v2 0/2] trace: fix Makefile dependencies Stefan Hajnoczi
  2015-10-30  9:53 ` [Qemu-devel] [PATCH v2 1/2] trace: fix make foo-timestamp rules Stefan Hajnoczi
  2015-10-30  9:53 ` [Qemu-devel] [PATCH v2 2/2] trace: add make dependencies on tracetool source Stefan Hajnoczi
@ 2015-10-30 19:31 ` Lluís Vilanova
  2015-11-02 13:24 ` Stefan Hajnoczi
  3 siblings, 0 replies; 5+ messages in thread
From: Lluís Vilanova @ 2015-10-30 19:31 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: qemu-devel, Markus Armbruster

Stefan Hajnoczi writes:

> v2:
>  * Use find(1) to catch all tracetool *.py files [Lluís]

>    Note: I used find -name instead of find -path for better POSIX compliance

Looks great to me.

Thanks,
  Lluis

-- 
"And it's much the same thing with knowledge, for whenever you learn
something new, the whole world becomes that much richer."
-- The Princess of Pure Reason, as told by Norton Juster in The Phantom
Tollbooth

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/2] trace: fix Makefile dependencies
  2015-10-30  9:53 [Qemu-devel] [PATCH v2 0/2] trace: fix Makefile dependencies Stefan Hajnoczi
                   ` (2 preceding siblings ...)
  2015-10-30 19:31 ` [Qemu-devel] [PATCH v2 0/2] trace: fix Makefile dependencies Lluís Vilanova
@ 2015-11-02 13:24 ` Stefan Hajnoczi
  3 siblings, 0 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2015-11-02 13:24 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Markus Armbruster, qemu-devel, Lluís Vilanova

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

On Fri, Oct 30, 2015 at 09:53:13AM +0000, Stefan Hajnoczi wrote:
> v2:
>  * Use find(1) to catch all tracetool *.py files [Lluís]
> 
>    Note: I used find -name instead of find -path for better POSIX compliance
> 
> Issues with trace/Makefile.objs:
> 
> 1. Generated code is not recreated when patches modify scripts/tracetool/*.py.
>    Typically such patches also modify trace/*.[ch] and the result is build
>    failures when new C code compiles against stale generated code.
> 
> 2. The timestamp mechanism used to avoid unnecessary rebuilding is broken, it
>    currently requires two make invocations for a full build.
> 
> Stefan Hajnoczi (2):
>   trace: fix make foo-timestamp rules
>   trace: add make dependencies on tracetool source
> 
>  trace/Makefile.objs | 48 ++++++++++++++++++++++++++++--------------------
>  1 file changed, 28 insertions(+), 20 deletions(-)
> 
> -- 
> 2.4.3
> 
> 

Thanks, applied to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-11-02 13:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-30  9:53 [Qemu-devel] [PATCH v2 0/2] trace: fix Makefile dependencies Stefan Hajnoczi
2015-10-30  9:53 ` [Qemu-devel] [PATCH v2 1/2] trace: fix make foo-timestamp rules Stefan Hajnoczi
2015-10-30  9:53 ` [Qemu-devel] [PATCH v2 2/2] trace: add make dependencies on tracetool source Stefan Hajnoczi
2015-10-30 19:31 ` [Qemu-devel] [PATCH v2 0/2] trace: fix Makefile dependencies Lluís Vilanova
2015-11-02 13:24 ` Stefan Hajnoczi

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).