| Message ID | 1342377272-32511-1-git-send-email-sw@weilnetz.de |
|---|---|
| State | Accepted |
| Headers | show |
On 15 July 2012 19:34, Stefan Weil <sw@weilnetz.de> wrote: > "+=" does not work with dash and other simple /bin/sh implementations. > > The new code prepends the flag while the old code either did not work > (it continued after an error message which typically was not read) or > appended the flag. That difference should not matter here. > > Signed-off-by: Stefan Weil <sw@weilnetz.de> > --- > configure | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/configure b/configure > index 12351f5..0269ba0 100755 > --- a/configure > +++ b/configure > @@ -2822,7 +2822,7 @@ int main(int argc, char **argv) > } > EOF > if ! compile_prog "" "" ; then > - CFLAGS+="-march=i486" > + CFLAGS="-march=i486 $CFLAGS" > fi > fi This is not quite the right fix for this. This flag should be in QEMU_CFLAGS, because it is a flag without which QEMU would be unable to compile. See previous discussion in this thread: http://lists.xen.org/archives/html/xen-devel/2012-04/msg00330.html (Unfortunately Olaf never submitted an updated patch.) -- PMM
Am 17.07.2012 20:43, schrieb Peter Maydell: > On 15 July 2012 19:34, Stefan Weil <sw@weilnetz.de> wrote: >> "+=" does not work with dash and other simple /bin/sh implementations. >> >> The new code prepends the flag while the old code either did not work >> (it continued after an error message which typically was not read) or >> appended the flag. That difference should not matter here. >> >> Signed-off-by: Stefan Weil <sw@weilnetz.de> >> --- >> configure | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/configure b/configure >> index 12351f5..0269ba0 100755 >> --- a/configure >> +++ b/configure >> @@ -2822,7 +2822,7 @@ int main(int argc, char **argv) >> } >> EOF >> if ! compile_prog "" "" ; then >> - CFLAGS+="-march=i486" >> + CFLAGS="-march=i486 $CFLAGS" >> fi >> fi > > This is not quite the right fix for this. This flag should be in > QEMU_CFLAGS, because it is a flag without which QEMU > would be unable to compile. See previous discussion in this > thread: > > http://lists.xen.org/archives/html/xen-devel/2012-04/msg00330.html > > (Unfortunately Olaf never submitted an updated patch.) > > -- PMM > If the user overrides CFLAGS, I expect that he/she will notice that compilation fails and hopefully find the cause (only expert users should override CFLAGS). Overriding -march=i486 might be useful to set -march=i686. Therefore it was not so obvious to me whether CFLAGS or QEMU_CFLAGS was the better choice. I don't mind if we decide to use QEMU_CFLAGS here, but suggest putting that change in a separate patch. This one fixes a shell syntax error without changing the macro name. Regards, Stefan W.
On 17 July 2012 20:07, Stefan Weil <sw@weilnetz.de> wrote: > If the user overrides CFLAGS, I expect that he/she will notice > that compilation fails and hopefully find the cause (only > expert users should override CFLAGS). Overriding -march=i486 > might be useful to set -march=i686. > > Therefore it was not so obvious to me whether CFLAGS or > QEMU_CFLAGS was the better choice. Yes, I had to dig to find out the underlying principle distinguishing the two. Having found it I would like us to fix this in a way which respects that principle. -- PMM
Am 17.07.2012 21:19, schrieb Peter Maydell: > On 17 July 2012 20:07, Stefan Weil <sw@weilnetz.de> wrote: >> If the user overrides CFLAGS, I expect that he/she will notice >> that compilation fails and hopefully find the cause (only >> expert users should override CFLAGS). Overriding -march=i486 >> might be useful to set -march=i686. >> >> Therefore it was not so obvious to me whether CFLAGS or >> QEMU_CFLAGS was the better choice. > > Yes, I had to dig to find out the underlying principle > distinguishing the two. Having found it I would like us to > fix this in a way which respects that principle. I was planning to pick up Olaf's patch doing just that but haven't got around to it in time for v1.1. Olaf, do you have time to resubmit a patch to upstream qemu-devel as requested by Peter and me? Regards, Andreas
On 20 July 2012 13:28, Andreas Färber <afaerber@suse.de> wrote: > Am 17.07.2012 21:19, schrieb Peter Maydell: >> On 17 July 2012 20:07, Stefan Weil <sw@weilnetz.de> wrote: >>> If the user overrides CFLAGS, I expect that he/she will notice >>> that compilation fails and hopefully find the cause (only >>> expert users should override CFLAGS). Overriding -march=i486 >>> might be useful to set -march=i686. >>> >>> Therefore it was not so obvious to me whether CFLAGS or >>> QEMU_CFLAGS was the better choice. >> >> Yes, I had to dig to find out the underlying principle >> distinguishing the two. Having found it I would like us to >> fix this in a way which respects that principle. > > I was planning to pick up Olaf's patch doing just that but haven't got > around to it in time for v1.1. Olaf, do you have time to resubmit a > patch to upstream qemu-devel as requested by Peter and me? I actually wrote a patch sitting on top of this one by Stefan which moves the flag to QEMU_CFLAGS; both are in the 11-patch series dealing with various configure Werror issues: http://patchwork.ozlabs.org/patch/171689/ http://patchwork.ozlabs.org/patch/171706/ -- PMM
On Fri, Jul 20, Peter Maydell wrote: > I actually wrote a patch sitting on top of this one by Stefan > which moves the flag to QEMU_CFLAGS; > both are in the 11-patch series dealing with various configure > Werror issues: > http://patchwork.ozlabs.org/patch/171689/ The actual CFLAGS issue had to be solved differently in the xen sources. And the patch above solves the bug in configure. Olaf
diff --git a/configure b/configure index 12351f5..0269ba0 100755 --- a/configure +++ b/configure @@ -2822,7 +2822,7 @@ int main(int argc, char **argv) } EOF if ! compile_prog "" "" ; then - CFLAGS+="-march=i486" + CFLAGS="-march=i486 $CFLAGS" fi fi
"+=" does not work with dash and other simple /bin/sh implementations. The new code prepends the flag while the old code either did not work (it continued after an error message which typically was not read) or appended the flag. That difference should not matter here. Signed-off-by: Stefan Weil <sw@weilnetz.de> --- configure | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)