Re: [PATCH] Documentation/Makefile: fix lint-docs mkdir dependency
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Oct 28, 2021, 07:48 UTC
- Message-ID
- <211028.861r45y3pt.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <xmqqlf2et3r3.fsf@gitster.g>
On Wed, Oct 27 2021, Junio C Hamano wrote:
Show 22 quoted lines
> Jeff King <peff@peff.net> writes: > >> Since 8650c6298c (doc lint: make "lint-docs" non-.PHONY, 2021-10-15), we >> put the output for gitlink linter into .build/lint-docs/gitlink. There >> are order-only dependencies to create the sequence of subdirs like: >> >> .build/lint-docs: | .build >> $(QUIET)mkdir $@ >> .build/lint-docs/gitlink: | .build/lint-docs >> $(QUIET)mkdir $@ >> >> where each level has to depend on the prior one (since the parent >> directory must exist for us to create something inside it). But the >> "howto" and "config" subdirectories of gitlink have the wrong >> dependency; they depend on "lint-docs", not "lint-docs/gitlink". > > Thanks. > > 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:
$ 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'
Benchmark #1: make -j8 lint-docs R=
Time (mean ± σ): 709.7 ms ± 25.7 ms [User: 3.584 s, System: 0.696 s]
Range (min … max): 691.4 ms … 833.1 ms 30 runs
Benchmark #2: make -j8 lint-docs R=~
Time (mean ± σ): 647.3 ms ± 5.5 ms [User: 3.081 s, System: 0.635 s]
Range (min … max): 638.4 ms … 670.6 ms 30 runs
Summary
'make -j8 lint-docs R=~' ran
1.10 ± 0.04 times faster than 'make -j8 lint-docs R='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.
diff --git a/Documentation/Makefile b/Documentation/Makefile index ed656db2ae9..8a185e89e55 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -463,26 +463,11 @@ 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 - $(QUIET_LINT_GITLINK)$(PERL_PATH) lint-gitlink.perl \ + $(QUIET_LINT_GITLINK)mkdir -p $(@D) && \ + $(PERL_PATH) lint-gitlink.perl \ $< \ $(HOWTO_TXT) $(DOC_DEP_TXT) \ --section=1 $(MAN1_TXT) \ @@ -492,24 +477,20 @@ $(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 $< >$@ + $(QUIET_LINT_MANEND)mkdir -p $(@D) && \ + $(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 - $(QUIET_LINT_MANSEC)$(PERL_PATH) lint-man-section-order.perl $< >$@ + $(QUIET_LINT_MANSEC)mkdir -p $(@D) && \ + $(PERL_PATH) lint-man-section-order.perl $< >$@ .PHONY: lint-docs-man-section-order lint-docs-man-section-order: $(LINT_DOCS_MAN_SECTION_ORDER)