[libvirt PATCH] meson: add -Wall and -Wextra explicitly for buildtype=plain
If someone runs `meson setup --buildtype plain` meson ignores warning_level=2 that is in our meson.build file. The implication is that Meson will not automatically add -Wall which enables -Wformat. This breaks building libvirt from git with the buildtype set to plain. There is an issue reported [1] to not ignore warning_level silently and the change to ignore it was done by upstream commit [2]. [1] <https://github.com/mesonbuild/meson/issues/7399> [2] <https://github.com/mesonbuild/meson/commit/8ee1c9a07a3a35e3ed262fbc358fd86c257a966e> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- meson.build | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/meson.build b/meson.build index e193166a9b..f38785eeae 100644 --- a/meson.build +++ b/meson.build @@ -503,10 +503,6 @@ cc_flags_disabled = [ # In meson this is specified using 'c_std=gnu99' in project() function. '-std=gnu99', - # In meson this is specified using 'warning_level=2' in project() function. - '-Wall', - '-Wextra', - # don't care about C++ compiler compat '-Wc++-compat', '-Wabi', @@ -573,6 +569,21 @@ cc_flags_disabled = [ '-Wsuggest-attribute=malloc', ] +# In meson this is specified using 'warning_level=2' in project() function. +# However, meson silently ignores 'warning_level' option with 'buildtype=plain' +# so we have to enable them explicitly. +if get_option('buildtype') == 'plain' + cc_flags += [ + '-Wall', + '-Wextra', + ] +else + cc_flags_disabled += [ + '-Wall', + '-Wextra', + ] +endif + foreach flag : cc_flags_disabled if cc_flags.contains(flag) error('@0@ is disabled but listed in cc_flags'.format(flag)) -- 2.26.2
On 9/1/20 8:08 AM, Pavel Hrdina wrote:
If someone runs `meson setup --buildtype plain` meson ignores warning_level=2 that is in our meson.build file. The implication is that Meson will not automatically add -Wall which enables -Wformat.
This breaks building libvirt from git with the buildtype set to plain.
There is an issue reported [1] to not ignore warning_level silently and the change to ignore it was done by upstream commit [2].
[1] <https://github.com/mesonbuild/meson/issues/7399> [2] <https://github.com/mesonbuild/meson/commit/8ee1c9a07a3a35e3ed262fbc358fd86c257a966e>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- meson.build | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
I'm just learning meson so not particularly confident to review patches, but will note this change causes a warning in downstream package build where the %meson rpm macro is used [ 23s] Compiler for C supports arguments -Wall: YES [ 23s] Compiler for C supports arguments -Wextra: YES [ 23s] meson.build:594: WARNING: Consider using the built-in warning_level option instead of using "-Wall". [ 23s] meson.build:594: WARNING: Consider using the built-in warning_level option instead of using "-Wextra". Regards, Jim
diff --git a/meson.build b/meson.build index e193166a9b..f38785eeae 100644 --- a/meson.build +++ b/meson.build @@ -503,10 +503,6 @@ cc_flags_disabled = [ # In meson this is specified using 'c_std=gnu99' in project() function. '-std=gnu99',
- # In meson this is specified using 'warning_level=2' in project() function. - '-Wall', - '-Wextra', - # don't care about C++ compiler compat '-Wc++-compat', '-Wabi', @@ -573,6 +569,21 @@ cc_flags_disabled = [ '-Wsuggest-attribute=malloc', ]
+# In meson this is specified using 'warning_level=2' in project() function. +# However, meson silently ignores 'warning_level' option with 'buildtype=plain' +# so we have to enable them explicitly. +if get_option('buildtype') == 'plain' + cc_flags += [ + '-Wall', + '-Wextra', + ] +else + cc_flags_disabled += [ + '-Wall', + '-Wextra', + ] +endif + foreach flag : cc_flags_disabled if cc_flags.contains(flag) error('@0@ is disabled but listed in cc_flags'.format(flag))
On Tue, Sep 01, 2020 at 04:37:06PM -0600, Jim Fehlig wrote:
On 9/1/20 8:08 AM, Pavel Hrdina wrote:
If someone runs `meson setup --buildtype plain` meson ignores warning_level=2 that is in our meson.build file. The implication is that Meson will not automatically add -Wall which enables -Wformat.
This breaks building libvirt from git with the buildtype set to plain.
There is an issue reported [1] to not ignore warning_level silently and the change to ignore it was done by upstream commit [2].
[1] <https://github.com/mesonbuild/meson/issues/7399> [2] <https://github.com/mesonbuild/meson/commit/8ee1c9a07a3a35e3ed262fbc358fd86c257a966e>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- meson.build | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
I'm just learning meson so not particularly confident to review patches, but will note this change causes a warning in downstream package build where the %meson rpm macro is used
[ 23s] Compiler for C supports arguments -Wall: YES [ 23s] Compiler for C supports arguments -Wextra: YES [ 23s] meson.build:594: WARNING: Consider using the built-in warning_level option instead of using "-Wall". [ 23s] meson.build:594: WARNING: Consider using the built-in warning_level option instead of using "-Wextra".
With the way how Meson handles buildtype=plain and warning_level it is expected to see the warning. They check the existence of these flags and print a warning regardless of the used buildtype but as explained in the commit message Meson ignores warning_level if buildtype=plain and doesn't add these flags even if warning_level is set. It is inconsistent behavior in Meson and it should be somehow fixed. Pavel
On Tue, Sep 01, 2020 at 04:08:04PM +0200, Pavel Hrdina wrote:
If someone runs `meson setup --buildtype plain` meson ignores warning_level=2 that is in our meson.build file. The implication is that Meson will not automatically add -Wall which enables -Wformat.
This breaks building libvirt from git with the buildtype set to plain.
There is an issue reported [1] to not ignore warning_level silently and the change to ignore it was done by upstream commit [2].
[1] <https://github.com/mesonbuild/meson/issues/7399> [2] <https://github.com/mesonbuild/meson/commit/8ee1c9a07a3a35e3ed262fbc358fd86c257a966e>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- meson.build | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/meson.build b/meson.build index e193166a9b..f38785eeae 100644 --- a/meson.build +++ b/meson.build @@ -503,10 +503,6 @@ cc_flags_disabled = [ # In meson this is specified using 'c_std=gnu99' in project() function. '-std=gnu99',
- # In meson this is specified using 'warning_level=2' in project() function. - '-Wall', - '-Wextra', - # don't care about C++ compiler compat '-Wc++-compat', '-Wabi', @@ -573,6 +569,21 @@ cc_flags_disabled = [ '-Wsuggest-attribute=malloc', ]
+# In meson this is specified using 'warning_level=2' in project() function. +# However, meson silently ignores 'warning_level' option with 'buildtype=plain' +# so we have to enable them explicitly. +if get_option('buildtype') == 'plain' + cc_flags += [ + '-Wall', + '-Wextra', + ] +else + cc_flags_disabled += [ + '-Wall', + '-Wextra', + ] +endif
I'd really just simplify this to remove the conditional and always add -Wall/-Wextra, and ignore the warning meson will print. I tend to think they should provide a way to squelch that warning, as it is too presumtous that apps will only ever need to set the basic warning flags, instead of warning to set many like libvirt does. Or maybe, meson can be enhanced to allow setting many warnings like libvirt uses
+ foreach flag : cc_flags_disabled if cc_flags.contains(flag) error('@0@ is disabled but listed in cc_flags'.format(flag)) -- 2.26.2
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (3)
-
Daniel P. Berrangé -
Jim Fehlig -
Pavel Hrdina