* [Qemu-devel] [PATCH 1/2] Makefile: Remove BUILD_DIR from qapi-dir
@ 2012-06-09 7:08 Stefan Weil
2012-06-09 7:08 ` [Qemu-devel] [PATCH 2/2] Makefile: Remove macro qapi-dir Stefan Weil
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Stefan Weil @ 2012-06-09 7:08 UTC (permalink / raw)
To: qemu-trivial; +Cc: Stefan Weil, qemu-devel
qapi-dir does not need an absolute path. All other build directories
are relative. When BUILD_DIR is removed, the build output looks better
(no long lines with absolute paths when everything else uses short
lines):
GEN qapi-generated/qga-qapi-types.c
CC qapi-generated/qga-qapi-types.o
GEN qapi-generated/qga-qapi-visit.c
CC qapi-generated/qga-qapi-visit.o
GEN qapi-generated/qga-qmp-marshal.c
CC qapi-generated/qga-qmp-marshal.o
Using a relative path also avoids potential problems when BUILD_DIR
includes blanks.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 32550cb..8880a3e 100644
--- a/Makefile
+++ b/Makefile
@@ -171,7 +171,7 @@ fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@," GEN $@")
-qapi-dir := $(BUILD_DIR)/qapi-generated
+qapi-dir := qapi-generated
qemu-ga$(EXESUF): LIBS = $(LIBS_QGA)
qemu-ga$(EXESUF): QEMU_CFLAGS += -I $(qapi-dir)
--
1.7.10
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 2/2] Makefile: Remove macro qapi-dir
2012-06-09 7:08 [Qemu-devel] [PATCH 1/2] Makefile: Remove BUILD_DIR from qapi-dir Stefan Weil
@ 2012-06-09 7:08 ` Stefan Weil
2012-06-12 9:46 ` [Qemu-devel] [PATCH 1/2] Makefile: Remove BUILD_DIR from qapi-dir Stefan Hajnoczi
2012-06-12 9:49 ` Stefan Hajnoczi
2 siblings, 0 replies; 6+ messages in thread
From: Stefan Weil @ 2012-06-09 7:08 UTC (permalink / raw)
To: qemu-trivial; +Cc: Stefan Weil, qemu-devel
As qapi-dir was now a constant, it can be replaced by its value.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
Makefile | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/Makefile b/Makefile
index 8880a3e..10874b8 100644
--- a/Makefile
+++ b/Makefile
@@ -171,9 +171,8 @@ fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@," GEN $@")
-qapi-dir := qapi-generated
qemu-ga$(EXESUF): LIBS = $(LIBS_QGA)
-qemu-ga$(EXESUF): QEMU_CFLAGS += -I $(qapi-dir)
+qemu-ga$(EXESUF): QEMU_CFLAGS += -I qapi-generated
gen-out-type = $(subst .,-,$(suffix $@))
@@ -181,15 +180,15 @@ ifneq ($(wildcard config-host.mak),)
include $(SRC_PATH)/tests/Makefile
endif
-$(qapi-dir)/qga-qapi-types.c $(qapi-dir)/qga-qapi-types.h :\
+qapi-generated/qga-qapi-types.c qapi-generated/qga-qapi-types.h :\
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-types.py
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o "$(qapi-dir)" -p "qga-" < $<, " GEN $@")
-$(qapi-dir)/qga-qapi-visit.c $(qapi-dir)/qga-qapi-visit.h :\
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o qapi-generated -p "qga-" < $<, " GEN $@")
+qapi-generated/qga-qapi-visit.c qapi-generated/qga-qapi-visit.h :\
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-visit.py
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "$(qapi-dir)" -p "qga-" < $<, " GEN $@")
-$(qapi-dir)/qga-qmp-commands.h $(qapi-dir)/qga-qmp-marshal.c :\
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o qapi-generated -p "qga-" < $<, " GEN $@")
+qapi-generated/qga-qmp-commands.h qapi-generated/qga-qmp-marshal.c :\
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -o "$(qapi-dir)" -p "qga-" < $<, " GEN $@")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -o qapi-generated -p "qga-" < $<, " GEN $@")
qapi-types.c qapi-types.h :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py
@@ -201,8 +200,8 @@ qmp-commands.h qmp-marshal.c :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o "." < $<, " GEN $@")
-QGALIB_OBJ=$(addprefix $(qapi-dir)/, qga-qapi-types.o qga-qapi-visit.o qga-qmp-marshal.o)
-QGALIB_GEN=$(addprefix $(qapi-dir)/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
+QGALIB_OBJ=$(addprefix qapi-generated/, qga-qapi-types.o qga-qapi-visit.o qga-qmp-marshal.o)
+QGALIB_GEN=$(addprefix qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
$(QGALIB_OBJ): $(QGALIB_GEN)
$(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
@@ -226,7 +225,7 @@ clean:
rm -f trace-dtrace.h trace-dtrace.h-timestamp
rm -f $(foreach f,$(GENERATED_HEADERS),$(f) $(f)-timestamp)
rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp)
- rm -rf $(qapi-dir)
+ rm -rf qapi-generated
$(MAKE) -C tests/tcg clean
for d in $(ALL_SUBDIRS) $(QEMULIBS) libcacard; do \
if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \
--
1.7.10
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Makefile: Remove BUILD_DIR from qapi-dir
2012-06-09 7:08 [Qemu-devel] [PATCH 1/2] Makefile: Remove BUILD_DIR from qapi-dir Stefan Weil
2012-06-09 7:08 ` [Qemu-devel] [PATCH 2/2] Makefile: Remove macro qapi-dir Stefan Weil
@ 2012-06-12 9:46 ` Stefan Hajnoczi
2012-06-12 9:49 ` Stefan Hajnoczi
2 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2012-06-12 9:46 UTC (permalink / raw)
To: Stefan Weil; +Cc: qemu-trivial, qemu-devel, Michael Roth
On Sat, Jun 09, 2012 at 09:08:38AM +0200, Stefan Weil wrote:
> qapi-dir does not need an absolute path. All other build directories
> are relative. When BUILD_DIR is removed, the build output looks better
> (no long lines with absolute paths when everything else uses short
> lines):
>
> GEN qapi-generated/qga-qapi-types.c
> CC qapi-generated/qga-qapi-types.o
> GEN qapi-generated/qga-qapi-visit.c
> CC qapi-generated/qga-qapi-visit.o
> GEN qapi-generated/qga-qmp-marshal.c
> CC qapi-generated/qga-qmp-marshal.o
>
> Using a relative path also avoids potential problems when BUILD_DIR
> includes blanks.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Makefile: Remove BUILD_DIR from qapi-dir
2012-06-09 7:08 [Qemu-devel] [PATCH 1/2] Makefile: Remove BUILD_DIR from qapi-dir Stefan Weil
2012-06-09 7:08 ` [Qemu-devel] [PATCH 2/2] Makefile: Remove macro qapi-dir Stefan Weil
2012-06-12 9:46 ` [Qemu-devel] [PATCH 1/2] Makefile: Remove BUILD_DIR from qapi-dir Stefan Hajnoczi
@ 2012-06-12 9:49 ` Stefan Hajnoczi
2012-06-12 14:14 ` Michael Roth
2 siblings, 1 reply; 6+ messages in thread
From: Stefan Hajnoczi @ 2012-06-12 9:49 UTC (permalink / raw)
To: Michael Roth; +Cc: qemu-trivial, Stefan Weil, qemu-devel
On Sat, Jun 09, 2012 at 09:08:38AM +0200, Stefan Weil wrote:
> qapi-dir does not need an absolute path. All other build directories
> are relative. When BUILD_DIR is removed, the build output looks better
> (no long lines with absolute paths when everything else uses short
> lines):
>
> GEN qapi-generated/qga-qapi-types.c
> CC qapi-generated/qga-qapi-types.o
> GEN qapi-generated/qga-qapi-visit.c
> CC qapi-generated/qga-qapi-visit.o
> GEN qapi-generated/qga-qmp-marshal.c
> CC qapi-generated/qga-qmp-marshal.o
>
> Using a relative path also avoids potential problems when BUILD_DIR
> includes blanks.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
I merged this but have CCed Michael Roth to take a look. qemu.git
contains an earlier commit to explicitly add $(BUILD_DIR):
commit 9b129408589b2ed7bb2cdea03d2aba46a5fd74d4
Author: Michael Roth <mdroth@linux.vnet.ibm.com>
Date: Tue Nov 29 16:47:49 2011 -0600
Makefile: use full path for qapi-generated directory
Generally $(BUILD_DIR) == $(CURDIR), but that isn't necessarilly the
case, so use $(BUILD_DIR)/qapi-generated for generated files to
avoid potentionally sticking generating files in odd places outside
the build's include paths.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Does this rationale still apply? Is it really a good ideal to remove
$(BUILD_DIR)?
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Makefile: Remove BUILD_DIR from qapi-dir
2012-06-12 9:49 ` Stefan Hajnoczi
@ 2012-06-12 14:14 ` Michael Roth
2012-06-13 9:24 ` Stefan Hajnoczi
0 siblings, 1 reply; 6+ messages in thread
From: Michael Roth @ 2012-06-12 14:14 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-trivial, Stefan Weil, qemu-devel
On Tue, Jun 12, 2012 at 10:49:57AM +0100, Stefan Hajnoczi wrote:
> On Sat, Jun 09, 2012 at 09:08:38AM +0200, Stefan Weil wrote:
> > qapi-dir does not need an absolute path. All other build directories
> > are relative. When BUILD_DIR is removed, the build output looks better
> > (no long lines with absolute paths when everything else uses short
> > lines):
> >
> > GEN qapi-generated/qga-qapi-types.c
> > CC qapi-generated/qga-qapi-types.o
> > GEN qapi-generated/qga-qapi-visit.c
> > CC qapi-generated/qga-qapi-visit.o
> > GEN qapi-generated/qga-qmp-marshal.c
> > CC qapi-generated/qga-qmp-marshal.o
> >
> > Using a relative path also avoids potential problems when BUILD_DIR
> > includes blanks.
> >
> > Signed-off-by: Stefan Weil <sw@weilnetz.de>
> > ---
> > Makefile | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
> I merged this but have CCed Michael Roth to take a look. qemu.git
> contains an earlier commit to explicitly add $(BUILD_DIR):
>
> commit 9b129408589b2ed7bb2cdea03d2aba46a5fd74d4
> Author: Michael Roth <mdroth@linux.vnet.ibm.com>
> Date: Tue Nov 29 16:47:49 2011 -0600
>
> Makefile: use full path for qapi-generated directory
>
> Generally $(BUILD_DIR) == $(CURDIR), but that isn't necessarilly the
> case, so use $(BUILD_DIR)/qapi-generated for generated files to
> avoid potentionally sticking generating files in odd places outside
> the build's include paths.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>
> Does this rationale still apply? Is it really a good ideal to remove
> $(BUILD_DIR)?
Here's some context for the patch:
http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg03474.html
Originally a change was made to add a $(SRC_DIR) prefix to fix a build.
This triggered an unrelated bug, and was not quite correct. I submitted
a patch to fix the bug it triggered, and then a patch that used
$(BUILD_DIR) instead.
I'm not sure the latter was ever needed however, it looks like it was
just the classic issue of someone having a dirty $(SRC_DIR) and
subsequently switching to out of tree builds. I think the original issue
was simple a stale qapi-generated/ directory sitting in $(SRC_DIR) that
was taking precedence over the one being created in $(BUILD_DIR)
So, while having the $(BUILD_DIR) prefix does make the build safer in
this regard, I think it addresses a use-case that was never supported to
begin with. So I'd be fine with applying Stefan's patch if it makes
things cleaner/more consistent with everything else.
>
> Stefan
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Makefile: Remove BUILD_DIR from qapi-dir
2012-06-12 14:14 ` Michael Roth
@ 2012-06-13 9:24 ` Stefan Hajnoczi
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2012-06-13 9:24 UTC (permalink / raw)
To: Michael Roth; +Cc: qemu-trivial, Stefan Weil, qemu-devel
On Tue, Jun 12, 2012 at 3:14 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> On Tue, Jun 12, 2012 at 10:49:57AM +0100, Stefan Hajnoczi wrote:
>> On Sat, Jun 09, 2012 at 09:08:38AM +0200, Stefan Weil wrote:
>> > qapi-dir does not need an absolute path. All other build directories
>> > are relative. When BUILD_DIR is removed, the build output looks better
>> > (no long lines with absolute paths when everything else uses short
>> > lines):
>> >
>> > GEN qapi-generated/qga-qapi-types.c
>> > CC qapi-generated/qga-qapi-types.o
>> > GEN qapi-generated/qga-qapi-visit.c
>> > CC qapi-generated/qga-qapi-visit.o
>> > GEN qapi-generated/qga-qmp-marshal.c
>> > CC qapi-generated/qga-qmp-marshal.o
>> >
>> > Using a relative path also avoids potential problems when BUILD_DIR
>> > includes blanks.
>> >
>> > Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> > ---
>> > Makefile | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> I merged this but have CCed Michael Roth to take a look. qemu.git
>> contains an earlier commit to explicitly add $(BUILD_DIR):
>>
>> commit 9b129408589b2ed7bb2cdea03d2aba46a5fd74d4
>> Author: Michael Roth <mdroth@linux.vnet.ibm.com>
>> Date: Tue Nov 29 16:47:49 2011 -0600
>>
>> Makefile: use full path for qapi-generated directory
>>
>> Generally $(BUILD_DIR) == $(CURDIR), but that isn't necessarilly the
>> case, so use $(BUILD_DIR)/qapi-generated for generated files to
>> avoid potentionally sticking generating files in odd places outside
>> the build's include paths.
>>
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>
>> Does this rationale still apply? Is it really a good ideal to remove
>> $(BUILD_DIR)?
>
> Here's some context for the patch:
>
> http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg03474.html
>
> Originally a change was made to add a $(SRC_DIR) prefix to fix a build.
> This triggered an unrelated bug, and was not quite correct. I submitted
> a patch to fix the bug it triggered, and then a patch that used
> $(BUILD_DIR) instead.
>
> I'm not sure the latter was ever needed however, it looks like it was
> just the classic issue of someone having a dirty $(SRC_DIR) and
> subsequently switching to out of tree builds. I think the original issue
> was simple a stale qapi-generated/ directory sitting in $(SRC_DIR) that
> was taking precedence over the one being created in $(BUILD_DIR)
>
> So, while having the $(BUILD_DIR) prefix does make the build safer in
> this regard, I think it addresses a use-case that was never supported to
> begin with. So I'd be fine with applying Stefan's patch if it makes
> things cleaner/more consistent with everything else.
Thanks for explaining. I will include Stefan Weil's patch.
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-06-13 9:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-09 7:08 [Qemu-devel] [PATCH 1/2] Makefile: Remove BUILD_DIR from qapi-dir Stefan Weil
2012-06-09 7:08 ` [Qemu-devel] [PATCH 2/2] Makefile: Remove macro qapi-dir Stefan Weil
2012-06-12 9:46 ` [Qemu-devel] [PATCH 1/2] Makefile: Remove BUILD_DIR from qapi-dir Stefan Hajnoczi
2012-06-12 9:49 ` Stefan Hajnoczi
2012-06-12 14:14 ` Michael Roth
2012-06-13 9:24 ` Stefan Hajnoczi
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).