1. Patchew
  2. Libvirt
  3. View series

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

Jiri Denemark posted 1 patch 3 years, 10 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/5870b054509855a8d22472d8af9ad8ae7d6f1769.1610958214.git.jdenemar@redhat.com
There is a newer version of this series
src/meson.build                 | 1 +
tests/meson.build               | 8 ++++++++
tools/nss/meson.build           | 2 ++
tools/wireshark/src/meson.build | 3 +++
4 files changed, 14 insertions(+)
[libvirt PATCH] meson: Fix build with -Dtest_coverage=true
Posted by Jiri Denemark 3 years, 10 months ago 
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@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
   ),
 )
 
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
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,
 )
-- 
2.30.0
Re: [libvirt PATCH] meson: Fix build with -Dtest_coverage=true
Posted by Pavel Hrdina 3 years, 10 months ago 
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@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.

>    ),
>  )
>  
> 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.

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

Pavel

> -- 
> 2.30.0
>
Re: [libvirt PATCH] meson: Fix build with -Dtest_coverage=true
Posted by Jiri Denemark 3 years, 10 months ago 
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@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

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.