[PATCH] Fix 'make coverage' when used with lcov version 2.0+

[PATCH] Fix 'make coverage' when used with lcov version 2.0+

[Aaron Merey]

Starting with version 2.0, various lcov warnings now trigger an error
exit.  This results in 'make coverage' terminating before completion.

Fix this by invoking lcov and genhtml with --ignore-errors to prevent
the error exit when version 2.0+ is in use.

Manually tested by running elfutils-htdocs/update-coverage.sh with
lcov 1.14 and 2.0.

Signed-off-by: Aaron Merey <amerey@redhat.com>
---
 Makefile.am  | 30 +++++++++++++++++++++++++++++-
 configure.ac |  4 ++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index c9d59d4a..a1f0b0c3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -77,6 +77,7 @@ coverage-clean:
 coverage: $(COVERAGE_OUTPUT_INDEX_HTML)
 	@echo 'file://$(abs_builddir)/$(COVERAGE_OUTPUT_INDEX_HTML)'
 
+if LCOV_OLD
 $(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
 	LC_ALL=C $(GENHTML) \
 		--legend \
@@ -89,7 +90,23 @@ $(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
 		--prefix='$(realpath $(abs_builddir)/..)' \
 		--output-directory='$(COVERAGE_OUTPUT_DIRECTORY)' \
 		$<
+else
+$(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
+	LC_ALL=C $(GENHTML) \
+		--legend \
+		--show-details \
+		--ignore-errors empty,negative \
+		--rc=genhtml_branch_coverage=1 \
+		--title='$(COVERAGE_TITLE)' \
+		--prefix='$(abspath $(abs_srcdir))' \
+		--prefix='$(realpath $(abs_srcdir))' \
+		--prefix='$(abspath $(abs_builddir)/..)' \
+		--prefix='$(realpath $(abs_builddir)/..)' \
+		--output-directory='$(COVERAGE_OUTPUT_DIRECTORY)' \
+		$<
+endif
 
+if LCOV_OLD
 $(COVERAGE_OUTPUT_FILE):
 	$(LCOV) \
 		--capture \
@@ -99,7 +116,18 @@ $(COVERAGE_OUTPUT_FILE):
 		--gcov-tool='$(GCOV)' \
 		--output-file='$@' \
 		$(LCOV_DIRS_ARGS)
-
+else
+$(COVERAGE_OUTPUT_FILE):
+	$(LCOV) \
+		--capture \
+		--no-external \
+		--no-checksum \
+		--ignore-errors empty,negative \
+		--rc=lcov_branch_coverage=1 \
+		--gcov-tool='$(GCOV)' \
+		--output-file='$@' \
+		$(LCOV_DIRS_ARGS)
+endif
 endif
 
 # Tell version 3.79 and up of GNU make to not build goals in this
diff --git a/configure.ac b/configure.ac
index a279bb52..2aa728bd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -322,6 +322,10 @@ if test "$use_gcov" = yes; then
 fi
 AM_CONDITIONAL(GCOV, test "$use_gcov" = yes)
 
+# Check if lcov/genhtml supports ignoring additional errors.
+AM_CONDITIONAL([LCOV_OLD],
+  [test "$LCOV" = "lcov" && lcov --version | grep -E -q "version 0|version 1"])
+
 AC_ARG_ENABLE([sanitize-undefined],
               AS_HELP_STRING([--enable-sanitize-undefined],
                              [Use gcc undefined behaviour sanitizer]),
-- 
2.43.0

Re: [PATCH] Fix 'make coverage' when used with lcov version 2.0+

[Dmitry V. Levin]

Hi,

On Mon, May 06, 2024 at 04:45:27PM -0400, Aaron Merey wrote:
> Starting with version 2.0, various lcov warnings now trigger an error
> exit.  This results in 'make coverage' terminating before completion.
> 
> Fix this by invoking lcov and genhtml with --ignore-errors to prevent
> the error exit when version 2.0+ is in use.
> 
> Manually tested by running elfutils-htdocs/update-coverage.sh with
> lcov 1.14 and 2.0.
> 
> Signed-off-by: Aaron Merey <amerey@redhat.com>
> ---
>  Makefile.am  | 30 +++++++++++++++++++++++++++++-
>  configure.ac |  4 ++++
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index c9d59d4a..a1f0b0c3 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -77,6 +77,7 @@ coverage-clean:
>  coverage: $(COVERAGE_OUTPUT_INDEX_HTML)
>  	@echo 'file://$(abs_builddir)/$(COVERAGE_OUTPUT_INDEX_HTML)'
>  
> +if LCOV_OLD
>  $(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
>  	LC_ALL=C $(GENHTML) \
>  		--legend \
> @@ -89,7 +90,23 @@ $(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
>  		--prefix='$(realpath $(abs_builddir)/..)' \
>  		--output-directory='$(COVERAGE_OUTPUT_DIRECTORY)' \
>  		$<
> +else
> +$(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
> +	LC_ALL=C $(GENHTML) \
> +		--legend \
> +		--show-details \
> +		--ignore-errors empty,negative \
> +		--rc=genhtml_branch_coverage=1 \
> +		--title='$(COVERAGE_TITLE)' \
> +		--prefix='$(abspath $(abs_srcdir))' \
> +		--prefix='$(realpath $(abs_srcdir))' \
> +		--prefix='$(abspath $(abs_builddir)/..)' \
> +		--prefix='$(realpath $(abs_builddir)/..)' \
> +		--output-directory='$(COVERAGE_OUTPUT_DIRECTORY)' \
> +		$<
> +endif
>  
> +if LCOV_OLD
>  $(COVERAGE_OUTPUT_FILE):
>  	$(LCOV) \
>  		--capture \
> @@ -99,7 +116,18 @@ $(COVERAGE_OUTPUT_FILE):
>  		--gcov-tool='$(GCOV)' \
>  		--output-file='$@' \
>  		$(LCOV_DIRS_ARGS)
> -
> +else
> +$(COVERAGE_OUTPUT_FILE):
> +	$(LCOV) \
> +		--capture \
> +		--no-external \
> +		--no-checksum \
> +		--ignore-errors empty,negative \
> +		--rc=lcov_branch_coverage=1 \
> +		--gcov-tool='$(GCOV)' \
> +		--output-file='$@' \
> +		$(LCOV_DIRS_ARGS)
> +endif
>  endif
>  
>  # Tell version 3.79 and up of GNU make to not build goals in this

Would it be possible to avoid code duplication by parametrizing just
the difference?


-- 
ldv

Re: [PATCH] Fix 'make coverage' when used with lcov version 2.0+

[Aaron Merey]

On Mon, May 6, 2024 at 4:45 PM Aaron Merey <amerey@redhat.com> wrote:
>
> Starting with version 2.0, various lcov warnings now trigger an error
> exit.  This results in 'make coverage' terminating before completion.
>
> Fix this by invoking lcov and genhtml with --ignore-errors to prevent
> the error exit when version 2.0+ is in use.
>
> Manually tested by running elfutils-htdocs/update-coverage.sh with
> lcov 1.14 and 2.0.
>
> Signed-off-by: Aaron Merey <amerey@redhat.com>
> ---
>  Makefile.am  | 30 +++++++++++++++++++++++++++++-
>  configure.ac |  4 ++++
>  2 files changed, 33 insertions(+), 1 deletion(-)

Pushed as commit ca8ad4648197

Aaron

Re: [PATCH] Fix 'make coverage' when used with lcov version 2.0+

[Dmitry V. Levin]

On Wed, May 08, 2024 at 12:28:04PM -0400, Aaron Merey wrote:
> On Mon, May 6, 2024 at 4:45 PM Aaron Merey <amerey@redhat.com> wrote:
> >
> > Starting with version 2.0, various lcov warnings now trigger an error
> > exit.  This results in 'make coverage' terminating before completion.
> >
> > Fix this by invoking lcov and genhtml with --ignore-errors to prevent
> > the error exit when version 2.0+ is in use.
> >
> > Manually tested by running elfutils-htdocs/update-coverage.sh with
> > lcov 1.14 and 2.0.
> >
> > Signed-off-by: Aaron Merey <amerey@redhat.com>
> > ---
> >  Makefile.am  | 30 +++++++++++++++++++++++++++++-
> >  configure.ac |  4 ++++
> >  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> Pushed as commit ca8ad4648197

What's the purpose of sending proposed patches to the mailing list
if reviews are silently ignored?


-- 
ldv

Re: [PATCH] Fix 'make coverage' when used with lcov version 2.0+

[Frank Ch. Eigler]

Hi -

On Fri, May 10, 2024 at 12:53:39AM +0300, Dmitry V. Levin wrote:
> > Pushed as commit ca8ad4648197
> 
> What's the purpose of sending proposed patches to the mailing list
> if reviews are silently ignored?

Please be collegial and don't exaggerate.

- FChE

Re: [PATCH] Fix 'make coverage' when used with lcov version 2.0+

[Dmitry V. Levin]

On Thu, May 09, 2024 at 06:08:05PM -0400, Frank Ch. Eigler wrote:
> Hi -
> 
> On Fri, May 10, 2024 at 12:53:39AM +0300, Dmitry V. Levin wrote:
> > > Pushed as commit ca8ad4648197
> > 
> > What's the purpose of sending proposed patches to the mailing list
> > if reviews are silently ignored?
> 
> Please be collegial and don't exaggerate.

The fact is that the review was silently ignored, which is, from my point
of view, an extraordinary event, and I hereby raise the question why it
was ignored, and request that the issue pointed out in the review to be
properly addressed.

Thanks,


-- 
ldv

Re: [PATCH] Fix 'make coverage' when used with lcov version 2.0+

[Frank Ch. Eigler]

Hi -

> > > What's the purpose of sending proposed patches to the mailing list
> > > if reviews are silently ignored?
> > 
> > Please be collegial and don't exaggerate.
> 
> The fact is that the review was silently ignored, which is, from my point
> of view, an extraordinary event, 

The review *singular*.  And you can't know whether it was ignored, or
never received or missed or considered.

> [...] and request that the issue pointed out in the review to be
> properly addressed.

May I suggest asking for that in a less accusatory and exaggerated
fashion next time.  Note also that being "properly addressed" is up to
the discretion of the maintainers - one of whom is Aaron.

- FChE

Re: [PATCH] Fix 'make coverage' when used with lcov version 2.0+

[Dmitry V. Levin]

Hi Frank,

On Thu, May 09, 2024 at 07:03:47PM -0400, Frank Ch. Eigler wrote:
> Hi -
> 
> > > > What's the purpose of sending proposed patches to the mailing list
> > > > if reviews are silently ignored?
> > > 
> > > Please be collegial and don't exaggerate.
> > 
> > The fact is that the review was silently ignored, which is, from my point
> > of view, an extraordinary event, 
> 
> The review *singular*.  And you can't know whether it was ignored, or
> never received or missed or considered.

Well, even if it happened unintentionally, it was ignored just the same
as if it happened on purpose.  I hope the technical reasons for that
are now fixed, and won't cause any problems next time.

> > [...] and request that the issue pointed out in the review to be
> > properly addressed.
> 
> May I suggest asking for that in a less accusatory and exaggerated
> fashion next time.  Note also that being "properly addressed" is up to
> the discretion of the maintainers - one of whom is Aaron.

Please note that elfutils is a community project, and the code in question
was contributed a few years ago by myself, see commit
2a16a0fc7e353f8fcfc27a57710e008840297847.
I'm glad the code duplication issue is now resolved.

Frank, please reconsider your approach.  Your comments are not just
unhelpful and unproductive, they can actually discourage contributors
which is the last thing one wants in a free software project.


-- 
ldv

Re: [PATCH] Fix 'make coverage' when used with lcov version 2.0+

[Frank Ch. Eigler]

Hi -

> Well, even if it happened unintentionally, it was ignored just the same
> as if it happened on purpose.  I hope the technical reasons for that
> are now fixed, and won't cause any problems next time.

You have been in the open source community long enough to know that
emails sometimes just get lost.  It might happen again, and one one
should not take too much offence.

> Please note that elfutils is a community project, and the code in
> question was contributed a few years ago by myself, see commit
> 2a16a0fc7e353f8fcfc27a57710e008840297847.

Yes, however we both know that this does not mean that a maintainer is
required to affirmatively consult you (or anyone) on changes years
after its contribution.

> Frank, please reconsider your approach.  Your comments are [...]

I disagree.


- FChE

Re: [PATCH] Fix 'make coverage' when used with lcov version 2.0+

[Mark Wielaard]

Hi,

I think things worked out in the end, so that is good.

Personally I didn't think Dmitry's request to take his review into
account was exaggerating. But that might be because I know him and am
happy with his suggestions in general.

If the tone of some request was interpreted as "not collegial and
exaggerated", then maybe next time also provide an example of how to
formulate the request that sounds better. Tone is a difficult thing,
especially on the mailinglist.

Cheers,

Mark

[COMMITTED] Makefile.am: Avoid code duplication

[Aaron Merey]

On Thu, May 9, 2024 at 6:59 PM Dmitry V. Levin <ldv@altlinux.org> wrote:
>
> On Thu, May 09, 2024 at 06:08:05PM -0400, Frank Ch. Eigler wrote:
> > Hi -
> >
> > On Fri, May 10, 2024 at 12:53:39AM +0300, Dmitry V. Levin wrote:
> > > > Pushed as commit ca8ad4648197
> > >
> > > What's the purpose of sending proposed patches to the mailing list
> > > if reviews are silently ignored?
> >
> > Please be collegial and don't exaggerate.
>
> The fact is that the review was silently ignored, which is, from my point
> of view, an extraordinary event, and I hereby raise the question why it
> was ignored, and request that the issue pointed out in the review to be
> properly addressed.

Apologies Dmitry, your review was mistakenly caught in my mail client's
spam filter and I did not see it until now.

I agree that we can avoid some code duplication here. I'm going to push
the following patch as obvious:

---
 Makefile.am | 33 ++++++---------------------------
 1 file changed, 6 insertions(+), 27 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index a1f0b0c3..69af63ac 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -78,24 +78,16 @@ coverage: $(COVERAGE_OUTPUT_INDEX_HTML)
 	@echo 'file://$(abs_builddir)/$(COVERAGE_OUTPUT_INDEX_HTML)'
 
 if LCOV_OLD
-$(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
-	LC_ALL=C $(GENHTML) \
-		--legend \
-		--show-details \
-		--rc=genhtml_branch_coverage=1 \
-		--title='$(COVERAGE_TITLE)' \
-		--prefix='$(abspath $(abs_srcdir))' \
-		--prefix='$(realpath $(abs_srcdir))' \
-		--prefix='$(abspath $(abs_builddir)/..)' \
-		--prefix='$(realpath $(abs_builddir)/..)' \
-		--output-directory='$(COVERAGE_OUTPUT_DIRECTORY)' \
-		$<
+ignore_errors =
 else
+ignore_errors = --ignore-errors empty,negative
+endif
+
 $(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
 	LC_ALL=C $(GENHTML) \
 		--legend \
 		--show-details \
-		--ignore-errors empty,negative \
+		$(ignore_errors) \
 		--rc=genhtml_branch_coverage=1 \
 		--title='$(COVERAGE_TITLE)' \
 		--prefix='$(abspath $(abs_srcdir))' \
@@ -104,30 +96,17 @@ $(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
 		--prefix='$(realpath $(abs_builddir)/..)' \
 		--output-directory='$(COVERAGE_OUTPUT_DIRECTORY)' \
 		$<
-endif
 
-if LCOV_OLD
 $(COVERAGE_OUTPUT_FILE):
 	$(LCOV) \
 		--capture \
 		--no-external \
 		--no-checksum \
+		$(ignore_errors) \
 		--rc=lcov_branch_coverage=1 \
 		--gcov-tool='$(GCOV)' \
 		--output-file='$@' \
 		$(LCOV_DIRS_ARGS)
-else
-$(COVERAGE_OUTPUT_FILE):
-	$(LCOV) \
-		--capture \
-		--no-external \
-		--no-checksum \
-		--ignore-errors empty,negative \
-		--rc=lcov_branch_coverage=1 \
-		--gcov-tool='$(GCOV)' \
-		--output-file='$@' \
-		$(LCOV_DIRS_ARGS)
-endif
 endif
 
 # Tell version 3.79 and up of GNU make to not build goals in this
-- 
2.43.0

Re: [COMMITTED] Makefile.am: Avoid code duplication

[Dmitry V. Levin]

On Thu, May 09, 2024 at 07:24:26PM -0400, Aaron Merey wrote:
> On Thu, May 9, 2024 at 6:59 PM Dmitry V. Levin <ldv@altlinux.org> wrote:
> > On Thu, May 09, 2024 at 06:08:05PM -0400, Frank Ch. Eigler wrote:
> > > On Fri, May 10, 2024 at 12:53:39AM +0300, Dmitry V. Levin wrote:
> > > > > Pushed as commit ca8ad4648197
> > > >
> > > > What's the purpose of sending proposed patches to the mailing list
> > > > if reviews are silently ignored?
> > >
> > > Please be collegial and don't exaggerate.
> >
> > The fact is that the review was silently ignored, which is, from my point
> > of view, an extraordinary event, and I hereby raise the question why it
> > was ignored, and request that the issue pointed out in the review to be
> > properly addressed.
> 
> Apologies Dmitry, your review was mistakenly caught in my mail client's
> spam filter and I did not see it until now.
> 
> I agree that we can avoid some code duplication here. I'm going to push
> the following patch as obvious:
> 
> ---
>  Makefile.am | 33 ++++++---------------------------
>  1 file changed, 6 insertions(+), 27 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index a1f0b0c3..69af63ac 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -78,24 +78,16 @@ coverage: $(COVERAGE_OUTPUT_INDEX_HTML)
>  	@echo 'file://$(abs_builddir)/$(COVERAGE_OUTPUT_INDEX_HTML)'
>  
>  if LCOV_OLD
> -$(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
> -	LC_ALL=C $(GENHTML) \
> -		--legend \
> -		--show-details \
> -		--rc=genhtml_branch_coverage=1 \
> -		--title='$(COVERAGE_TITLE)' \
> -		--prefix='$(abspath $(abs_srcdir))' \
> -		--prefix='$(realpath $(abs_srcdir))' \
> -		--prefix='$(abspath $(abs_builddir)/..)' \
> -		--prefix='$(realpath $(abs_builddir)/..)' \
> -		--output-directory='$(COVERAGE_OUTPUT_DIRECTORY)' \
> -		$<
> +ignore_errors =
>  else
> +ignore_errors = --ignore-errors empty,negative
> +endif
> +
>  $(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
>  	LC_ALL=C $(GENHTML) \
>  		--legend \
>  		--show-details \
> -		--ignore-errors empty,negative \
> +		$(ignore_errors) \
>  		--rc=genhtml_branch_coverage=1 \
>  		--title='$(COVERAGE_TITLE)' \
>  		--prefix='$(abspath $(abs_srcdir))' \
> @@ -104,30 +96,17 @@ $(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
>  		--prefix='$(realpath $(abs_builddir)/..)' \
>  		--output-directory='$(COVERAGE_OUTPUT_DIRECTORY)' \
>  		$<
> -endif
>  
> -if LCOV_OLD
>  $(COVERAGE_OUTPUT_FILE):
>  	$(LCOV) \
>  		--capture \
>  		--no-external \
>  		--no-checksum \
> +		$(ignore_errors) \
>  		--rc=lcov_branch_coverage=1 \
>  		--gcov-tool='$(GCOV)' \
>  		--output-file='$@' \
>  		$(LCOV_DIRS_ARGS)
> -else
> -$(COVERAGE_OUTPUT_FILE):
> -	$(LCOV) \
> -		--capture \
> -		--no-external \
> -		--no-checksum \
> -		--ignore-errors empty,negative \
> -		--rc=lcov_branch_coverage=1 \
> -		--gcov-tool='$(GCOV)' \
> -		--output-file='$@' \
> -		$(LCOV_DIRS_ARGS)
> -endif
>  endif
>  
>  # Tell version 3.79 and up of GNU make to not build goals in this

That's much better, thanks!


-- 
ldv