Meson build

[Jim Fehlig]

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} \
            ...

Regards,
Jim