[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/2] Makefile: Remove BUILD_DIR f
From: |
Michael Roth |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/2] Makefile: Remove BUILD_DIR from qapi-dir |
Date: |
Tue, 12 Jun 2012 09:14:39 -0500 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> > ---
> > 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 <address@hidden>
> 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 <address@hidden>
>
> 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
>