diff mbox

[for,2.6] configure: Allow builds -Weverything

Message ID 1461879221-13338-1-git-send-email-sw@weilnetz.de (mailing list archive)
State New, archived
Headers show

Commit Message

Stefan Weil April 28, 2016, 9:33 p.m. UTC
The clang compiler supports a useful compiler option -Weverything.

As this option triggers warnings in glib header files, too, testing
glib with -Werror will always fail. A size mismatch is also detected
without -Werror, so simply remove it.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---

With this patch, it is possible to build QEMU using this configuration:

./configure' --cc=clang-3.7 --cxx=clang++-3.7 --extra-cflags="-Weverything -DHAVE_FSXATTR" --disable-werror

(HAVE_FSXATTR avoids a fatal build error because of structure redefinition)

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

Comments

Daniel P. Berrangé April 29, 2016, 8:29 a.m. UTC | #1
On Thu, Apr 28, 2016 at 11:33:41PM +0200, Stefan Weil wrote:
> The clang compiler supports a useful compiler option -Weverything.
> 
> As this option triggers warnings in glib header files, too, testing
> glib with -Werror will always fail. A size mismatch is also detected
> without -Werror, so simply remove it.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> 
> With this patch, it is possible to build QEMU using this configuration:
> 
> ./configure' --cc=clang-3.7 --cxx=clang++-3.7 --extra-cflags="-Weverything -DHAVE_FSXATTR" --disable-werror
> 
> (HAVE_FSXATTR avoids a fatal build error because of structure redefinition)
> 
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index ab54f3c..abd0eff 100755
> --- a/configure
> +++ b/configure
> @@ -2967,7 +2967,7 @@ int main(void) {
>  }
>  EOF
>  
> -if ! compile_prog "-Werror $CFLAGS" "$LIBS" ; then
> +if ! compile_prog "$CFLAGS" "$LIBS" ; then
>      error_exit "sizeof(size_t) doesn't match GLIB_SIZEOF_SIZE_T."\
>                 "You probably need to set PKG_CONFIG_LIBDIR"\
>  	       "to point to the right pkg-config files for your"\

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>


I've confirmed that this sizeof test still works as intended when the
-Werror is removed. IIRC, the -Werror usage was left over from an
earlier version of the patch

Regards,
Daniel
Peter Maydell April 29, 2016, 8:51 a.m. UTC | #2
On 28 April 2016 at 22:33, Stefan Weil <sw@weilnetz.de> wrote:
> The clang compiler supports a useful compiler option -Weverything.
>
> As this option triggers warnings in glib header files, too, testing
> glib with -Werror will always fail. A size mismatch is also detected
> without -Werror, so simply remove it.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>
> With this patch, it is possible to build QEMU using this configuration:
>
> ./configure' --cc=clang-3.7 --cxx=clang++-3.7 --extra-cflags="-Weverything -DHAVE_FSXATTR" --disable-werror
>
> (HAVE_FSXATTR avoids a fatal build error because of structure redefinition)

No objection to the patch, but not for 2.6 at this stage I think.

(Does the configure patch re fsxattr that got a ping today
avoid your need to define HAVE_FSXATTR by hand?)

thanks
-- PMM
Stefan Weil April 29, 2016, 9:22 a.m. UTC | #3
Am 29.04.2016 um 10:51 schrieb Peter Maydell:
> On 28 April 2016 at 22:33, Stefan Weil <sw@weilnetz.de> wrote:
>> The clang compiler supports a useful compiler option -Weverything.
>>
>> As this option triggers warnings in glib header files, too, testing
>> glib with -Werror will always fail. A size mismatch is also detected
>> without -Werror, so simply remove it.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>
>> With this patch, it is possible to build QEMU using this configuration:
>>
>> ./configure' --cc=clang-3.7 --cxx=clang++-3.7 --extra-cflags="-Weverything -DHAVE_FSXATTR" --disable-werror
>>
>> (HAVE_FSXATTR avoids a fatal build error because of structure redefinition)
> No objection to the patch, but not for 2.6 at this stage I think.
>
> (Does the configure patch re fsxattr that got a ping today
> avoid your need to define HAVE_FSXATTR by hand?)
>
> thanks
> -- PMM


IMHO the patch is save for 2.6, but I don't insist on getting it into
that version - although I already had the same problem with gcc +
increased warning level (without finding the reason), so it is not
specific for clang.

The configure patch which you are referring to is this one:

"configure: Check if struct fsxattr is available from linux header"

It can be found on qemu-trivial, but not on qemu-devel, patchworks or in
my personal QEMU mails. Obviously this patch only reached qemu-trivial,
but not qemu-devel. Perhaps that's why it did not get more attention.

The patch looks good, and I expect that it would solve the problem which
I observed. Jan, could you please send it a 2nd time (cc me), so I can
test it?

Thanks

Stefan
Stefan Weil May 16, 2016, 11:27 a.m. UTC | #4
Am 29.04.2016 um 10:51 schrieb Peter Maydell:
> On 28 April 2016 at 22:33, Stefan Weil <sw@weilnetz.de> wrote:
>> The clang compiler supports a useful compiler option -Weverything.
>>
>> As this option triggers warnings in glib header files, too, testing
>> glib with -Werror will always fail. A size mismatch is also detected
>> without -Werror, so simply remove it.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>
>> With this patch, it is possible to build QEMU using this configuration:
>>
>> ./configure' --cc=clang-3.7 --cxx=clang++-3.7 --extra-cflags="-Weverything -DHAVE_FSXATTR" --disable-werror
>>
>> (HAVE_FSXATTR avoids a fatal build error because of structure redefinition)
> 
> No objection to the patch, but not for 2.6 at this stage I think.
> 
> (Does the configure patch re fsxattr that got a ping today
> avoid your need to define HAVE_FSXATTR by hand?)
> 
> thanks
> -- PMM
> 

I think this patch which was too late for 2.6.0 is a candidate for
qemu-trivial, maybe also for qemu-stable. CC both mailing lists.

Stefan
Paolo Bonzini May 16, 2016, 12:22 p.m. UTC | #5
On 16/05/2016 13:27, Stefan Weil wrote:
> I think this patch which was too late for 2.6.0 is a candidate for
> qemu-trivial, maybe also for qemu-stable. CC both mailing lists.

Yes, it is confusing to have --disable-werror compilations break because
of warnings becoming errors.  However, the subject is imprecise, what about:

----
configure: Allow builds with extra warnings

The clang compiler supports a useful compiler option -Weverything,
and GCC also has other warnings not enabled by -Wall.

If glib header files trigger a warning, however, testing glib with
-Werror will always fail. A size mismatch is also detected without
-Werror, so simply remove it.
----

Thanks,

Paolo
Stefan Weil May 16, 2016, 12:32 p.m. UTC | #6
Am 16.05.2016 um 14:22 schrieb Paolo Bonzini:
> 
> 
> On 16/05/2016 13:27, Stefan Weil wrote:
>> I think this patch which was too late for 2.6.0 is a candidate for
>> qemu-trivial, maybe also for qemu-stable. CC both mailing lists.
> 
> Yes, it is confusing to have --disable-werror compilations break because
> of warnings becoming errors.  However, the subject is imprecise, what about:
> 
> ----
> configure: Allow builds with extra warnings
> 
> The clang compiler supports a useful compiler option -Weverything,
> and GCC also has other warnings not enabled by -Wall.
> 
> If glib header files trigger a warning, however, testing glib with
> -Werror will always fail. A size mismatch is also detected without
> -Werror, so simply remove it.
> ----
> 
> Thanks,
> 
> Paolo


Yes, that's a better description. Maybe Michael can use it when
applying the patch.

Stefan
Paolo Bonzini May 16, 2016, 1:06 p.m. UTC | #7
On 16/05/2016 14:32, Stefan Weil wrote:
> Am 16.05.2016 um 14:22 schrieb Paolo Bonzini:
>>
>>
>> On 16/05/2016 13:27, Stefan Weil wrote:
>>> I think this patch which was too late for 2.6.0 is a candidate for
>>> qemu-trivial, maybe also for qemu-stable. CC both mailing lists.
>>
>> Yes, it is confusing to have --disable-werror compilations break because
>> of warnings becoming errors.  However, the subject is imprecise, what about:
>>
>> ----
>> configure: Allow builds with extra warnings
>>
>> The clang compiler supports a useful compiler option -Weverything,
>> and GCC also has other warnings not enabled by -Wall.
>>
>> If glib header files trigger a warning, however, testing glib with
>> -Werror will always fail. A size mismatch is also detected without
>> -Werror, so simply remove it.
>> ----
>>
>> Thanks,
>>
>> Paolo
> 
> 
> Yes, that's a better description. Maybe Michael can use it when
> applying the patch.

I'm sending a pull request with all sort of patches in a couple days, I
can include this one too.

Paolo
diff mbox

Patch

diff --git a/configure b/configure
index ab54f3c..abd0eff 100755
--- a/configure
+++ b/configure
@@ -2967,7 +2967,7 @@  int main(void) {
 }
 EOF
 
-if ! compile_prog "-Werror $CFLAGS" "$LIBS" ; then
+if ! compile_prog "$CFLAGS" "$LIBS" ; then
     error_exit "sizeof(size_t) doesn't match GLIB_SIZEOF_SIZE_T."\
                "You probably need to set PKG_CONFIG_LIBDIR"\
 	       "to point to the right pkg-config files for your"\