| Message ID | 28cdb5f0-d6f3-6d0f-7179-db2385a1fd1e@gmail.com |
|---|---|
| State | Accepted |
| Commit | ad56e8057d8af0201ed0cb65acc12e5889d4afcc |
| Headers | show |
On 10/13/2017 12:37 PM, James Almer wrote: > On 10/13/2017 11:30 AM, Hendrik Leppkes wrote: >> On Fri, Oct 13, 2017 at 4:14 PM, James Almer <jamrial@gmail.com> wrote: >>> On 10/12/2017 6:30 PM, James Almer wrote: >>>> Should prevent some options from being added to cflags when they >>>> don't exist and the compiler only warns about it. >>>> >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> I figure this is safer than adding >>>> -Werror=unused-command-line-argument -Werror=unknown-warning-option >>>> as Ronald suggested. >>>> >>>> configure | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/configure b/configure >>>> index ade67a31bb..c7962665f1 100755 >>>> --- a/configure >>>> +++ b/configure >>>> @@ -6370,7 +6370,7 @@ fi >>>> >>>> check_disable_warning(){ >>>> warning_flag=-W${1#-Wno-} >>>> - test_cflags $warning_flag && add_cflags $1 >>>> + test_cflags -Werror $warning_flag && add_cflags $1 >>>> } >>>> >>>> check_disable_warning -Wno-parentheses >>> Ping. This or a similar solution has been annoying Clang users for some >>> days now and should be part of the 3.4 release. >> I wonder if a general -Werror is really "safer", do these tests really >> execute without any other warnings, which might trigger a false >> negative? We're not exactly making it compile a proper source file, so >> it might warn about random things. >> >> - Hendrik > Ok, what about the attached patch, then? > > Both -Werror=unused-command-line-argument and > -Werror=unknown-warning-option are not supported by gcc, so they > generate an error and would break every check. > This hopefully only uses them where they exist. > > > 0001-configure-force-erroring-out-in-check_disable_warnin.patch > > > From 0e305676e6b30ed3274fcbb4f31e318d41cac66d Mon Sep 17 00:00:00 2001 > From: James Almer <jamrial@gmail.com> > Date: Fri, 13 Oct 2017 12:34:34 -0300 > Subject: [PATCH] configure: force erroring out in check_disable_warning() if > an option doesn't exists > > Should prevent some options from being added to cflags when they > don't exist and the compiler only warns about it. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > configure | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/configure b/configure > index 64fd088208..87087f02e4 100755 > --- a/configure > +++ b/configure > @@ -6370,9 +6370,14 @@ fi > > check_disable_warning(){ > warning_flag=-W${1#-Wno-} > - test_cflags $warning_flag && add_cflags $1 > + test_cflags $unknown_warning_flags $warning_flag && add_cflags $1 > } > > +test_cflags -Werror=unused-command-line-argument && > + append unknown_warning_flags "-Werror=unused-command-line-argument" > +test_cflags -Werror=unknown-warning-option && > + append unknown_warning_flags "-Werror=unknown-warning-option" > + > check_disable_warning -Wno-parentheses > check_disable_warning -Wno-switch > check_disable_warning -Wno-format-zero-length > -- 2.14.2 I'll push this version later today if nobody comments.
On Fri, Oct 13, 2017 at 12:37:18PM -0300, James Almer wrote: > On 10/13/2017 11:30 AM, Hendrik Leppkes wrote: > > On Fri, Oct 13, 2017 at 4:14 PM, James Almer <jamrial@gmail.com> wrote: > >> On 10/12/2017 6:30 PM, James Almer wrote: > >>> Should prevent some options from being added to cflags when they > >>> don't exist and the compiler only warns about it. > >>> > >>> Signed-off-by: James Almer <jamrial@gmail.com> > >>> --- > >>> I figure this is safer than adding > >>> -Werror=unused-command-line-argument -Werror=unknown-warning-option > >>> as Ronald suggested. > >>> > >>> configure | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/configure b/configure > >>> index ade67a31bb..c7962665f1 100755 > >>> --- a/configure > >>> +++ b/configure > >>> @@ -6370,7 +6370,7 @@ fi > >>> > >>> check_disable_warning(){ > >>> warning_flag=-W${1#-Wno-} > >>> - test_cflags $warning_flag && add_cflags $1 > >>> + test_cflags -Werror $warning_flag && add_cflags $1 > >>> } > >>> > >>> check_disable_warning -Wno-parentheses > >> > >> Ping. This or a similar solution has been annoying Clang users for some > >> days now and should be part of the 3.4 release. > > > > I wonder if a general -Werror is really "safer", do these tests really > > execute without any other warnings, which might trigger a false > > negative? We're not exactly making it compile a proper source file, so > > it might warn about random things. > > > > - Hendrik > > Ok, what about the attached patch, then? > > Both -Werror=unused-command-line-argument and > -Werror=unknown-warning-option are not supported by gcc, so they > generate an error and would break every check. > This hopefully only uses them where they exist. > configure | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > 11cd9cc220b675d4cbe171b61c969e024458afad 0001-configure-force-erroring-out-in-check_disable_warnin.patch > From 0e305676e6b30ed3274fcbb4f31e318d41cac66d Mon Sep 17 00:00:00 2001 > From: James Almer <jamrial@gmail.com> > Date: Fri, 13 Oct 2017 12:34:34 -0300 > Subject: [PATCH] configure: force erroring out in check_disable_warning() if > an option doesn't exists > > Should prevent some options from being added to cflags when they > don't exist and the compiler only warns about it. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > configure | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) iam a bit surprised this has not been fixed long ago but LGTM [...]
On 10/13/2017 8:54 PM, Michael Niedermayer wrote: > On Fri, Oct 13, 2017 at 12:37:18PM -0300, James Almer wrote: >> On 10/13/2017 11:30 AM, Hendrik Leppkes wrote: >>> On Fri, Oct 13, 2017 at 4:14 PM, James Almer <jamrial@gmail.com> wrote: >>>> On 10/12/2017 6:30 PM, James Almer wrote: >>>>> Should prevent some options from being added to cflags when they >>>>> don't exist and the compiler only warns about it. >>>>> >>>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>>> --- >>>>> I figure this is safer than adding >>>>> -Werror=unused-command-line-argument -Werror=unknown-warning-option >>>>> as Ronald suggested. >>>>> >>>>> configure | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/configure b/configure >>>>> index ade67a31bb..c7962665f1 100755 >>>>> --- a/configure >>>>> +++ b/configure >>>>> @@ -6370,7 +6370,7 @@ fi >>>>> >>>>> check_disable_warning(){ >>>>> warning_flag=-W${1#-Wno-} >>>>> - test_cflags $warning_flag && add_cflags $1 >>>>> + test_cflags -Werror $warning_flag && add_cflags $1 >>>>> } >>>>> >>>>> check_disable_warning -Wno-parentheses >>>> >>>> Ping. This or a similar solution has been annoying Clang users for some >>>> days now and should be part of the 3.4 release. >>> >>> I wonder if a general -Werror is really "safer", do these tests really >>> execute without any other warnings, which might trigger a false >>> negative? We're not exactly making it compile a proper source file, so >>> it might warn about random things. >>> >>> - Hendrik >> >> Ok, what about the attached patch, then? >> >> Both -Werror=unused-command-line-argument and >> -Werror=unknown-warning-option are not supported by gcc, so they >> generate an error and would break every check. >> This hopefully only uses them where they exist. > >> configure | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> 11cd9cc220b675d4cbe171b61c969e024458afad 0001-configure-force-erroring-out-in-check_disable_warnin.patch >> From 0e305676e6b30ed3274fcbb4f31e318d41cac66d Mon Sep 17 00:00:00 2001 >> From: James Almer <jamrial@gmail.com> >> Date: Fri, 13 Oct 2017 12:34:34 -0300 >> Subject: [PATCH] configure: force erroring out in check_disable_warning() if >> an option doesn't exists >> >> Should prevent some options from being added to cflags when they >> don't exist and the compiler only warns about it. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> configure | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) > > iam a bit surprised this has not been fixed long ago > but LGTM Applied and backported, thanks!
diff --git a/configure b/configure index 64fd088208..87087f02e4 100755 --- a/configure +++ b/configure @@ -6370,9 +6370,14 @@ fi check_disable_warning(){ warning_flag=-W${1#-Wno-} - test_cflags $warning_flag && add_cflags $1 + test_cflags $unknown_warning_flags $warning_flag && add_cflags $1 } +test_cflags -Werror=unused-command-line-argument && + append unknown_warning_flags "-Werror=unused-command-line-argument" +test_cflags -Werror=unknown-warning-option && + append unknown_warning_flags "-Werror=unknown-warning-option" + check_disable_warning -Wno-parentheses check_disable_warning -Wno-switch check_disable_warning -Wno-format-zero-length