[Qemu-devel] [PATCH v2] qemu/configure: fix CFLAGS handling for i386

[Qemu-devel] [PATCH v2] qemu/configure: fix CFLAGS handling for i386

[Olaf Hering]

configure will generate incorrect CFLAGS which will lead to compile
errors due to unknown gcc options, iff CFLAGS was already in the
environment during configure invocation.

Add a space before the -march=i486 gcc option.
Remove += operator usage because configure is not a bash script.

This patch is against the qemu-xen tree, but it should apply also to
qemu.git since it has the same issue. Please apply to both trees.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

---
 configure |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: qemu-xen-dir-remote/configure
===================================================================
--- qemu-xen-dir-remote.orig/configure
+++ qemu-xen-dir-remote/configure
@@ -2637,7 +2637,7 @@ int main(int argc, char **argv)
 }
 EOF
   if ! compile_prog "" "" ; then
-    CFLAGS+="-march=i486"
+    CFLAGS="$CFLAGS -march=i486"
   fi
 fi
 

[Qemu-devel] [Xen-devel] [PATCH v2] qemu/configure: fix CFLAGS handling for i386

[Ian Jackson]

Olaf Hering writes ("[Xen-devel] [PATCH v2] qemu/configure: fix CFLAGS handling for i386"):
> configure will generate incorrect CFLAGS which will lead to compile
> errors due to unknown gcc options, iff CFLAGS was already in the
> environment during configure invocation.

Don't do that then.

In general, CFLAGS is not a variable you can safely set in your
environment when invoking a nonconsenting build system.

Ian.

Re: [Qemu-devel] [Xen-devel] [PATCH v2] qemu/configure: fix CFLAGS handling for i386

[Peter Maydell]

On 4 April 2012 16:16, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> Olaf Hering writes ("[Xen-devel] [PATCH v2] qemu/configure: fix CFLAGS handling for i386"):
>> configure will generate incorrect CFLAGS which will lead to compile
>> errors due to unknown gcc options, iff CFLAGS was already in the
>> environment during configure invocation.
>
> Don't do that then.
>
> In general, CFLAGS is not a variable you can safely set in your
> environment when invoking a nonconsenting build system.

Yes. You probably wanted configure's --extra-cflags option.
However that doesn't mean the code in configure at the moment
is actually right...

Having looked at configure I'm pretty sure what we want here is
  QEMU_CFLAGS="-march=i486 $QEMU_CFLAGS"

because we're only doing this for the benefit of a particular bit
of code in hw/vhost.c and so QEMU_CFLAGS is sufficient. Also this
brings it into line with other places where we add a -march flag,
which use QEMU_CFLAGS, not CFLAGS.

-- PMM

Re: [Qemu-devel] [Xen-devel] [PATCH v2] qemu/configure: fix CFLAGS handling for i386

[Peter Maydell]

On 4 April 2012 16:40, Peter Maydell <peter.maydell@linaro.org> wrote:
> Having looked at configure I'm pretty sure what we want here is
>  QEMU_CFLAGS="-march=i486 $QEMU_CFLAGS"
>
> because we're only doing this for the benefit of a particular bit
> of code in hw/vhost.c and so QEMU_CFLAGS is sufficient. Also this
> brings it into line with other places where we add a -march flag,
> which use QEMU_CFLAGS, not CFLAGS.

...and having dug around in the git history we find that QEMU_CFLAGS
were introduced in commit a558ee1, whose commit message defines the
difference like this:
    QEMU_CFLAGS: flags without which we can't compile
    CFLAGS: "-g -O2"

"-march=i486" is clearly "flags without which we can't compile",
so we should be setting it in QEMU_CFLAGS, not CFLAGS.

Olaf, if you want to submit a fixed patch I think we should apply
it to upstream qemu.

PS: remarks like
"This patch is against the qemu-xen tree, but it should apply also to
qemu.git since it has the same issue. Please apply to both trees."

should go below the '---' in a patch email so they don't appear
in the git commit message when the patch is applied.

thanks
-- PMM

Re: [Qemu-devel] [Xen-devel] [PATCH v2] qemu/configure: fix CFLAGS handling for i386

[Andreas Färber]

Am 04.04.2012 18:09, schrieb Peter Maydell:
> On 4 April 2012 16:40, Peter Maydell <peter.maydell@linaro.org> wrote:
>> Having looked at configure I'm pretty sure what we want here is
>>  QEMU_CFLAGS="-march=i486 $QEMU_CFLAGS"
>>
>> because we're only doing this for the benefit of a particular bit
>> of code in hw/vhost.c and so QEMU_CFLAGS is sufficient. Also this
>> brings it into line with other places where we add a -march flag,
>> which use QEMU_CFLAGS, not CFLAGS.
> 
> ...and having dug around in the git history we find that QEMU_CFLAGS
> were introduced in commit a558ee1, whose commit message defines the
> difference like this:
>     QEMU_CFLAGS: flags without which we can't compile
>     CFLAGS: "-g -O2"
> 
> "-march=i486" is clearly "flags without which we can't compile",
> so we should be setting it in QEMU_CFLAGS, not CFLAGS.
> 
> Olaf, if you want to submit a fixed patch I think we should apply
> it to upstream qemu.
> 
> PS: remarks like
> "This patch is against the qemu-xen tree, but it should apply also to
> qemu.git since it has the same issue. Please apply to both trees."
> 
> should go below the '---' in a patch email so they don't appear
> in the git commit message when the patch is applied.

And you also forgot to update the subject to something like:
"configure: Fix CFLAGS handling for i386" - this is for the QEMU
repository after all, so no need to put that into the commit message.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg