{"thread":{"id":"54780","subject":"[PATCH v2 2/5] Documentation/Makefile: conditionally include ../GIT-VERSION-FILE","startedAt":"2020-12-07T00:33:17Z","lastAt":"2020-12-08T22:24:28Z","messageCount":4,"participants":["Ramsay Jones","Felipe Contreras","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":5},"messages":[{"id":"411574","messageId":"3e085045-99d5-29ee-ed3f-076b1b8bb6b6@ramsayjones.plus.com","threadId":"54780","inReplyTo":null,"subject":"[PATCH v2 2/5] Documentation/Makefile: conditionally include ../GIT-VERSION-FILE","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2020-12-07T00:32:54Z","receivedAt":"2020-12-07T00:33:17Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nThe 'clean' target is still noticeably slow on cygwin, despite the\nsubstantial improvement made by the previous patch. For example, the\nsecond invocation of 'make clean' below:\n\n $ make clean >/dev/null 2>&1\n $ make clean\n ...\n make[1]: Entering directory '/home/ramsay/git/Documentation'\n make[2]: Entering directory '/home/ramsay/git'\n make[2]: 'GIT-VERSION-FILE' is up to date.\n make[2]: Leaving directory '/home/ramsay/git'\n ...\n $\n\nhas been timed at 12.364s on my laptop (on old core i5-4200M @ 2.50GHz,\n8GB RAM, 1TB HDD).\n\nNotice that the 'clean' target is making a nested call to the parent\nMakefile to ensure that the GIT-VERSION-FILE is up-to-date (prior to\nthe previous patch, there would have been _two_ such invocations).\nThis is to ensure that the $(GIT_VERSION) make variable is set, once\nthat file had been included. However, the 'clean' target does not use\nthe $(GIT_VERSION) variable, so this is wasted effort.\n\nIn order to eliminate such wasted effort, use the value of the internal\n$(MAKECMDGOALS) variable to only '-include ../GIT-VERSION-FILE' when the\ntarget is not 'clean'. (This drops the time down to 10.361s, on my laptop,\ngiving an improvement of 16.20%).\n\nSigned-off-by: Ramsay Jones \n---\n Documentation/Makefile | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 652d57a1b6..5c680024eb 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -272,7 +272,9 @@ install-html: html\n ../GIT-VERSION-FILE: FORCE\n \t$(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE\n \n+ifneq ($(MAKECMDGOALS),clean)\n -include ../GIT-VERSION-FILE\n+endif\n \n #\n # Determine \"include::\" file references in asciidoc files.\n-- \n2.29.0\n"},{"id":"411582","messageId":"CAMP44s3hCm_2=jRv-4knNWe+V5vR_+XWjHVFkVm_LNzJ+n1sDg@mail.gmail.com","threadId":"54780","inReplyTo":"3e085045-99d5-29ee-ed3f-076b1b8bb6b6@ramsayjones.plus.com","subject":"Re: [PATCH v2 2/5] Documentation/Makefile: conditionally include ../GIT-VERSION-FILE","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-12-07T03:21:06Z","receivedAt":"2020-12-07T03:22:05Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Dec 6, 2020 at 6:35 PM Ramsay Jones wrote:\n>\n>\n> The 'clean' target is still noticeably slow on cygwin, despite the\n> substantial improvement made by the previous patch. For example, the\n> second invocation of 'make clean' below:\n>\n> $ make clean >/dev/null 2>&1\n> $ make clean\n> ...\n> make[1]: Entering directory '/home/ramsay/git/Documentation'\n> make[2]: Entering directory '/home/ramsay/git'\n> make[2]: 'GIT-VERSION-FILE' is up to date.\n> make[2]: Leaving directory '/home/ramsay/git'\n> ...\n> $\n>\n> has been timed at 12.364s on my laptop (on old core i5-4200M @ 2.50GHz,\n> 8GB RAM, 1TB HDD).\n>\n> Notice that the 'clean' target is making a nested call to the parent\n> Makefile to ensure that the GIT-VERSION-FILE is up-to-date (prior to\n> the previous patch, there would have been _two_ such invocations).\n> This is to ensure that the $(GIT_VERSION) make variable is set, once\n> that file had been included. However, the 'clean' target does not use\n> the $(GIT_VERSION) variable, so this is wasted effort.\n\nYes, this is the important information: \"the 'clean' target does not use\nthe $(GIT_VERSION) variable\". I would feature it at the start of the\ncommit message.\n\n-- \nFelipe Contreras\n"},{"id":"411589","messageId":"xmqqy2iaawk8.fsf@gitster.c.googlers.com","threadId":"54780","inReplyTo":"3e085045-99d5-29ee-ed3f-076b1b8bb6b6@ramsayjones.plus.com","subject":"Re: [PATCH v2 2/5] Documentation/Makefile: conditionally include ../GIT-VERSION-FILE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-07T07:45:43Z","receivedAt":"2020-12-07T07:46:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> The 'clean' target is still noticeably slow on cygwin, despite the\n> substantial improvement made by the previous patch. For example, the\n> second invocation of 'make clean' below:\n>\n> $ make clean >/dev/null 2>&1\n> $ make clean\n> ...\n> make[1]: Entering directory '/home/ramsay/git/Documentation'\n> make[2]: Entering directory '/home/ramsay/git'\n> make[2]: 'GIT-VERSION-FILE' is up to date.\n> make[2]: Leaving directory '/home/ramsay/git'\n> ...\n> $\n>\n> has been timed at 12.364s on my laptop (on old core i5-4200M @ 2.50GHz,\n> 8GB RAM, 1TB HDD).\n>\n> Notice that the 'clean' target is making a nested call to the parent\n> Makefile to ensure that the GIT-VERSION-FILE is up-to-date (prior to\n> the previous patch, there would have been _two_ such invocations).\n> This is to ensure that the $(GIT_VERSION) make variable is set, once\n> that file had been included. However, the 'clean' target does not use\n> the $(GIT_VERSION) variable, so this is wasted effort.\n>\n> In order to eliminate such wasted effort, use the value of the internal\n> $(MAKECMDGOALS) variable to only '-include ../GIT-VERSION-FILE' when the\n> target is not 'clean'. (This drops the time down to 10.361s, on my laptop,\n> giving an improvement of 16.20%).\n\nAgain, nicely explained.\n\n>\n> Signed-off-by: Ramsay Jones \n> ---\n> Documentation/Makefile | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/Documentation/Makefile b/Documentation/Makefile\n> index 652d57a1b6..5c680024eb 100644\n> --- a/Documentation/Makefile\n> +++ b/Documentation/Makefile\n> @@ -272,7 +272,9 @@ install-html: html\n> ../GIT-VERSION-FILE: FORCE\n> \t$(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE\n> \n> +ifneq ($(MAKECMDGOALS),clean)\n> -include ../GIT-VERSION-FILE\n> +endif\n> \n> #\n> # Determine \"include::\" file references in asciidoc files.\n"},{"id":"411783","messageId":"9cde91bf-8808-6110-b9ea-a3385225f0a2@ramsayjones.plus.com","threadId":"54780","inReplyTo":"CAMP44s3hCm_2=jRv-4knNWe+V5vR_+XWjHVFkVm_LNzJ+n1sDg@mail.gmail.com","subject":"Re: [PATCH v2 2/5] Documentation/Makefile: conditionally include ../GIT-VERSION-FILE","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2020-12-08T22:23:16Z","receivedAt":"2020-12-08T22:24:28Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 07/12/2020 03:21, Felipe Contreras wrote:\n> On Sun, Dec 6, 2020 at 6:35 PM Ramsay Jones wrote:\n[snip]\n>> Notice that the 'clean' target is making a nested call to the parent\n>> Makefile to ensure that the GIT-VERSION-FILE is up-to-date (prior to\n>> the previous patch, there would have been _two_ such invocations).\n>> This is to ensure that the $(GIT_VERSION) make variable is set, once\n>> that file had been included. However, the 'clean' target does not use\n>> the $(GIT_VERSION) variable, so this is wasted effort.\n> \n> Yes, this is the important information: \"the 'clean' target does not use\n> the $(GIT_VERSION) variable\". I would feature it at the start of the\n> commit message.\n\nHeh, I didn't intend to 'bury the lead' here. I spent about 30 minutes\nre-writing this message to come up with an improvement; I'm not sure\nthat I succeeded. ;-)\n\nv3 comming soon ...\n\nThanks!\n\nATB,\nRamsay Jones\n\n"}]}