Re: [libvirt PATCH 021/351] meson: add compiler warnings
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 :|