| Message ID | 1339225719-5434-1-git-send-email-sw@weilnetz.de |
|---|---|
| State | Accepted |
| Headers | show |
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
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
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 >
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
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)
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(-)