Re: [Qemu-devel] [PATCH 2/2] [trivial] Simpler handling of tracetool-generated files in makefiles
On Sat, Apr 14, 2012 at 12:19:08AM +0200, Lluís Vilanova wrote: Adds 'tracetool-gen' to generate files with tracetool into a temporary file, and 'tracetool-ci' to commit the generation from the temporaty file into the actual destination file if there were any changes in the produced file. Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu --- Makefile.objs | 19 +-- rules.mak | 17 + 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/Makefile.objs b/Makefile.objs index 6d6f24d..b98e905 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -373,18 +373,17 @@ libdis-$(CONFIG_LM32_DIS) += lm32-dis.o # trace ifeq ($(TRACE_BACKEND),dtrace) -trace.h: trace.h-timestamp trace-dtrace.h -else -trace.h: trace.h-timestamp +TRACE_H_EXTRA_DEPS=trace-dtrace.h endif +trace.h: trace.h-timestamp $(TRACE_H_EXTRA_DEPS) I like this. trace.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --format=h --backend=$(TRACE_BACKEND) $ $@, GEN trace.h) - @cmp -s $@ trace.h || cp $@ trace.h + $(call tracetool-gen,h,$(TRACE_BACKEND)) + $(call tracetool-ci) Here I don't think it's worth introducing an abstraction. While there is a pattern I think the abstraction actually hides what is going on rather than being useful. The macros are hiding output generation, I find that especially troubling because you can't really tell what is going to happen. It's clearer to leave these statements open coded. Stefan
Re: [Qemu-devel] [PATCH 2/2] [trivial] Simpler handling of tracetool-generated files in makefiles
Stefan Hajnoczi writes: On Sat, Apr 14, 2012 at 12:19:08AM +0200, Lluís Vilanova wrote: trace.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak -$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --format=h --backend=$(TRACE_BACKEND) $ $@, GEN trace.h) -@cmp -s $@ trace.h || cp $@ trace.h +$(call tracetool-gen,h,$(TRACE_BACKEND)) +$(call tracetool-ci) Here I don't think it's worth introducing an abstraction. While there is a pattern I think the abstraction actually hides what is going on rather than being useful. The macros are hiding output generation, I find that especially troubling because you can't really tell what is going to happen. It's clearer to leave these statements open coded. I just thought it was excessively verbose. Would it work for you making it more explicit? $(call tracetool-gen,$,$@,h,$(TRACE_BACKEND)) $(call tracetool-ci,$@) The main points bugging me were: * use of quiet-command plus explicit quiet compilation string ( GEN whatever). * use of $(PYTHON) $(SRC_PATH)/scripts/tracetool.py. If not, I'll just drop it and instead simply replace calls to tracetool with $(TRACETOOL). 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
Re: [Qemu-devel] [PATCH 2/2] [trivial] Simpler handling of tracetool-generated files in makefiles
On Wed, Apr 18, 2012 at 2:45 PM, Lluís Vilanova vilan...@ac.upc.edu wrote: If not, I'll just drop it and instead simply replace calls to tracetool with $(TRACETOOL). If you're willing to do this I would prefer it. Thanks, Stefan
[Qemu-devel] [PATCH 2/2] [trivial] Simpler handling of tracetool-generated files in makefiles
Adds 'tracetool-gen' to generate files with tracetool into a temporary file, and 'tracetool-ci' to commit the generation from the temporaty file into the actual destination file if there were any changes in the produced file. Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu --- Makefile.objs | 19 +-- rules.mak | 17 + 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/Makefile.objs b/Makefile.objs index 6d6f24d..b98e905 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -373,18 +373,17 @@ libdis-$(CONFIG_LM32_DIS) += lm32-dis.o # trace ifeq ($(TRACE_BACKEND),dtrace) -trace.h: trace.h-timestamp trace-dtrace.h -else -trace.h: trace.h-timestamp +TRACE_H_EXTRA_DEPS=trace-dtrace.h endif +trace.h: trace.h-timestamp $(TRACE_H_EXTRA_DEPS) trace.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --format=h --backend=$(TRACE_BACKEND) $ $@, GEN trace.h) - @cmp -s $@ trace.h || cp $@ trace.h + $(call tracetool-gen,h,$(TRACE_BACKEND)) + $(call tracetool-ci) trace.c: trace.c-timestamp trace.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --format=c --backend=$(TRACE_BACKEND) $ $@, GEN trace.c) - @cmp -s $@ trace.c || cp $@ trace.c + $(call tracetool-gen,c,$(TRACE_BACKEND)) + $(call tracetool-ci) trace.o: trace.c $(GENERATED_HEADERS) @@ -396,11 +395,11 @@ trace-dtrace.h: trace-dtrace.dtrace # rule file. So we use '.dtrace' instead trace-dtrace.dtrace: trace-dtrace.dtrace-timestamp trace-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/tracetool.py --format=d --backend=$(TRACE_BACKEND) $ $@, GEN trace-dtrace.dtrace) - @cmp -s $@ trace-dtrace.dtrace || cp $@ trace-dtrace.dtrace + $(call tracetool-gen,d,$(TRACE_BACKEND)) + $(call tracetool-ci) trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS) - $(call quiet-command,dtrace -o $@ -G -s $, GEN trace-dtrace.o) + $(call quiet-command,dtrace -o $@ -G -s $, GEN trace-dtrace.o) ifeq ($(LIBTOOL),) trace-dtrace.lo: trace-dtrace.dtrace diff --git a/rules.mak b/rules.mak index c30093c..2cbceab 100644 --- a/rules.mak +++ b/rules.mak @@ -59,6 +59,23 @@ find-in-path = $(if $(find-string /, $1), \ $(wildcard $1), \ $(wildcard $(patsubst %, %/$1, $(subst :, ,$(PATH) +# Generate files with tracetool + +TRACETOOL=$(SRC_PATH)/scripts/tracetool.py + +# Generate a file with tracetool. +# Assumes: +# * $@ : ends with -timestamp +# * $ : is the trace-events file +# Takes: +# * 1 : format +# * 2 : backend +# * 1 : extra tracetool arguments (optional) +tracetool-gen=$(call quiet-command,$(PYTHON) $(TRACETOOL) $(3) --format=$(1) --backend=$(2) $ $@, GEN $(subst -timestamp,,$@)) + +# Commits the file generated by tracetool with 'tracetool-gen' +tracetool-ci=@cmp -s $@ $(subst -timestamp,,$@) || cp $@ $(subst -timestamp,,$@) + # Generate timestamp files for .h include files %.h: %.h-timestamp
