{"thread":{"id":"61586","subject":"[PATCH 0/4] Documentation: improve linting of manpage existence","startedAt":"2024-06-05T05:16:49Z","lastAt":"2024-06-07T16:51:56Z","messageCount":18,"participants":["Patrick Steinhardt","Junio C Hamano","James Liu"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"496322","messageId":"cover.1717564310.git.ps@pks.im","threadId":"61586","inReplyTo":null,"subject":"[PATCH 0/4] Documentation: improve linting of manpage existence","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-06-05T05:16:42Z","receivedAt":"2024-06-05T05:16:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nin [1], Junio noticed that `make check-docs` was failing for a recent\npatch series of mine that introduces the new git-refs(1) command.\nCuriously though, while the checks do print errors, the Makefile target\nitself succeeded and thus did not make the corresponding CI job fail.\n\nThis patch series fixes that issue and also refactors the infrastructure\nsuch that it fits better into our existing set of linter targets for our\ndocumentation. Finally, this series then adds a job to GitLab CI that is\nequivalent to the job we already have on GitHub Actions.\n\n[1]: <xmqqjzj9czop.fsf@gitster.g>\n\nPatrick\n\nPatrick Steinhardt (4):\n  Makefile: extract script to lint missing/extraneous manpages\n  Documentation/lint-manpages: bubble up errors\n  gitlab-ci: add job to run `make check-docs`\n  ci/test-documentation: work around SyntaxWarning in Python 3.12\n\n .gitlab-ci.yml                 |   9 +++\n Documentation/Makefile         |   4 ++\n Documentation/lint-manpages.sh | 107 +++++++++++++++++++++++++++++++++\n Makefile                       |  36 -----------\n ci/test-documentation.sh       |   1 +\n 5 files changed, 121 insertions(+), 36 deletions(-)\n create mode 100755 Documentation/lint-manpages.sh\n\n-- \n2.45.2.409.g7b0defb391.dirty\n\n"},{"id":"496323","messageId":"b06088a2ff65a3455f0f5db2a9b752901f2af14b.1717564310.git.ps@pks.im","threadId":"61586","inReplyTo":"cover.1717564310.git.ps@pks.im","subject":"[PATCH 1/4] Makefile: extract script to lint missing/extraneous manpages","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-06-05T05:16:47Z","receivedAt":"2024-06-05T05:16:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"check-docs\" target of our top-level Makefile fulfills two different\nroles. For one it runs the \"lint-docs\" target of the \"Documentation/\"\nMakefile. And second it performs some checks of whether there are any\nmanpages that are missing or extraneous via some inline scripts.\n\nThe second set of checks feels quite misplaced in the top-level Makefile\nas it would fit in much better with our \"lint-docs\" target. Back when\nthe checks were introduced in 8c989ec528 (Makefile: $(MAKE) check-docs,\n2006-04-13), that target did not yet exist though.\n\nFurthermore, the script makes use of several Makefile variables which\nare defined in the top-level Makefile, which makes it hard to access\ntheir contents from elsewhere. There is a trick though that we already\nuse in \"check-builtins.sh\" to gain access: we can create an ad-hoc\nMakefile that has an extra target to print those variables.\n\nPull out the script into a separate \"lint-manpages.sh\" script by using\nthat trick. Wire up that script via the \"lint-docs\" target. For one,\nnormal shell scripts are way easier to reason about than those which are\nembedded in a Makefile. Second, it allows one to easily execute the\nscript standalone without any of the other checks.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/Makefile         |  4 ++\n Documentation/lint-manpages.sh | 82 ++++++++++++++++++++++++++++++++++\n Makefile                       | 36 ---------------\n 3 files changed, 86 insertions(+), 36 deletions(-)\n create mode 100755 Documentation/lint-manpages.sh\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex a04da672c6..a3868a462f 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -485,12 +485,16 @@ $(LINT_DOCS_FSCK_MSGIDS): ../fsck.h fsck-msgids.txt\n \n lint-docs-fsck-msgids: $(LINT_DOCS_FSCK_MSGIDS)\n \n+lint-docs-manpages:\n+\t$(QUIET_GEN)./lint-manpages.sh\n+\n ## Lint: list of targets above\n .PHONY: lint-docs\n lint-docs: lint-docs-fsck-msgids\n lint-docs: lint-docs-gitlink\n lint-docs: lint-docs-man-end-blurb\n lint-docs: lint-docs-man-section-order\n+lint-docs: lint-docs-manpages\n \n ifeq ($(wildcard po/Makefile),po/Makefile)\n doc-l10n install-l10n::\ndiff --git a/Documentation/lint-manpages.sh b/Documentation/lint-manpages.sh\nnew file mode 100755\nindex 0000000000..f720a3f3d6\n--- /dev/null\n+++ b/Documentation/lint-manpages.sh\n@@ -0,0 +1,82 @@\n+#!/usr/bin/env bash\n+\n+cd \"$(dirname \"${BASH_SOURCE[0]}\")\"/..\n+\n+extract_variable () {\n+\t(\n+\t\tcat Makefile\n+\t\tcat <<EOF\n+print_variable:\n+\t\\$(foreach b,\\$($1),echo XXX \\$(b:\\$X=) YYY;)\n+EOF\n+\t) |\n+\tmake -f - print_variable 2>/dev/null |\n+\tsed -n -e 's/.*XXX \\(.*\\) YYY.*/\\1/p'\n+}\n+\n+check_missing_docs () {\n+\tfor v in $BUILT_INS\n+\tdo\n+\t\tcase \"$v\" in\n+\t\tgit-merge-octopus) continue;;\n+\t\tgit-merge-ours) continue;;\n+\t\tgit-merge-recursive) continue;;\n+\t\tgit-merge-resolve) continue;;\n+\t\tgit-merge-subtree) continue;;\n+\t\tgit-fsck-objects) continue;;\n+\t\tgit-init-db) continue;;\n+\t\tgit-remote-*) continue;;\n+\t\tgit-stage) continue;;\n+\t\tgit-legacy-*) continue;;\n+\t\tgit-?*--?* ) continue ;;\n+\t\tesac\n+\n+\t\tif ! test -f \"Documentation/$v.txt\"\n+\t\tthen\n+\t\t\techo \"no doc: $v\"\n+\t\tfi\n+\n+\t\tif ! sed -e '1,/^### command list/d' -e '/^#/d' command-list.txt |\n+\t\t\tgrep -q \"^$v[ \t]\"\n+\t\tthen\n+\t\t\tcase \"$v\" in\n+\t\t\tgit)\n+\t\t\t\t;;\n+\t\t\t*)\n+\t\t\t\techo \"no link: $v\";;\n+\t\t\tesac\n+\t\tfi\n+\tdone\n+}\n+\n+check_extraneous_docs () {\n+\tlocal commands=\"$(printf \"%s\\n\" \"$ALL_COMMANDS\" \"$BUILT_INS\" \"$EXCLUDED_PROGRAMS\")\"\n+\n+\twhile read how cmd\n+\tdo\n+\t\tif ! [[ $commands = *\"$cmd\"* ]]\n+\t\tthen\n+\t\t\techo \"removed but $how: $cmd\"\n+\t\tfi\n+\tdone < <(\n+\t\tsed -e '1,/^### command list/d' \\\n+\t\t    -e '/^#/d' \\\n+\t\t    -e '/guide$/d' \\\n+\t\t    -e '/interfaces$/d' \\\n+\t\t    -e 's/[ \t].*//' \\\n+\t\t    -e 's/^/listed /' command-list.txt\n+\t\tmake -C Documentation print-man1 |\n+\t\tgrep '\\.txt$' |\n+\t\tsed -e 's|^|documented |' \\\n+\t\t    -e 's/\\.txt//'\n+\t)\n+}\n+\n+BUILT_INS=\"$(extract_variable BUILT_INS)\"\n+ALL_COMMANDS=\"$(extract_variable ALL_COMMANDS)\"\n+EXCLUDED_PROGRAMS=\"$(extract_variable EXCLUDED_PROGRAMS)\"\n+\n+{\n+\tcheck_missing_docs\n+\tcheck_extraneous_docs\n+} | sort\ndiff --git a/Makefile b/Makefile\nindex 59d98ba688..84e2aa9686 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3757,42 +3757,6 @@ ALL_COMMANDS += scalar\n .PHONY: check-docs\n check-docs::\n \t$(MAKE) -C Documentation lint-docs\n-\t@(for v in $(patsubst %$X,%,$(ALL_COMMANDS)); \\\n-\tdo \\\n-\t\tcase \"$$v\" in \\\n-\t\tgit-merge-octopus | git-merge-ours | git-merge-recursive | \\\n-\t\tgit-merge-resolve | git-merge-subtree | \\\n-\t\tgit-fsck-objects | git-init-db | \\\n-\t\tgit-remote-* | git-stage | git-legacy-* | \\\n-\t\tgit-?*--?* ) continue ;; \\\n-\t\tesac ; \\\n-\t\ttest -f \"Documentation/$$v.txt\" || \\\n-\t\techo \"no doc: $$v\"; \\\n-\t\tsed -e '1,/^### command list/d' -e '/^#/d' command-list.txt | \\\n-\t\tgrep -q \"^$$v[ \t]\" || \\\n-\t\tcase \"$$v\" in \\\n-\t\tgit) ;; \\\n-\t\t*) echo \"no link: $$v\";; \\\n-\t\tesac ; \\\n-\tdone; \\\n-\t( \\\n-\t\tsed -e '1,/^### command list/d' \\\n-\t\t    -e '/^#/d' \\\n-\t\t    -e '/guide$$/d' \\\n-\t\t    -e '/interfaces$$/d' \\\n-\t\t    -e 's/[ \t].*//' \\\n-\t\t    -e 's/^/listed /' command-list.txt; \\\n-\t\t$(MAKE) -C Documentation print-man1 | \\\n-\t\tgrep '\\.txt$$' | \\\n-\t\tsed -e 's|^|documented |' \\\n-\t\t    -e 's/\\.txt//'; \\\n-\t) | while read how cmd; \\\n-\tdo \\\n-\t\tcase \" $(patsubst %$X,%,$(ALL_COMMANDS) $(BUILT_INS) $(EXCLUDED_PROGRAMS)) \" in \\\n-\t\t*\" $$cmd \"*)\t;; \\\n-\t\t*) echo \"removed but $$how: $$cmd\" ;; \\\n-\t\tesac; \\\n-\tdone ) | sort\n \n ### Make sure built-ins do not have dups and listed in git.c\n #\n-- \n2.45.2.409.g7b0defb391.dirty\n\n"},{"id":"496324","messageId":"b39a780d33e9ff00bc44886ed7e10904470c75e6.1717564310.git.ps@pks.im","threadId":"61586","inReplyTo":"cover.1717564310.git.ps@pks.im","subject":"[PATCH 2/4] Documentation/lint-manpages: bubble up errors","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-06-05T05:16:51Z","receivedAt":"2024-06-05T05:16:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"lint-manpages.sh\" script does not return an error in case any of\nits checks fail. While this is faithful to the implementation that we\nhad as part of the \"check-docs\" target before the preceding commit, it\nmakes it hard to spot any violations of the rules via the corresponding\nCI job, which will of course exit successfully, too.\n\nAdapt the script to bubble up errors.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/lint-manpages.sh | 35 +++++++++++++++++++++++++++++-----\n 1 file changed, 30 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/lint-manpages.sh b/Documentation/lint-manpages.sh\nindex f720a3f3d6..c7ae8ee17a 100755\n--- a/Documentation/lint-manpages.sh\n+++ b/Documentation/lint-manpages.sh\n@@ -15,6 +15,8 @@ EOF\n }\n \n check_missing_docs () {\n+\tlocal ret=0\n+\n \tfor v in $BUILT_INS\n \tdo\n \t\tcase \"$v\" in\n@@ -34,6 +36,7 @@ check_missing_docs () {\n \t\tif ! test -f \"Documentation/$v.txt\"\n \t\tthen\n \t\t\techo \"no doc: $v\"\n+\t\t\tret=1\n \t\tfi\n \n \t\tif ! sed -e '1,/^### command list/d' -e '/^#/d' command-list.txt |\n@@ -43,20 +46,26 @@ check_missing_docs () {\n \t\t\tgit)\n \t\t\t\t;;\n \t\t\t*)\n-\t\t\t\techo \"no link: $v\";;\n+\t\t\t\techo \"no link: $v\"\n+\t\t\t\tret=1\n+\t\t\t\t;;\n \t\t\tesac\n \t\tfi\n \tdone\n+\n+\treturn $ret\n }\n \n check_extraneous_docs () {\n \tlocal commands=\"$(printf \"%s\\n\" \"$ALL_COMMANDS\" \"$BUILT_INS\" \"$EXCLUDED_PROGRAMS\")\"\n+\tlocal ret=0\n \n \twhile read how cmd\n \tdo\n \t\tif ! [[ $commands = *\"$cmd\"* ]]\n \t\tthen\n \t\t\techo \"removed but $how: $cmd\"\n+\t\t\tret=1\n \t\tfi\n \tdone < <(\n \t\tsed -e '1,/^### command list/d' \\\n@@ -70,13 +79,29 @@ check_extraneous_docs () {\n \t\tsed -e 's|^|documented |' \\\n \t\t    -e 's/\\.txt//'\n \t)\n+\n+\treturn $ret\n }\n \n BUILT_INS=\"$(extract_variable BUILT_INS)\"\n ALL_COMMANDS=\"$(extract_variable ALL_COMMANDS)\"\n EXCLUDED_PROGRAMS=\"$(extract_variable EXCLUDED_PROGRAMS)\"\n \n-{\n-\tcheck_missing_docs\n-\tcheck_extraneous_docs\n-} | sort\n+findings=$(\n+\tif ! check_missing_docs\n+\tthen\n+\t\tret=1\n+\tfi\n+\n+\tif ! check_extraneous_docs\n+\tthen\n+\t\tret=1\n+\tfi\n+\n+\texit $ret\n+)\n+ret=$?\n+\n+echo \"$findings\" | sort\n+\n+exit $ret\n-- \n2.45.2.409.g7b0defb391.dirty\n\n"},{"id":"496325","messageId":"a44d57a7328e5964111ae9b3abc87cba93d9a1d7.1717564310.git.ps@pks.im","threadId":"61586","inReplyTo":"cover.1717564310.git.ps@pks.im","subject":"[PATCH 3/4] gitlab-ci: add job to run `make check-docs`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-06-05T05:16:56Z","receivedAt":"2024-06-05T05:17:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Add another job to execute `make check-docs`, which lints our\ndocumentation and makes sure that expected manpages exist. This job\nmirrors the same job that we already have for GitHub Actions.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n .gitlab-ci.yml | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\nindex f676959ca0..37b991e080 100644\n--- a/.gitlab-ci.yml\n+++ b/.gitlab-ci.yml\n@@ -122,3 +122,12 @@ check-whitespace:\n     - ./ci/check-whitespace.sh \"$CI_MERGE_REQUEST_TARGET_BRANCH_SHA\"\n   rules:\n     - if: $CI_PIPELINE_SOURCE == 'merge_request_event'\n+\n+documentation:\n+  image: ubuntu:latest\n+  variables:\n+    jobname: Documentation\n+  before_script:\n+    - ./ci/install-dependencies.sh\n+  script:\n+    - ./ci/test-documentation.sh\n-- \n2.45.2.409.g7b0defb391.dirty\n\n"},{"id":"496326","messageId":"c758b45282f8eb5fec401da6021d7ded6cebb243.1717564310.git.ps@pks.im","threadId":"61586","inReplyTo":"cover.1717564310.git.ps@pks.im","subject":"[PATCH 4/4] ci/test-documentation: work around SyntaxWarning in Python 3.12","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-06-05T05:17:01Z","receivedAt":"2024-06-05T05:17:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In Python 3.6, unrecognized escape sequences in regular expressions\nstarted to produce a DeprecationWarning [1]. In Python 3.12, this was\nupgraded to a SyntaxWarning and will eventually be raised even further\nto a SyntaxError. We indirectly hit such unrecognized escape sequences\nvia Asciidoc, which results in a bunch of warnings:\n\n    $ asciidoc -o /dev/null git-cat-file.txt\n    <unknown>:1: SyntaxWarning: invalid escape sequence '\\S'\n    <unknown>:1: SyntaxWarning: invalid escape sequence '\\S'\n\nThis in turn causes our \"ci/test-documentation.sh\" script to fail, as it\nchecks that stderr of `make doc` is empty.\n\nThese escape sequences seem to be part of Asciidoc itself. In the long\nterm, we should probably consider dropping support for Asciidoc in favor\nof Asciidoctor. Upstream also considers itself to be legacy software and\nrecommends to move away from it [2]:\n\n    It is suggested that unless you specifically require the AsciiDoc.py\n    toolchain, you should find a processor that handles the modern\n    AsciiDoc syntax.\n\nFor now though, let's expand its lifetime a little bit more by filtering\nout these new warnings. We should probably reconsider once the warnings\nare upgraded to errors by Python.\n\n[1]: https://docs.python.org/3/reference/lexical_analysis.html#string-and-bytes-literals\n[2]: https://github.com/asciidoc-py/asciidoc-py/blob/6d9f76cff0dc3b7ca21bdd570200f8518464d99b/README.md#asciidocpy\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n ci/test-documentation.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/ci/test-documentation.sh b/ci/test-documentation.sh\nindex de41888430..02b3af3941 100755\n--- a/ci/test-documentation.sh\n+++ b/ci/test-documentation.sh\n@@ -11,6 +11,7 @@ filter_log () {\n \t    -e '/^    \\* new asciidoc flags$/d' \\\n \t    -e '/stripped namespace before processing/d' \\\n \t    -e '/Attributed.*IDs for element/d' \\\n+\t    -e '/SyntaxWarning: invalid escape sequence/d' \\\n \t    \"$1\"\n }\n \n-- \n2.45.2.409.g7b0defb391.dirty\n\n"},{"id":"496327","messageId":"xmqqzfs0x8w9.fsf@gitster.g","threadId":"61586","inReplyTo":"b06088a2ff65a3455f0f5db2a9b752901f2af14b.1717564310.git.ps@pks.im","subject":"Re: [PATCH 1/4] Makefile: extract script to lint missing/extraneous manpages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-05T05:20:54Z","receivedAt":"2024-06-05T05:20:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> +++ b/Documentation/lint-manpages.sh\n> @@ -0,0 +1,82 @@\n> +#!/usr/bin/env bash\n\nI do not see much bash-ism here.  Unless absolutely needed, please\nuse \"#!/bin/sh\" instead.\n\n"},{"id":"496328","messageId":"xmqqtti8x8tx.fsf@gitster.g","threadId":"61586","inReplyTo":"b39a780d33e9ff00bc44886ed7e10904470c75e6.1717564310.git.ps@pks.im","subject":"Re: [PATCH 2/4] Documentation/lint-manpages: bubble up errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-05T05:22:18Z","receivedAt":"2024-06-05T05:22:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> +echo \"$findings\" | sort\n> + ...\n> +exit $ret\n\nAll make sense.\n\n"},{"id":"496329","messageId":"xmqqplswx8qn.fsf@gitster.g","threadId":"61586","inReplyTo":"c758b45282f8eb5fec401da6021d7ded6cebb243.1717564310.git.ps@pks.im","subject":"Re: [PATCH 4/4] ci/test-documentation: work around SyntaxWarning in Python 3.12","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-05T05:24:16Z","receivedAt":"2024-06-05T05:24:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> These escape sequences seem to be part of Asciidoc itself. In the long\n> term, we should probably consider dropping support for Asciidoc in favor\n> of Asciidoctor. Upstream also considers itself to be legacy software and\n> recommends to move away from it [2]:\n> ...\n> For now though, let's expand its lifetime a little bit more by filtering\n> out these new warnings. We should probably reconsider once the warnings\n> are upgraded to errors by Python.\n\nSounds sensible.  We can throw it in to the Git 3.0 list, perhaps\n;-)\n"},{"id":"496330","messageId":"Zl_3TQrUHdb5osDA@tanuki","threadId":"61586","inReplyTo":"xmqqzfs0x8w9.fsf@gitster.g","subject":"Re: [PATCH 1/4] Makefile: extract script to lint missing/extraneous manpages","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-06-05T05:27:41Z","receivedAt":"2024-06-05T05:27:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Jun 04, 2024 at 10:20:54PM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > +++ b/Documentation/lint-manpages.sh\n> > @@ -0,0 +1,82 @@\n> > +#!/usr/bin/env bash\n> \n> I do not see much bash-ism here.  Unless absolutely needed, please\n> use \"#!/bin/sh\" instead.\n\nAh, true. I initially did have some bash-isms, but got rid of them. Will\nadapt.\n\nPatrick\n"},{"id":"496430","messageId":"D1SPGQ8OOLLS.I6HUJZW81XLH@jamesliu.io","threadId":"61586","inReplyTo":"Zl_3TQrUHdb5osDA@tanuki","subject":"Re: [PATCH 1/4] Makefile: extract script to lint missing/extraneous manpages","fromName":"James Liu","fromEmail":"james@jamesliu.io","sentAt":"2024-06-06T06:03:45Z","receivedAt":"2024-06-06T06:03:50Z","isPatch":true,"sender":{"key":"james@jamesliu.io","avatar":"https://avatars.githubusercontent.com/u/4476568?v=4"},"body":"On Wed Jun 5, 2024 at 3:27 PM AEST, Patrick Steinhardt wrote:\n> On Tue, Jun 04, 2024 at 10:20:54PM -0700, Junio C Hamano wrote:\n> > Patrick Steinhardt <ps@pks.im> writes:\n> > \n> > > +++ b/Documentation/lint-manpages.sh\n> > > @@ -0,0 +1,82 @@\n> > > +#!/usr/bin/env bash\n> > \n> > I do not see much bash-ism here.  Unless absolutely needed, please\n> > use \"#!/bin/sh\" instead.\n>\n> Ah, true. I initially did have some bash-isms, but got rid of them. Will\n> adapt.\n>\n> Patrick\n\nIt looks like the script fails to run under /bin/sh:\nhttps://gitlab.com/gitlab-org/git/-/jobs/7021474555#L4365\n\nCheers,\nJames\n"},{"id":"496432","messageId":"xmqqikymppl7.fsf@gitster.g","threadId":"61586","inReplyTo":"D1SPGQ8OOLLS.I6HUJZW81XLH@jamesliu.io","subject":"Re: [PATCH 1/4] Makefile: extract script to lint missing/extraneous manpages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-06T06:12:04Z","receivedAt":"2024-06-06T06:12:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"James Liu\" <james@jamesliu.io> writes:\n\n> On Wed Jun 5, 2024 at 3:27 PM AEST, Patrick Steinhardt wrote:\n>> On Tue, Jun 04, 2024 at 10:20:54PM -0700, Junio C Hamano wrote:\n>> > Patrick Steinhardt <ps@pks.im> writes:\n>> > \n>> > > +++ b/Documentation/lint-manpages.sh\n>> > > @@ -0,0 +1,82 @@\n>> > > +#!/usr/bin/env bash\n>> > \n>> > I do not see much bash-ism here.  Unless absolutely needed, please\n>> > use \"#!/bin/sh\" instead.\n>>\n>> Ah, true. I initially did have some bash-isms, but got rid of them. Will\n>> adapt.\n>>\n>> Patrick\n>\n> It looks like the script fails to run under /bin/sh:\n> https://gitlab.com/gitlab-org/git/-/jobs/7021474555#L4365\n\nNice to know.  In any case, the original was an inline scriptlet in\nthe Makefile and should have been happily working for folks whose\n/bin/sh is different from bash, so the version that is extracted out\nshouldn't have to rely on bashisms.\n\nThanks.\n"},{"id":"496451","messageId":"cover.1717660597.git.ps@pks.im","threadId":"61586","inReplyTo":"cover.1717564310.git.ps@pks.im","subject":"[PATCH v2 0/4] Documentation: improve linting for manpage existence","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-06-06T08:00:56Z","receivedAt":"2024-06-06T08:01:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is the second version of my patch series that improves linting for\nthe existence and/or absence of manpages.\n\nChanges compared to v1:\n\n  - Converted the extracted script to be compatible with POSIX shells.\n\n  - Fix an empty line being printed when there are no findings.\n\n  - Silence the patched-in Makefile recipe, which otherwise leads to one\n    value existing twice in the converted output.\n\n  - Fix `check_missing_docs()` to use `ALL_COMMANDS`, not `BUILT_INS`.\n\nThanks!\n\nPatrick\n\nPatrick Steinhardt (4):\n  Makefile: extract script to lint missing/extraneous manpages\n  Documentation/lint-manpages: bubble up errors\n  gitlab-ci: add job to run `make check-docs`\n  ci/test-documentation: work around SyntaxWarning in Python 3.12\n\n .gitlab-ci.yml                 |   9 +++\n Documentation/Makefile         |   4 ++\n Documentation/lint-manpages.sh | 108 +++++++++++++++++++++++++++++++++\n Makefile                       |  36 -----------\n ci/test-documentation.sh       |   1 +\n 5 files changed, 122 insertions(+), 36 deletions(-)\n create mode 100755 Documentation/lint-manpages.sh\n\nRange-diff against v1:\n1:  b06088a2ff ! 1:  489a6eaf2d Makefile: extract script to lint missing/extraneous manpages\n    @@ Documentation/Makefile: $(LINT_DOCS_FSCK_MSGIDS): ../fsck.h fsck-msgids.txt\n     \n      ## Documentation/lint-manpages.sh (new) ##\n     @@\n    -+#!/usr/bin/env bash\n    -+\n    -+cd \"$(dirname \"${BASH_SOURCE[0]}\")\"/..\n    ++#!/bin/sh\n     +\n     +extract_variable () {\n     +\t(\n    -+\t\tcat Makefile\n    ++\t\tcat ../Makefile\n     +\t\tcat <<EOF\n     +print_variable:\n    -+\t\\$(foreach b,\\$($1),echo XXX \\$(b:\\$X=) YYY;)\n    ++\t@\\$(foreach b,\\$($1),echo XXX \\$(b:\\$X=) YYY;)\n     +EOF\n     +\t) |\n    -+\tmake -f - print_variable 2>/dev/null |\n    ++\tmake -C .. -f - print_variable 2>/dev/null |\n     +\tsed -n -e 's/.*XXX \\(.*\\) YYY.*/\\1/p'\n     +}\n     +\n     +check_missing_docs () {\n    -+\tfor v in $BUILT_INS\n    ++\tfor v in $ALL_COMMANDS\n     +\tdo\n     +\t\tcase \"$v\" in\n     +\t\tgit-merge-octopus) continue;;\n    @@ Documentation/lint-manpages.sh (new)\n     +\t\tgit-?*--?* ) continue ;;\n     +\t\tesac\n     +\n    -+\t\tif ! test -f \"Documentation/$v.txt\"\n    ++\t\tif ! test -f \"$v.txt\"\n     +\t\tthen\n     +\t\t\techo \"no doc: $v\"\n     +\t\tfi\n     +\n    -+\t\tif ! sed -e '1,/^### command list/d' -e '/^#/d' command-list.txt |\n    ++\t\tif ! sed -e '1,/^### command list/d' -e '/^#/d' ../command-list.txt |\n     +\t\t\tgrep -q \"^$v[ \t]\"\n     +\t\tthen\n     +\t\t\tcase \"$v\" in\n    @@ Documentation/lint-manpages.sh (new)\n     +}\n     +\n     +check_extraneous_docs () {\n    -+\tlocal commands=\"$(printf \"%s\\n\" \"$ALL_COMMANDS\" \"$BUILT_INS\" \"$EXCLUDED_PROGRAMS\")\"\n    -+\n    -+\twhile read how cmd\n    -+\tdo\n    -+\t\tif ! [[ $commands = *\"$cmd\"* ]]\n    -+\t\tthen\n    -+\t\t\techo \"removed but $how: $cmd\"\n    -+\t\tfi\n    -+\tdone < <(\n    ++\t(\n     +\t\tsed -e '1,/^### command list/d' \\\n     +\t\t    -e '/^#/d' \\\n     +\t\t    -e '/guide$/d' \\\n     +\t\t    -e '/interfaces$/d' \\\n     +\t\t    -e 's/[ \t].*//' \\\n    -+\t\t    -e 's/^/listed /' command-list.txt\n    -+\t\tmake -C Documentation print-man1 |\n    ++\t\t    -e 's/^/listed /' ../command-list.txt\n    ++\t\tmake print-man1 |\n     +\t\tgrep '\\.txt$' |\n     +\t\tsed -e 's|^|documented |' \\\n     +\t\t    -e 's/\\.txt//'\n    ++\t) | (\n    ++\t\tall_commands=\"$(printf \"%s \" \"$ALL_COMMANDS\" \"$BUILT_INS\" \"$EXCLUDED_PROGRAMS\" | tr '\\n' ' ')\"\n    ++\n    ++\t\twhile read how cmd\n    ++\t\tdo\n    ++\t\t\tcase \" $all_commands \" in\n    ++\t\t\t*\" $cmd \"*) ;;\n    ++\t\t\t*)\n    ++\t\t\t\techo \"removed but $how: $cmd\";;\n    ++\t\t\tesac\n    ++\t\tdone\n     +\t)\n     +}\n     +\n2:  b39a780d33 ! 2:  9f21c305b9 Documentation/lint-manpages: bubble up errors\n    @@ Commit message\n     \n      ## Documentation/lint-manpages.sh ##\n     @@ Documentation/lint-manpages.sh: EOF\n    + \tsed -n -e 's/.*XXX \\(.*\\) YYY.*/\\1/p'\n      }\n      \n    - check_missing_docs () {\n    -+\tlocal ret=0\n    +-check_missing_docs () {\n    ++check_missing_docs () (\n    ++\tret=0\n     +\n    - \tfor v in $BUILT_INS\n    + \tfor v in $ALL_COMMANDS\n      \tdo\n      \t\tcase \"$v\" in\n     @@ Documentation/lint-manpages.sh: check_missing_docs () {\n    - \t\tif ! test -f \"Documentation/$v.txt\"\n    + \t\tif ! test -f \"$v.txt\"\n      \t\tthen\n      \t\t\techo \"no doc: $v\"\n     +\t\t\tret=1\n      \t\tfi\n      \n    - \t\tif ! sed -e '1,/^### command list/d' -e '/^#/d' command-list.txt |\n    + \t\tif ! sed -e '1,/^### command list/d' -e '/^#/d' ../command-list.txt |\n     @@ Documentation/lint-manpages.sh: check_missing_docs () {\n      \t\t\tgit)\n      \t\t\t\t;;\n    @@ Documentation/lint-manpages.sh: check_missing_docs () {\n      \t\t\tesac\n      \t\tfi\n      \tdone\n    +-}\n     +\n    -+\treturn $ret\n    - }\n    ++\texit $ret\n    ++)\n      \n      check_extraneous_docs () {\n    - \tlocal commands=\"$(printf \"%s\\n\" \"$ALL_COMMANDS\" \"$BUILT_INS\" \"$EXCLUDED_PROGRAMS\")\"\n    -+\tlocal ret=0\n    - \n    - \twhile read how cmd\n    - \tdo\n    - \t\tif ! [[ $commands = *\"$cmd\"* ]]\n    - \t\tthen\n    - \t\t\techo \"removed but $how: $cmd\"\n    -+\t\t\tret=1\n    - \t\tfi\n    - \tdone < <(\n    - \t\tsed -e '1,/^### command list/d' \\\n    + \t(\n     @@ Documentation/lint-manpages.sh: check_extraneous_docs () {\n    - \t\tsed -e 's|^|documented |' \\\n      \t\t    -e 's/\\.txt//'\n    - \t)\n    + \t) | (\n    + \t\tall_commands=\"$(printf \"%s \" \"$ALL_COMMANDS\" \"$BUILT_INS\" \"$EXCLUDED_PROGRAMS\" | tr '\\n' ' ')\"\n    ++\t\tret=0\n    + \n    + \t\twhile read how cmd\n    + \t\tdo\n    + \t\t\tcase \" $all_commands \" in\n    + \t\t\t*\" $cmd \"*) ;;\n    + \t\t\t*)\n    +-\t\t\t\techo \"removed but $how: $cmd\";;\n    ++\t\t\t\techo \"removed but $how: $cmd\"\n    ++\t\t\t\tret=1;;\n    + \t\t\tesac\n    + \t\tdone\n     +\n    -+\treturn $ret\n    ++\t\texit $ret\n    + \t)\n      }\n      \n    - BUILT_INS=\"$(extract_variable BUILT_INS)\"\n    +@@ Documentation/lint-manpages.sh: BUILT_INS=\"$(extract_variable BUILT_INS)\"\n      ALL_COMMANDS=\"$(extract_variable ALL_COMMANDS)\"\n      EXCLUDED_PROGRAMS=\"$(extract_variable EXCLUDED_PROGRAMS)\"\n      \n    @@ Documentation/lint-manpages.sh: check_extraneous_docs () {\n     +)\n     +ret=$?\n     +\n    -+echo \"$findings\" | sort\n    ++printf \"%s\" \"$findings\" | sort\n     +\n     +exit $ret\n3:  a44d57a732 = 3:  d6d3628797 gitlab-ci: add job to run `make check-docs`\n4:  c758b45282 = 4:  c13fed9ebf ci/test-documentation: work around SyntaxWarning in Python 3.12\n-- \n2.45.2.409.g7b0defb391.dirty\n\n"},{"id":"496452","messageId":"489a6eaf2d328044307f38ff23dd35ecc5a4c515.1717660597.git.ps@pks.im","threadId":"61586","inReplyTo":"cover.1717660597.git.ps@pks.im","subject":"[PATCH v2 1/4] Makefile: extract script to lint missing/extraneous manpages","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-06-06T08:01:00Z","receivedAt":"2024-06-06T08:01:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"check-docs\" target of our top-level Makefile fulfills two different\nroles. For one it runs the \"lint-docs\" target of the \"Documentation/\"\nMakefile. And second it performs some checks of whether there are any\nmanpages that are missing or extraneous via some inline scripts.\n\nThe second set of checks feels quite misplaced in the top-level Makefile\nas it would fit in much better with our \"lint-docs\" target. Back when\nthe checks were introduced in 8c989ec528 (Makefile: $(MAKE) check-docs,\n2006-04-13), that target did not yet exist though.\n\nFurthermore, the script makes use of several Makefile variables which\nare defined in the top-level Makefile, which makes it hard to access\ntheir contents from elsewhere. There is a trick though that we already\nuse in \"check-builtins.sh\" to gain access: we can create an ad-hoc\nMakefile that has an extra target to print those variables.\n\nPull out the script into a separate \"lint-manpages.sh\" script by using\nthat trick. Wire up that script via the \"lint-docs\" target. For one,\nnormal shell scripts are way easier to reason about than those which are\nembedded in a Makefile. Second, it allows one to easily execute the\nscript standalone without any of the other checks.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/Makefile         |  4 ++\n Documentation/lint-manpages.sh | 83 ++++++++++++++++++++++++++++++++++\n Makefile                       | 36 ---------------\n 3 files changed, 87 insertions(+), 36 deletions(-)\n create mode 100755 Documentation/lint-manpages.sh\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex a04da672c6..a3868a462f 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -485,12 +485,16 @@ $(LINT_DOCS_FSCK_MSGIDS): ../fsck.h fsck-msgids.txt\n \n lint-docs-fsck-msgids: $(LINT_DOCS_FSCK_MSGIDS)\n \n+lint-docs-manpages:\n+\t$(QUIET_GEN)./lint-manpages.sh\n+\n ## Lint: list of targets above\n .PHONY: lint-docs\n lint-docs: lint-docs-fsck-msgids\n lint-docs: lint-docs-gitlink\n lint-docs: lint-docs-man-end-blurb\n lint-docs: lint-docs-man-section-order\n+lint-docs: lint-docs-manpages\n \n ifeq ($(wildcard po/Makefile),po/Makefile)\n doc-l10n install-l10n::\ndiff --git a/Documentation/lint-manpages.sh b/Documentation/lint-manpages.sh\nnew file mode 100755\nindex 0000000000..0abb4e0b4c\n--- /dev/null\n+++ b/Documentation/lint-manpages.sh\n@@ -0,0 +1,83 @@\n+#!/bin/sh\n+\n+extract_variable () {\n+\t(\n+\t\tcat ../Makefile\n+\t\tcat <<EOF\n+print_variable:\n+\t@\\$(foreach b,\\$($1),echo XXX \\$(b:\\$X=) YYY;)\n+EOF\n+\t) |\n+\tmake -C .. -f - print_variable 2>/dev/null |\n+\tsed -n -e 's/.*XXX \\(.*\\) YYY.*/\\1/p'\n+}\n+\n+check_missing_docs () {\n+\tfor v in $ALL_COMMANDS\n+\tdo\n+\t\tcase \"$v\" in\n+\t\tgit-merge-octopus) continue;;\n+\t\tgit-merge-ours) continue;;\n+\t\tgit-merge-recursive) continue;;\n+\t\tgit-merge-resolve) continue;;\n+\t\tgit-merge-subtree) continue;;\n+\t\tgit-fsck-objects) continue;;\n+\t\tgit-init-db) continue;;\n+\t\tgit-remote-*) continue;;\n+\t\tgit-stage) continue;;\n+\t\tgit-legacy-*) continue;;\n+\t\tgit-?*--?* ) continue ;;\n+\t\tesac\n+\n+\t\tif ! test -f \"$v.txt\"\n+\t\tthen\n+\t\t\techo \"no doc: $v\"\n+\t\tfi\n+\n+\t\tif ! sed -e '1,/^### command list/d' -e '/^#/d' ../command-list.txt |\n+\t\t\tgrep -q \"^$v[ \t]\"\n+\t\tthen\n+\t\t\tcase \"$v\" in\n+\t\t\tgit)\n+\t\t\t\t;;\n+\t\t\t*)\n+\t\t\t\techo \"no link: $v\";;\n+\t\t\tesac\n+\t\tfi\n+\tdone\n+}\n+\n+check_extraneous_docs () {\n+\t(\n+\t\tsed -e '1,/^### command list/d' \\\n+\t\t    -e '/^#/d' \\\n+\t\t    -e '/guide$/d' \\\n+\t\t    -e '/interfaces$/d' \\\n+\t\t    -e 's/[ \t].*//' \\\n+\t\t    -e 's/^/listed /' ../command-list.txt\n+\t\tmake print-man1 |\n+\t\tgrep '\\.txt$' |\n+\t\tsed -e 's|^|documented |' \\\n+\t\t    -e 's/\\.txt//'\n+\t) | (\n+\t\tall_commands=\"$(printf \"%s \" \"$ALL_COMMANDS\" \"$BUILT_INS\" \"$EXCLUDED_PROGRAMS\" | tr '\\n' ' ')\"\n+\n+\t\twhile read how cmd\n+\t\tdo\n+\t\t\tcase \" $all_commands \" in\n+\t\t\t*\" $cmd \"*) ;;\n+\t\t\t*)\n+\t\t\t\techo \"removed but $how: $cmd\";;\n+\t\t\tesac\n+\t\tdone\n+\t)\n+}\n+\n+BUILT_INS=\"$(extract_variable BUILT_INS)\"\n+ALL_COMMANDS=\"$(extract_variable ALL_COMMANDS)\"\n+EXCLUDED_PROGRAMS=\"$(extract_variable EXCLUDED_PROGRAMS)\"\n+\n+{\n+\tcheck_missing_docs\n+\tcheck_extraneous_docs\n+} | sort\ndiff --git a/Makefile b/Makefile\nindex 59d98ba688..84e2aa9686 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3757,42 +3757,6 @@ ALL_COMMANDS += scalar\n .PHONY: check-docs\n check-docs::\n \t$(MAKE) -C Documentation lint-docs\n-\t@(for v in $(patsubst %$X,%,$(ALL_COMMANDS)); \\\n-\tdo \\\n-\t\tcase \"$$v\" in \\\n-\t\tgit-merge-octopus | git-merge-ours | git-merge-recursive | \\\n-\t\tgit-merge-resolve | git-merge-subtree | \\\n-\t\tgit-fsck-objects | git-init-db | \\\n-\t\tgit-remote-* | git-stage | git-legacy-* | \\\n-\t\tgit-?*--?* ) continue ;; \\\n-\t\tesac ; \\\n-\t\ttest -f \"Documentation/$$v.txt\" || \\\n-\t\techo \"no doc: $$v\"; \\\n-\t\tsed -e '1,/^### command list/d' -e '/^#/d' command-list.txt | \\\n-\t\tgrep -q \"^$$v[ \t]\" || \\\n-\t\tcase \"$$v\" in \\\n-\t\tgit) ;; \\\n-\t\t*) echo \"no link: $$v\";; \\\n-\t\tesac ; \\\n-\tdone; \\\n-\t( \\\n-\t\tsed -e '1,/^### command list/d' \\\n-\t\t    -e '/^#/d' \\\n-\t\t    -e '/guide$$/d' \\\n-\t\t    -e '/interfaces$$/d' \\\n-\t\t    -e 's/[ \t].*//' \\\n-\t\t    -e 's/^/listed /' command-list.txt; \\\n-\t\t$(MAKE) -C Documentation print-man1 | \\\n-\t\tgrep '\\.txt$$' | \\\n-\t\tsed -e 's|^|documented |' \\\n-\t\t    -e 's/\\.txt//'; \\\n-\t) | while read how cmd; \\\n-\tdo \\\n-\t\tcase \" $(patsubst %$X,%,$(ALL_COMMANDS) $(BUILT_INS) $(EXCLUDED_PROGRAMS)) \" in \\\n-\t\t*\" $$cmd \"*)\t;; \\\n-\t\t*) echo \"removed but $$how: $$cmd\" ;; \\\n-\t\tesac; \\\n-\tdone ) | sort\n \n ### Make sure built-ins do not have dups and listed in git.c\n #\n-- \n2.45.2.409.g7b0defb391.dirty\n\n"},{"id":"496453","messageId":"9f21c305b9e0e63ddf1f1b78535f09ae957812d4.1717660597.git.ps@pks.im","threadId":"61586","inReplyTo":"cover.1717660597.git.ps@pks.im","subject":"[PATCH v2 2/4] Documentation/lint-manpages: bubble up errors","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-06-06T08:01:05Z","receivedAt":"2024-06-06T08:01:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"lint-manpages.sh\" script does not return an error in case any of\nits checks fail. While this is faithful to the implementation that we\nhad as part of the \"check-docs\" target before the preceding commit, it\nmakes it hard to spot any violations of the rules via the corresponding\nCI job, which will of course exit successfully, too.\n\nAdapt the script to bubble up errors.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/lint-manpages.sh | 41 +++++++++++++++++++++++++++-------\n 1 file changed, 33 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/lint-manpages.sh b/Documentation/lint-manpages.sh\nindex 0abb4e0b4c..92cfc0a15a 100755\n--- a/Documentation/lint-manpages.sh\n+++ b/Documentation/lint-manpages.sh\n@@ -12,7 +12,9 @@ EOF\n \tsed -n -e 's/.*XXX \\(.*\\) YYY.*/\\1/p'\n }\n \n-check_missing_docs () {\n+check_missing_docs () (\n+\tret=0\n+\n \tfor v in $ALL_COMMANDS\n \tdo\n \t\tcase \"$v\" in\n@@ -32,6 +34,7 @@ check_missing_docs () {\n \t\tif ! test -f \"$v.txt\"\n \t\tthen\n \t\t\techo \"no doc: $v\"\n+\t\t\tret=1\n \t\tfi\n \n \t\tif ! sed -e '1,/^### command list/d' -e '/^#/d' ../command-list.txt |\n@@ -41,11 +44,15 @@ check_missing_docs () {\n \t\t\tgit)\n \t\t\t\t;;\n \t\t\t*)\n-\t\t\t\techo \"no link: $v\";;\n+\t\t\t\techo \"no link: $v\"\n+\t\t\t\tret=1\n+\t\t\t\t;;\n \t\t\tesac\n \t\tfi\n \tdone\n-}\n+\n+\texit $ret\n+)\n \n check_extraneous_docs () {\n \t(\n@@ -61,15 +68,19 @@ check_extraneous_docs () {\n \t\t    -e 's/\\.txt//'\n \t) | (\n \t\tall_commands=\"$(printf \"%s \" \"$ALL_COMMANDS\" \"$BUILT_INS\" \"$EXCLUDED_PROGRAMS\" | tr '\\n' ' ')\"\n+\t\tret=0\n \n \t\twhile read how cmd\n \t\tdo\n \t\t\tcase \" $all_commands \" in\n \t\t\t*\" $cmd \"*) ;;\n \t\t\t*)\n-\t\t\t\techo \"removed but $how: $cmd\";;\n+\t\t\t\techo \"removed but $how: $cmd\"\n+\t\t\t\tret=1;;\n \t\t\tesac\n \t\tdone\n+\n+\t\texit $ret\n \t)\n }\n \n@@ -77,7 +88,21 @@ BUILT_INS=\"$(extract_variable BUILT_INS)\"\n ALL_COMMANDS=\"$(extract_variable ALL_COMMANDS)\"\n EXCLUDED_PROGRAMS=\"$(extract_variable EXCLUDED_PROGRAMS)\"\n \n-{\n-\tcheck_missing_docs\n-\tcheck_extraneous_docs\n-} | sort\n+findings=$(\n+\tif ! check_missing_docs\n+\tthen\n+\t\tret=1\n+\tfi\n+\n+\tif ! check_extraneous_docs\n+\tthen\n+\t\tret=1\n+\tfi\n+\n+\texit $ret\n+)\n+ret=$?\n+\n+printf \"%s\" \"$findings\" | sort\n+\n+exit $ret\n-- \n2.45.2.409.g7b0defb391.dirty\n\n"},{"id":"496454","messageId":"d6d3628797a70ffff32676d3fd64f5179b19c94d.1717660597.git.ps@pks.im","threadId":"61586","inReplyTo":"cover.1717660597.git.ps@pks.im","subject":"[PATCH v2 3/4] gitlab-ci: add job to run `make check-docs`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-06-06T08:01:10Z","receivedAt":"2024-06-06T08:01:15Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Add another job to execute `make check-docs`, which lints our\ndocumentation and makes sure that expected manpages exist. This job\nmirrors the same job that we already have for GitHub Actions.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n .gitlab-ci.yml | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\nindex f676959ca0..37b991e080 100644\n--- a/.gitlab-ci.yml\n+++ b/.gitlab-ci.yml\n@@ -122,3 +122,12 @@ check-whitespace:\n     - ./ci/check-whitespace.sh \"$CI_MERGE_REQUEST_TARGET_BRANCH_SHA\"\n   rules:\n     - if: $CI_PIPELINE_SOURCE == 'merge_request_event'\n+\n+documentation:\n+  image: ubuntu:latest\n+  variables:\n+    jobname: Documentation\n+  before_script:\n+    - ./ci/install-dependencies.sh\n+  script:\n+    - ./ci/test-documentation.sh\n-- \n2.45.2.409.g7b0defb391.dirty\n\n"},{"id":"496455","messageId":"c13fed9ebf2ff4a68e12661c31ac4be4405028b3.1717660597.git.ps@pks.im","threadId":"61586","inReplyTo":"cover.1717660597.git.ps@pks.im","subject":"[PATCH v2 4/4] ci/test-documentation: work around SyntaxWarning in Python 3.12","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-06-06T08:01:14Z","receivedAt":"2024-06-06T08:01:19Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In Python 3.6, unrecognized escape sequences in regular expressions\nstarted to produce a DeprecationWarning [1]. In Python 3.12, this was\nupgraded to a SyntaxWarning and will eventually be raised even further\nto a SyntaxError. We indirectly hit such unrecognized escape sequences\nvia Asciidoc, which results in a bunch of warnings:\n\n    $ asciidoc -o /dev/null git-cat-file.txt\n    <unknown>:1: SyntaxWarning: invalid escape sequence '\\S'\n    <unknown>:1: SyntaxWarning: invalid escape sequence '\\S'\n\nThis in turn causes our \"ci/test-documentation.sh\" script to fail, as it\nchecks that stderr of `make doc` is empty.\n\nThese escape sequences seem to be part of Asciidoc itself. In the long\nterm, we should probably consider dropping support for Asciidoc in favor\nof Asciidoctor. Upstream also considers itself to be legacy software and\nrecommends to move away from it [2]:\n\n    It is suggested that unless you specifically require the AsciiDoc.py\n    toolchain, you should find a processor that handles the modern\n    AsciiDoc syntax.\n\nFor now though, let's expand its lifetime a little bit more by filtering\nout these new warnings. We should probably reconsider once the warnings\nare upgraded to errors by Python.\n\n[1]: https://docs.python.org/3/reference/lexical_analysis.html#string-and-bytes-literals\n[2]: https://github.com/asciidoc-py/asciidoc-py/blob/6d9f76cff0dc3b7ca21bdd570200f8518464d99b/README.md#asciidocpy\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n ci/test-documentation.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/ci/test-documentation.sh b/ci/test-documentation.sh\nindex de41888430..02b3af3941 100755\n--- a/ci/test-documentation.sh\n+++ b/ci/test-documentation.sh\n@@ -11,6 +11,7 @@ filter_log () {\n \t    -e '/^    \\* new asciidoc flags$/d' \\\n \t    -e '/stripped namespace before processing/d' \\\n \t    -e '/Attributed.*IDs for element/d' \\\n+\t    -e '/SyntaxWarning: invalid escape sequence/d' \\\n \t    \"$1\"\n }\n \n-- \n2.45.2.409.g7b0defb391.dirty\n\n"},{"id":"496595","messageId":"D1TDLY0U13M8.34988PP0ORBD9@jamesliu.io","threadId":"61586","inReplyTo":"cover.1717660597.git.ps@pks.im","subject":"Re: [PATCH v2 0/4] Documentation: improve linting for manpage existence","fromName":"James Liu","fromEmail":"james@jamesliu.io","sentAt":"2024-06-07T00:59:01Z","receivedAt":"2024-06-07T00:59:05Z","isPatch":true,"sender":{"key":"james@jamesliu.io","avatar":"https://avatars.githubusercontent.com/u/4476568?v=4"},"body":"On Thu Jun 6, 2024 at 6:00 PM AEST, Patrick Steinhardt wrote:\n> Hi,\n>\n> this is the second version of my patch series that improves linting for\n> the existence and/or absence of manpages.\n>\n> Changes compared to v1:\n>\n>   - Converted the extracted script to be compatible with POSIX shells.\n>\n>   - Fix an empty line being printed when there are no findings.\n>\n>   - Silence the patched-in Makefile recipe, which otherwise leads to one\n>     value existing twice in the converted output.\n>\n>   - Fix `check_missing_docs()` to use `ALL_COMMANDS`, not `BUILT_INS`.\n>\n> Thanks!\n>\n> Patrick\n\nThanks Patrick, this looks good to me! Learnt a few shell tricks too.\n\nCheers,\nJames\n"},{"id":"496656","messageId":"xmqqsexoelw5.fsf@gitster.g","threadId":"61586","inReplyTo":"489a6eaf2d328044307f38ff23dd35ecc5a4c515.1717660597.git.ps@pks.im","subject":"Re: [PATCH v2 1/4] Makefile: extract script to lint missing/extraneous manpages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-07T16:51:54Z","receivedAt":"2024-06-07T16:51:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> The \"check-docs\" target of our top-level Makefile fulfills two different\n> roles. For one it runs the \"lint-docs\" target of the \"Documentation/\"\n> Makefile. And second it performs some checks of whether there are any\n> manpages that are missing or extraneous via some inline scripts.\n\nYes.\n\n> The second set of checks feels quite misplaced in the top-level Makefile\n> as it would fit in much better with our \"lint-docs\" target. Back when\n> the checks were introduced in 8c989ec528 (Makefile: $(MAKE) check-docs,\n> 2006-04-13), that target did not yet exist though.\n\nCorrect.\n\n> Furthermore, the script makes use of several Makefile variables which\n> are defined in the top-level Makefile, which makes it hard to access\n> their contents from elsewhere. There is a trick though that we already\n> use in \"check-builtins.sh\" to gain access: we can create an ad-hoc\n> Makefile that has an extra target to print those variables.\n\nYes.\n\n> Pull out the script into a separate \"lint-manpages.sh\" script by using\n> that trick. Wire up that script via the \"lint-docs\" target. For one,\n> normal shell scripts are way easier to reason about than those which are\n> embedded in a Makefile. Second, it allows one to easily execute the\n> script standalone without any of the other checks.\n\nNicely done.  We might want to stop doing \"make print-man1\" in the\nlint script, remove \"print-man1\" target from Documentation/Makefile,\nand instead extract MAN1_TXT using the same \"it is unlikely to have\nan output line enclosing something in between XXX and YYY\" trick,\nbut it may not be worth it.\n\n> +check_missing_docs () {\n> +...\n> +\t\tgit-merge-octopus) continue;;\n> +\t\tgit-merge-ours) continue;;\n> +\t\tgit-merge-recursive) continue;;\n> +\t\tgit-merge-resolve) continue;;\n> +\t\tgit-merge-subtree) continue;;\n> +\t\tgit-fsck-objects) continue;;\n> +\t\tgit-init-db) continue;;\n> +\t\tgit-remote-*) continue;;\n> +\t\tgit-stage) continue;;\n> +\t\tgit-legacy-*) continue;;\n> +\t\tgit-?*--?* ) continue ;;\n\nThis looks easier to maintain than the original by listing one\npattern per line.  Again, nicely done.\n\n\n"}]}