All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>, git@vger.kernel.org
Subject: Re: [PATCH] Documentation/Makefile: fix lint-docs mkdir dependency
Date: Thu, 28 Oct 2021 09:48:51 +0200	[thread overview]
Message-ID: <211028.861r45y3pt.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <xmqqlf2et3r3.fsf@gitster.g>


On Wed, Oct 27 2021, Junio C Hamano wrote:

> 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)
 

  reply	other threads:[~2021-10-28  8:05 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-26  7:31 [PATCH] Documentation/Makefile: fix lint-docs mkdir dependency Jeff King
2021-10-26 10:05 ` Ævar Arnfjörð Bjarmason
2021-10-26 21:18   ` Jeff King
2021-10-28  0:03 ` Junio C Hamano
2021-10-28  7:48   ` Ævar Arnfjörð Bjarmason [this message]
2021-10-28 14:35     ` Jeff King
2021-10-28 16:45       ` Junio C Hamano
2021-10-28 17:06         ` Jeff King
2021-10-28 18:30           ` Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=211028.861r45y3pt.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.