Re: [PATCH] Fix 'make coverage' when used with lcov version 2.0+
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
Re: [PATCH] Fix 'make coverage' when used with lcov version 2.0+
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+
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+
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+
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+
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+
On Wed, May 08, 2024 at 12:28:04PM -0400, Aaron Merey wrote: > On Mon, May 6, 2024 at 4:45 PM 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 > > --- > > 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+
On Mon, May 6, 2024 at 4:45 PM 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 > --- > 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+
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 > --- > 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
[PATCH] Fix 'make coverage' when used with lcov version 2.0+
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 --- 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
