[libvirt PATCH] meson: Fix build with -Dtest_coverage=true

[Jiri Denemark]

On Mon, Jan 18, 2021 at 09:56:56 +0100, Pavel Hrdina wrote:
> On Mon, Jan 18, 2021 at 09:23:34AM +0100, Jiri Denemark wrote:
> > As can be seen in commit 8a62a1592ae00eab4eb153c02661e56b9d8d9032 (from
> > autoconf era), the coverage flags have to be used also when linking
> > objects. However, this was not reflected when we switched to meson.
> > 
> > Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> > ---
> >  src/meson.build                 | 1 +
> >  tests/meson.build               | 8 ++++++++
> >  tools/nss/meson.build           | 2 ++
> >  tools/wireshark/src/meson.build | 3 +++
> >  4 files changed, 14 insertions(+)
> > 
> > diff --git a/src/meson.build b/src/meson.build
> > index 7c478219d6..980578d5d6 100644
> > --- a/src/meson.build
> > +++ b/src/meson.build
> > @@ -21,6 +21,7 @@ src_dep = declare_dependency(
> >      + coverage_flags
> >      + driver_modules_flags
> >      + win32_link_flags
> > +    + coverage_flags
> 
> You can see that it is already included few lines above.

Well, clearly I didn't see it :-)

> >    ),
> >  )
> >  
> > diff --git a/tests/meson.build b/tests/meson.build
> > index f1d91ca50d..c65487f5c2 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -202,6 +202,9 @@ foreach mock : mock_libs
> >        libvirt_lib,
> >        mock.get('link_with', []),
> >      ],
> > +    link_args: [
> > +      coverage_flags,
> > +    ],
> >    )
> >  endforeach
> >  
> > @@ -218,6 +221,7 @@ executable(
> >    ],
> >    link_args: [
> >      libvirt_no_indirect,
> > +    coverage_flags
> >    ],
> >  )
> >  
> > @@ -566,6 +570,7 @@ foreach data : tests
> >      ],
> >      link_args: [
> >        libvirt_no_indirect,
> > +      coverage_flags,
> >      ],
> >      link_with: [
> >        libvirt_lib,
> > @@ -644,6 +649,9 @@ foreach data : helpers
> >      link_with: [
> >        data['link_with'],
> >      ],
> > +    link_args: [
> > +      coverage_flags,
> > +    ],
> >      export_dynamic: true,
> >    )
> >  endforeach
> 
> This should not be needed as well because coverage_flags is part of
> tests_dep in "compile_args" which is the same as CFLAGS in automake.
> 
> The only place that uses COVERAGE_LDFLAGS is our fake SSH binary which
> should be translated to link_args and we already do that.
> 
> If it has to be included when linking then we should remove the usage of
> coverage_flags from the fake SSH and add it into tests_dep as link_args
> in addition to compile_args.

Hmm, good point. It's needed everywhere, otherwise linking fails with
undefined references to __gcov_*. I moved it to tests_dep.

> > diff --git a/tools/nss/meson.build b/tools/nss/meson.build
> > index cf3eec9b24..198936f3d4 100644
> > --- a/tools/nss/meson.build
> > +++ b/tools/nss/meson.build
> > @@ -66,6 +66,7 @@ nss_libvirt_lib = shared_module(
> >    link_args: [
> >      nss_libvirt_syms,
> >      libvirt_export_dynamic,
> > +    coverage_flags,
> >    ],
> >    link_whole: [
> >      nss_libvirt_impl,
> > @@ -81,6 +82,7 @@ nss_libvirt_guest_lib = shared_library(
> >    link_args: [
> >      nss_libvirt_guest_syms,
> >      libvirt_export_dynamic,
> > +    coverage_flags,
> >    ],
> >    link_whole: [
> >      nss_libvirt_guest_impl,
> > diff --git a/tools/wireshark/src/meson.build b/tools/wireshark/src/meson.build
> > index 49ccc9bb86..9b452dc5ca 100644
> > --- a/tools/wireshark/src/meson.build
> > +++ b/tools/wireshark/src/meson.build
> > @@ -12,6 +12,9 @@ shared_library(
> >      xdr_dep,
> >      tools_dep,
> >    ],
> > +  link_args: [
> > +    coverage_flags
> > +  ],
> >    install: true,
> >    install_dir: wireshark_plugindir,
> >  )
> 
> The commit mentioned doesn't add COVERAGE_LDFLAGS into nss or wireshark
> libs and checking our code before the meson switch we don't have it
> there as well. It's not even in AM_LDFLAGS so looks like preexisting.

Coverage builds were working before the meson rewrite, while linking
fails without explicitly adding coverage_flags to link_args for nss and
wireshark libs now. I guess it must have been done via some libtool
dependency magic which brought the right flags from the internal libs
we're linking with.

Jirka