[libvirt] [PATCH] Revert "configure: Remove --enable-test-coverage"

Jiri Denemark posted 1 patch 4 years, 8 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/f2e287b9c8ecb201362e0ae4b44e9402acd69279.1565280764.git.jdenemar@redhat.com
There is a newer version of this series
Makefile.am                | 20 +++++++++++++++++---
configure.ac               | 18 ++++++++++++++++++
src/Makefile.am            |  3 ++-
src/remote/Makefile.inc.am |  2 ++
tests/Makefile.am          |  2 ++
tools/Makefile.am          |  6 ++++++
6 files changed, 47 insertions(+), 4 deletions(-)
[libvirt] [PATCH] Revert "configure: Remove --enable-test-coverage"
Posted by Jiri Denemark 4 years, 8 months ago
This reverts commit f38d553e2d6ec2f041cb7947b5eafcdd3b26ae65.

Gnulib's make coverage (or init-coverage, build-coverage, gen-coverage)
is not a 1-1 replacement for the original configure option. Our old
--enable-test-coverage seems to be close to gnulib's make build-coverage
except gnulib runs lcov in that phase and the build actually fails for
me even before lcov is run. And since we want to be able to just build
libvirt without running lcov, I suggest reverting to our own
implementation.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---

Notes:
    I admit the best solution would be to somehow make gnulib support what
    we need (and fix the actual build), but I don't feel brave enough to do
    that. Eric? :-)
    
    Also if we ever switch to glib and drop gnulib completely, we would need
    to reintroduce our own implementation anyway.

 Makefile.am                | 20 +++++++++++++++++---
 configure.ac               | 18 ++++++++++++++++++
 src/Makefile.am            |  3 ++-
 src/remote/Makefile.inc.am |  2 ++
 tests/Makefile.am          |  2 ++
 tools/Makefile.am          |  6 ++++++
 6 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index dedd8d2ff8..27c49280c4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -16,15 +16,15 @@
 ## License along with this library.  If not, see
 ## <http://www.gnu.org/licenses/>.
 
+LCOV = lcov
+GENHTML = genhtml
+
 SUBDIRS = . gnulib/lib include/libvirt src tools docs gnulib/tests \
   tests po examples
 
 XZ_OPT ?= -v -T0
 export XZ_OPT
 
-# have gnulib 'make coverage' output to 'cov' dir
-COVERAGE_OUT = "cov"
-
 ACLOCAL_AMFLAGS = -I m4
 
 EXTRA_DIST = \
@@ -87,6 +87,20 @@ check-local: all tests
 check-access: all
 	@($(MAKE) $(AM_MAKEFLAGS) -C tests check-access)
 
+cov: clean-cov
+	$(MKDIR_P) $(top_builddir)/coverage
+	$(LCOV) -c -o $(top_builddir)/coverage/libvirt.info.tmp \
+	  -d $(top_builddir)/src \
+	  -d $(top_builddir)/tests
+	$(LCOV) -r $(top_builddir)/coverage/libvirt.info.tmp \
+	  -o $(top_builddir)/coverage/libvirt.info
+	rm $(top_builddir)/coverage/libvirt.info.tmp
+	$(GENHTML) --show-details -t "libvirt" -o $(top_builddir)/coverage \
+	  --legend $(top_builddir)/coverage/libvirt.info
+
+clean-cov:
+	rm -rf $(top_builddir)/coverage
+
 MAINTAINERCLEANFILES = .git-module-status
 
 dist-hook: gen-AUTHORS
diff --git a/configure.ac b/configure.ac
index ebb2e24e21..f41c6d5d86 100644
--- a/configure.ac
+++ b/configure.ac
@@ -726,6 +726,23 @@ fi
 AC_SUBST([VIR_TEST_EXPENSIVE_DEFAULT])
 AM_CONDITIONAL([WITH_EXPENSIVE_TESTS], [test $VIR_TEST_EXPENSIVE_DEFAULT = 1])
 
+LIBVIRT_ARG_ENABLE([TEST_COVERAGE], [turn on code coverage instrumentation], [no])
+case "$enable_test_coverage" in
+  yes|no) ;;
+  *) AC_MSG_ERROR([bad value ${enable_test_coverga} for test-coverage option]) ;;
+esac
+
+if test "$enable_test_coverage" = yes; then
+  save_WARN_CFLAGS=$WARN_CFLAGS
+  WARN_CFLAGS=
+  gl_WARN_ADD([-fprofile-arcs])
+  gl_WARN_ADD([-ftest-coverage])
+  COVERAGE_FLAGS=$WARN_CFLAGS
+  AC_SUBST([COVERAGE_CFLAGS], [$COVERAGE_FLAGS])
+  AC_SUBST([COVERAGE_LDFLAGS], [$COVERAGE_FLAGS])
+  WARN_CFLAGS=$save_WARN_CFLAGS
+fi
+
 LIBVIRT_ARG_ENABLE([TEST_OOM], [memory allocation failure checking], [no])
 case "$enable_test_oom" in
   yes|no) ;;
@@ -1011,6 +1028,7 @@ LIBVIRT_WIN_RESULT_WINDRES
 AC_MSG_NOTICE([])
 AC_MSG_NOTICE([Test suite])
 AC_MSG_NOTICE([])
+AC_MSG_NOTICE([         Coverage: $enable_test_coverage])
 AC_MSG_NOTICE([        Alloc OOM: $enable_test_oom])
 AC_MSG_NOTICE([])
 AC_MSG_NOTICE([Miscellaneous])
diff --git a/src/Makefile.am b/src/Makefile.am
index f111b2a1b4..5ddc06f629 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -37,8 +37,9 @@ AM_CFLAGS =	$(LIBXML_CFLAGS) \
 		$(WARN_CFLAGS) \
 		$(LOCK_CHECKING_CFLAGS) \
 		$(WIN32_EXTRA_CFLAGS) \
-		$(NULL)
+		$(COVERAGE_CFLAGS)
 AM_LDFLAGS =	$(DRIVER_MODULES_LDFLAGS) \
+		$(COVERAGE_LDFLAGS) \
 		$(RELRO_LDFLAGS) \
 		$(NO_INDIRECT_LDFLAGS) \
 		$(CYGWIN_EXTRA_LDFLAGS) \
diff --git a/src/remote/Makefile.inc.am b/src/remote/Makefile.inc.am
index 0cf00cb902..4d3a450461 100644
--- a/src/remote/Makefile.inc.am
+++ b/src/remote/Makefile.inc.am
@@ -145,6 +145,7 @@ libvirtd_CFLAGS = \
 	$(LIBNL_CFLAGS) \
 	$(WARN_CFLAGS) \
 	$(PIE_CFLAGS) \
+	$(COVERAGE_CFLAGS) \
 	-I$(srcdir)/access \
 	-I$(srcdir)/conf \
 	-I$(srcdir)/rpc \
@@ -153,6 +154,7 @@ libvirtd_CFLAGS = \
 libvirtd_LDFLAGS = \
 	$(RELRO_LDFLAGS) \
 	$(PIE_LDFLAGS) \
+	$(COVERAGE_LDFLAGS) \
 	$(NO_INDIRECT_LDFLAGS) \
 	$(NO_UNDEFINED_LDFLAGS) \
 	$(NULL)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1c92e3ca6f..23e7c422ea 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -41,6 +41,7 @@ AM_CFLAGS = \
 	$(SELINUX_CFLAGS) \
 	$(APPARMOR_CFLAGS) \
 	$(YAJL_CFLAGS) \
+	$(COVERAGE_CFLAGS) \
 	$(XDR_CFLAGS) \
 	$(WARN_CFLAGS)
 
@@ -267,6 +268,7 @@ endif WITH_SECDRIVER_SELINUX
 
 # This is a fake SSH we use from virnetsockettest
 ssh_SOURCES = ssh.c
+ssh_LDADD = $(COVERAGE_LDFLAGS)
 
 if WITH_LIBXL
 test_programs += xlconfigtest \
diff --git a/tools/Makefile.am b/tools/Makefile.am
index df3d628bab..9cf6baf35b 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -33,6 +33,7 @@ WARN_CFLAGS += $(STRICT_FRAME_LIMIT_CFLAGS)
 
 AM_CFLAGS = \
 	$(WARN_CFLAGS) \
+	$(COVERAGE_CFLAGS) \
 	$(PIE_CFLAGS) \
 	$(LIBXML_CFLAGS) \
 	$(NULL)
@@ -143,6 +144,7 @@ libvirt_shell_la_CFLAGS = \
 libvirt_shell_la_LDFLAGS = \
 		$(AM_LDFLAGS) \
 		$(PIE_LDFLAGS) \
+		$(COVERAGE_LDFLAGS) \
 		$(NULL)
 libvirt_shell_la_LIBADD = \
 		../src/libvirt.la \
@@ -188,6 +190,7 @@ endif ! WITH_BHYVE
 virt_host_validate_LDFLAGS = \
 		$(AM_LDFLAGS) \
 		$(PIE_LDFLAGS) \
+		$(COVERAGE_LDFLAGS) \
 		$(NULL)
 
 virt_host_validate_LDADD = \
@@ -213,6 +216,7 @@ virt_login_shell_helper_SOURCES = \
 virt_login_shell_helper_LDFLAGS = \
 		$(AM_LDFLAGS) \
 		$(PIE_LDFLAGS) \
+		$(COVERAGE_LDFLAGS) \
 		$(NULL)
 virt_login_shell_helper_LDADD = \
 		../src/libvirt.la \
@@ -245,6 +249,7 @@ virsh_SOURCES = \
 virsh_LDFLAGS = \
 		$(AM_LDFLAGS) \
 		$(PIE_LDFLAGS) \
+		$(COVERAGE_LDFLAGS) \
 		$(NULL)
 virsh_LDADD = \
 		$(STATIC_BINARIES) \
@@ -262,6 +267,7 @@ virt_admin_SOURCES = \
 
 virt_admin_LDFLAGS = \
 		$(AM_LDFLAGS) \
+		$(COVERAGE_LDFLAGS) \
 		$(STATIC_BINARIES) \
 		$(PIE_LDFLAGS) \
 		$(NULL)
-- 
2.22.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Revert "configure: Remove --enable-test-coverage"
Posted by Michal Privoznik 4 years, 8 months ago
On 8/8/19 6:12 PM, Jiri Denemark wrote:
> This reverts commit f38d553e2d6ec2f041cb7947b5eafcdd3b26ae65.
> 
> Gnulib's make coverage (or init-coverage, build-coverage, gen-coverage)
> is not a 1-1 replacement for the original configure option. Our old
> --enable-test-coverage seems to be close to gnulib's make build-coverage
> except gnulib runs lcov in that phase and the build actually fails for
> me even before lcov is run. And since we want to be able to just build
> libvirt without running lcov, I suggest reverting to our own
> implementation.
> 
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
> 
> Notes:
>      I admit the best solution would be to somehow make gnulib support what
>      we need (and fix the actual build), but I don't feel brave enough to do
>      that. Eric? :-)
>      
>      Also if we ever switch to glib and drop gnulib completely, we would need
>      to reintroduce our own implementation anyway.
> 
>   Makefile.am                | 20 +++++++++++++++++---
>   configure.ac               | 18 ++++++++++++++++++
>   src/Makefile.am            |  3 ++-
>   src/remote/Makefile.inc.am |  2 ++
>   tests/Makefile.am          |  2 ++
>   tools/Makefile.am          |  6 ++++++
>   6 files changed, 47 insertions(+), 4 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list