| Message ID | 20120413221908.1079.50464.stgit@ginnungagap.bsc.es |
|---|---|
| State | New |
| Headers | show |
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 <vilanova@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
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
On Wed, Apr 18, 2012 at 2:45 PM, Lluís Vilanova <vilanova@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
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
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 <vilanova@ac.upc.edu> --- Makefile.objs | 19 +++++++++---------- rules.mak | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 10 deletions(-)