Xen devel - Apr 2012 - [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.

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

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.

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

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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