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

[Jeff King <peff@peff.net>]

On Thu, Oct 28, 2021 at 09:48:51AM +0200, Ævar Arnfjörð Bjarmason wrote:

> > I wonder if we can somehow avoid this unwieldy chain of commands,
> > perhaps with using "mkdir -p" somewhere, or make the lint scripts
> > create the necessary leading paths.  From the looks of the tail end
> > of Documentation/Makefile, the latter may be the cleaner solution.
> 
> Simplest would be to simply do the "mkdir -p" unconditionally, which we
> do in some other places. The diff below on top of next would do that.
> 
> I didn't do it because it slows things down quite a bit. Here HEAD is
> the diff below:

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:

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 911b6bf79c..a70e418af6 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -463,19 +463,13 @@ 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
-	$(QUIET)mkdir $@
-.build/lint-docs/gitlink/config: | .build/lint-docs
-	$(QUIET)mkdir $@
+.build/lint-docs/gitlink:
+	$(QUIET)mkdir -p $@
+.build/lint-docs/gitlink/howto:
+	$(QUIET)mkdir -p $@
+.build/lint-docs/gitlink/config:
+	$(QUIET)mkdir -p $@
 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
@@ -492,8 +486,8 @@ $(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 $@
+.build/lint-docs/man-end-blurb:
+	$(QUIET)mkdir -p $@
 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
@@ -503,8 +497,8 @@ $(LINT_DOCS_MAN_END_BLURB): .build/lint-docs/man-end-blurb/%.ok: %.txt
 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 $@
+.build/lint-docs/man-section-order:
+	$(QUIET)mkdir -p $@
 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

 We could technically even drop the .build/lint-docs/gitlink rule,
 because it's a parent of other directories built by the same rule. But
 that's a bit too clever and magical for my tastes.

 All that said, I'm not that unhappy with the current state, and I think
 with my patch it should be correct / robust.

> You can do this with make macros via $(eval) calling a $(foreach) loop,
> i.e. just generate the boilerplate we have now. For this case I thought
> it wasn't worth it, but figured I could eventually loop back to it
> if/when we use a nested structure inside a ".build directory more
> widely.

Yeah, I think for the scope of the problem here (remembering that "mkdir
foo/bar" needs to depend on "mkdir foo") that a macro would probably
just confuse things. IMHO the more subtle maintenance trap is that
LINT_DOCS_GITLINK needs to remember to depend on the "config/" and
"howto/" directories, because that's where we keep source files. It
would be easy to add a source file to DOC_DEP_TXT that mentions a new
subdirectory, but not realize it needs an extra rule.

If the macro magic went as far as actually mapping all of
LINT_DOCS_GITLINK into their .build/ dirname counterparts, and then
automatically generated the right rules, that would make it truly
turnkey. It would probably also be a pretty gross macro, but maybe worth
it if nobody ever needed to touch it. :)

At any rate, I don't think there's any urgency on that.

-Peff