[libvirt PATCH 021/351] meson: add compiler warnings

[Daniel P. Berrangé]

On Wed, Jul 22, 2020 at 05:49:59PM +0200, Pavel Hrdina wrote:

> > > +  # -Wformat=2 implies -Wformat-nonliteral so we need to manually exclude it
> > > +  '-Wno-format-nonliteral',
> > > +
> > > +  # -Wformat enables this by default, and we should keep it,
> > > +  # but need to rewrite various areas of code first
> > > +  '-Wno-format-truncation',
> > > +
> > > +  # This should be < 256 really. Currently we're down to 4096,
> > > +  # but using 1024 bytes sized buffers (mostly for virStrerror)
> > > +  # stops us from going down further
> > > +  '-Wframe-larger-than=4096',
> > > +
> > > +  # extra special flags
> > > +  '-fexceptions',
> > > +  '-fasynchronous-unwind-tables',
> > > +
> > > +  # Need -fipa-pure-const in order to make -Wsuggest-attribute=pure
> > > +  # fire even without -O.
> > > +  '-fipa-pure-const',
> > > +
> > > +  # We should eventually enable this, but right now there are at
> > > +  # least 75 functions triggering warnings.
> > > +  '-Wno-suggest-attribute=pure',
> > > +  '-Wno-suggest-attribute=const',
> > > +]
> > > +
> > > +if git
> > > +  cc_flags += [ '-Werror' ]
> > > +endif
> > 
> > This doesn't seem right. We definitely don't want this to be based
> > solely on whether we're building from git.
> 
> So the thing with Meson is that it has build-in option --werror with
> default value 'false'. It is possible to change the default value
> in the project() function. Unfortunately it is not possible to call
> anything else before project() function so we cannot detect if building
> from GIT before we can change the default value.
> 
> There are three solutions:
> 
>     1) Keep it as it is, it would not be possible to disable -Werror if
>     building from GIT.

That mostly shouldn't be a problem for upstream GIT since we expect to
be -Werror clean, but new GCCs break us sometimes. Today in fact with
GCC 10 :-). Could also be a problem for downstreams, eg we use git to
maintain patches in RPMs.

So I think this is not viable.

>     2) Drop the GIT detection and require developers to use --werror to
>     have this functionality.

The main goal of defaulting to Werror was to make sure developers
didn't accidentally introduce build warnings.  As long as we don't
have gating pre-merge CI, I think need to try really hard to keep
it.


> 
>     3) Introduce our own -Dgit_werror which would allow us to change the
>     default behavior and disable -Werror for build from GIT.

This duplicates --werror behaviour, but I think it is the least
worst option right now.

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 :|