{"thread":{"id":"57729","subject":"[PATCH v3] Makefile: add a prerequisite to the coverage-report target","startedAt":"2022-04-14T02:25:28Z","lastAt":"2022-04-14T05:16:17Z","messageCount":3,"participants":["Elia Pinto","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"453622","messageId":"20220414022513.31465-1-gitter.spiros@gmail.com","threadId":"57729","inReplyTo":null,"subject":"[PATCH v3] Makefile: add a prerequisite to the coverage-report target","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2022-04-14T02:25:13Z","receivedAt":"2022-04-14T02:25:28Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"Directly invoking make coverage-report as a target results in an error because\nits prerequisites are missing,\n\nThis patch adds the compile-test prerequisite, which is run only once each time\nthe compile-report target is invoked. In practice, the developer may decide to\nreview the coverage-report results without necessarily rerunning for this\ncoverage-test, if it has already been run.\n\nSigned-off-by: Elia Pinto \n---\nThis is the third revision of the patch. Compared to the second:\n\n- deleted the Makefile coverage-test.file variable, replaced directly by the\n coverage-test.made file\n- added in coverage-compile as last command the touch of coverage-test.made, so\n that even if coverage-test is run first, invoking indirectly it by the first coverage-report does not\n call coverage-test and so the test suite gain.\n\n Makefile | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex f8bccfab5e..d6ae228c74 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3407,6 +3407,7 @@ coverage-clean-results:\n \t$(RM) coverage-untested-functions\n \t$(RM) -r cover_db/\n \t$(RM) -r cover_db_html/\n+\t$(RM) coverage-test.made\n \n coverage-clean: coverage-clean-results\n \t$(RM) $(addsuffix *.gcno,$(object_dirs))\n@@ -3421,13 +3422,18 @@ coverage-compile:\n coverage-test: coverage-clean-results coverage-compile\n \t$(MAKE) CFLAGS=\"$(COVERAGE_CFLAGS)\" LDFLAGS=\"$(COVERAGE_LDFLAGS)\" \\\n \t\tDEFAULT_TEST_TARGET=test -j1 test\n+\ttouch coverage-test.made\n+\n+coverage-test.made:\n+\t@make coverage-test\n+\ttouch coverage-test.made\n \n coverage-prove: coverage-clean-results coverage-compile\n \t$(MAKE) CFLAGS=\"$(COVERAGE_CFLAGS)\" LDFLAGS=\"$(COVERAGE_LDFLAGS)\" \\\n \t\tDEFAULT_TEST_TARGET=prove GIT_PROVE_OPTS=\"$(GIT_PROVE_OPTS) -j1\" \\\n \t\t-j1 test\n \n-coverage-report:\n+coverage-report: coverage-test.made\n \t$(QUIET_GCOV)for dir in $(object_dirs); do \\\n \t\t$(GCOV) $(GCOVFLAGS) --object-directory=$$dir $$dir*.c || exit; \\\n \tdone\n-- \n2.35.1\n\n"},{"id":"453625","messageId":"xmqqsfqg2r0i.fsf@gitster.g","threadId":"57729","inReplyTo":"20220414022513.31465-1-gitter.spiros@gmail.com","subject":"Re: [PATCH v3] Makefile: add a prerequisite to the coverage-report target","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-14T05:06:05Z","receivedAt":"2022-04-14T05:06:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elia Pinto writes:\n\n> Directly invoking make coverage-report as a target results in an error because\n> its prerequisites are missing,\n>\n> This patch adds the compile-test prerequisite, which is run only once each time\n> the compile-report target is invoked. In practice, the developer may decide to\n> review the coverage-report results without necessarily rerunning for this\n> coverage-test, if it has already been run.\n>\n> Signed-off-by: Elia Pinto \n> ---\n> This is the third revision of the patch. Compared to the second:\n>\n> - deleted the Makefile coverage-test.file variable, replaced directly by the\n> coverage-test.made file\n> - added in coverage-compile as last command the touch of coverage-test.made, so\n> that even if coverage-test is run first, invoking indirectly it by the first coverage-report does not\n> call coverage-test and so the test suite gain.\n>\n> Makefile | 8 +++++++-\n> 1 file changed, 7 insertions(+), 1 deletion(-)\n>\n> diff --git a/Makefile b/Makefile\n> index f8bccfab5e..d6ae228c74 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -3407,6 +3407,7 @@ coverage-clean-results:\n> \t$(RM) coverage-untested-functions\n> \t$(RM) -r cover_db/\n> \t$(RM) -r cover_db_html/\n> +\t$(RM) coverage-test.made\n> \n> coverage-clean: coverage-clean-results\n> \t$(RM) $(addsuffix *.gcno,$(object_dirs))\n> @@ -3421,13 +3422,18 @@ coverage-compile:\n> coverage-test: coverage-clean-results coverage-compile\n> \t$(MAKE) CFLAGS=\"$(COVERAGE_CFLAGS)\" LDFLAGS=\"$(COVERAGE_LDFLAGS)\" \\\n> \t\tDEFAULT_TEST_TARGET=test -j1 test\n> +\ttouch coverage-test.made\n> +\n> +coverage-test.made:\n> +\t@make coverage-test\n\ns/@make/$(MAKE)/, I think.\n\n> +\ttouch coverage-test.made\n\nOther than that, looking good.\n\nThanks.\n"},{"id":"453626","messageId":"xmqqlew82qjp.fsf@gitster.g","threadId":"57729","inReplyTo":"20220414022513.31465-1-gitter.spiros@gmail.com","subject":"Re: [PATCH v3] Makefile: add a prerequisite to the coverage-report target","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-14T05:16:10Z","receivedAt":"2022-04-14T05:16:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elia Pinto writes:\n\n> @@ -3421,13 +3422,18 @@ coverage-compile:\n> coverage-test: coverage-clean-results coverage-compile\n> \t$(MAKE) CFLAGS=\"$(COVERAGE_CFLAGS)\" LDFLAGS=\"$(COVERAGE_LDFLAGS)\" \\\n> \t\tDEFAULT_TEST_TARGET=test -j1 test\n> +\ttouch coverage-test.made\n> +\n> +coverage-test.made:\n> +\t@make coverage-test\n> +\ttouch coverage-test.made\n\nAs this target runs \"$(MAKE) coverage-test\", at the end of which the\n.made file is touched anyway, I wonder if we need the second line\nhere.\n\nI just tried to run this sequence:\n\n $ make distclean\n $ make coverage-report\n $ make coverage-report\n $ make coverage-test\n\nwithout the \"touch\" in the recipe for the coverage-test.made target,\nand it behaved as I expected them to, i.e.\n\n * got an pristine tree\n * built, ran tests and showed report\n * showed report without building or running testing\n * cleared the gcov files, ran tests and showed report\n\nSo, perhaps this patch on top?\n\nThanks.\n\n Makefile | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git i/Makefile w/Makefile\nindex fa8cc86276..72ae7fa886 100644\n--- i/Makefile\n+++ w/Makefile\n@@ -3416,8 +3416,7 @@ coverage-test: coverage-clean-results coverage-compile\n \ttouch coverage-test.made\n \n coverage-test.made:\n-\t@make coverage-test\n-\ttouch coverage-test.made\n+\t$(MAKE) coverage-test\n \n coverage-prove: coverage-clean-results coverage-compile\n \t$(MAKE) CFLAGS=\"$(COVERAGE_CFLAGS)\" LDFLAGS=\"$(COVERAGE_LDFLAGS)\" \\\n\n"}]}