Re: [PATCH] Documentation/Makefile: fix lint-docs mkdir dependency

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Oct 28 2021, Jeff King wrote:

> On Thu, Oct 28, 2021 at 09:45:14AM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > There's an in-between, I'd think, where the many "foo/bar/baz/$@"
>> > targets have an order-dependency on "foo/bar/baz", and that single rule
>> > uses "mkdir -p" to create all of the directories.
>> >
>> > It doesn't buy us much simplification in this case, though, because
>> > various rules independently depend on .build/gitlink/lint-docs/howto,
>> > .built/gitlink/lint-docs, and .build/gitlink, etc. So we still end up
>> > with roughly the same number of rules, though the directory rules don't
>> > have to depend on one another.
>> >
>> > It also means that these "mkdir -p" may race with each other, though in
>> > general I'd hope that most "mkdir" implements could handle this.
>> >
>> > Something like this works, I think:
>> 
>> Hmph, what I actually meant was to make sure that the recipe to
>> create the files to have "mkdir -p $(basename $@)" in front, instead
>> of having "we need to prepare the containing directory in order to
>> have a file there" in the makefile.
>
> Yeah, I agree that's simpler, and is what Ævar showed. But it is slower,
> because we run a bunch of redundant "mkdir -p" calls, one per file.

Here's a method that's both less verbose in lines & also faster, but
maybe too clever a use of GNU make features, on top of "next".

It relies on $(wildcard) returning an empty list on a non-existing
filename, and then on $(if) to either expand to "mkdir -p $(@D)", or
nothing, and (perhaps in an ugly way?) piggy-backs on an existing $@
rule, so one rule has two $(QUIET_*) uses.

With:

    HEAD~2 = next
    HEAD~1 = unconditional mkdir -p, upthread <211028.861r45y3pt.gmgdl@evledraar.gmail.com>
    HEAD = the below patch

We get these results, i.e. it's also faster:
    
    $ 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'
    Benchmark #1: make -j8 lint-docs R=
      Time (mean ± σ):     628.5 ms ±  43.2 ms    [User: 2.385 s, System: 0.445 s]
      Range (min … max):   605.6 ms … 787.7 ms    30 runs
    Benchmark #2: make -j8 lint-docs R=~
      Time (mean ± σ):     770.2 ms ±  12.7 ms    [User: 3.446 s, System: 0.666 s]
      Range (min … max):   756.3 ms … 831.4 ms    30 runs
    Benchmark #3: make -j8 lint-docs R=~2
      Time (mean ± σ):     696.9 ms ±   3.5 ms    [User: 2.967 s, System: 0.600 s]
      Range (min … max):   691.2 ms … 706.2 ms    30 runs
    Summary
      'make -j8 lint-docs R=' ran
        1.11 ± 0.08 times faster than 'make -j8 lint-docs R=~2'
        1.23 ± 0.09 times faster than 'make -j8 lint-docs R=~'

The one negative thing is that we'll return an inconsistent set of
"mkdir" lines, since it's racy, but here we're using the race to our
benefit. It doesn't matter for the end result if we e.g. created a more
nested directory first, or needed two "mkdir -p" calls because we did a
shallower one first.

Do you think it's worth submitting that as a non-throwaway?

diff --git a/Documentation/Makefile b/Documentation/Makefile
index ed656db2ae9..99c2f9d02cf 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -234,6 +234,7 @@ ifndef V
 	QUIET_DBLATEX	= @echo '   ' DBLATEX $@;
 	QUIET_XSLTPROC	= @echo '   ' XSLTPROC $@;
 	QUIET_GEN	= @echo '   ' GEN $@;
+	QUIET_MKDIR_P	= @echo '   ' MKDIR -p $(@D);
 	QUIET_STDERR	= 2> /dev/null
 	QUIET_SUBDIR0	= +@subdir=
 	QUIET_SUBDIR1	= ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
@@ -463,25 +464,10 @@ quick-install-html: require-htmlrepo
 print-man1:
 	@for i in $(MAN1_TXT); do echo $$i; done
 
-## Lint: Common
-.build:
-	$(QUIET)mkdir $@
-.build/lint-docs: | .build
-	$(QUIET)mkdir $@
-
-## Lint: gitlink
-.build/lint-docs/gitlink: | .build/lint-docs
-	$(QUIET)mkdir $@
-.build/lint-docs/gitlink/howto: | .build/lint-docs/gitlink
-	$(QUIET)mkdir $@
-.build/lint-docs/gitlink/config: | .build/lint-docs/gitlink
-	$(QUIET)mkdir $@
 LINT_DOCS_GITLINK = $(patsubst %.txt,.build/lint-docs/gitlink/%.ok,$(HOWTO_TXT) $(DOC_DEP_TXT))
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/howto
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/config
 $(LINT_DOCS_GITLINK): lint-gitlink.perl
 $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt
+	$(if $(wildcard $(@D)),,$(QUIET_MKDIR_P)$(shell mkdir -p $(@D)))
 	$(QUIET_LINT_GITLINK)$(PERL_PATH) lint-gitlink.perl \
 		$< \
 		$(HOWTO_TXT) $(DOC_DEP_TXT) \
@@ -492,23 +478,19 @@ $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt
 lint-docs-gitlink: $(LINT_DOCS_GITLINK)
 
 ## Lint: man-end-blurb
-.build/lint-docs/man-end-blurb: | .build/lint-docs
-	$(QUIET)mkdir $@
 LINT_DOCS_MAN_END_BLURB = $(patsubst %.txt,.build/lint-docs/man-end-blurb/%.ok,$(MAN_TXT))
-$(LINT_DOCS_MAN_END_BLURB): | .build/lint-docs/man-end-blurb
 $(LINT_DOCS_MAN_END_BLURB): lint-man-end-blurb.perl
 $(LINT_DOCS_MAN_END_BLURB): .build/lint-docs/man-end-blurb/%.ok: %.txt
-	$(QUIET_LINT_MANEND)$(PERL_PATH) lint-man-end-blurb.perl $< >$@
+	$(if $(wildcard $(@D)),,$(QUIET_MKDIR_P)$(shell mkdir -p $(@D)))
+	$(QUIET_LINT_MANEND)$($(PERL_PATH) lint-man-end-blurb.perl $< >$@
 .PHONY: lint-docs-man-end-blurb
 lint-docs-man-end-blurb: $(LINT_DOCS_MAN_END_BLURB)
 
 ## Lint: man-section-order
-.build/lint-docs/man-section-order: | .build/lint-docs
-	$(QUIET)mkdir $@
 LINT_DOCS_MAN_SECTION_ORDER = $(patsubst %.txt,.build/lint-docs/man-section-order/%.ok,$(MAN_TXT))
-$(LINT_DOCS_MAN_SECTION_ORDER): | .build/lint-docs/man-section-order
 $(LINT_DOCS_MAN_SECTION_ORDER): lint-man-section-order.perl
 $(LINT_DOCS_MAN_SECTION_ORDER): .build/lint-docs/man-section-order/%.ok: %.txt
+	$(if $(wildcard $(@D)),,$(QUIET_MKDIR_P)$(shell mkdir -p $(@D)))
 	$(QUIET_LINT_MANSEC)$(PERL_PATH) lint-man-section-order.perl $< >$@
 .PHONY: lint-docs-man-section-order
 lint-docs-man-section-order: $(LINT_DOCS_MAN_SECTION_ORDER)