threads / patch / 56720

patch, 4 partsdoc lint: fix recent regression, make non-.PHONY

Subject: [PATCH 0/4] doc lint: fix recent regression, make non-.PHONY

## tl;dr

8 messages between Oct 15, 2021 and Oct 16, 2021. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Ævar Arnfjörð Bjarmason· Oct 15, 2021, 12:39 UTC · lore

Recent patches of mine broke the "check-docs" target by using ;-chaining instead of &&-chaining, d'oh! That's currently hiding a breakage in "seen"[1], sorry!

In 1/4 that's fixed, 2/4 emits errors to STDERR instead of STDOUT, 3/4 speeds up the target by ~2x by skipping unnecessary work, and 4/4 makes it non-.PHONY.

The technique in 4/4 of exhaustively generating a dependency graph per-file is something I'm also using in some yet-to-be-submitted series of other Makefile fixes, that similarly speed things up.

1. https://lore.kernel.org/git/87lf2ueafl.fsf@evledraar.gmail.com/
Ævar Arnfjörð Bjarmason (4):
  doc lint: fix error-hiding regression
  doc lint: emit errors on STDERR
  doc build: speed up "make lint-docs"
  doc lint: make "lint-docs" non-.PHONY
 Documentation/.gitignore                  |  1 +
 Documentation/Makefile                    | 67 ++++++++++++++++++++---
 Documentation/lint-gitlink.perl           | 10 ++--
 Documentation/lint-man-end-blurb.perl     |  2 +-
 Documentation/lint-man-section-order.perl |  2 +-
 5 files changed, 69 insertions(+), 13 deletions(-)
-- 
2.33.1.1338.g20da966911a
Ævar Arnfjörð Bjarmason· Oct 15, 2021, 12:39 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 1/4] doc lint: fix error-hiding regression

Fix the broken "make lint-docs" (or "make check-docs" at the top-level) target, which has been broken since my cafd9828e89 (doc lint: lint and fix missing "GIT" end sections, 2021-04-09).

The CI for "seen" is emitting an error about a broken gitlink, but due to there being 3x scripts chained via ";" instead of "&&" we're not carrying forward the non-zero exit code.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Show changes to Documentation/Makefile +3 −3
diff --git a/Documentation/Makefile b/Documentation/Makefile
index f5605b7767f..78324934d9f 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -461,9 +461,9 @@ lint-docs::
 		$(HOWTO_TXT) $(DOC_DEP_TXT) \
 		--section=1 $(MAN1_TXT) \
 		--section=5 $(MAN5_TXT) \
-		--section=7 $(MAN7_TXT); \
-	$(PERL_PATH) lint-man-end-blurb.perl $(MAN_TXT); \
-	$(PERL_PATH) lint-man-section-order.perl $(MAN_TXT);
+		--section=7 $(MAN7_TXT) && \
+	$(PERL_PATH) lint-man-end-blurb.perl $(MAN_TXT) && \
+	$(PERL_PATH) lint-man-section-order.perl $(MAN_TXT)
 
 ifeq ($(wildcard po/Makefile),po/Makefile)
 doc-l10n install-l10n::
-- 
2.33.1.1338.g20da966911a
Ævar Arnfjörð Bjarmason· Oct 15, 2021, 12:39 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 2/4] doc lint: emit errors on STDERR

Have all of the scripts invoked by "make check-docs" emit their output on STDERR. This does not currently matter due to the way we're invoking them, but will in a subsequent change. It's a good idea to do this in any case for consistency with other tools we invoke.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/lint-gitlink.perl           | 4 ++--
 Documentation/lint-man-end-blurb.perl     | 2 +-
 Documentation/lint-man-section-order.perl | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)
Show changes to 3 files +4 −4

Documentation/lint-gitlink.perl, Documentation/lint-man-end-blurb.perl, Documentation/lint-man-section-order.perl

diff --git a/Documentation/lint-gitlink.perl b/Documentation/lint-gitlink.perl
index b22a367844a..076701ccfac 100755
--- a/Documentation/lint-gitlink.perl
+++ b/Documentation/lint-gitlink.perl
@@ -30,8 +30,8 @@ sub report {
 	my ($pos, $line, $target, $msg) = @_;
 	substr($line, $pos) = "' <-- HERE";
 	$line =~ s/^\s+//;
-	print "$ARGV:$.: error: $target: $msg, shown with 'HERE' below:\n";
-	print "$ARGV:$.:\t'$line\n";
+	print STDERR "$ARGV:$.: error: $target: $msg, shown with 'HERE' below:\n";
+	print STDERR "$ARGV:$.:\t'$line\n";
 	$exit_code = 1;
 }
 
diff --git a/Documentation/lint-man-end-blurb.perl b/Documentation/lint-man-end-blurb.perl
index d69312e5db5..6bdb13ad9fd 100755
--- a/Documentation/lint-man-end-blurb.perl
+++ b/Documentation/lint-man-end-blurb.perl
@@ -6,7 +6,7 @@
 my $exit_code = 0;
 sub report {
 	my ($target, $msg) = @_;
-	print "error: $target: $msg\n";
+	print STDERR "error: $target: $msg\n";
 	$exit_code = 1;
 }
 
diff --git a/Documentation/lint-man-section-order.perl b/Documentation/lint-man-section-order.perl
index b05f9156dd9..425377dfeb7 100755
--- a/Documentation/lint-man-section-order.perl
+++ b/Documentation/lint-man-section-order.perl
@@ -46,7 +46,7 @@
 my $exit_code = 0;
 sub report {
 	my ($msg) = @_;
-	print "$ARGV:$.: $msg\n";
+	print STDERR "$ARGV:$.: $msg\n";
 	$exit_code = 1;
 }
 
-- 
2.33.1.1338.g20da966911a
Ævar Arnfjörð Bjarmason· Oct 15, 2021, 12:39 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 4/4] doc lint: make "lint-docs" non-.PHONY

Speed up the "lint-docs" target by making it non-.PHONY. Similar to my c234e8a0ecf (Makefile: make the "sparse" target non-.PHONY, 2021-09-23). We'll now create empty files corresponding to a dependency graph for each of these lint scripts.

This speeds things up a bit[1], and makes the output correspond to any in-tree changes we have:

    $ touch git-add.txt; make lint-docs; make lint-docs
        GEN cmd-list.made
        GEN doc.dep
        LINT GITLINK git-add.txt
        LINT MAN END git-add.txt
        LINT MAN SEC git-add.txt
    make: Nothing to be done for 'lint-docs'.

As with the "sparse" target changes this has a hard dependency on the use of ".DELETE_ON_ERROR" in the Makefile, added here in db10fc6c09f (doc: simplify Makefile using .DELETE_ON_ERROR, 2021-05-21). This method also depends on the output for us emitting any errors on STDERR (fixed in a preceding commit), as well us these scripts exiting with non-zero on any errors (which they were already doing).

1.
$ git show HEAD~:Documentation/Makefile >Makefile.old
$ hyperfine --warmup 2 -L f ",.old" 'make -j1 -f Makefile{f} lint-docs'
Benchmark #1: make -j1 -f Makefile lint-docs
  Time (mean ± σ):      60.8 ms ±   1.4 ms    [User: 58.7 ms, System: 2.5 ms]
  Range (min … max):    58.9 ms …  64.0 ms    48 runs
Benchmark #2: make -j1 -f Makefile.old lint-docs
  Time (mean ± σ):      84.0 ms ±   1.5 ms    [User: 78.6 ms, System: 5.7 ms]
  Range (min … max):    81.8 ms …  87.8 ms    35 runs
Summary
  'make -j1 -f Makefile lint-docs' ran
    1.38 ± 0.04 times faster than 'make -j1 -f Makefile.old lint-docs'
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/.gitignore        |  1 +
 Documentation/Makefile          | 65 ++++++++++++++++++++++++++++++---
 Documentation/lint-gitlink.perl |  6 ++-
 3 files changed, 64 insertions(+), 8 deletions(-)
Show changes to 3 files +64 −8

Documentation/.gitignore, Documentation/Makefile, Documentation/lint-gitlink.perl

diff --git a/Documentation/.gitignore b/Documentation/.gitignore
index 9022d483554..1c3771e7d72 100644
--- a/Documentation/.gitignore
+++ b/Documentation/.gitignore
@@ -14,4 +14,5 @@ manpage-base-url.xsl
 SubmittingPatches.txt
 tmp-doc-diff/
 GIT-ASCIIDOCFLAGS
+/.build/
 /GIT-EXCLUDED-PROGRAMS
diff --git a/Documentation/Makefile b/Documentation/Makefile
index e8e54e053a6..46c91f32566 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -225,6 +225,7 @@ endif
 
 ifneq ($(findstring $(MAKEFLAGS),s),s)
 ifndef V
+	QUIET		= @
 	QUIET_ASCIIDOC	= @echo '   ' ASCIIDOC $@;
 	QUIET_XMLTO	= @echo '   ' XMLTO $@;
 	QUIET_DB2TEXI	= @echo '   ' DB2TEXI $@;
@@ -232,11 +233,15 @@ ifndef V
 	QUIET_DBLATEX	= @echo '   ' DBLATEX $@;
 	QUIET_XSLTPROC	= @echo '   ' XSLTPROC $@;
 	QUIET_GEN	= @echo '   ' GEN $@;
-	QUIET_LINT	= @echo '   ' LINT $@;
 	QUIET_STDERR	= 2> /dev/null
 	QUIET_SUBDIR0	= +@subdir=
 	QUIET_SUBDIR1	= ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
 			  $(MAKE) $(PRINT_DIR) -C $$subdir
+
+	QUIET_LINT_GITLINK	= @echo '   ' LINT GITLINK $<;
+	QUIET_LINT_MANSEC	= @echo '   ' LINT MAN SEC $<;
+	QUIET_LINT_MANEND	= @echo '   ' LINT MAN END $<;
+
 	export V
 endif
 endif
@@ -343,6 +348,7 @@ GIT-ASCIIDOCFLAGS: FORCE
             fi
 
 clean:
+	$(RM) -rf .build/
 	$(RM) *.xml *.xml+ *.html *.html+ *.1 *.5 *.7
 	$(RM) *.texi *.texi+ *.texi++ git.info gitman.info
 	$(RM) *.pdf
@@ -456,14 +462,61 @@ quick-install-html: require-htmlrepo
 print-man1:
 	@for i in $(MAN1_TXT); do echo $$i; done
 
-lint-docs::
-	$(QUIET_LINT)$(PERL_PATH) lint-gitlink.perl \
+## 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 $@
+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 \
+		$< \
 		$(HOWTO_TXT) $(DOC_DEP_TXT) \
 		--section=1 $(MAN1_TXT) \
 		--section=5 $(MAN5_TXT) \
-		--section=7 $(MAN7_TXT) && \
-	$(PERL_PATH) lint-man-end-blurb.perl $(MAN_TXT) && \
-	$(PERL_PATH) lint-man-section-order.perl $(MAN_TXT)
+		--section=7 $(MAN7_TXT) >$@
+.PHONY: lint-docs-gitlink
+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 $< >$@
+.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 $< >$@
+.PHONY: lint-docs-man-section-order
+lint-docs-man-section-order: $(LINT_DOCS_MAN_SECTION_ORDER)
+
+## Lint: list of targets above
+.PHONY: lint-docs
+lint-docs: lint-docs-gitlink
+lint-docs: lint-docs-man-end-blurb
+lint-docs: lint-docs-man-section-order
 
 ifeq ($(wildcard po/Makefile),po/Makefile)
 doc-l10n install-l10n::
diff --git a/Documentation/lint-gitlink.perl b/Documentation/lint-gitlink.perl
index 076701ccfac..1c61dd9512b 100755
--- a/Documentation/lint-gitlink.perl
+++ b/Documentation/lint-gitlink.perl
@@ -5,11 +5,12 @@
 
 # Parse arguments, a simple state machine for input like:
 #
-# howto/*.txt config/*.txt --section=1 git.txt git-add.txt [...] --to-lint git-add.txt a-file.txt [...]
+# <file-to-check.txt> <valid-files-to-link-to> --section=1 git.txt git-add.txt [...] --to-lint git-add.txt a-file.txt [...]
 my %TXT;
 my %SECTION;
 my $section;
 my $lint_these = 0;
+my $to_check = shift @ARGV;
 for my $arg (@ARGV) {
 	if (my ($sec) = $arg =~ /^--section=(\d+)$/s) {
 		$section = $sec;
@@ -36,7 +37,8 @@ sub report {
 }
 
 @ARGV = sort values %TXT;
-die "BUG: Nothing to process!" unless @ARGV;
+die "BUG: No list of valid linkgit:* files given" unless @ARGV;
+@ARGV = $to_check;
 while (<>) {
 	my $line = $_;
 	while ($line =~ m/linkgit:((.*?)\[(\d)\])/g) {
-- 
2.33.1.1338.g20da966911a
Bagas Sanjaya· Oct 16, 2021, 01:57 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 4/4] doc lint: make "lint-docs" non-.PHONY

On 15/10/21 19.39, Ævar Arnfjörð Bjarmason wrote:
Show 8 quoted lines
> @@ -343,6 +348,7 @@ GIT-ASCIIDOCFLAGS: FORCE
>               fi
>   
>   clean:
> +	$(RM) -rf .build/
>   	$(RM) *.xml *.xml+ *.html *.html+ *.1 *.5 *.7
>   	$(RM) *.texi *.texi+ *.texi++ git.info gitman.info
>   	$(RM) *.pdf
Why did you clean the build directory?
-- 
An old man doll... just what I always wanted! - Clara
Ævar Arnfjörð Bjarmason· Oct 16, 2021, 05:13 UTC · re: Bagas Sanjaya · lore

Re: [PATCH 4/4] doc lint: make "lint-docs" non-.PHONY

On Sat, Oct 16 2021, Bagas Sanjaya wrote:
Show 10 quoted lines
> On 15/10/21 19.39, Ævar Arnfjörð Bjarmason wrote:
>> @@ -343,6 +348,7 @@ GIT-ASCIIDOCFLAGS: FORCE
>>               fi
>>     clean:
>> +	$(RM) -rf .build/
>>   	$(RM) *.xml *.xml+ *.html *.html+ *.1 *.5 *.7
>>   	$(RM) *.texi *.texi+ *.texi++ git.info gitman.info
>>   	$(RM) *.pdf
>
> Why did you clean the build directory?

The Documentation/.build directory is added by this patch, for the lint scratch files, but the structure is such that it can be used by other things in the future.

Ævar Arnfjörð Bjarmason· Oct 15, 2021, 12:39 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 3/4] doc build: speed up "make lint-docs"

Extend the trick we use to speed up the "clean" target to also extend to the "lint-docs" target. See 54df87555b1 (Documentation/Makefile: conditionally include doc.dep, 2020-12-08) for the "clean" implementation.

The "doc-lint" target only depends on *.txt files, so we don't need to generate GIT-VERSION-FILE etc. if that's all we're doing. This makes the "make lint-docs" target more than 2x as fast:

$ git show HEAD~:Documentation/Makefile >Makefile.old
$ hyperfine -L f ",.old" 'make -f Makefile{f} lint-docs'
Benchmark #1: make -f Makefile lint-docs
  Time (mean ± σ):     100.2 ms ±   1.3 ms    [User: 93.7 ms, System: 6.7 ms]
  Range (min … max):    98.4 ms … 103.1 ms    29 runs
Benchmark #2: make -f Makefile.old lint-docs
  Time (mean ± σ):     220.0 ms ±  20.0 ms    [User: 206.0 ms, System: 18.0 ms]
  Range (min … max):   206.6 ms … 267.5 ms    11 runs
Summary
  'make -f Makefile lint-docs' ran
    2.19 ± 0.20 times faster than 'make -f Makefile.old lint-docs'
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to Documentation/Makefile +1 −1
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 78324934d9f..e8e54e053a6 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -284,7 +284,7 @@ install-html: html
 ../GIT-VERSION-FILE: FORCE
 	$(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE
 
-ifneq ($(MAKECMDGOALS),clean)
+ifneq ($(filter-out lint-docs clean,$(MAKECMDGOALS)),)
 -include ../GIT-VERSION-FILE
 endif
 
-- 
2.33.1.1338.g20da966911a
Junio C Hamano· Oct 15, 2021, 17:21 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 3/4] doc build: speed up "make lint-docs"

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
Show 8 quoted lines
> Extend the trick we use to speed up the "clean" target to also extend
> to the "lint-docs" target. See 54df87555b1 (Documentation/Makefile:
> conditionally include doc.dep, 2020-12-08) for the "clean"
> implementation.
>
> The "doc-lint" target only depends on *.txt files, so we don't need to
> generate GIT-VERSION-FILE etc. if that's all we're doing. This makes
> the "make lint-docs" target more than 2x as fast:
Nice. 0.2s down to 0.1 is a 2x speedup.
Show 31 quoted lines
> $ git show HEAD~:Documentation/Makefile >Makefile.old
> $ hyperfine -L f ",.old" 'make -f Makefile{f} lint-docs'
> Benchmark #1: make -f Makefile lint-docs
>   Time (mean ± σ):     100.2 ms ±   1.3 ms    [User: 93.7 ms, System: 6.7 ms]
>   Range (min … max):    98.4 ms … 103.1 ms    29 runs
>
> Benchmark #2: make -f Makefile.old lint-docs
>   Time (mean ± σ):     220.0 ms ±  20.0 ms    [User: 206.0 ms, System: 18.0 ms]
>   Range (min … max):   206.6 ms … 267.5 ms    11 runs
>
> Summary
>   'make -f Makefile lint-docs' ran
>     2.19 ± 0.20 times faster than 'make -f Makefile.old lint-docs'
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  Documentation/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 78324934d9f..e8e54e053a6 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -284,7 +284,7 @@ install-html: html
>  ../GIT-VERSION-FILE: FORCE
>  	$(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE
>  
> -ifneq ($(MAKECMDGOALS),clean)
> +ifneq ($(filter-out lint-docs clean,$(MAKECMDGOALS)),)
>  -include ../GIT-VERSION-FILE
>  endif

← back to recent threads