From: Paolo Bonzini <pbonzini@redhat.com>
To: "Lluís Vilanova" <vilanova@ac.upc.edu>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] build: Use separate makefile for "trace/"
Date: Wed, 12 Dec 2012 09:43:21 +0100 [thread overview]
Message-ID: <50C843A9.9020608@redhat.com> (raw)
In-Reply-To: <20121211214412.32738.80113.stgit@fimbulvetr.bsc.es>
Il 11/12/2012 22:44, Lluís Vilanova ha scritto:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
> .gitignore | 6 +--
> Makefile | 8 ++-
> Makefile.objs | 64 +---------------------------
> scripts/tracetool/backend/dtrace.py | 2 -
> trace/Makefile.objs | 81 +++++++++++++++++++++++++++++++++++
> 5 files changed, 92 insertions(+), 69 deletions(-)
> create mode 100644 trace/Makefile.objs
>
> diff --git a/.gitignore b/.gitignore
> index bd6ba1c..b67a37e 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -3,9 +3,9 @@ config-all-devices.*
> config-host.*
> config-target.*
> trace.h
> -trace.c
> -trace-dtrace.h
> -trace-dtrace.dtrace
> +trace/generated.c
> +trace/generated-dtrace.h
> +trace/generated-dtrace.dtrace
> *-timestamp
> *-softmmu
> *-darwin-user
> diff --git a/Makefile b/Makefile
> index e9d6848..9dbcca3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -33,10 +33,10 @@ endif
>
> GENERATED_HEADERS = config-host.h trace.h qemu-options.def
> ifeq ($(TRACE_BACKEND),dtrace)
> -GENERATED_HEADERS += trace-dtrace.h
> +GENERATED_HEADERS += trace/generated-dtrace.h
> endif
> GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
> -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c trace.c
> +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c trace/generated.c
>
> # Don't try to regenerate Makefile or configure
> # We don't generate any of them
> @@ -252,9 +252,9 @@ clean:
> rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~
> rm -Rf .libs
> rm -f qemu-img-cmds.h
> - rm -f trace-dtrace.dtrace trace-dtrace.dtrace-timestamp
> @# May not be present in GENERATED_HEADERS
> - rm -f trace-dtrace.h trace-dtrace.h-timestamp
> + rm -f trace/generated-dtrace.dtrace trace/generated-dtrace.dtrace-timestamp
> + rm -f trace/generated-dtrace.h trace/generated-dtrace.h-timestamp
> rm -f $(foreach f,$(GENERATED_HEADERS),$(f) $(f)-timestamp)
> rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp)
> rm -rf qapi-generated
> diff --git a/Makefile.objs b/Makefile.objs
> index 3c7abca..24832a2 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -147,66 +147,7 @@ libdis-$(CONFIG_LM32_DIS) += lm32-dis.o
> ######################################################################
> # trace
>
> -ifeq ($(TRACE_BACKEND),dtrace)
> -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,$(TRACETOOL) \
> - --format=h \
> - --backend=$(TRACE_BACKEND) \
> - < $< > $@," GEN trace.h")
> - @cmp -s $@ trace.h || cp $@ trace.h
> -
> -trace.c: trace.c-timestamp
> -trace.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
> - $(call quiet-command,$(TRACETOOL) \
> - --format=c \
> - --backend=$(TRACE_BACKEND) \
> - < $< > $@," GEN trace.c")
> - @cmp -s $@ trace.c || cp $@ trace.c
> -
> -trace.o: trace.c $(GENERATED_HEADERS)
> -
> -trace-dtrace.h: trace-dtrace.dtrace
> - $(call quiet-command,dtrace -o $@ -h -s $<, " GEN trace-dtrace.h")
> -
> -# Normal practice is to name DTrace probe file with a '.d' extension
> -# but that gets picked up by QEMU's Makefile as an external dependency
> -# 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,$(TRACETOOL) \
> - --format=d \
> - --backend=$(TRACE_BACKEND) \
> - < $< > $@," GEN trace-dtrace.dtrace")
> - @cmp -s $@ trace-dtrace.dtrace || cp $@ trace-dtrace.dtrace
> -
> -trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS)
> - $(call quiet-command,dtrace -o $@ -G -s $<, " GEN trace-dtrace.o")
> -
> -ifeq ($(LIBTOOL),)
> -trace-dtrace.lo: trace-dtrace.dtrace
> - @echo "missing libtool. please install and rerun configure."; exit 1
> -else
> -trace-dtrace.lo: trace-dtrace.dtrace
> - $(call quiet-command,$(LIBTOOL) --mode=compile --tag=CC dtrace -o $@ -G -s $<, " lt GEN trace-dtrace.o")
> -endif
> -
> -trace/simple.o: trace/simple.c $(GENERATED_HEADERS)
> -
> -trace-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace.o
> -ifneq ($(TRACE_BACKEND),dtrace)
> -trace-obj-y = trace.o
> -endif
> -
> -trace-obj-$(CONFIG_TRACE_DEFAULT) += trace/default.o
> -trace-obj-$(CONFIG_TRACE_SIMPLE) += trace/simple.o
> -trace-obj-$(CONFIG_TRACE_SIMPLE) += qemu-timer-common.o
> -trace-obj-$(CONFIG_TRACE_STDERR) += trace/stderr.o
> -trace-obj-y += trace/control.o
> -
> -$(trace-obj-y): $(GENERATED_HEADERS)
> +trace-obj-y += trace/
Please just put it into oslib-obj-y.
> ######################################################################
> # smartcard
> @@ -250,5 +191,6 @@ nested-vars += \
> block-obj-y \
> user-obj-y \
> common-obj-y \
> - extra-obj-y
> + extra-obj-y \
> + trace-obj-y
> dummy := $(call unnest-vars)
> diff --git a/scripts/tracetool/backend/dtrace.py b/scripts/tracetool/backend/dtrace.py
> index 23c43e2..0cbc68a 100644
> --- a/scripts/tracetool/backend/dtrace.py
> +++ b/scripts/tracetool/backend/dtrace.py
> @@ -37,7 +37,7 @@ def c(events):
>
>
> def h(events):
> - out('#include "trace-dtrace.h"',
> + out('#include "trace/generated-dtrace.h"',
> '')
>
> for e in events:
> diff --git a/trace/Makefile.objs b/trace/Makefile.objs
> new file mode 100644
> index 0000000..d7e8cdf
> --- /dev/null
> +++ b/trace/Makefile.objs
> @@ -0,0 +1,81 @@
> +# -*- mode: makefile -*-
> +
> +trace/simple.o: trace/simple.c $(GENERATED_HEADERS)
> +
> +trace-obj-$(CONFIG_TRACE_DEFAULT) += default.o
> +trace-obj-$(CONFIG_TRACE_SIMPLE) += simple.o
> +trace-obj-$(CONFIG_TRACE_SIMPLE) += ../qemu-timer-common.o
Is this line needed? (Actually, it is obviously unneeded if you use
oslib-obj-y). I am also worried that it causes duplicate symbols when
you link in both qemu-timer-common.o and trace/../qemu-timer-common.o.
The $(sort) invocation in the LINK macro of rules.mak will not treat
these two paths as duplicate.
> +trace-obj-$(CONFIG_TRACE_STDERR) += stderr.o
> +trace-obj-y += control.o
> +
> +
> +######################################################################
> +# Auto-generated tracing routines
> +
> +# NOTE: "trace.h" is kept in the top-level dir to shorten common include path
Ok, I'll take care of moving it to trace/ later when I do the same for
all other includes.
> +ifeq ($(TRACE_BACKEND),dtrace)
> +TRACE_H_EXTRA_DEPS=trace/generated-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,$(TRACETOOL) \
> + --format=h \
> + --backend=$(TRACE_BACKEND) \
> + < $< > $@," GEN trace.h")
> + @cmp -s $@ trace.h || cp $@ trace.h
> +
> +
> +trace/generated.c: trace/generated.c-timestamp
Please use $(obj)/generated.c, and similarly for everything else.
> +trace/generated.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
> + $(call quiet-command,$(TRACETOOL) \
> + --format=c \
> + --backend=$(TRACE_BACKEND) \
> + < $< > $@," GEN trace/generated.c")
> + @cmp -s $@ trace/generated.c || cp $@ trace/generated.c
> +
> +trace/generated.o: trace/generated.c trace.h
> +
> +
> +ifneq ($(TRACE_BACKEND),dtrace)
> +trace-obj-y += generated.o
> +endif
> +
> +
> +######################################################################
> +# Auto-generated DTrace code
> +
> +# Normal practice is to name DTrace probe file with a '.d' extension
> +# but that gets picked up by QEMU's Makefile as an external dependency
> +# rule file. So we use '.dtrace' instead
> +trace/generated-dtrace.dtrace: trace/generated-dtrace.dtrace-timestamp
> +trace/generated-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
> + $(call quiet-command,$(TRACETOOL) \
> + --format=d \
> + --backend=$(TRACE_BACKEND) \
> + < $< > $@," GEN trace/generated-dtrace.dtrace")
> + @cmp -s $@ trace/generated-dtrace.dtrace || cp $@ trace/generated-dtrace.dtrace
> +
> +
> +trace/generated-dtrace.h: trace/generated-dtrace.dtrace
> + $(call quiet-command,dtrace -o $@ -h -s $<, " GEN trace/generated-dtrace.h")
> +
> +trace/generated-dtrace.o: trace/generated-dtrace.dtrace $(GENERATED_HEADERS)
> + $(call quiet-command,dtrace -o $@ -G -s $<, " GEN trace/generated-dtrace.o")
> +
> +
> +ifeq ($(LIBTOOL),)
> +trace/generated-dtrace.lo: trace/generated-dtrace.dtrace
> + @echo "missing libtool. please install and rerun configure."; exit 1
> +else
> +trace/generated-dtrace.lo: trace/generated-dtrace.dtrace
> + $(call quiet-command,$(LIBTOOL) --mode=compile --tag=CC dtrace -o $@ -G -s $<, " lt GEN trace/generated-dtrace.lo")
> +endif
> +
> +
> +trace-obj-$(CONFIG_TRACE_DTRACE) += generated-dtrace.o
> +
> +
> +######################################################################
> +# Keep at bottom
> +
> +$(trace-obj-y): $(GENERATED_HEADERS)
This does not do what you think, because trace-obj-y includes a relative
path. It can be simply
trace/%.o: $(GENERATED_HEADERS)
but it is actually unnecessary because of this in the toplevel:
# Add a dependency on the generated files, so that they are always
# rebuilt before other object files
ifneq ($(filter-out %clean,$(MAKECMDGOALS)),$(if $(MAKECMDGOALS),,fail))
Makefile: $(GENERATED_HEADERS)
endif
Paolo
next prev parent reply other threads:[~2012-12-12 8:43 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-11 21:44 [Qemu-devel] [PATCH] build: Use separate makefile for "trace/" Lluís Vilanova
2012-12-12 8:43 ` Paolo Bonzini [this message]
2012-12-12 14:53 ` Lluís Vilanova
2012-12-13 8:44 ` Paolo Bonzini
2012-12-13 14:38 ` Lluís Vilanova
2012-12-14 8:14 ` Paolo Bonzini
2012-12-14 13:46 ` Lluís Vilanova
2012-12-14 13:50 ` Paolo Bonzini
2012-12-14 15:04 ` Lluís Vilanova
-- strict thread matches above, loose matches on Subject: below --
2012-12-11 21:49 Lluís Vilanova
2012-12-14 18:34 Lluís Vilanova
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50C843A9.9020608@redhat.com \
--to=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=vilanova@ac.upc.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.