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@xxxxxxxxxx>
> > ---
> > 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
