{"thread":{"id":"56786","subject":"[PATCH] Documentation/Makefile: fix lint-docs mkdir dependency","startedAt":"2021-10-26T07:31:47Z","lastAt":"2021-10-28T18:38:33Z","messageCount":9,"participants":["Jeff King","Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"439630","messageId":"YXeu4Hl2cmIPqobd@coredump.intra.peff.net","threadId":"56786","inReplyTo":null,"subject":"[PATCH] Documentation/Makefile: fix lint-docs mkdir dependency","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-26T07:31:44Z","receivedAt":"2021-10-26T07:31:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Since 8650c6298c (doc lint: make \"lint-docs\" non-.PHONY, 2021-10-15), we\nput the output for gitlink linter into .build/lint-docs/gitlink. There\nare order-only dependencies to create the sequence of subdirs like:\n\n  .build/lint-docs: | .build\n          $(QUIET)mkdir $@\n  .build/lint-docs/gitlink: | .build/lint-docs\n          $(QUIET)mkdir $@\n\nwhere each level has to depend on the prior one (since the parent\ndirectory must exist for us to create something inside it). But the\n\"howto\" and \"config\" subdirectories of gitlink have the wrong\ndependency; they depend on \"lint-docs\", not \"lint-docs/gitlink\".\n\nThis usually works out, because the LINT_DOCS_GITLINK targets which\ndepend on \"gitlink/howto\" also depend on just \"gitlink\", so the\ndirectory gets created anyway. But since we haven't given make an\nexplicit ordering, things can racily happen out of order.\n\nIf you stick a \"sleep 1\" in the rule to build \"gitlink\" like this:\n\n   ## Lint: gitlink\n   .build/lint-docs/gitlink: | .build/lint-docs\n  -\t$(QUIET)mkdir $@\n  +\t$(QUIET)sleep 1 && mkdir $@\n\nthen \"make clean; make lint-docs\" will fail reliably. Or you can see it\nas-is just by building the directory in isolation:\n\n  $ make clean\n  [...]\n  $ make .build/lint-docs/gitlink/howto\n      GEN mergetools-list.made\n      GEN cmd-list.made\n      GEN doc.dep\n      SUBDIR ../\n  make[1]: 'GIT-VERSION-FILE' is up to date.\n      SUBDIR ../\n  make[1]: 'GIT-VERSION-FILE' is up to date.\n  mkdir: cannot create directory ‘.build/lint-docs/gitlink/howto’: No such file or directory\n  make: *** [Makefile:476: .build/lint-docs/gitlink/howto] Error 1\n\nThe fix is easy: we just need to depend on the correct parent directory.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThe problem starts in ab/fix-make-lint-docs, which is in master.\n\nI wasn't able to trigger the problem locally even with running 'make\nclean; make lint-docs' in a loop, but I did see it in the wild in a CI\ndocumentation job:\n\n  https://github.com/peff/git/runs/4005766641?check_suite_focus=true#step:4:60\n\nIt would have been a lot easier to diagnose from the CI output if the\nmkdir lines weren't silent. I.e., if we had a $(QUIET_MKDIR) which\nprinted \"MKDIR $@\" rather than nothing at all.\n\n Documentation/Makefile | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 911b6bf79c..ed656db2ae 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -472,9 +472,9 @@ print-man1:\n ## Lint: gitlink\n .build/lint-docs/gitlink: | .build/lint-docs\n \t$(QUIET)mkdir $@\n-.build/lint-docs/gitlink/howto: | .build/lint-docs\n+.build/lint-docs/gitlink/howto: | .build/lint-docs/gitlink\n \t$(QUIET)mkdir $@\n-.build/lint-docs/gitlink/config: | .build/lint-docs\n+.build/lint-docs/gitlink/config: | .build/lint-docs/gitlink\n \t$(QUIET)mkdir $@\n LINT_DOCS_GITLINK = $(patsubst %.txt,.build/lint-docs/gitlink/%.ok,$(HOWTO_TXT) $(DOC_DEP_TXT))\n $(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink\n-- \n2.33.1.1387.g97d4a0c3a8\n"},{"id":"439634","messageId":"211026.86fsso14m1.gmgdl@evledraar.gmail.com","threadId":"56786","inReplyTo":"YXeu4Hl2cmIPqobd@coredump.intra.peff.net","subject":"Re: [PATCH] Documentation/Makefile: fix lint-docs mkdir dependency","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-26T10:05:40Z","receivedAt":"2021-10-26T10:07:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Oct 26 2021, Jeff King wrote:\n\n> Since 8650c6298c (doc lint: make \"lint-docs\" non-.PHONY, 2021-10-15), we\n> put the output for gitlink linter into .build/lint-docs/gitlink. There\n> are order-only dependencies to create the sequence of subdirs like:\n>\n>   .build/lint-docs: | .build\n>           $(QUIET)mkdir $@\n>   .build/lint-docs/gitlink: | .build/lint-docs\n>           $(QUIET)mkdir $@\n>\n> where each level has to depend on the prior one (since the parent\n> directory must exist for us to create something inside it). But the\n> \"howto\" and \"config\" subdirectories of gitlink have the wrong\n> dependency; they depend on \"lint-docs\", not \"lint-docs/gitlink\".\n>\n> This usually works out, because the LINT_DOCS_GITLINK targets which\n> depend on \"gitlink/howto\" also depend on just \"gitlink\", so the\n> directory gets created anyway. But since we haven't given make an\n> explicit ordering, things can racily happen out of order.\n>\n> If you stick a \"sleep 1\" in the rule to build \"gitlink\" like this:\n>\n>    ## Lint: gitlink\n>    .build/lint-docs/gitlink: | .build/lint-docs\n>   -\t$(QUIET)mkdir $@\n>   +\t$(QUIET)sleep 1 && mkdir $@\n>\n> then \"make clean; make lint-docs\" will fail reliably. Or you can see it\n> as-is just by building the directory in isolation:\n>\n>   $ make clean\n>   [...]\n>   $ make .build/lint-docs/gitlink/howto\n>       GEN mergetools-list.made\n>       GEN cmd-list.made\n>       GEN doc.dep\n>       SUBDIR ../\n>   make[1]: 'GIT-VERSION-FILE' is up to date.\n>       SUBDIR ../\n>   make[1]: 'GIT-VERSION-FILE' is up to date.\n>   mkdir: cannot create directory ‘.build/lint-docs/gitlink/howto’: No such file or directory\n>   make: *** [Makefile:476: .build/lint-docs/gitlink/howto] Error 1\n>\n> The fix is easy: we just need to depend on the correct parent directory.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> The problem starts in ab/fix-make-lint-docs, which is in master.\n>\n> I wasn't able to trigger the problem locally even with running 'make\n> clean; make lint-docs' in a loop, but I did see it in the wild in a CI\n> documentation job:\n>\n>   https://github.com/peff/git/runs/4005766641?check_suite_focus=true#step:4:60\n>\n> It would have been a lot easier to diagnose from the CI output if the\n> mkdir lines weren't silent. I.e., if we had a $(QUIET_MKDIR) which\n> printed \"MKDIR $@\" rather than nothing at all.\n>\n>  Documentation/Makefile | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/Makefile b/Documentation/Makefile\n> index 911b6bf79c..ed656db2ae 100644\n> --- a/Documentation/Makefile\n> +++ b/Documentation/Makefile\n> @@ -472,9 +472,9 @@ print-man1:\n>  ## Lint: gitlink\n>  .build/lint-docs/gitlink: | .build/lint-docs\n>  \t$(QUIET)mkdir $@\n> -.build/lint-docs/gitlink/howto: | .build/lint-docs\n> +.build/lint-docs/gitlink/howto: | .build/lint-docs/gitlink\n>  \t$(QUIET)mkdir $@\n> -.build/lint-docs/gitlink/config: | .build/lint-docs\n> +.build/lint-docs/gitlink/config: | .build/lint-docs/gitlink\n>  \t$(QUIET)mkdir $@\n>  LINT_DOCS_GITLINK = $(patsubst %.txt,.build/lint-docs/gitlink/%.ok,$(HOWTO_TXT) $(DOC_DEP_TXT))\n>  $(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink\n\nThanks a lot for fixing that bug, and sorry for not spotting it. This\nfix LGTM and is obviously correct.\n\nI'll try to do something about the $(QUIET*) as a follow-up, I was\ntrying to find the right balance between over-verbosity & \"tracing\".\n"},{"id":"439686","messageId":"YXhwqVUNaoSZpdex@coredump.intra.peff.net","threadId":"56786","inReplyTo":"211026.86fsso14m1.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] Documentation/Makefile: fix lint-docs mkdir dependency","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-26T21:18:33Z","receivedAt":"2021-10-26T21:18:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 26, 2021 at 12:05:40PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> I'll try to do something about the $(QUIET*) as a follow-up, I was\n> trying to find the right balance between over-verbosity & \"tracing\".\n\nYeah, I wondered if it would end up super-verbose. But I tried it out\nand IMHO it looks just fine to print mkdir lines. In the initial build,\n\"make\" realizes we only need to run each rule once, and in subsequent\nbuilds it doesn't run them at all.\n\n-Peff\n"},{"id":"439849","messageId":"xmqqlf2et3r3.fsf@gitster.g","threadId":"56786","inReplyTo":"YXeu4Hl2cmIPqobd@coredump.intra.peff.net","subject":"Re: [PATCH] Documentation/Makefile: fix lint-docs mkdir dependency","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-28T00:03:12Z","receivedAt":"2021-10-28T00:03:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Since 8650c6298c (doc lint: make \"lint-docs\" non-.PHONY, 2021-10-15), we\n> put the output for gitlink linter into .build/lint-docs/gitlink. There\n> are order-only dependencies to create the sequence of subdirs like:\n>\n>   .build/lint-docs: | .build\n>           $(QUIET)mkdir $@\n>   .build/lint-docs/gitlink: | .build/lint-docs\n>           $(QUIET)mkdir $@\n>\n> where each level has to depend on the prior one (since the parent\n> directory must exist for us to create something inside it). But the\n> \"howto\" and \"config\" subdirectories of gitlink have the wrong\n> dependency; they depend on \"lint-docs\", not \"lint-docs/gitlink\".\n\nThanks.\n\nI wonder if we can somehow avoid this unwieldy chain of commands,\nperhaps with using \"mkdir -p\" somewhere, or make the lint scripts\ncreate the necessary leading paths.  From the looks of the tail end\nof Documentation/Makefile, the latter may be the cleaner solution.\n\n"},{"id":"439862","messageId":"211028.861r45y3pt.gmgdl@evledraar.gmail.com","threadId":"56786","inReplyTo":"xmqqlf2et3r3.fsf@gitster.g","subject":"Re: [PATCH] Documentation/Makefile: fix lint-docs mkdir dependency","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-28T07:48:51Z","receivedAt":"2021-10-28T08:05:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 27 2021, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> Since 8650c6298c (doc lint: make \"lint-docs\" non-.PHONY, 2021-10-15), we\n>> put the output for gitlink linter into .build/lint-docs/gitlink. There\n>> are order-only dependencies to create the sequence of subdirs like:\n>>\n>>   .build/lint-docs: | .build\n>>           $(QUIET)mkdir $@\n>>   .build/lint-docs/gitlink: | .build/lint-docs\n>>           $(QUIET)mkdir $@\n>>\n>> where each level has to depend on the prior one (since the parent\n>> directory must exist for us to create something inside it). But the\n>> \"howto\" and \"config\" subdirectories of gitlink have the wrong\n>> dependency; they depend on \"lint-docs\", not \"lint-docs/gitlink\".\n>\n> Thanks.\n>\n> I wonder if we can somehow avoid this unwieldy chain of commands,\n> perhaps with using \"mkdir -p\" somewhere, or make the lint scripts\n> create the necessary leading paths.  From the looks of the tail end\n> of Documentation/Makefile, the latter may be the cleaner solution.\n\nSimplest would be to simply do the \"mkdir -p\" unconditionally, which we\ndo in some other places. The diff below on top of next would do that.\n\nI didn't do it because it slows things down quite a bit. Here HEAD is\nthe diff below:\n    \n    $ hyperfine --warmup 5 -m 30 -L s \",~\" -p 'git checkout HEAD{s} -- Makefile; rm -rf .build' 'make -j8 lint-docs R={s}' -c 'git checkout HEAD -- Makefile'\n    Benchmark #1: make -j8 lint-docs R=\n      Time (mean ± σ):     709.7 ms ±  25.7 ms    [User: 3.584 s, System: 0.696 s]\n      Range (min … max):   691.4 ms … 833.1 ms    30 runs\n     \n    Benchmark #2: make -j8 lint-docs R=~\n      Time (mean ± σ):     647.3 ms ±   5.5 ms    [User: 3.081 s, System: 0.635 s]\n      Range (min … max):   638.4 ms … 670.6 ms    30 runs\n     \n    Summary\n      'make -j8 lint-docs R=~' ran\n        1.10 ± 0.04 times faster than 'make -j8 lint-docs R='\n\nYou can do this with make macros via $(eval) calling a $(foreach) loop,\ni.e. just generate the boilerplate we have now. For this case I thought\nit wasn't worth it, but figured I could eventually loop back to it\nif/when we use a nested structure inside a \".build directory more\nwidely.\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex ed656db2ae9..8a185e89e55 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -463,26 +463,11 @@ quick-install-html: require-htmlrepo\n print-man1:\n \t@for i in $(MAN1_TXT); do echo $$i; done\n \n-## Lint: Common\n-.build:\n-\t$(QUIET)mkdir $@\n-.build/lint-docs: | .build\n-\t$(QUIET)mkdir $@\n-\n-## Lint: gitlink\n-.build/lint-docs/gitlink: | .build/lint-docs\n-\t$(QUIET)mkdir $@\n-.build/lint-docs/gitlink/howto: | .build/lint-docs/gitlink\n-\t$(QUIET)mkdir $@\n-.build/lint-docs/gitlink/config: | .build/lint-docs/gitlink\n-\t$(QUIET)mkdir $@\n LINT_DOCS_GITLINK = $(patsubst %.txt,.build/lint-docs/gitlink/%.ok,$(HOWTO_TXT) $(DOC_DEP_TXT))\n-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink\n-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/howto\n-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/config\n $(LINT_DOCS_GITLINK): lint-gitlink.perl\n $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt\n-\t$(QUIET_LINT_GITLINK)$(PERL_PATH) lint-gitlink.perl \\\n+\t$(QUIET_LINT_GITLINK)mkdir -p $(@D) && \\\n+\t$(PERL_PATH) lint-gitlink.perl \\\n \t\t$< \\\n \t\t$(HOWTO_TXT) $(DOC_DEP_TXT) \\\n \t\t--section=1 $(MAN1_TXT) \\\n@@ -492,24 +477,20 @@ $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt\n lint-docs-gitlink: $(LINT_DOCS_GITLINK)\n \n ## Lint: man-end-blurb\n-.build/lint-docs/man-end-blurb: | .build/lint-docs\n-\t$(QUIET)mkdir $@\n LINT_DOCS_MAN_END_BLURB = $(patsubst %.txt,.build/lint-docs/man-end-blurb/%.ok,$(MAN_TXT))\n-$(LINT_DOCS_MAN_END_BLURB): | .build/lint-docs/man-end-blurb\n $(LINT_DOCS_MAN_END_BLURB): lint-man-end-blurb.perl\n $(LINT_DOCS_MAN_END_BLURB): .build/lint-docs/man-end-blurb/%.ok: %.txt\n-\t$(QUIET_LINT_MANEND)$(PERL_PATH) lint-man-end-blurb.perl $< >$@\n+\t$(QUIET_LINT_MANEND)mkdir -p $(@D) && \\\n+\t$(PERL_PATH) lint-man-end-blurb.perl $< >$@\n .PHONY: lint-docs-man-end-blurb\n lint-docs-man-end-blurb: $(LINT_DOCS_MAN_END_BLURB)\n \n ## Lint: man-section-order\n-.build/lint-docs/man-section-order: | .build/lint-docs\n-\t$(QUIET)mkdir $@\n LINT_DOCS_MAN_SECTION_ORDER = $(patsubst %.txt,.build/lint-docs/man-section-order/%.ok,$(MAN_TXT))\n-$(LINT_DOCS_MAN_SECTION_ORDER): | .build/lint-docs/man-section-order\n $(LINT_DOCS_MAN_SECTION_ORDER): lint-man-section-order.perl\n $(LINT_DOCS_MAN_SECTION_ORDER): .build/lint-docs/man-section-order/%.ok: %.txt\n-\t$(QUIET_LINT_MANSEC)$(PERL_PATH) lint-man-section-order.perl $< >$@\n+\t$(QUIET_LINT_MANSEC)mkdir -p $(@D) && \\\n+\t$(PERL_PATH) lint-man-section-order.perl $< >$@\n .PHONY: lint-docs-man-section-order\n lint-docs-man-section-order: $(LINT_DOCS_MAN_SECTION_ORDER)\n \n"},{"id":"439873","messageId":"YXq1OE/p5VoPR3WZ@coredump.intra.peff.net","threadId":"56786","inReplyTo":"211028.861r45y3pt.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] Documentation/Makefile: fix lint-docs mkdir dependency","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-28T14:35:36Z","receivedAt":"2021-10-28T14:36:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 28, 2021 at 09:48:51AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > I wonder if we can somehow avoid this unwieldy chain of commands,\n> > perhaps with using \"mkdir -p\" somewhere, or make the lint scripts\n> > create the necessary leading paths.  From the looks of the tail end\n> > of Documentation/Makefile, the latter may be the cleaner solution.\n> \n> Simplest would be to simply do the \"mkdir -p\" unconditionally, which we\n> do in some other places. The diff below on top of next would do that.\n> \n> I didn't do it because it slows things down quite a bit. Here HEAD is\n> the diff below:\n\nThere's an in-between, I'd think, where the many \"foo/bar/baz/$@\"\ntargets have an order-dependency on \"foo/bar/baz\", and that single rule\nuses \"mkdir -p\" to create all of the directories.\n\nIt doesn't buy us much simplification in this case, though, because\nvarious rules independently depend on .build/gitlink/lint-docs/howto,\n.built/gitlink/lint-docs, and .build/gitlink, etc. So we still end up\nwith roughly the same number of rules, though the directory rules don't\nhave to depend on one another.\n\nIt also means that these \"mkdir -p\" may race with each other, though in\ngeneral I'd hope that most \"mkdir\" implements could handle this.\n\nSomething like this works, I think:\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 911b6bf79c..a70e418af6 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -463,19 +463,13 @@ quick-install-html: require-htmlrepo\n print-man1:\n \t@for i in $(MAN1_TXT); do echo $$i; done\n \n-## Lint: Common\n-.build:\n-\t$(QUIET)mkdir $@\n-.build/lint-docs: | .build\n-\t$(QUIET)mkdir $@\n-\n ## Lint: gitlink\n-.build/lint-docs/gitlink: | .build/lint-docs\n-\t$(QUIET)mkdir $@\n-.build/lint-docs/gitlink/howto: | .build/lint-docs\n-\t$(QUIET)mkdir $@\n-.build/lint-docs/gitlink/config: | .build/lint-docs\n-\t$(QUIET)mkdir $@\n+.build/lint-docs/gitlink:\n+\t$(QUIET)mkdir -p $@\n+.build/lint-docs/gitlink/howto:\n+\t$(QUIET)mkdir -p $@\n+.build/lint-docs/gitlink/config:\n+\t$(QUIET)mkdir -p $@\n LINT_DOCS_GITLINK = $(patsubst %.txt,.build/lint-docs/gitlink/%.ok,$(HOWTO_TXT) $(DOC_DEP_TXT))\n $(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink\n $(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/howto\n@@ -492,8 +486,8 @@ $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt\n lint-docs-gitlink: $(LINT_DOCS_GITLINK)\n \n ## Lint: man-end-blurb\n-.build/lint-docs/man-end-blurb: | .build/lint-docs\n-\t$(QUIET)mkdir $@\n+.build/lint-docs/man-end-blurb:\n+\t$(QUIET)mkdir -p $@\n LINT_DOCS_MAN_END_BLURB = $(patsubst %.txt,.build/lint-docs/man-end-blurb/%.ok,$(MAN_TXT))\n $(LINT_DOCS_MAN_END_BLURB): | .build/lint-docs/man-end-blurb\n $(LINT_DOCS_MAN_END_BLURB): lint-man-end-blurb.perl\n@@ -503,8 +497,8 @@ $(LINT_DOCS_MAN_END_BLURB): .build/lint-docs/man-end-blurb/%.ok: %.txt\n lint-docs-man-end-blurb: $(LINT_DOCS_MAN_END_BLURB)\n \n ## Lint: man-section-order\n-.build/lint-docs/man-section-order: | .build/lint-docs\n-\t$(QUIET)mkdir $@\n+.build/lint-docs/man-section-order:\n+\t$(QUIET)mkdir -p $@\n LINT_DOCS_MAN_SECTION_ORDER = $(patsubst %.txt,.build/lint-docs/man-section-order/%.ok,$(MAN_TXT))\n $(LINT_DOCS_MAN_SECTION_ORDER): | .build/lint-docs/man-section-order\n $(LINT_DOCS_MAN_SECTION_ORDER): lint-man-section-order.perl\n\n We could technically even drop the .build/lint-docs/gitlink rule,\n because it's a parent of other directories built by the same rule. But\n that's a bit too clever and magical for my tastes.\n\n All that said, I'm not that unhappy with the current state, and I think\n with my patch it should be correct / robust.\n\n> You can do this with make macros via $(eval) calling a $(foreach) loop,\n> i.e. just generate the boilerplate we have now. For this case I thought\n> it wasn't worth it, but figured I could eventually loop back to it\n> if/when we use a nested structure inside a \".build directory more\n> widely.\n\nYeah, I think for the scope of the problem here (remembering that \"mkdir\nfoo/bar\" needs to depend on \"mkdir foo\") that a macro would probably\njust confuse things. IMHO the more subtle maintenance trap is that\nLINT_DOCS_GITLINK needs to remember to depend on the \"config/\" and\n\"howto/\" directories, because that's where we keep source files. It\nwould be easy to add a source file to DOC_DEP_TXT that mentions a new\nsubdirectory, but not realize it needs an extra rule.\n\nIf the macro magic went as far as actually mapping all of\nLINT_DOCS_GITLINK into their .build/ dirname counterparts, and then\nautomatically generated the right rules, that would make it truly\nturnkey. It would probably also be a pretty gross macro, but maybe worth\nit if nobody ever needed to touch it. :)\n\nAt any rate, I don't think there's any urgency on that.\n\n-Peff\n"},{"id":"439895","messageId":"xmqqzgqtrtd1.fsf@gitster.g","threadId":"56786","inReplyTo":"YXq1OE/p5VoPR3WZ@coredump.intra.peff.net","subject":"Re: [PATCH] Documentation/Makefile: fix lint-docs mkdir dependency","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-28T16:45:14Z","receivedAt":"2021-10-28T16:45:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> There's an in-between, I'd think, where the many \"foo/bar/baz/$@\"\n> targets have an order-dependency on \"foo/bar/baz\", and that single rule\n> uses \"mkdir -p\" to create all of the directories.\n>\n> It doesn't buy us much simplification in this case, though, because\n> various rules independently depend on .build/gitlink/lint-docs/howto,\n> .built/gitlink/lint-docs, and .build/gitlink, etc. So we still end up\n> with roughly the same number of rules, though the directory rules don't\n> have to depend on one another.\n>\n> It also means that these \"mkdir -p\" may race with each other, though in\n> general I'd hope that most \"mkdir\" implements could handle this.\n>\n> Something like this works, I think:\n\nHmph, what I actually meant was to make sure that the recipe to\ncreate the files to have \"mkdir -p $(basename $@)\" in front, instead\nof having \"we need to prepare the containing directory in order to\nhave a file there\" in the makefile.\n\n> ...\n> At any rate, I don't think there's any urgency on that.\n\nSure.  I think I've picked up the one at the start of this thread\nalready, so we should be good.\n\nThanks.\n"},{"id":"439898","messageId":"YXrYh9aPA8csq+UQ@coredump.intra.peff.net","threadId":"56786","inReplyTo":"xmqqzgqtrtd1.fsf@gitster.g","subject":"Re: [PATCH] Documentation/Makefile: fix lint-docs mkdir dependency","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-28T17:06:15Z","receivedAt":"2021-10-28T17:06:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 28, 2021 at 09:45:14AM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > There's an in-between, I'd think, where the many \"foo/bar/baz/$@\"\n> > targets have an order-dependency on \"foo/bar/baz\", and that single rule\n> > uses \"mkdir -p\" to create all of the directories.\n> >\n> > It doesn't buy us much simplification in this case, though, because\n> > various rules independently depend on .build/gitlink/lint-docs/howto,\n> > .built/gitlink/lint-docs, and .build/gitlink, etc. So we still end up\n> > with roughly the same number of rules, though the directory rules don't\n> > have to depend on one another.\n> >\n> > It also means that these \"mkdir -p\" may race with each other, though in\n> > general I'd hope that most \"mkdir\" implements could handle this.\n> >\n> > Something like this works, I think:\n> \n> Hmph, what I actually meant was to make sure that the recipe to\n> create the files to have \"mkdir -p $(basename $@)\" in front, instead\n> of having \"we need to prepare the containing directory in order to\n> have a file there\" in the makefile.\n\nYeah, I agree that's simpler, and is what Ævar showed. But it is slower,\nbecause we run a bunch of redundant \"mkdir -p\" calls, one per file.\n\n-Peff\n"},{"id":"439916","messageId":"211028.86o879vvtp.gmgdl@evledraar.gmail.com","threadId":"56786","inReplyTo":"YXrYh9aPA8csq+UQ@coredump.intra.peff.net","subject":"Re: [PATCH] Documentation/Makefile: fix lint-docs mkdir dependency","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-28T18:30:13Z","receivedAt":"2021-10-28T18:38:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Oct 28 2021, Jeff King wrote:\n\n> On Thu, Oct 28, 2021 at 09:45:14AM -0700, Junio C Hamano wrote:\n>\n>> Jeff King <peff@peff.net> writes:\n>> \n>> > There's an in-between, I'd think, where the many \"foo/bar/baz/$@\"\n>> > targets have an order-dependency on \"foo/bar/baz\", and that single rule\n>> > uses \"mkdir -p\" to create all of the directories.\n>> >\n>> > It doesn't buy us much simplification in this case, though, because\n>> > various rules independently depend on .build/gitlink/lint-docs/howto,\n>> > .built/gitlink/lint-docs, and .build/gitlink, etc. So we still end up\n>> > with roughly the same number of rules, though the directory rules don't\n>> > have to depend on one another.\n>> >\n>> > It also means that these \"mkdir -p\" may race with each other, though in\n>> > general I'd hope that most \"mkdir\" implements could handle this.\n>> >\n>> > Something like this works, I think:\n>> \n>> Hmph, what I actually meant was to make sure that the recipe to\n>> create the files to have \"mkdir -p $(basename $@)\" in front, instead\n>> of having \"we need to prepare the containing directory in order to\n>> have a file there\" in the makefile.\n>\n> Yeah, I agree that's simpler, and is what Ævar showed. But it is slower,\n> because we run a bunch of redundant \"mkdir -p\" calls, one per file.\n\nHere's a method that's both less verbose in lines & also faster, but\nmaybe too clever a use of GNU make features, on top of \"next\".\n\nIt relies on $(wildcard) returning an empty list on a non-existing\nfilename, and then on $(if) to either expand to \"mkdir -p $(@D)\", or\nnothing, and (perhaps in an ugly way?) piggy-backs on an existing $@\nrule, so one rule has two $(QUIET_*) uses.\n\nWith:\n\n    HEAD~2 = next\n    HEAD~1 = unconditional mkdir -p, upthread <211028.861r45y3pt.gmgdl@evledraar.gmail.com>\n    HEAD = the below patch\n\nWe get these results, i.e. it's also faster:\n    \n    $ hyperfine --warmup 5 -m 30 -L s \",~,~2\" -p 'git checkout HEAD{s} -- Makefile; rm -rf .build' 'make -j8 lint-docs R={s}' -c 'git checkout HEAD -- Makefile'\n    Benchmark #1: make -j8 lint-docs R=\n      Time (mean ± σ):     628.5 ms ±  43.2 ms    [User: 2.385 s, System: 0.445 s]\n      Range (min … max):   605.6 ms … 787.7 ms    30 runs\n    Benchmark #2: make -j8 lint-docs R=~\n      Time (mean ± σ):     770.2 ms ±  12.7 ms    [User: 3.446 s, System: 0.666 s]\n      Range (min … max):   756.3 ms … 831.4 ms    30 runs\n    Benchmark #3: make -j8 lint-docs R=~2\n      Time (mean ± σ):     696.9 ms ±   3.5 ms    [User: 2.967 s, System: 0.600 s]\n      Range (min … max):   691.2 ms … 706.2 ms    30 runs\n    Summary\n      'make -j8 lint-docs R=' ran\n        1.11 ± 0.08 times faster than 'make -j8 lint-docs R=~2'\n        1.23 ± 0.09 times faster than 'make -j8 lint-docs R=~'\n\nThe one negative thing is that we'll return an inconsistent set of\n\"mkdir\" lines, since it's racy, but here we're using the race to our\nbenefit. It doesn't matter for the end result if we e.g. created a more\nnested directory first, or needed two \"mkdir -p\" calls because we did a\nshallower one first.\n\nDo you think it's worth submitting that as a non-throwaway?\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex ed656db2ae9..99c2f9d02cf 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -234,6 +234,7 @@ ifndef V\n \tQUIET_DBLATEX\t= @echo '   ' DBLATEX $@;\n \tQUIET_XSLTPROC\t= @echo '   ' XSLTPROC $@;\n \tQUIET_GEN\t= @echo '   ' GEN $@;\n+\tQUIET_MKDIR_P\t= @echo '   ' MKDIR -p $(@D);\n \tQUIET_STDERR\t= 2> /dev/null\n \tQUIET_SUBDIR0\t= +@subdir=\n \tQUIET_SUBDIR1\t= ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \\\n@@ -463,25 +464,10 @@ quick-install-html: require-htmlrepo\n print-man1:\n \t@for i in $(MAN1_TXT); do echo $$i; done\n \n-## Lint: Common\n-.build:\n-\t$(QUIET)mkdir $@\n-.build/lint-docs: | .build\n-\t$(QUIET)mkdir $@\n-\n-## Lint: gitlink\n-.build/lint-docs/gitlink: | .build/lint-docs\n-\t$(QUIET)mkdir $@\n-.build/lint-docs/gitlink/howto: | .build/lint-docs/gitlink\n-\t$(QUIET)mkdir $@\n-.build/lint-docs/gitlink/config: | .build/lint-docs/gitlink\n-\t$(QUIET)mkdir $@\n LINT_DOCS_GITLINK = $(patsubst %.txt,.build/lint-docs/gitlink/%.ok,$(HOWTO_TXT) $(DOC_DEP_TXT))\n-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink\n-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/howto\n-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/config\n $(LINT_DOCS_GITLINK): lint-gitlink.perl\n $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt\n+\t$(if $(wildcard $(@D)),,$(QUIET_MKDIR_P)$(shell mkdir -p $(@D)))\n \t$(QUIET_LINT_GITLINK)$(PERL_PATH) lint-gitlink.perl \\\n \t\t$< \\\n \t\t$(HOWTO_TXT) $(DOC_DEP_TXT) \\\n@@ -492,23 +478,19 @@ $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt\n lint-docs-gitlink: $(LINT_DOCS_GITLINK)\n \n ## Lint: man-end-blurb\n-.build/lint-docs/man-end-blurb: | .build/lint-docs\n-\t$(QUIET)mkdir $@\n LINT_DOCS_MAN_END_BLURB = $(patsubst %.txt,.build/lint-docs/man-end-blurb/%.ok,$(MAN_TXT))\n-$(LINT_DOCS_MAN_END_BLURB): | .build/lint-docs/man-end-blurb\n $(LINT_DOCS_MAN_END_BLURB): lint-man-end-blurb.perl\n $(LINT_DOCS_MAN_END_BLURB): .build/lint-docs/man-end-blurb/%.ok: %.txt\n-\t$(QUIET_LINT_MANEND)$(PERL_PATH) lint-man-end-blurb.perl $< >$@\n+\t$(if $(wildcard $(@D)),,$(QUIET_MKDIR_P)$(shell mkdir -p $(@D)))\n+\t$(QUIET_LINT_MANEND)$($(PERL_PATH) lint-man-end-blurb.perl $< >$@\n .PHONY: lint-docs-man-end-blurb\n lint-docs-man-end-blurb: $(LINT_DOCS_MAN_END_BLURB)\n \n ## Lint: man-section-order\n-.build/lint-docs/man-section-order: | .build/lint-docs\n-\t$(QUIET)mkdir $@\n LINT_DOCS_MAN_SECTION_ORDER = $(patsubst %.txt,.build/lint-docs/man-section-order/%.ok,$(MAN_TXT))\n-$(LINT_DOCS_MAN_SECTION_ORDER): | .build/lint-docs/man-section-order\n $(LINT_DOCS_MAN_SECTION_ORDER): lint-man-section-order.perl\n $(LINT_DOCS_MAN_SECTION_ORDER): .build/lint-docs/man-section-order/%.ok: %.txt\n+\t$(if $(wildcard $(@D)),,$(QUIET_MKDIR_P)$(shell mkdir -p $(@D)))\n \t$(QUIET_LINT_MANSEC)$(PERL_PATH) lint-man-section-order.perl $< >$@\n .PHONY: lint-docs-man-section-order\n lint-docs-man-section-order: $(LINT_DOCS_MAN_SECTION_ORDER)\n"}]}