| Message ID | 20120403173224.GA22140@aepfle.de |
|---|---|
| State | New |
| Headers | show |
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
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
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
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(-)