From 2dd100c5139c164ce4f86734daec522a8236982e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 6 Jun 2024 10:01:00 +0200 Subject: [PATCH 1/4] Makefile: extract script to lint missing/extraneous manpages The "check-docs" target of our top-level Makefile fulfills two different roles. For one it runs the "lint-docs" target of the "Documentation/" Makefile. And second it performs some checks of whether there are any manpages that are missing or extraneous via some inline scripts. The second set of checks feels quite misplaced in the top-level Makefile as it would fit in much better with our "lint-docs" target. Back when the checks were introduced in 8c989ec528 (Makefile: $(MAKE) check-docs, 2006-04-13), that target did not yet exist though. Furthermore, the script makes use of several Makefile variables which are defined in the top-level Makefile, which makes it hard to access their contents from elsewhere. There is a trick though that we already use in "check-builtins.sh" to gain access: we can create an ad-hoc Makefile that has an extra target to print those variables. Pull out the script into a separate "lint-manpages.sh" script by using that trick. Wire up that script via the "lint-docs" target. For one, normal shell scripts are way easier to reason about than those which are embedded in a Makefile. Second, it allows one to easily execute the script standalone without any of the other checks. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- Documentation/Makefile | 4 ++ Documentation/lint-manpages.sh | 83 ++++++++++++++++++++++++++++++++++ Makefile | 36 --------------- 3 files changed, 87 insertions(+), 36 deletions(-) create mode 100755 Documentation/lint-manpages.sh diff --git a/Documentation/Makefile b/Documentation/Makefile index a04da672c6..a3868a462f 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -485,12 +485,16 @@ $(LINT_DOCS_FSCK_MSGIDS): ../fsck.h fsck-msgids.txt lint-docs-fsck-msgids: $(LINT_DOCS_FSCK_MSGIDS) +lint-docs-manpages: + $(QUIET_GEN)./lint-manpages.sh + ## Lint: list of targets above .PHONY: lint-docs lint-docs: lint-docs-fsck-msgids lint-docs: lint-docs-gitlink lint-docs: lint-docs-man-end-blurb lint-docs: lint-docs-man-section-order +lint-docs: lint-docs-manpages ifeq ($(wildcard po/Makefile),po/Makefile) doc-l10n install-l10n:: diff --git a/Documentation/lint-manpages.sh b/Documentation/lint-manpages.sh new file mode 100755 index 0000000000..0abb4e0b4c --- /dev/null +++ b/Documentation/lint-manpages.sh @@ -0,0 +1,83 @@ +#!/bin/sh + +extract_variable () { + ( + cat ../Makefile + cat </dev/null | + sed -n -e 's/.*XXX \(.*\) YYY.*/\1/p' +} + +check_missing_docs () { + for v in $ALL_COMMANDS + do + case "$v" in + git-merge-octopus) continue;; + git-merge-ours) continue;; + git-merge-recursive) continue;; + git-merge-resolve) continue;; + git-merge-subtree) continue;; + git-fsck-objects) continue;; + git-init-db) continue;; + git-remote-*) continue;; + git-stage) continue;; + git-legacy-*) continue;; + git-?*--?* ) continue ;; + esac + + if ! test -f "$v.txt" + then + echo "no doc: $v" + fi + + if ! sed -e '1,/^### command list/d' -e '/^#/d' ../command-list.txt | + grep -q "^$v[ ]" + then + case "$v" in + git) + ;; + *) + echo "no link: $v";; + esac + fi + done +} + +check_extraneous_docs () { + ( + sed -e '1,/^### command list/d' \ + -e '/^#/d' \ + -e '/guide$/d' \ + -e '/interfaces$/d' \ + -e 's/[ ].*//' \ + -e 's/^/listed /' ../command-list.txt + make print-man1 | + grep '\.txt$' | + sed -e 's|^|documented |' \ + -e 's/\.txt//' + ) | ( + all_commands="$(printf "%s " "$ALL_COMMANDS" "$BUILT_INS" "$EXCLUDED_PROGRAMS" | tr '\n' ' ')" + + while read how cmd + do + case " $all_commands " in + *" $cmd "*) ;; + *) + echo "removed but $how: $cmd";; + esac + done + ) +} + +BUILT_INS="$(extract_variable BUILT_INS)" +ALL_COMMANDS="$(extract_variable ALL_COMMANDS)" +EXCLUDED_PROGRAMS="$(extract_variable EXCLUDED_PROGRAMS)" + +{ + check_missing_docs + check_extraneous_docs +} | sort diff --git a/Makefile b/Makefile index 59d98ba688..84e2aa9686 100644 --- a/Makefile +++ b/Makefile @@ -3757,42 +3757,6 @@ ALL_COMMANDS += scalar .PHONY: check-docs check-docs:: $(MAKE) -C Documentation lint-docs - @(for v in $(patsubst %$X,%,$(ALL_COMMANDS)); \ - do \ - case "$$v" in \ - git-merge-octopus | git-merge-ours | git-merge-recursive | \ - git-merge-resolve | git-merge-subtree | \ - git-fsck-objects | git-init-db | \ - git-remote-* | git-stage | git-legacy-* | \ - git-?*--?* ) continue ;; \ - esac ; \ - test -f "Documentation/$$v.txt" || \ - echo "no doc: $$v"; \ - sed -e '1,/^### command list/d' -e '/^#/d' command-list.txt | \ - grep -q "^$$v[ ]" || \ - case "$$v" in \ - git) ;; \ - *) echo "no link: $$v";; \ - esac ; \ - done; \ - ( \ - sed -e '1,/^### command list/d' \ - -e '/^#/d' \ - -e '/guide$$/d' \ - -e '/interfaces$$/d' \ - -e 's/[ ].*//' \ - -e 's/^/listed /' command-list.txt; \ - $(MAKE) -C Documentation print-man1 | \ - grep '\.txt$$' | \ - sed -e 's|^|documented |' \ - -e 's/\.txt//'; \ - ) | while read how cmd; \ - do \ - case " $(patsubst %$X,%,$(ALL_COMMANDS) $(BUILT_INS) $(EXCLUDED_PROGRAMS)) " in \ - *" $$cmd "*) ;; \ - *) echo "removed but $$how: $$cmd" ;; \ - esac; \ - done ) | sort ### Make sure built-ins do not have dups and listed in git.c # From 64239209743d4b07518786400290f58d1955eeb9 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 6 Jun 2024 10:01:05 +0200 Subject: [PATCH 2/4] Documentation/lint-manpages: bubble up errors The "lint-manpages.sh" script does not return an error in case any of its checks fail. While this is faithful to the implementation that we had as part of the "check-docs" target before the preceding commit, it makes it hard to spot any violations of the rules via the corresponding CI job, which will of course exit successfully, too. Adapt the script to bubble up errors. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- Documentation/lint-manpages.sh | 41 +++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/Documentation/lint-manpages.sh b/Documentation/lint-manpages.sh index 0abb4e0b4c..92cfc0a15a 100755 --- a/Documentation/lint-manpages.sh +++ b/Documentation/lint-manpages.sh @@ -12,7 +12,9 @@ EOF sed -n -e 's/.*XXX \(.*\) YYY.*/\1/p' } -check_missing_docs () { +check_missing_docs () ( + ret=0 + for v in $ALL_COMMANDS do case "$v" in @@ -32,6 +34,7 @@ check_missing_docs () { if ! test -f "$v.txt" then echo "no doc: $v" + ret=1 fi if ! sed -e '1,/^### command list/d' -e '/^#/d' ../command-list.txt | @@ -41,11 +44,15 @@ check_missing_docs () { git) ;; *) - echo "no link: $v";; + echo "no link: $v" + ret=1 + ;; esac fi done -} + + exit $ret +) check_extraneous_docs () { ( @@ -61,15 +68,19 @@ check_extraneous_docs () { -e 's/\.txt//' ) | ( all_commands="$(printf "%s " "$ALL_COMMANDS" "$BUILT_INS" "$EXCLUDED_PROGRAMS" | tr '\n' ' ')" + ret=0 while read how cmd do case " $all_commands " in *" $cmd "*) ;; *) - echo "removed but $how: $cmd";; + echo "removed but $how: $cmd" + ret=1;; esac done + + exit $ret ) } @@ -77,7 +88,21 @@ BUILT_INS="$(extract_variable BUILT_INS)" ALL_COMMANDS="$(extract_variable ALL_COMMANDS)" EXCLUDED_PROGRAMS="$(extract_variable EXCLUDED_PROGRAMS)" -{ - check_missing_docs - check_extraneous_docs -} | sort +findings=$( + if ! check_missing_docs + then + ret=1 + fi + + if ! check_extraneous_docs + then + ret=1 + fi + + exit $ret +) +ret=$? + +printf "%s" "$findings" | sort + +exit $ret From 401151de9e58f5da7dd6c3d6ce1ffba94c0ed740 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 6 Jun 2024 10:01:10 +0200 Subject: [PATCH 3/4] gitlab-ci: add job to run `make check-docs` Add another job to execute `make check-docs`, which lints our documentation and makes sure that expected manpages exist. This job mirrors the same job that we already have for GitHub Actions. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- .gitlab-ci.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f676959ca0..37b991e080 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -122,3 +122,12 @@ check-whitespace: - ./ci/check-whitespace.sh "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA" rules: - if: $CI_PIPELINE_SOURCE == 'merge_request_event' + +documentation: + image: ubuntu:latest + variables: + jobname: Documentation + before_script: + - ./ci/install-dependencies.sh + script: + - ./ci/test-documentation.sh From f60fec6a1665891e50b94ff553b4165fe8ab3f2d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 6 Jun 2024 10:01:14 +0200 Subject: [PATCH 4/4] ci/test-documentation: work around SyntaxWarning in Python 3.12 In Python 3.6, unrecognized escape sequences in regular expressions started to produce a DeprecationWarning [1]. In Python 3.12, this was upgraded to a SyntaxWarning and will eventually be raised even further to a SyntaxError. We indirectly hit such unrecognized escape sequences via Asciidoc, which results in a bunch of warnings: $ asciidoc -o /dev/null git-cat-file.txt :1: SyntaxWarning: invalid escape sequence '\S' :1: SyntaxWarning: invalid escape sequence '\S' This in turn causes our "ci/test-documentation.sh" script to fail, as it checks that stderr of `make doc` is empty. These escape sequences seem to be part of Asciidoc itself. In the long term, we should probably consider dropping support for Asciidoc in favor of Asciidoctor. Upstream also considers itself to be legacy software and recommends to move away from it [2]: It is suggested that unless you specifically require the AsciiDoc.py toolchain, you should find a processor that handles the modern AsciiDoc syntax. For now though, let's expand its lifetime a little bit more by filtering out these new warnings. We should probably reconsider once the warnings are upgraded to errors by Python. [1]: https://docs.python.org/3/reference/lexical_analysis.html#string-and-bytes-literals [2]: https://github.com/asciidoc-py/asciidoc-py/blob/6d9f76cff0dc3b7ca21bdd570200f8518464d99b/README.md#asciidocpy Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- ci/test-documentation.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/test-documentation.sh b/ci/test-documentation.sh index de41888430..02b3af3941 100755 --- a/ci/test-documentation.sh +++ b/ci/test-documentation.sh @@ -11,6 +11,7 @@ filter_log () { -e '/^ \* new asciidoc flags$/d' \ -e '/stripped namespace before processing/d' \ -e '/Attributed.*IDs for element/d' \ + -e '/SyntaxWarning: invalid escape sequence/d' \ "$1" }