| Message ID | YXeu4Hl2cmIPqobd@coredump.intra.peff.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 04f0be2a8baae2c2ffe4d3735c623b09783ab74c |
| Headers | show |
| Series | Documentation/Makefile: fix lint-docs mkdir dependency | expand |
On Tue, Oct 26 2021, Jeff King wrote: > 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". > > This usually works out, because the LINT_DOCS_GITLINK targets which > depend on "gitlink/howto" also depend on just "gitlink", so the > directory gets created anyway. But since we haven't given make an > explicit ordering, things can racily happen out of order. > > If you stick a "sleep 1" in the rule to build "gitlink" like this: > > ## Lint: gitlink > .build/lint-docs/gitlink: | .build/lint-docs > - $(QUIET)mkdir $@ > + $(QUIET)sleep 1 && mkdir $@ > > then "make clean; make lint-docs" will fail reliably. Or you can see it > as-is just by building the directory in isolation: > > $ make clean > [...] > $ make .build/lint-docs/gitlink/howto > GEN mergetools-list.made > GEN cmd-list.made > GEN doc.dep > SUBDIR ../ > make[1]: 'GIT-VERSION-FILE' is up to date. > SUBDIR ../ > make[1]: 'GIT-VERSION-FILE' is up to date. > mkdir: cannot create directory ‘.build/lint-docs/gitlink/howto’: No such file or directory > make: *** [Makefile:476: .build/lint-docs/gitlink/howto] Error 1 > > The fix is easy: we just need to depend on the correct parent directory. > > Signed-off-by: Jeff King <peff@peff.net> > --- > The problem starts in ab/fix-make-lint-docs, which is in master. > > I wasn't able to trigger the problem locally even with running 'make > clean; make lint-docs' in a loop, but I did see it in the wild in a CI > documentation job: > > https://github.com/peff/git/runs/4005766641?check_suite_focus=true#step:4:60 > > It would have been a lot easier to diagnose from the CI output if the > mkdir lines weren't silent. I.e., if we had a $(QUIET_MKDIR) which > printed "MKDIR $@" rather than nothing at all. > > Documentation/Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/Makefile b/Documentation/Makefile > index 911b6bf79c..ed656db2ae 100644 > --- a/Documentation/Makefile > +++ b/Documentation/Makefile > @@ -472,9 +472,9 @@ print-man1: > ## Lint: gitlink > .build/lint-docs/gitlink: | .build/lint-docs > $(QUIET)mkdir $@ > -.build/lint-docs/gitlink/howto: | .build/lint-docs > +.build/lint-docs/gitlink/howto: | .build/lint-docs/gitlink > $(QUIET)mkdir $@ > -.build/lint-docs/gitlink/config: | .build/lint-docs > +.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 Thanks a lot for fixing that bug, and sorry for not spotting it. This fix LGTM and is obviously correct. I'll try to do something about the $(QUIET*) as a follow-up, I was trying to find the right balance between over-verbosity & "tracing".
On Tue, Oct 26, 2021 at 12:05:40PM +0200, Ævar Arnfjörð Bjarmason wrote: > I'll try to do something about the $(QUIET*) as a follow-up, I was > trying to find the right balance between over-verbosity & "tracing". Yeah, I wondered if it would end up super-verbose. But I tried it out and IMHO it looks just fine to print mkdir lines. In the initial build, "make" realizes we only need to run each rule once, and in subsequent builds it doesn't run them at all. -Peff
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.
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)
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
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. > ... > At any rate, I don't think there's any urgency on that. Sure. I think I've picked up the one at the start of this thread already, so we should be good. Thanks.
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. -Peff
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)
diff --git a/Documentation/Makefile b/Documentation/Makefile index 911b6bf79c..ed656db2ae 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -472,9 +472,9 @@ print-man1: ## Lint: gitlink .build/lint-docs/gitlink: | .build/lint-docs $(QUIET)mkdir $@ -.build/lint-docs/gitlink/howto: | .build/lint-docs +.build/lint-docs/gitlink/howto: | .build/lint-docs/gitlink $(QUIET)mkdir $@ -.build/lint-docs/gitlink/config: | .build/lint-docs +.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
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". This usually works out, because the LINT_DOCS_GITLINK targets which depend on "gitlink/howto" also depend on just "gitlink", so the directory gets created anyway. But since we haven't given make an explicit ordering, things can racily happen out of order. If you stick a "sleep 1" in the rule to build "gitlink" like this: ## Lint: gitlink .build/lint-docs/gitlink: | .build/lint-docs - $(QUIET)mkdir $@ + $(QUIET)sleep 1 && mkdir $@ then "make clean; make lint-docs" will fail reliably. Or you can see it as-is just by building the directory in isolation: $ make clean [...] $ make .build/lint-docs/gitlink/howto GEN mergetools-list.made GEN cmd-list.made GEN doc.dep SUBDIR ../ make[1]: 'GIT-VERSION-FILE' is up to date. SUBDIR ../ make[1]: 'GIT-VERSION-FILE' is up to date. mkdir: cannot create directory ‘.build/lint-docs/gitlink/howto’: No such file or directory make: *** [Makefile:476: .build/lint-docs/gitlink/howto] Error 1 The fix is easy: we just need to depend on the correct parent directory. Signed-off-by: Jeff King <peff@peff.net> --- The problem starts in ab/fix-make-lint-docs, which is in master. I wasn't able to trigger the problem locally even with running 'make clean; make lint-docs' in a loop, but I did see it in the wild in a CI documentation job: https://github.com/peff/git/runs/4005766641?check_suite_focus=true#step:4:60 It would have been a lot easier to diagnose from the CI output if the mkdir lines weren't silent. I.e., if we had a $(QUIET_MKDIR) which printed "MKDIR $@" rather than nothing at all. Documentation/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)