Patchwork [2/2,trivial] Simpler handling of tracetool-generated files in makefiles

login
register
mail settings
Submitter Lluís Vilanova
Date April 13, 2012, 10:19 p.m.
Message ID <20120413221908.1079.50464.stgit@ginnungagap.bsc.es>
Download mbox | patch
Permalink /patch/152437/
State New
Headers show

Comments

Lluís Vilanova - April 13, 2012, 10:19 p.m.
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(-)
Stefan Hajnoczi - April 18, 2012, 1:10 p.m.
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
Lluís Vilanova - April 18, 2012, 1:45 p.m.
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
Stefan Hajnoczi - April 18, 2012, 2:08 p.m.
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

Patch

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