Documentation: improve linting of manpage existence

Hi,

in [1], Junio noticed that make check-docs was failing for a recent patch series of mine that introduces the new git-refs(1) command. Curiously though, while the checks do print errors, the Makefile target itself succeeded and thus did not make the corresponding CI job fail.

This patch series fixes that issue and also refactors the infrastructure such that it fits better into our existing set of linter targets for our documentation. Finally, this series then adds a job to GitLab CI that is equivalent to the job we already have on GitHub Actions.

[1]: <[email protected]>

Patrick

Patrick Steinhardt (4):
Makefile: extract script to lint missing/extraneous manpages Documentation/lint-manpages: bubble up errors
gitlab-ci: add job to run make check-docs ci/test-documentation: work around SyntaxWarning in Python 3.12

 .gitlab-ci.yml                 |   9 +++
 Documentation/Makefile         |   4 ++
 Documentation/lint-manpages.sh | 107 +++++++++++++++++++++++++++++++++
 Makefile                       |  36 -----------
 ci/test-documentation.sh       |   1 +
 5 files changed, 121 insertions(+), 36 deletions(-)
 create mode 100755 Documentation/lint-manpages.sh

2.45.2.409.g7b0defb391.dirty

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.

---
 Documentation/Makefile         |  4 ++
 Documentation/lint-manpages.sh | 82 ++++++++++++++++++++++++++++++++++
 Makefile                       | 36 ---------------
 3 files changed, 86 insertions(+), 36 deletions(-)
 create mode 100755 Documentation/lint-manpages.sh

index a04da672c6..a3868a462f 100644
@@ -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::

index 59d98ba688..84e2aa9686 100644
@@ -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
 #

2.45.2.409.g7b0defb391.dirty

Ah, true. I initially did have some bash-isms, but got rid of them. Will adapt.

Patrick

It looks like the script fails to run under /bin/sh:
https://gitlab.com/gitlab-org/git/-/jobs/7021474555#L4365

Cheers,
James

Nice to know. In any case, the original was an inline scriptlet in the Makefile and should have been happily working for folks whose /bin/sh is different from bash, so the version that is extracted out shouldn't have to rely on bashisms.

Thanks.

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.

---
 Documentation/lint-manpages.sh | 35 +++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

index f720a3f3d6..c7ae8ee17a 100755
@@ -15,6 +15,8 @@ EOF
 }
 
 check_missing_docs () {
+	local ret=0
+
 	for v in $BUILT_INS
 	do
 		case "$v" in

@@ -34,6 +36,7 @@ check_missing_docs () {
 		if ! test -f "Documentation/$v.txt"
 		then
 			echo "no doc: $v"
+			ret=1
 		fi
 
 		if ! sed -e '1,/^### command list/d' -e '/^#/d' command-list.txt |

@@ -43,20 +46,26 @@ check_missing_docs () {
 			git)
 				;;
 			*)
-				echo "no link: $v";;
+				echo "no link: $v"
+				ret=1
+				;;
 			esac
 		fi
 	done
+
+	return $ret
 }
 
 check_extraneous_docs () {
 	local commands="$(printf "%s\n" "$ALL_COMMANDS" "$BUILT_INS" "$EXCLUDED_PROGRAMS")"
+	local ret=0
 
 	while read how cmd
 	do
 		if ! [[ $commands = *"$cmd"* ]]
 		then
 			echo "removed but $how: $cmd"
+			ret=1
 		fi
 	done < <(
 		sed -e '1,/^### command list/d' \

@@ -70,13 +79,29 @@ check_extraneous_docs () {
 		sed -e 's|^|documented |' \
 		    -e 's/\.txt//'
 	)
+
+	return $ret
 }
 
 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=$?
+
+echo "$findings" | sort
+
+exit $ret

2.45.2.409.g7b0defb391.dirty

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.

---
 .gitlab-ci.yml | 9 +++++++++
 1 file changed, 9 insertions(+)

index f676959ca0..37b991e080 100644
@@ -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

2.45.2.409.g7b0defb391.dirty

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
    <unknown>:1: SyntaxWarning: invalid escape sequence '\S'
    <unknown>: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

---
 ci/test-documentation.sh | 1 +
 1 file changed, 1 insertion(+)

index de41888430..02b3af3941 100755
@@ -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"
 }
 

2.45.2.409.g7b0defb391.dirty

Sounds sensible. We can throw it in to the Git 3.0 list, perhaps ;-)

Hi,

this is the second version of my patch series that improves linting for the existence and/or absence of manpages.

Changes compared to v1:

- Converted the extracted script to be compatible with POSIX shells.

- Fix an empty line being printed when there are no findings.

- Silence the patched-in Makefile recipe, which otherwise leads to one value existing twice in the converted output.

- Fix check_missing_docs() to use ALL_COMMANDS, not BUILT_INS.

Thanks!

Patrick

Patrick Steinhardt (4):
Makefile: extract script to lint missing/extraneous manpages Documentation/lint-manpages: bubble up errors
gitlab-ci: add job to run make check-docs ci/test-documentation: work around SyntaxWarning in Python 3.12

 .gitlab-ci.yml                 |   9 +++
 Documentation/Makefile         |   4 ++
 Documentation/lint-manpages.sh | 108 +++++++++++++++++++++++++++++++++
 Makefile                       |  36 -----------
 ci/test-documentation.sh       |   1 +
 5 files changed, 122 insertions(+), 36 deletions(-)
 create mode 100755 Documentation/lint-manpages.sh

Range-diff against v1:
1: b06088a2ff ! 1: 489a6eaf2d Makefile: extract script to lint missing/extraneous manpages

    @@ Documentation/Makefile: $(LINT_DOCS_FSCK_MSGIDS): ../fsck.h fsck-msgids.txt
     
      ## Documentation/lint-manpages.sh (new) ##
     @@
    -+#!/usr/bin/env bash
    -+
    -+cd "$(dirname "${BASH_SOURCE[0]}")"/..
    ++#!/bin/sh
     +
     +extract_variable () {
     +	(
    -+		cat Makefile
    ++		cat ../Makefile
     +		cat <<EOF
     +print_variable:
    -+	\$(foreach b,\$($1),echo XXX \$(b:\$X=) YYY;)
    ++	@\$(foreach b,\$($1),echo XXX \$(b:\$X=) YYY;)
     +EOF
     +	) |
    -+	make -f - print_variable 2>/dev/null |
    ++	make -C .. -f - print_variable 2>/dev/null |
     +	sed -n -e 's/.*XXX \(.*\) YYY.*/\1/p'
     +}
     +
     +check_missing_docs () {
    -+	for v in $BUILT_INS
    ++	for v in $ALL_COMMANDS
     +	do
     +		case "$v" in
     +		git-merge-octopus) continue;;
    @@ Documentation/lint-manpages.sh (new)
     +		git-?*--?* ) continue ;;
     +		esac
     +
    -+		if ! test -f "Documentation/$v.txt"
    ++		if ! test -f "$v.txt"
     +		then
     +			echo "no doc: $v"
     +		fi
     +
    -+		if ! sed -e '1,/^### command list/d' -e '/^#/d' command-list.txt |
    ++		if ! sed -e '1,/^### command list/d' -e '/^#/d' ../command-list.txt |
     +			grep -q "^$v[ 	]"
     +		then
     +			case "$v" in
    @@ Documentation/lint-manpages.sh (new)
     +}
     +
     +check_extraneous_docs () {
    -+	local commands="$(printf "%s\n" "$ALL_COMMANDS" "$BUILT_INS" "$EXCLUDED_PROGRAMS")"
    -+
    -+	while read how cmd
    -+	do
    -+		if ! [[ $commands = *"$cmd"* ]]
    -+		then
    -+			echo "removed but $how: $cmd"
    -+		fi
    -+	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 |
    ++		    -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
     +	)
     +}
     +
2:  b39a780d33 ! 2:  9f21c305b9 Documentation/lint-manpages: bubble up errors
    @@ Commit message
     
      ## Documentation/lint-manpages.sh ##
     @@ Documentation/lint-manpages.sh: EOF
    + 	sed -n -e 's/.*XXX \(.*\) YYY.*/\1/p'
      }
      
    - check_missing_docs () {
    -+	local ret=0
    +-check_missing_docs () {
    ++check_missing_docs () (
    ++	ret=0
     +
    - 	for v in $BUILT_INS
    + 	for v in $ALL_COMMANDS
      	do
      		case "$v" in
     @@ Documentation/lint-manpages.sh: check_missing_docs () {
    - 		if ! test -f "Documentation/$v.txt"
    + 		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 |
    + 		if ! sed -e '1,/^### command list/d' -e '/^#/d' ../command-list.txt |
     @@ Documentation/lint-manpages.sh: check_missing_docs () {
      			git)
      				;;
    @@ Documentation/lint-manpages.sh: check_missing_docs () {
      			esac
      		fi
      	done
    +-}
     +
    -+	return $ret
    - }
    ++	exit $ret
    ++)
      
      check_extraneous_docs () {
    - 	local commands="$(printf "%s\n" "$ALL_COMMANDS" "$BUILT_INS" "$EXCLUDED_PROGRAMS")"
    -+	local ret=0
    - 
    - 	while read how cmd
    - 	do
    - 		if ! [[ $commands = *"$cmd"* ]]
    - 		then
    - 			echo "removed but $how: $cmd"
    -+			ret=1
    - 		fi
    - 	done < <(
    - 		sed -e '1,/^### command list/d' \
    + 	(
     @@ Documentation/lint-manpages.sh: check_extraneous_docs () {
    - 		sed -e 's|^|documented |' \
      		    -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
     +
    -+	return $ret
    ++		exit $ret
    + 	)
      }
      
    - BUILT_INS="$(extract_variable BUILT_INS)"
    +@@ Documentation/lint-manpages.sh: BUILT_INS="$(extract_variable BUILT_INS)"
      ALL_COMMANDS="$(extract_variable ALL_COMMANDS)"
      EXCLUDED_PROGRAMS="$(extract_variable EXCLUDED_PROGRAMS)"
      
    @@ Documentation/lint-manpages.sh: check_extraneous_docs () {
     +)
     +ret=$?
     +
    -+echo "$findings" | sort
    ++printf "%s" "$findings" | sort
     +
     +exit $ret
3:  a44d57a732 = 3:  d6d3628797 gitlab-ci: add job to run `make check-docs`
4:  c758b45282 = 4:  c13fed9ebf ci/test-documentation: work around SyntaxWarning in Python 3.12

2.45.2.409.g7b0defb391.dirty

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.

---
 Documentation/Makefile         |  4 ++
 Documentation/lint-manpages.sh | 83 ++++++++++++++++++++++++++++++++++
 Makefile                       | 36 ---------------
 3 files changed, 87 insertions(+), 36 deletions(-)
 create mode 100755 Documentation/lint-manpages.sh

index a04da672c6..a3868a462f 100644
@@ -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::

index 59d98ba688..84e2aa9686 100644
@@ -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
 #

2.45.2.409.g7b0defb391.dirty

Yes.

Correct.

Yes.

Nicely done. We might want to stop doing "make print-man1" in the lint script, remove "print-man1" target from Documentation/Makefile, and instead extract MAN1_TXT using the same "it is unlikely to have an output line enclosing something in between XXX and YYY" trick, but it may not be worth it.

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.

---
 Documentation/lint-manpages.sh | 41 +++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 8 deletions(-)

index 0abb4e0b4c..92cfc0a15a 100755
@@ -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

2.45.2.409.g7b0defb391.dirty

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.

---
 .gitlab-ci.yml | 9 +++++++++
 1 file changed, 9 insertions(+)

index f676959ca0..37b991e080 100644
@@ -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

2.45.2.409.g7b0defb391.dirty

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
    <unknown>:1: SyntaxWarning: invalid escape sequence '\S'
    <unknown>: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

---
 ci/test-documentation.sh | 1 +
 1 file changed, 1 insertion(+)

index de41888430..02b3af3941 100755
@@ -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"
 }
 

2.45.2.409.g7b0defb391.dirty

Thanks Patrick, this looks good to me! Learnt a few shell tricks too.

Cheers,
James