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