Hi Mark,
On Mon, Jan 11, 2021 at 04:07:38PM +0100, Mark Wielaard wrote:
> Hi Dmitry,
>
> On Wed, 2020-12-23 at 00:25 +0300, Dmitry V. Levin wrote:
> > Implement a target for capturing code coverage using lcov.
> > It is available when elfutils is configured using --enable-gcov.
> >
> > Signed-off-by: Dmitry V. Levin <l...@altlinux.org>
> > ---
> > Rather than trying to salvage the coverage target, implement a
> > different
> > target for capturing code coverage similar to those I use in other
> > projects. I'm sure you'll like it, too.
>
> I like it. We need to integrate it with the elfutils-htdocs update-
> coverage script that is responsible for generating:
> https://sourceware.org/elfutils/coverage/
I didn't know it exists. :)
> See below.
>
> > ChangeLog | 4 ++++
> > configure.ac | 3 +++
> > tests/.gitignore | 2 ++
> > tests/ChangeLog | 5 +++++
> > tests/Makefile.am | 40 ++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 54 insertions(+)
> >
> > diff --git a/ChangeLog b/ChangeLog
> > index 03c90b6b..fe4f1829 100644
> > --- a/ChangeLog
> > +++ b/ChangeLog
> > @@ -1,3 +1,7 @@
> > +2020-12-22 Dmitry V. Levin <l...@altlinux.org>
> > +
> > + * configure.ac [--enable-gcov]: Check for gcov, lcov, and
> > genhtml.
> > +
> > 2020-12-20 Dmitry V. Levin <l...@altlinux.org>
> >
> > * .gitignore: Move subdirectory patterns to separate .gitignore
> > files.
> > diff --git a/configure.ac b/configure.ac
> > index 60747bc8..346ab800 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -311,6 +311,9 @@ if test "$use_gcov" = yes; then
> > CFLAGS="$CFLAGS -fprofile-arcs -ftest-coverage"
> > CXXFLAGS="$CXXFLAGS -fprofile-arcs -ftest-coverage"
> > LDFLAGS="$LDFLAGS -fprofile-arcs"
> > + AC_CHECK_PROG([GCOV], [gcov], [gcov])
> > + AC_CHECK_PROG([LCOV], [lcov], [lcov])
> > + AC_CHECK_PROG([GENHTML], [genhtml], [genhtml])
> > fi
> > AM_CONDITIONAL(GCOV, test "$use_gcov" = yes)
>
> This is a good idea. But do we need a particular version of lcov?
> I get lcov: Unknown option: exclude
> $ lcov --version
> lcov: LCOV version 1.13
> I don't insist on having it working with old lcov, just that we detect
> it.
lcov --exclude was introduced by lcov commit v1.14~19; yes, we can check
whether e.g. "lcov --exclude=/ --version" works. Alternatively, we could
use --directory option, but that would require some discipline in
maintaining the list of directories. What's your preferred choice?
> > diff --git a/tests/.gitignore b/tests/.gitignore
> > index d0e83da2..4aca5c7c 100644
> > --- a/tests/.gitignore
> > +++ b/tests/.gitignore
> > @@ -1,3 +1,5 @@
> > +/*-coverage
> > +/*.lcov
> > /*.log
> > /*.trs
> > /addrcfi
> >
> > diff --git a/tests/ChangeLog b/tests/ChangeLog
> > index 4688b50a..c0d9d4b8 100644
> > --- a/tests/ChangeLog
> > +++ b/tests/ChangeLog
> > @@ -1,3 +1,8 @@
> > +2020-12-22 Dmitry V. Levin <l...@altlinux.org>
> > +
> > + * Makefile.am [GCOV] (coverage-html): New target.
> > + * .gitignore: Update.
> > +
> > 2020-12-20 Dmitry V. Levin <l...@altlinux.org>
> >
> > * .gitignore: New file.
> > diff --git a/tests/Makefile.am b/tests/Makefile.am
> > index 502becff..293b4225 100644
> > --- a/tests/Makefile.am
> > +++ b/tests/Makefile.am
> > @@ -695,4 +695,44 @@ check: check-am coverage
> > .PHONY: coverage
> > coverage:
> > -$(srcdir)/coverage.sh
> > +
> > +COVERAGE_OUTPUT_FILE = $(PACKAGE_NAME)-$(PACKAGE_VERSION).lcov
> > +COVERAGE_OUTPUT_DIRECTORY = $(PACKAGE_NAME)-$(PACKAGE_VERSION)-coverage
By the way, I'm not sure whether we need -$(PACKAGE_VERSION) in
COVERAGE_OUTPUT_FILE and COVERAGE_OUTPUT_DIRECTORY.
> > +COVERAGE_OUTPUT_INDEX_HTML = $(COVERAGE_OUTPUT_DIRECTORY)/index.html
> > +COVERAGE_TITLE = $(PACKAGE_NAME)-$(PACKAGE_VERSION) coverage
> > +
> > +CLEANFILES += $(COVERAGE_OUTPUT_FILE)
> > +
> > +clean-local: coverage-clean
> > +distclean-local: coverage-clean
> > +coverage-clean:
> > + -rm -rf $(COVERAGE_OUTPUT_DIRECTORY)
> > +
> > +coverage-html: $(COVERAGE_OUTPUT_INDEX_HTML)
> > + @echo 'file://$(abs_builddir)/$(COVERAGE_OUTPUT_INDEX_HTML)'
> > +
> > +$(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
> > + LC_ALL=C $(GENHTML) \
> > + --legend \
> > + --show-details \
> > + --rc=genhtml_branch_coverage=1 \
> > + --title='$(COVERAGE_TITLE)' \
> > + --prefix='$(top_builddir)' \
> > + --prefix='$(abspath $(abs_top_srcdir))' \
> > + --output-directory='$(COVERAGE_OUTPUT_DIRECTORY)' \
> > + $<
> > +
> > +$(COVERAGE_OUTPUT_FILE):
> > + $(LCOV) \
> > + --capture \
> > + --no-checksum \
> > + --rc=lcov_branch_coverage=1 \
> > + --gcov-tool='$(GCOV)' \
> > + --exclude="$$TMPDIR/*" \
> > + --exclude='/tmp/*' \
> > + --exclude='/usr/include/*' \
> > + --exclude='*/tests/*' \
> > + --directory='$(top_builddir)' \
> > + --output-file='$@'
> > +
> > endif
>
> Note that the elfutils-htdocs git repo (which holds the website)
> contains an update-coverage.sh script that does:
>
> # Note we want srcdir == builddir for better output.
> ${tempdir}/elfutils/configure --enable-maintainer-mode --enable-gcov
> make -j$(nproc)
> make check
> lcov -c -d backends -d lib -d libasm -d libcpu -d libdw -d libdwelf
> \
> -d libdwfl -d libebl -d libelf -d src -d debuginfod \
> --no-external > lcov.out
> genhtml -s --legend -t "elfutils-${version}" -o coverage lcov.out
>
> So that is executed at the top-level instead of inside the tests dir
> and instead of --exclude it uses -d which IMHO gives slightly nicer
> output by only showing the top-level dirs.
I suppose the difference between --exclude and -d is only visible when
srcdir != builddir. If these directories are different, then
update-coverage.sh will likely miss some information because of this
explicit list of directories.
--
ldv
