# [PATCH v2 2/5] Documentation/Makefile: conditionally include ../GIT-VERSION-FILE 4 messages from 2020-12-07 to 2020-12-08. Participants: Ramsay Jones, Felipe Contreras, Junio C Hamano. Thread: https://gitlist.dev/t/54780 ## Ramsay Jones, 2020-12-07 00:32 Subject: [PATCH v2 2/5] Documentation/Makefile: conditionally include ../GIT-VERSION-FILE Message-ID: <3e085045-99d5-29ee-ed3f-076b1b8bb6b6@ramsayjones.plus.com> URL: https://gitlist.dev/e/3e085045-99d5-29ee-ed3f-076b1b8bb6b6%40ramsayjones.plus.com ``` The 'clean' target is still noticeably slow on cygwin, despite the substantial improvement made by the previous patch. For example, the second invocation of 'make clean' below: $ make clean >/dev/null 2>&1 $ make clean ... make[1]: Entering directory '/home/ramsay/git/Documentation' make[2]: Entering directory '/home/ramsay/git' make[2]: 'GIT-VERSION-FILE' is up to date. make[2]: Leaving directory '/home/ramsay/git' ... $ has been timed at 12.364s on my laptop (on old core i5-4200M @ 2.50GHz, 8GB RAM, 1TB HDD). Notice that the 'clean' target is making a nested call to the parent Makefile to ensure that the GIT-VERSION-FILE is up-to-date (prior to the previous patch, there would have been _two_ such invocations). This is to ensure that the $(GIT_VERSION) make variable is set, once that file had been included. However, the 'clean' target does not use the $(GIT_VERSION) variable, so this is wasted effort. In order to eliminate such wasted effort, use the value of the internal $(MAKECMDGOALS) variable to only '-include ../GIT-VERSION-FILE' when the target is not 'clean'. (This drops the time down to 10.361s, on my laptop, giving an improvement of 16.20%). Signed-off-by: Ramsay Jones --- Documentation/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/Makefile b/Documentation/Makefile index 652d57a1b6..5c680024eb 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -272,7 +272,9 @@ install-html: html ../GIT-VERSION-FILE: FORCE $(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE +ifneq ($(MAKECMDGOALS),clean) -include ../GIT-VERSION-FILE +endif # # Determine "include::" file references in asciidoc files. -- 2.29.0 ``` ## Felipe Contreras, 2020-12-07 03:21 Subject: Re: [PATCH v2 2/5] Documentation/Makefile: conditionally include ../GIT-VERSION-FILE Message-ID: URL: https://gitlist.dev/e/CAMP44s3hCm_2%3DjRv-4knNWe%2BV5vR_%2BXWjHVFkVm_LNzJ%2Bn1sDg%40mail.gmail.com In-Reply-To: <3e085045-99d5-29ee-ed3f-076b1b8bb6b6@ramsayjones.plus.com> ``` On Sun, Dec 6, 2020 at 6:35 PM Ramsay Jones wrote: > > > The 'clean' target is still noticeably slow on cygwin, despite the > substantial improvement made by the previous patch. For example, the > second invocation of 'make clean' below: > > $ make clean >/dev/null 2>&1 > $ make clean > ... > make[1]: Entering directory '/home/ramsay/git/Documentation' > make[2]: Entering directory '/home/ramsay/git' > make[2]: 'GIT-VERSION-FILE' is up to date. > make[2]: Leaving directory '/home/ramsay/git' > ... > $ > > has been timed at 12.364s on my laptop (on old core i5-4200M @ 2.50GHz, > 8GB RAM, 1TB HDD). > > Notice that the 'clean' target is making a nested call to the parent > Makefile to ensure that the GIT-VERSION-FILE is up-to-date (prior to > the previous patch, there would have been _two_ such invocations). > This is to ensure that the $(GIT_VERSION) make variable is set, once > that file had been included. However, the 'clean' target does not use > the $(GIT_VERSION) variable, so this is wasted effort. Yes, this is the important information: "the 'clean' target does not use the $(GIT_VERSION) variable". I would feature it at the start of the commit message. -- Felipe Contreras ``` ## Junio C Hamano, 2020-12-07 07:45 Subject: Re: [PATCH v2 2/5] Documentation/Makefile: conditionally include ../GIT-VERSION-FILE Message-ID: URL: https://gitlist.dev/e/xmqqy2iaawk8.fsf%40gitster.c.googlers.com In-Reply-To: <3e085045-99d5-29ee-ed3f-076b1b8bb6b6@ramsayjones.plus.com> ``` Ramsay Jones writes: > The 'clean' target is still noticeably slow on cygwin, despite the > substantial improvement made by the previous patch. For example, the > second invocation of 'make clean' below: > > $ make clean >/dev/null 2>&1 > $ make clean > ... > make[1]: Entering directory '/home/ramsay/git/Documentation' > make[2]: Entering directory '/home/ramsay/git' > make[2]: 'GIT-VERSION-FILE' is up to date. > make[2]: Leaving directory '/home/ramsay/git' > ... > $ > > has been timed at 12.364s on my laptop (on old core i5-4200M @ 2.50GHz, > 8GB RAM, 1TB HDD). > > Notice that the 'clean' target is making a nested call to the parent > Makefile to ensure that the GIT-VERSION-FILE is up-to-date (prior to > the previous patch, there would have been _two_ such invocations). > This is to ensure that the $(GIT_VERSION) make variable is set, once > that file had been included. However, the 'clean' target does not use > the $(GIT_VERSION) variable, so this is wasted effort. > > In order to eliminate such wasted effort, use the value of the internal > $(MAKECMDGOALS) variable to only '-include ../GIT-VERSION-FILE' when the > target is not 'clean'. (This drops the time down to 10.361s, on my laptop, > giving an improvement of 16.20%). Again, nicely explained. > > Signed-off-by: Ramsay Jones > --- > Documentation/Makefile | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/Makefile b/Documentation/Makefile > index 652d57a1b6..5c680024eb 100644 > --- a/Documentation/Makefile > +++ b/Documentation/Makefile > @@ -272,7 +272,9 @@ install-html: html > ../GIT-VERSION-FILE: FORCE > $(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE > > +ifneq ($(MAKECMDGOALS),clean) > -include ../GIT-VERSION-FILE > +endif > > # > # Determine "include::" file references in asciidoc files. ``` ## Ramsay Jones, 2020-12-08 22:23 Subject: Re: [PATCH v2 2/5] Documentation/Makefile: conditionally include ../GIT-VERSION-FILE Message-ID: <9cde91bf-8808-6110-b9ea-a3385225f0a2@ramsayjones.plus.com> URL: https://gitlist.dev/e/9cde91bf-8808-6110-b9ea-a3385225f0a2%40ramsayjones.plus.com In-Reply-To: ``` On 07/12/2020 03:21, Felipe Contreras wrote: > On Sun, Dec 6, 2020 at 6:35 PM Ramsay Jones wrote: [snip] >> Notice that the 'clean' target is making a nested call to the parent >> Makefile to ensure that the GIT-VERSION-FILE is up-to-date (prior to >> the previous patch, there would have been _two_ such invocations). >> This is to ensure that the $(GIT_VERSION) make variable is set, once >> that file had been included. However, the 'clean' target does not use >> the $(GIT_VERSION) variable, so this is wasted effort. > > Yes, this is the important information: "the 'clean' target does not use > the $(GIT_VERSION) variable". I would feature it at the start of the > commit message. Heh, I didn't intend to 'bury the lead' here. I spent about 30 minutes re-writing this message to come up with an improvement; I'm not sure that I succeeded. ;-) v3 comming soon ... Thanks! ATB, Ramsay Jones ```