* [Qemu-devel] [PULL 0/3] Tracing patches
@ 2017-03-16 7:04 Stefan Hajnoczi
2017-03-16 7:04 ` [Qemu-devel] [PULL 1/3] makefile: merge GENERATED_HEADERS & GENERATED_SOURCES variables Stefan Hajnoczi
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2017-03-16 7:04 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Stefan Hajnoczi
The following changes since commit 1883ff34b540daacae948f493b0ba525edf5f642:
Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2017-03-15 18:44:05 +0000)
are available in the git repository at:
git://github.com/stefanha/qemu.git tags/tracing-pull-request
for you to fetch changes up to 8755b4afbdcf1c274cab7545a9f76d3d6c7f5c29:
trace: ensure $(tracetool-y) is defined in top level makefile (2017-03-16 11:51:26 +0800)
----------------------------------------------------------------
Pull request
Tracing makefile fixes for QEMU 2.9.
----------------------------------------------------------------
Daniel P. Berrange (3):
makefile: merge GENERATED_HEADERS & GENERATED_SOURCES variables
makefile: generate trace-events-all upfront
trace: ensure $(tracetool-y) is defined in top level makefile
Makefile | 42 ++++++++++++++++++++++--------------------
Makefile.target | 6 +++---
target/s390x/Makefile.objs | 2 +-
tests/Makefile.include | 2 +-
trace/Makefile.objs | 8 --------
5 files changed, 27 insertions(+), 33 deletions(-)
--
2.9.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PULL 1/3] makefile: merge GENERATED_HEADERS & GENERATED_SOURCES variables
2017-03-16 7:04 [Qemu-devel] [PULL 0/3] Tracing patches Stefan Hajnoczi
@ 2017-03-16 7:04 ` Stefan Hajnoczi
2017-03-16 9:08 ` Markus Armbruster
2017-03-16 7:04 ` [Qemu-devel] [PULL 2/3] makefile: generate trace-events-all upfront Stefan Hajnoczi
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Stefan Hajnoczi @ 2017-03-16 7:04 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi
From: "Daniel P. Berrange" <berrange@redhat.com>
The only functional difference between the GENERATED_HEADERS
and GENERATED_SOURCES variables is that 'Makefile' has a
dependancy on GENERATED_HEADERS, causing generated header files
to be created immediatey at the start of the build process.
There is no reason why this early creation should be restricted
to the .h files, and not include .c files too. Merge both of
the variables into a single GENERATED_FILES variable to make
it clear it is for any type of generated file.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 20170228122901.24520-2-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
Makefile | 35 +++++++++++++++++------------------
Makefile.target | 6 +++---
target/s390x/Makefile.objs | 2 +-
tests/Makefile.include | 2 +-
4 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/Makefile b/Makefile
index 1c4c04f..a8024c0 100644
--- a/Makefile
+++ b/Makefile
@@ -50,24 +50,24 @@ endif
include $(SRC_PATH)/rules.mak
-GENERATED_HEADERS = qemu-version.h config-host.h qemu-options.def
-GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
-GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
-GENERATED_HEADERS += qmp-introspect.h
-GENERATED_SOURCES += qmp-introspect.c
+GENERATED_FILES = qemu-version.h config-host.h qemu-options.def
+GENERATED_FILES += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
+GENERATED_FILES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
+GENERATED_FILES += qmp-introspect.h
+GENERATED_FILES += qmp-introspect.c
-GENERATED_HEADERS += trace/generated-tcg-tracers.h
+GENERATED_FILES += trace/generated-tcg-tracers.h
-GENERATED_HEADERS += trace/generated-helpers-wrappers.h
-GENERATED_HEADERS += trace/generated-helpers.h
-GENERATED_SOURCES += trace/generated-helpers.c
+GENERATED_FILES += trace/generated-helpers-wrappers.h
+GENERATED_FILES += trace/generated-helpers.h
+GENERATED_FILES += trace/generated-helpers.c
ifdef CONFIG_TRACE_UST
-GENERATED_HEADERS += trace-ust-all.h
-GENERATED_SOURCES += trace-ust-all.c
+GENERATED_FILES += trace-ust-all.h
+GENERATED_FILES += trace-ust-all.c
endif
-GENERATED_HEADERS += module_block.h
+GENERATED_FILES += module_block.h
TRACE_HEADERS = trace-root.h $(trace-events-subdirs:%=%/trace.h)
TRACE_SOURCES = trace-root.c $(trace-events-subdirs:%=%/trace.c)
@@ -80,8 +80,8 @@ ifdef CONFIG_TRACE_UST
TRACE_HEADERS += trace-ust-root.h $(trace-events-subdirs:%=%/trace-ust.h)
endif
-GENERATED_HEADERS += $(TRACE_HEADERS)
-GENERATED_SOURCES += $(TRACE_SOURCES)
+GENERATED_FILES += $(TRACE_HEADERS)
+GENERATED_FILES += $(TRACE_SOURCES)
trace-group-name = $(shell dirname $1 | sed -e 's/[^a-zA-Z0-9]/_/g')
@@ -485,11 +485,10 @@ clean:
rm -f fsdev/*.pod
rm -f qemu-img-cmds.h
rm -f ui/shader/*-vert.h ui/shader/*-frag.h
- @# May not be present in GENERATED_HEADERS
+ @# May not be present in GENERATED_FILES
rm -f trace/generated-tracers-dtrace.dtrace*
rm -f trace/generated-tracers-dtrace.h*
- rm -f $(foreach f,$(GENERATED_HEADERS),$(f) $(f)-timestamp)
- rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp)
+ rm -f $(foreach f,$(GENERATED_FILES),$(f) $(f)-timestamp)
rm -rf qapi-generated
rm -rf qga/qapi-generated
for d in $(ALL_SUBDIRS); do \
@@ -784,7 +783,7 @@ endif # CONFIG_WIN
# Add a dependency on the generated files, so that they are always
# rebuilt before other object files
ifneq ($(filter-out $(UNCHECKED_GOALS),$(MAKECMDGOALS)),$(if $(MAKECMDGOALS),,fail))
-Makefile: $(GENERATED_HEADERS)
+Makefile: $(GENERATED_FILES)
endif
.SECONDARY: $(TRACE_HEADERS) $(TRACE_HEADERS:%=%-timestamp) \
diff --git a/Makefile.target b/Makefile.target
index 924304c..7df2b8c 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -162,7 +162,7 @@ else
obj-y += hw/$(TARGET_BASE_ARCH)/
endif
-GENERATED_HEADERS += hmp-commands.h hmp-commands-info.h
+GENERATED_FILES += hmp-commands.h hmp-commands-info.h
endif # CONFIG_SOFTMMU
@@ -238,5 +238,5 @@ ifdef CONFIG_TRACE_SYSTEMTAP
$(INSTALL_DATA) $(QEMU_PROG)-simpletrace.stp "$(DESTDIR)$(qemu_datadir)/../systemtap/tapset/$(QEMU_PROG)-simpletrace.stp"
endif
-GENERATED_HEADERS += config-target.h
-Makefile: $(GENERATED_HEADERS)
+GENERATED_FILES += config-target.h
+Makefile: $(GENERATED_FILES)
diff --git a/target/s390x/Makefile.objs b/target/s390x/Makefile.objs
index c573633..46f6a8c 100644
--- a/target/s390x/Makefile.objs
+++ b/target/s390x/Makefile.objs
@@ -8,7 +8,7 @@ obj-$(CONFIG_KVM) += kvm.o
feat-src = $(SRC_PATH)/target/$(TARGET_BASE_ARCH)/
feat-dst = $(BUILD_DIR)/$(TARGET_DIR)
ifneq ($(MAKECMDGOALS),clean)
-GENERATED_HEADERS += $(feat-dst)gen-features.h
+GENERATED_FILES += $(feat-dst)gen-features.h
endif
$(feat-dst)gen-features.h: $(feat-dst)gen-features.h-timestamp
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 346345e..479dcd9 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -482,7 +482,7 @@ qapi-schema += unknown-expr-key.json
check-qapi-schema-y := $(addprefix tests/qapi-schema/, $(qapi-schema))
-GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h \
+GENERATED_FILES += tests/test-qapi-types.h tests/test-qapi-visit.h \
tests/test-qmp-commands.h tests/test-qapi-event.h \
tests/test-qmp-introspect.h
--
2.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PULL 2/3] makefile: generate trace-events-all upfront
2017-03-16 7:04 [Qemu-devel] [PULL 0/3] Tracing patches Stefan Hajnoczi
2017-03-16 7:04 ` [Qemu-devel] [PULL 1/3] makefile: merge GENERATED_HEADERS & GENERATED_SOURCES variables Stefan Hajnoczi
@ 2017-03-16 7:04 ` Stefan Hajnoczi
2017-03-16 7:04 ` [Qemu-devel] [PULL 3/3] trace: ensure $(tracetool-y) is defined in top level makefile Stefan Hajnoczi
2017-03-16 15:31 ` [Qemu-devel] [PULL 0/3] Tracing patches Peter Maydell
3 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2017-03-16 7:04 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi
From: "Daniel P. Berrange" <berrange@redhat.com>
Files should not be created in the build dir during the
'make install' phase. List 'trace-events-all' as a
generated file so that it gets created upfront during
build.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 20170228122901.24520-3-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index a8024c0..6291764 100644
--- a/Makefile
+++ b/Makefile
@@ -82,6 +82,7 @@ endif
GENERATED_FILES += $(TRACE_HEADERS)
GENERATED_FILES += $(TRACE_SOURCES)
+GENERATED_FILES += $(BUILD_DIR)/trace-events-all
trace-group-name = $(shell dirname $1 | sed -e 's/[^a-zA-Z0-9]/_/g')
@@ -592,8 +593,7 @@ endif
endif
-install: all $(if $(BUILD_DOCS),install-doc) $(BUILD_DIR)/trace-events-all \
-install-datadir install-localstatedir
+install: all $(if $(BUILD_DOCS),install-doc) install-datadir install-localstatedir
ifneq ($(TOOLS),)
$(call install-prog,$(subst qemu-ga,qemu-ga$(EXESUF),$(TOOLS)),$(DESTDIR)$(bindir))
endif
--
2.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PULL 3/3] trace: ensure $(tracetool-y) is defined in top level makefile
2017-03-16 7:04 [Qemu-devel] [PULL 0/3] Tracing patches Stefan Hajnoczi
2017-03-16 7:04 ` [Qemu-devel] [PULL 1/3] makefile: merge GENERATED_HEADERS & GENERATED_SOURCES variables Stefan Hajnoczi
2017-03-16 7:04 ` [Qemu-devel] [PULL 2/3] makefile: generate trace-events-all upfront Stefan Hajnoczi
@ 2017-03-16 7:04 ` Stefan Hajnoczi
2017-03-16 15:31 ` [Qemu-devel] [PULL 0/3] Tracing patches Peter Maydell
3 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2017-03-16 7:04 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrange, Stefan Hajnoczi
From: "Daniel P. Berrange" <berrange@redhat.com>
The build rules for trace files have a dependancy on $(tracetool-y).
This variable populated in the trace/Makefile.objs file and thus its
definition gets pulled into the top level makefile. This happens too
late in the process though, so by the time $(tracetool-y) is defined,
make has already evaluated $(tracetool-y) in the dependancies and
found it to be empty. The result is that when the tracetool source
is changed, the generated files are not rebuilt. The solution is to
define the variable in the top level makefile too
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Tested-by: Eric Blake <eblake@redhat.com>
Message-id: 20170315123421.28815-1-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
Makefile | 3 +++
trace/Makefile.objs | 8 --------
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile
index 6291764..108b984 100644
--- a/Makefile
+++ b/Makefile
@@ -86,6 +86,9 @@ GENERATED_FILES += $(BUILD_DIR)/trace-events-all
trace-group-name = $(shell dirname $1 | sed -e 's/[^a-zA-Z0-9]/_/g')
+tracetool-y = $(SRC_PATH)/scripts/tracetool.py
+tracetool-y += $(shell find $(SRC_PATH)/scripts/tracetool -name "*.py")
+
%/trace.h: %/trace.h-timestamp
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
%/trace.h-timestamp: $(SRC_PATH)/%/trace-events $(tracetool-y)
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 7de840a..1b8eb4a 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -1,13 +1,5 @@
# -*- mode: makefile -*-
-######################################################################
-# tracetool source files
-# Every rule that invokes tracetool must depend on this so code is regenerated
-# if tracetool itself changes.
-
-tracetool-y = $(SRC_PATH)/scripts/tracetool.py
-tracetool-y += $(shell find $(SRC_PATH)/scripts/tracetool -name "*.py")
-
$(BUILD_DIR)/trace-events-all: $(trace-events-files)
$(call quiet-command,cat $^ > $@)
--
2.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PULL 1/3] makefile: merge GENERATED_HEADERS & GENERATED_SOURCES variables
2017-03-16 7:04 ` [Qemu-devel] [PULL 1/3] makefile: merge GENERATED_HEADERS & GENERATED_SOURCES variables Stefan Hajnoczi
@ 2017-03-16 9:08 ` Markus Armbruster
2017-03-21 13:48 ` Eric Blake
0 siblings, 1 reply; 7+ messages in thread
From: Markus Armbruster @ 2017-03-16 9:08 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Peter Maydell
Sorry for chiming in late, I had missed this change.
Stefan Hajnoczi <stefanha@redhat.com> writes:
> From: "Daniel P. Berrange" <berrange@redhat.com>
>
> The only functional difference between the GENERATED_HEADERS
> and GENERATED_SOURCES variables is that 'Makefile' has a
> dependancy on GENERATED_HEADERS, causing generated header files
> to be created immediatey at the start of the build process.
> There is no reason why this early creation should be restricted
> to the .h files, and not include .c files too.
Actually, there is.
Any prerequisites of Makefile are made even by make -n. Restricting
them to the ones make -n absolutely needs is good practice.
Generated headers must be prerequisites of Makefile, because automatic
dependency generation may fail without them.
There is no such reason for generated non-headers.
> Merge both of
> the variables into a single GENERATED_FILES variable to make
> it clear it is for any type of generated file.
I don't hate this quite enough for an outright NAK at this late stage.
I do hate it enough to ask you to think about it once more.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PULL 0/3] Tracing patches
2017-03-16 7:04 [Qemu-devel] [PULL 0/3] Tracing patches Stefan Hajnoczi
` (2 preceding siblings ...)
2017-03-16 7:04 ` [Qemu-devel] [PULL 3/3] trace: ensure $(tracetool-y) is defined in top level makefile Stefan Hajnoczi
@ 2017-03-16 15:31 ` Peter Maydell
3 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2017-03-16 15:31 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: QEMU Developers
On 16 March 2017 at 07:04, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The following changes since commit 1883ff34b540daacae948f493b0ba525edf5f642:
>
> Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2017-03-15 18:44:05 +0000)
>
> are available in the git repository at:
>
> git://github.com/stefanha/qemu.git tags/tracing-pull-request
>
> for you to fetch changes up to 8755b4afbdcf1c274cab7545a9f76d3d6c7f5c29:
>
> trace: ensure $(tracetool-y) is defined in top level makefile (2017-03-16 11:51:26 +0800)
>
> ----------------------------------------------------------------
> Pull request
>
> Tracing makefile fixes for QEMU 2.9.
>
> ----------------------------------------------------------------
>
> Daniel P. Berrange (3):
> makefile: merge GENERATED_HEADERS & GENERATED_SOURCES variables
> makefile: generate trace-events-all upfront
> trace: ensure $(tracetool-y) is defined in top level makefile
>
> Makefile | 42 ++++++++++++++++++++++--------------------
> Makefile.target | 6 +++---
> target/s390x/Makefile.objs | 2 +-
> tests/Makefile.include | 2 +-
> trace/Makefile.objs | 8 --------
> 5 files changed, 27 insertions(+), 33 deletions(-)
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PULL 1/3] makefile: merge GENERATED_HEADERS & GENERATED_SOURCES variables
2017-03-16 9:08 ` Markus Armbruster
@ 2017-03-21 13:48 ` Eric Blake
0 siblings, 0 replies; 7+ messages in thread
From: Eric Blake @ 2017-03-21 13:48 UTC (permalink / raw)
To: Markus Armbruster, Stefan Hajnoczi; +Cc: Peter Maydell, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1774 bytes --]
On 03/16/2017 04:08 AM, Markus Armbruster wrote:
> Sorry for chiming in late, I had missed this change.
>
> Stefan Hajnoczi <stefanha@redhat.com> writes:
>
>> From: "Daniel P. Berrange" <berrange@redhat.com>
>>
>> The only functional difference between the GENERATED_HEADERS
>> and GENERATED_SOURCES variables is that 'Makefile' has a
>> dependancy on GENERATED_HEADERS, causing generated header files
>> to be created immediatey at the start of the build process.
>> There is no reason why this early creation should be restricted
>> to the .h files, and not include .c files too.
>
> Actually, there is.
>
> Any prerequisites of Makefile are made even by make -n. Restricting
> them to the ones make -n absolutely needs is good practice.
In fact, this is part of what tripped me up in trying to get to a root
cause in why modifying scripts/tracetool/format/h.py didn't cause
trace.h to be regenerated. When trace.h is treated as a prerequisite of
Makefile, it's rules get run automatically and don't show up in 'make
--debug' output (you have to resort to the noisier 'make -d' to see that
the rule was run).
>
> Generated headers must be prerequisites of Makefile, because automatic
> dependency generation may fail without them.
>
> There is no such reason for generated non-headers.
>
>> Merge both of
>> the variables into a single GENERATED_FILES variable to make
>> it clear it is for any type of generated file.
>
> I don't hate this quite enough for an outright NAK at this late stage.
> I do hate it enough to ask you to think about it once more.
>
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-03-21 13:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-16 7:04 [Qemu-devel] [PULL 0/3] Tracing patches Stefan Hajnoczi
2017-03-16 7:04 ` [Qemu-devel] [PULL 1/3] makefile: merge GENERATED_HEADERS & GENERATED_SOURCES variables Stefan Hajnoczi
2017-03-16 9:08 ` Markus Armbruster
2017-03-21 13:48 ` Eric Blake
2017-03-16 7:04 ` [Qemu-devel] [PULL 2/3] makefile: generate trace-events-all upfront Stefan Hajnoczi
2017-03-16 7:04 ` [Qemu-devel] [PULL 3/3] trace: ensure $(tracetool-y) is defined in top level makefile Stefan Hajnoczi
2017-03-16 15:31 ` [Qemu-devel] [PULL 0/3] Tracing patches Peter Maydell
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.