On Tue, Sep 01, 2020 at 11:38:59AM -0600, Jim Fehlig wrote: > On 9/1/20 7:55 AM, Daniel P. Berrangé wrote: > > On Tue, Sep 01, 2020 at 03:50:36PM +0200, Ján Tomko wrote: > > > On a Tuesday in 2020, Pavel Hrdina wrote: > > > > On Tue, Sep 01, 2020 at 03:13:39PM +0200, Ján Tomko wrote: > > > > > On a Tuesday in 2020, Pavel Hrdina wrote: > > > > > > On Sun, Aug 30, 2020 at 02:34:56AM +0200, Toolybird wrote: > > > > > > > Hi, > > > > > > > > > > > > > > Just a heads up on my experiences with the new build system. > > > > > > > > > > > > > > Arch Linux > > > > > > > meson-0.55.1 > > > > > > > > > > > > > > Overall, it looks good, so well done! > > > > > > > > > > > > > > Just a couple of minor things I noted: > > > > > > > > > > > > > > 1. Arch uses a meson wrapper script (arch-meson) that sets: > > > > > > > > > > > > > > --buildtype plain > > > > > > > > > > > > > > This seems to trigger a bug in meson that results in copious bogus compiler warnings: > > > > > > > > > > > > > > cc1: warning: ‘-Wformat-y2k’ ignored without ‘-Wformat’ [-Wformat-y2k] > > > > > > > cc1: warning: ‘-Wformat-extra-args’ ignored without ‘-Wformat’ [-Wformat-extra-args] > > > > > > > cc1: warning: ‘-Wformat-zero-length’ ignored without ‘-Wformat’ [-Wformat-zero-length] > > > > > > > cc1: warning: ‘-Wformat-contains-nul’ ignored without ‘-Wformat’ [-Wformat-contains-nul] > > > > > > > cc1: warning: ‘-Wformat-security’ ignored without ‘-Wformat’ [-Wformat-security] > > > > > > > > > > > > > > which of course breaks -Werror builds. > > > > > > > > > > > > > > I can easily work around it by setting `-D git_werror=disabled' or even better still: > > > > > > > > > > > > > > CFLAGS+=" -Wall" arch-meson... > > > > > > > > > > > > This should be easily fixable by adding -Wformat in our list of flags > > > > > > that we pass to compiler. We were incorrectly relying on meson to add > > > > > > -Wformat automatically and since we add the other -Wformat* flags > > > > > > manually we should add -Wformat as well. I'll post a patch to fix this. > > > > > > > > > > > > > > > > What would that fix? > > > > > > > > > > It would neither be a plain build, nor a full -Werror build (since we're > > > > > missing -Wall). > > > > > > > > It would fix building libvirt from git in case user uses > > > > buildtype=plain. > > > > > > > > > > For a build from git, we expect people to build with all the warnings > > > and -Werror. > > > > > > Per your comment in the below issue: > > > https://github.com/mesonbuild/meson/issues/7399#issuecomment-684856012 > > > meson does not add the warnings there on purpose. > > > > > > The combination of disabling the most important warnings and enabling > > > just the extras doesn't make much sense. Especially if we still pretend > > > it's a -Werror build. > > > > Yeah, I'd recommend distros to NOT use buildtype=plain. > > AFAICT, rpm builds that use the %meson macro will also have buildtype=plain. > From /usr/lib/rpm/macros.d/macros.meson of the meson-0.54 package > > %meson \ > %set_build_flags > %{shrink:%{__meson} \ > --buildtype=plain \ > --prefix=%{_prefix} \ > ... > > > Having the full set of warnings including -Werror enabled is a good thing > > in general, even for distros. There have certainly been cases where > > distros backported a patch incorrectly and warning flags would have > > identified it, especially combined with -Werror. > > Nod. Should we adjust the upstream spec file to enable the full set of > warnings? And if so what would be the best way to do that? Add > --buildtype=debug to the meson options? E.g. > > %meson \ > --buildtype=debug \ > -Drunstatedir=%{_rundir} \ > %{?arg_qemu} \ > ... There is no need to do anything for spec file, see the patch I posted to libvir-list [1]. Pavel [1] <https://www.redhat.com/archives/libvir-list/2020-September/msg00045.html>
Attachment:
signature.asc
Description: PGP signature