diff mbox

fix out of tree build

Message ID alpine.DEB.2.00.1111221726190.31179@kaball-desktop
State New
Headers show

Commit Message

Stefano Stabellini Nov. 22, 2011, 5:27 p.m. UTC 
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Comments

Stefan Hajnoczi Nov. 23, 2011, 9:48 a.m. UTC | #1 
On Tue, Nov 22, 2011 at 05:27:15PM +0000, Stefano Stabellini wrote:
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Thanks, applied to the trivial patches -next tree:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches-next

Stefan
Stefan Weil Nov. 28, 2011, 6:34 p.m. UTC | #2 
Am 22.11.2011 18:27, schrieb Stefano Stabellini:
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
> diff --git a/Makefile b/Makefile
> index 168093c..885065f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -167,7 +167,7 @@ check-qjson: check-qjson.o $(qobject-obj-y) 
> $(tools-obj-y)
> test-coroutine: test-coroutine.o qemu-timer-common.o async.o 
> $(coroutine-obj-y) $(tools-obj-y)
>
> $(qapi-obj-y): $(GENERATED_HEADERS)
> -qapi-dir := qapi-generated
> +qapi-dir := $(SRC_PATH)/qapi-generated
> test-visitor.o test-qmp-commands.o qemu-ga$(EXESUF): QEMU_CFLAGS += -I 
> $(qapi-dir)
> qemu-ga$(EXESUF): LIBS = $(LIBS_QGA)

This patch was committed to latest QEMU and breaks in-tree and 
out-of-tree builds.
Without it, both kinds of builds work in my standard Debian configuration.

Regards,
Stefan Weil


gcc output (extract):

CC /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.o
/home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c: 
In function ‘qapi_free_GuestAgentInfoList’:
/home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c:39: 
warning: implicit declaration of function ‘visit_type_GuestAgentInfoList’
/home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c:39: 
warning: nested extern declaration of ‘visit_type_GuestAgentInfoList’
/home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c: 
In function ‘qapi_free_GuestAgentInfo’:
/home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c:54: 
warning: implicit declaration of function ‘visit_type_GuestAgentInfo’
/home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c:54: 
warning: nested extern declaration of ‘visit_type_GuestAgentInfo’
/home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c: 
In function ‘qapi_free_GuestFileReadList’:
/home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c:69: 
warning: implicit declaration of function ‘visit_type_GuestFileReadList’
Anthony Liguori Nov. 28, 2011, 6:45 p.m. UTC | #3 
On 11/28/2011 12:34 PM, Stefan Weil wrote:
> Am 22.11.2011 18:27, schrieb Stefano Stabellini:
>>
>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>
>> diff --git a/Makefile b/Makefile
>> index 168093c..885065f 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -167,7 +167,7 @@ check-qjson: check-qjson.o $(qobject-obj-y) $(tools-obj-y)
>> test-coroutine: test-coroutine.o qemu-timer-common.o async.o
>> $(coroutine-obj-y) $(tools-obj-y)
>>
>> $(qapi-obj-y): $(GENERATED_HEADERS)
>> -qapi-dir := qapi-generated
>> +qapi-dir := $(SRC_PATH)/qapi-generated
>> test-visitor.o test-qmp-commands.o qemu-ga$(EXESUF): QEMU_CFLAGS += -I
>> $(qapi-dir)
>> qemu-ga$(EXESUF): LIBS = $(LIBS_QGA)
>
> This patch was committed to latest QEMU and breaks in-tree and out-of-tree builds.
> Without it, both kinds of builds work in my standard Debian configuration.

The patch is obviously incorrect.  I'm going to revert it since the commit 
message has no explanation of why it's fixing anything.

Regards,

Anthony Liguori

>
> Regards,
> Stefan Weil
>
>
> gcc output (extract):
>
> CC /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.o
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c: In
> function ‘qapi_free_GuestAgentInfoList’:
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c:39:
> warning: implicit declaration of function ‘visit_type_GuestAgentInfoList’
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c:39:
> warning: nested extern declaration of ‘visit_type_GuestAgentInfoList’
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c: In
> function ‘qapi_free_GuestAgentInfo’:
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c:54:
> warning: implicit declaration of function ‘visit_type_GuestAgentInfo’
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c:54:
> warning: nested extern declaration of ‘visit_type_GuestAgentInfo’
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c: In
> function ‘qapi_free_GuestFileReadList’:
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c:69:
> warning: implicit declaration of function ‘visit_type_GuestFileReadList’
>
>
Stefan Weil Nov. 28, 2011, 8:40 p.m. UTC | #4 
Am 28.11.2011 19:34, schrieb Stefan Weil:
> Am 22.11.2011 18:27, schrieb Stefano Stabellini:
>>
>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>
>> diff --git a/Makefile b/Makefile
>> index 168093c..885065f 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -167,7 +167,7 @@ check-qjson: check-qjson.o $(qobject-obj-y) 
>> $(tools-obj-y)
>> test-coroutine: test-coroutine.o qemu-timer-common.o async.o 
>> $(coroutine-obj-y) $(tools-obj-y)
>>
>> $(qapi-obj-y): $(GENERATED_HEADERS)
>> -qapi-dir := qapi-generated
>> +qapi-dir := $(SRC_PATH)/qapi-generated
>> test-visitor.o test-qmp-commands.o qemu-ga$(EXESUF): QEMU_CFLAGS += 
>> -I $(qapi-dir)
>> qemu-ga$(EXESUF): LIBS = $(LIBS_QGA)
>
> This patch was committed to latest QEMU and breaks in-tree and 
> out-of-tree builds.
> Without it, both kinds of builds work in my standard Debian 
> configuration.
>
> Regards,
> Stefan Weil
>
>
> gcc output (extract):
>
> CC /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.o
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c: In 
> function ‘qapi_free_GuestAgentInfoList’:
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c:39: 
> warning: implicit declaration of function ‘visit_type_GuestAgentInfoList’
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c:39: 
> warning: nested extern declaration of ‘visit_type_GuestAgentInfoList’
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c: In 
> function ‘qapi_free_GuestAgentInfo’:
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c:54: 
> warning: implicit declaration of function ‘visit_type_GuestAgentInfo’
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c:54: 
> warning: nested extern declaration of ‘visit_type_GuestAgentInfo’
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c: In 
> function ‘qapi_free_GuestFileReadList’:
> /home/stefan/src/qemu/qemu.org/patches/qapi-generated/qga-qapi-types.c:69: 
> warning: implicit declaration of function ‘visit_type_GuestFileReadList’

The broken build can be reproduced with a source path containing at 
least one dot.
Source paths without any dot will show no problems. My source path was 
~/src/qemu/qemu.org/qemu.

File scripts/qapi.py fails to create unique guard names when given a 
filename with 2 or more dots:

def guardname(filename):
     if filename.startswith('./'):
         filename = filename[2:]
     return filename.replace("/", "_").replace("-", 
"_").split(".")[0].upper() + '_H'

The function always returns the same guard name (in my case 
_HOME_STEFAN_SRC_QEMU_QEMU_H)
which won't work when used for more than one header file.

I expect that blanks in the source path will also give unexpected results.

Even when this function were fixed, I think that
qapi-dir := $(BUILD_DIR)/qapi-generated
would be better.

Regards,
Stefan Weil
Stefano Stabellini Nov. 29, 2011, 4:23 p.m. UTC | #5 
On Mon, 28 Nov 2011, Stefan Weil wrote:
> The broken build can be reproduced with a source path containing at 
> least one dot.
> Source paths without any dot will show no problems. My source path was 
> ~/src/qemu/qemu.org/qemu.

Sorry for the build break, I didn't have any dots in my source path so
I didn't notice it :-/


> File scripts/qapi.py fails to create unique guard names when given a 
> filename with 2 or more dots:
> 
> def guardname(filename):
>      if filename.startswith('./'):
>          filename = filename[2:]
>      return filename.replace("/", "_").replace("-", 
> "_").split(".")[0].upper() + '_H'
> 
> The function always returns the same guard name (in my case 
> _HOME_STEFAN_SRC_QEMU_QEMU_H)
> which won't work when used for more than one header file.
> 
> I expect that blanks in the source path will also give unexpected results.
> 
> Even when this function were fixed, I think that
> qapi-dir := $(BUILD_DIR)/qapi-generated
> would be better.
 
Doing a more careful analysis of the out-of-tree build, I noticed that
the system actually works correctly if the source directory is
"distclean".
I don't think we support building qemu out-of-tree if the source dir
hasn't been cleaned properly, so I don't think is worth pursuing this
patch anymore.
Just be aware that a "make clean" is not enough.
diff mbox

Patch

diff --git a/Makefile b/Makefile
index 168093c..885065f 100644
--- a/Makefile
+++ b/Makefile
@@ -167,7 +167,7 @@  check-qjson: check-qjson.o $(qobject-obj-y) $(tools-obj-y)
 test-coroutine: test-coroutine.o qemu-timer-common.o async.o $(coroutine-obj-y) $(tools-obj-y)
 
 $(qapi-obj-y): $(GENERATED_HEADERS)
-qapi-dir := qapi-generated
+qapi-dir := $(SRC_PATH)/qapi-generated
 test-visitor.o test-qmp-commands.o qemu-ga$(EXESUF): QEMU_CFLAGS += -I $(qapi-dir)
 qemu-ga$(EXESUF): LIBS = $(LIBS_QGA)