* [PATCH v3] Makefile: add a prerequisite to the coverage-report target
@ 2022-04-14 2:25 Elia Pinto
2022-04-14 5:06 ` Junio C Hamano
2022-04-14 5:16 ` Junio C Hamano
0 siblings, 2 replies; 3+ messages in thread
From: Elia Pinto @ 2022-04-14 2:25 UTC (permalink / raw)
To: git; +Cc: gitster, Elia Pinto
Directly invoking make coverage-report as a target results in an error because
its prerequisites are missing,
This patch adds the compile-test prerequisite, which is run only once each time
the compile-report target is invoked. In practice, the developer may decide to
review the coverage-report results without necessarily rerunning for this
coverage-test, if it has already been run.
Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
This is the third revision of the patch. Compared to the second:
- deleted the Makefile coverage-test.file variable, replaced directly by the
coverage-test.made file
- added in coverage-compile as last command the touch of coverage-test.made, so
that even if coverage-test is run first, invoking indirectly it by the first coverage-report does not
call coverage-test and so the test suite gain.
Makefile | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index f8bccfab5e..d6ae228c74 100644
--- a/Makefile
+++ b/Makefile
@@ -3407,6 +3407,7 @@ coverage-clean-results:
$(RM) coverage-untested-functions
$(RM) -r cover_db/
$(RM) -r cover_db_html/
+ $(RM) coverage-test.made
coverage-clean: coverage-clean-results
$(RM) $(addsuffix *.gcno,$(object_dirs))
@@ -3421,13 +3422,18 @@ coverage-compile:
coverage-test: coverage-clean-results coverage-compile
$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
DEFAULT_TEST_TARGET=test -j1 test
+ touch coverage-test.made
+
+coverage-test.made:
+ @make coverage-test
+ touch coverage-test.made
coverage-prove: coverage-clean-results coverage-compile
$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
DEFAULT_TEST_TARGET=prove GIT_PROVE_OPTS="$(GIT_PROVE_OPTS) -j1" \
-j1 test
-coverage-report:
+coverage-report: coverage-test.made
$(QUIET_GCOV)for dir in $(object_dirs); do \
$(GCOV) $(GCOVFLAGS) --object-directory=$$dir $$dir*.c || exit; \
done
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] Makefile: add a prerequisite to the coverage-report target
2022-04-14 2:25 [PATCH v3] Makefile: add a prerequisite to the coverage-report target Elia Pinto
@ 2022-04-14 5:06 ` Junio C Hamano
2022-04-14 5:16 ` Junio C Hamano
1 sibling, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2022-04-14 5:06 UTC (permalink / raw)
To: Elia Pinto; +Cc: git
Elia Pinto <gitter.spiros@gmail.com> writes:
> Directly invoking make coverage-report as a target results in an error because
> its prerequisites are missing,
>
> This patch adds the compile-test prerequisite, which is run only once each time
> the compile-report target is invoked. In practice, the developer may decide to
> review the coverage-report results without necessarily rerunning for this
> coverage-test, if it has already been run.
>
> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
> ---
> This is the third revision of the patch. Compared to the second:
>
> - deleted the Makefile coverage-test.file variable, replaced directly by the
> coverage-test.made file
> - added in coverage-compile as last command the touch of coverage-test.made, so
> that even if coverage-test is run first, invoking indirectly it by the first coverage-report does not
> call coverage-test and so the test suite gain.
>
> Makefile | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index f8bccfab5e..d6ae228c74 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3407,6 +3407,7 @@ coverage-clean-results:
> $(RM) coverage-untested-functions
> $(RM) -r cover_db/
> $(RM) -r cover_db_html/
> + $(RM) coverage-test.made
>
> coverage-clean: coverage-clean-results
> $(RM) $(addsuffix *.gcno,$(object_dirs))
> @@ -3421,13 +3422,18 @@ coverage-compile:
> coverage-test: coverage-clean-results coverage-compile
> $(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
> DEFAULT_TEST_TARGET=test -j1 test
> + touch coverage-test.made
> +
> +coverage-test.made:
> + @make coverage-test
s/@make/$(MAKE)/, I think.
> + touch coverage-test.made
Other than that, looking good.
Thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] Makefile: add a prerequisite to the coverage-report target
2022-04-14 2:25 [PATCH v3] Makefile: add a prerequisite to the coverage-report target Elia Pinto
2022-04-14 5:06 ` Junio C Hamano
@ 2022-04-14 5:16 ` Junio C Hamano
1 sibling, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2022-04-14 5:16 UTC (permalink / raw)
To: Elia Pinto; +Cc: git
Elia Pinto <gitter.spiros@gmail.com> writes:
> @@ -3421,13 +3422,18 @@ coverage-compile:
> coverage-test: coverage-clean-results coverage-compile
> $(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
> DEFAULT_TEST_TARGET=test -j1 test
> + touch coverage-test.made
> +
> +coverage-test.made:
> + @make coverage-test
> + touch coverage-test.made
As this target runs "$(MAKE) coverage-test", at the end of which the
.made file is touched anyway, I wonder if we need the second line
here.
I just tried to run this sequence:
$ make distclean
$ make coverage-report
$ make coverage-report
$ make coverage-test
without the "touch" in the recipe for the coverage-test.made target,
and it behaved as I expected them to, i.e.
* got an pristine tree
* built, ran tests and showed report
* showed report without building or running testing
* cleared the gcov files, ran tests and showed report
So, perhaps this patch on top?
Thanks.
Makefile | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git i/Makefile w/Makefile
index fa8cc86276..72ae7fa886 100644
--- i/Makefile
+++ w/Makefile
@@ -3416,8 +3416,7 @@ coverage-test: coverage-clean-results coverage-compile
touch coverage-test.made
coverage-test.made:
- @make coverage-test
- touch coverage-test.made
+ $(MAKE) coverage-test
coverage-prove: coverage-clean-results coverage-compile
$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-04-14 5:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-14 2:25 [PATCH v3] Makefile: add a prerequisite to the coverage-report target Elia Pinto
2022-04-14 5:06 ` Junio C Hamano
2022-04-14 5:16 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).