[Qemu-devel] [PATCH] configure: fix breakage of --extra-{cflags, ldflags}

[Qemu-devel] [PATCH] configure: fix breakage of --extra-{cflags, ldflags}

[Nathan Froyd]

Commit e3fc14c3a1a77a1cda669a3e16f1f6b82577e4ec broke the use of
--extra-{cflags,ldflags} in the case where those flags were necessary
for bits of configure to succeed.  For instance, if you did:

  configure [...] --extra-cflags=-I/path/to/zlib/headers \
    --extra-ldflags=-L/path/to/zlib/libraries

prior to the above commit, the paths would go into CFLAGS and LDFLAGS
for use by the rest of the configure tests.  After the above commit,
that's no longer true, because the EXTRA_{CFLAGS,LDFLAGS} variables are
only used for creating makefiles.

Fixed by making sure that EXTRA_{CFLAGS,LDFLAGS} find their way into
{CFLAGS,LDFLAGS}, respectively.

Signed-off-by: Nathan Froyd <froydnj@codesourcery.com>
---
 configure |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 0db885b..dd2b1d9 100755
--- a/configure
+++ b/configure
@@ -520,8 +520,8 @@ CFLAGS="$CFLAGS -g -fno-strict-aliasing"
 if test "$debug" = "no" ; then
   CFLAGS="$CFLAGS -O2"
 fi
-CFLAGS="$CFLAGS -Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls"
-LDFLAGS="$LDFLAGS -g"
+CFLAGS="$CFLAGS $EXTRA_CFLAGS -Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls"
+LDFLAGS="$LDFLAGS $EXTRA_LDFLAGS -g"
 
 # Consult white-list to determine whether to enable werror
 # by default.  Only enable by default for git builds
-- 
1.6.2.4

[Qemu-devel] Re: [PATCH] configure: fix breakage of --extra-{cflags, ldflags}

[Jan Kiszka]

Nathan Froyd wrote:
> Commit e3fc14c3a1a77a1cda669a3e16f1f6b82577e4ec broke the use of
> --extra-{cflags,ldflags} in the case where those flags were necessary
> for bits of configure to succeed.  For instance, if you did:
> 
>   configure [...] --extra-cflags=-I/path/to/zlib/headers \
>     --extra-ldflags=-L/path/to/zlib/libraries
> 
> prior to the above commit, the paths would go into CFLAGS and LDFLAGS
> for use by the rest of the configure tests.  After the above commit,
> that's no longer true, because the EXTRA_{CFLAGS,LDFLAGS} variables are
> only used for creating makefiles.

Yep, that's true...

> 
> Fixed by making sure that EXTRA_{CFLAGS,LDFLAGS} find their way into
> {CFLAGS,LDFLAGS}, respectively.
> 
> Signed-off-by: Nathan Froyd <froydnj@codesourcery.com>
> ---
>  configure |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index 0db885b..dd2b1d9 100755
> --- a/configure
> +++ b/configure
> @@ -520,8 +520,8 @@ CFLAGS="$CFLAGS -g -fno-strict-aliasing"
>  if test "$debug" = "no" ; then
>    CFLAGS="$CFLAGS -O2"
>  fi
> -CFLAGS="$CFLAGS -Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls"
> -LDFLAGS="$LDFLAGS -g"
> +CFLAGS="$CFLAGS $EXTRA_CFLAGS -Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls"
> +LDFLAGS="$LDFLAGS $EXTRA_LDFLAGS -g"

...but now we add the extra flags twice. So please remove the injection
into the flags for $config_mak.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH] configure: fix breakage of --extra-{cflags, ldflags}

[Nathan Froyd]

On Thu, Jul 23, 2009 at 02:43:26PM +0200, Juan Quintela wrote:
> Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >> -CFLAGS="$CFLAGS -Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls"
> >> -LDFLAGS="$LDFLAGS -g"
> >> +CFLAGS="$CFLAGS $EXTRA_CFLAGS -Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls"
> >> +LDFLAGS="$LDFLAGS $EXTRA_LDFLAGS -g"
> >
> > ...but now we add the extra flags twice. So please remove the injection
> > into the flags for $config_mak.
>
> It is still broken.  Thinking of a solution.
> People wanted EXTRA_CFLAGS to be added at the end of CFLAGS, not in the
> middle.  Trying to find a way that is consistent for everything.
>
> Could you told in what test do you need extra_cflags?

The particular instance was for detecting zlib in a cross
configuration.  But that's not necessarily the only test where
--extra-{cflags,ldflags} would be useful: every test that needs external
libraries is subject to this bug (zlib, TLS, SASL, SDL, VDE libraries,
BrlAPI?, Xen?, bluez?, etc.).

If adding EXTRA_{CFLAGS,LDFLAGS} to all such tests is necessary and
people feel that's the best solution, then I can do that.

-Nathan

[Qemu-devel] Re: [PATCH] configure: fix breakage of --extra-{cflags, ldflags}

[Jan Kiszka]

Juan Quintela wrote:
> Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> Nathan Froyd wrote:
>>> Commit e3fc14c3a1a77a1cda669a3e16f1f6b82577e4ec broke the use of
>>> --extra-{cflags,ldflags} in the case where those flags were necessary
>>> for bits of configure to succeed.  For instance, if you did:
>>>
>>>   configure [...] --extra-cflags=-I/path/to/zlib/headers \
>>>     --extra-ldflags=-L/path/to/zlib/libraries
>>>
>>> prior to the above commit, the paths would go into CFLAGS and LDFLAGS
>>> for use by the rest of the configure tests.  After the above commit,
>>> that's no longer true, because the EXTRA_{CFLAGS,LDFLAGS} variables are
>>> only used for creating makefiles.
>> Yep, that's true...
>>
>>> Fixed by making sure that EXTRA_{CFLAGS,LDFLAGS} find their way into
>>> {CFLAGS,LDFLAGS}, respectively.
>>>
>>> Signed-off-by: Nathan Froyd <froydnj@codesourcery.com>
>>> ---
>>>  configure |    4 ++--
>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/configure b/configure
>>> index 0db885b..dd2b1d9 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -520,8 +520,8 @@ CFLAGS="$CFLAGS -g -fno-strict-aliasing"
>>>  if test "$debug" = "no" ; then
>>>    CFLAGS="$CFLAGS -O2"
>>>  fi
>>> -CFLAGS="$CFLAGS -Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls"
>>> -LDFLAGS="$LDFLAGS -g"
>>> +CFLAGS="$CFLAGS $EXTRA_CFLAGS -Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls"
>>> +LDFLAGS="$LDFLAGS $EXTRA_LDFLAGS -g"
>> ...but now we add the extra flags twice. So please remove the injection
>> into the flags for $config_mak.
> 
> It is still broken.  Thinking of a solution.

Good that someone is more awake than /me. :)

> People wanted EXTRA_CFLAGS to be added at the end of CFLAGS, not in the
> middle.  Trying to find a way that is consistent for everything.
> 
> Could you told in what test do you need extra_cflags?

I think the only consistent solution (besides stopping to re-invent
autotools) is explicitly appending EXTRA_CFLAGS to all tests.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH] configure: fix breakage of --extra-{cflags, ldflags}

[Jan Kiszka]

Juan Quintela wrote:
> Nathan Froyd <froydnj@codesourcery.com> wrote:
>> On Thu, Jul 23, 2009 at 02:43:26PM +0200, Juan Quintela wrote:
>>> Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> -CFLAGS="$CFLAGS -Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls"
>>>>> -LDFLAGS="$LDFLAGS -g"
>>>>> +CFLAGS="$CFLAGS $EXTRA_CFLAGS -Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls"
>>>>> +LDFLAGS="$LDFLAGS $EXTRA_LDFLAGS -g"
>>>> ...but now we add the extra flags twice. So please remove the injection
>>>> into the flags for $config_mak.
>>> It is still broken.  Thinking of a solution.
>>> People wanted EXTRA_CFLAGS to be added at the end of CFLAGS, not in the
>>> middle.  Trying to find a way that is consistent for everything.
>>>
>>> Could you told in what test do you need extra_cflags?
>> The particular instance was for detecting zlib in a cross
>> configuration.  But that's not necessarily the only test where
>> --extra-{cflags,ldflags} would be useful: every test that needs external
>> libraries is subject to this bug (zlib, TLS, SASL, SDL, VDE libraries,
>> BrlAPI?, Xen?, bluez?, etc.).
>>
>> If adding EXTRA_{CFLAGS,LDFLAGS} to all such tests is necessary and
>> people feel that's the best solution, then I can do that.
> 
> I think that the solution is the other way around :)
> 
> call configure with:
> 
> CFLAGS="foo bar" ./configure
> 
> and use CFLAGS for all configure script
> 
> That was what I had in mind.
> 
> If we do always in configure:
> 
> CFLAGS="baz goo $CFLAGS"
> 
> We also maintain the propierte that whatever you pass to CFLAGS is the
> last option in CFLAG (i.e. can overwrote whatever found by configure.
> 
> Then we can:
> - kill extra cflags,
> - win your feature,
> -  and have user wrote at the end of CFLAGS variable
> 
> What do you think?

No concerns. My only additional requirement is that configure should not
forget about the provided CFLAGS when going through automatic reconfig,
just like it's currently the case with --extra-* - and like autoconf
behaves, too.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH] configure: fix breakage of --extra-{cflags, ldflags}

[Nathan Froyd]

On Thu, Jul 23, 2009 at 04:40:41AM -0700, Nathan Froyd wrote:
> Commit e3fc14c3a1a77a1cda669a3e16f1f6b82577e4ec broke the use of
> --extra-{cflags,ldflags} in the case where those flags were necessary
> for bits of configure to succeed.  For instance, if you did:
>
>   configure [...] --extra-cflags=-I/path/to/zlib/headers \
>     --extra-ldflags=-L/path/to/zlib/libraries
>
> prior to the above commit, the paths would go into CFLAGS and LDFLAGS
> for use by the rest of the configure tests.

I spoke too soon; it turns out that the use of CFLAGS in the zlib
configure test is a local patch we have to QEMU.  So this is not an
issue for upstream QEMU at the moment.

It does raise the question of how configure is supposed to find things
in non-standard locations, but since none of the other configure tests
seem to care about non-standard compilation scenarios (e.g. using
--extra-* to specify the location of the SASL libraries won't
work)...this is probably not worth addressing unless people feel it's
worthwhile for the sake of Truth and Beauty.

Sorry about the noise.

-Nathan