All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Fernando Ramos" <greenfoo@u92.eu>,
	"Matthias Aßhauer" <mha1993@live.de>,
	"Lars Schneider" <larsxschneider@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH] tests: do roundtrip builtin doc & sanity checking
Date: Sun, 27 Mar 2022 15:18:20 +0200	[thread overview]
Message-ID: <patch-1.1-4a5d219dfe7-20220327T131751Z-avarab@gmail.com> (raw)
In-Reply-To: <xmqq4k3k6xw3.fsf@gitster.g>

Replace the part of "make check-docs" first added in [1] and the
entirety of "make check-builtins" first added in [2] with checks that
do checking against --list-cmds=main (see [3]) and that our generated
"man" docs instead.

The "check-docs" part of this was mostly broken anyway, in that we
were ignoring the exit code. Since b6d8887d3d6 (documentation: add
documentation for 'git version', 2021-09-14) we started emitting:

	$ make -s check-docs
	removed but documented: git-version

But as noted in [4] the check didn't fail.

By checking that we get the man page when running "git $cmd --help" we
can be sure that we don't miss anything. To do this add a
GIT_TEST_MANPATH, as we don't want to append to the manpath as we'd
pick up the MANPATH on the system.

There is no replacement provided for the "$base is builtin but
$builtin.$sfx still exists" part of the "check-builtins" check being
removed here. Back when it was added various commands were being
migrated to C at a rapid rate, now we have just a few stragglers
in *.sh and *.perl. It's unlikely that we'd forget to remove the old
code.

1. 8c989ec5288 (Makefile: $(MAKE) check-docs, 2006-04-13)
2. c74390e4a1d (cherry is built-in, do not ship git-cherry.sh,
   2006-11-05)
3. 6bb2dc0b947 (completion: implement and use --list-cmds=main,others,
   2018-05-20)
4. https://lore.kernel.org/git/87o88i2keu.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile     |  3 --
 Makefile                   | 39 +----------------
 builtin/help.c             | 12 +++++-
 check-builtins.sh          | 34 ---------------
 ci/install-dependencies.sh |  3 ++
 t/t0012-help.sh            | 86 +++++++++++++++++++++++++++++++++++++-
 6 files changed, 101 insertions(+), 76 deletions(-)
 delete mode 100755 check-builtins.sh

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 1eb9192dae8..a2c6ef479e0 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -431,9 +431,6 @@ require-htmlrepo::
 quick-install-html: require-htmlrepo
 	'$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REPO) $(DESTDIR)$(htmldir) $(GIT_MAN_REF)
 
-print-man1:
-	@for i in $(MAN1_TXT); do echo $$i; done
-
 ## Lint: gitlink
 LINT_DOCS_GITLINK = $(patsubst %.txt,.build/lint-docs/gitlink/%.ok,$(HOWTO_TXT) $(DOC_DEP_TXT))
 $(LINT_DOCS_GITLINK): lint-gitlink.perl
diff --git a/Makefile b/Makefile
index e8aba291d7f..c5b83f80a3a 100644
--- a/Makefile
+++ b/Makefile
@@ -3331,46 +3331,11 @@ ALL_COMMANDS += gitweb
 .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 '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
 #
-check-builtins::
-	./check-builtins.sh
+check-builtins:: all man
+	$(MAKE) -C t T=t0012-help.sh
 
 ### Test suite coverage testing
 #
diff --git a/builtin/help.c b/builtin/help.c
index 222f994f863..775eee76c05 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -434,7 +434,7 @@ static const char *cmd_to_page(const char *git_cmd)
 		return xstrfmt("git%s", git_cmd);
 }
 
-static void setup_man_path(void)
+static void setup_real_man_path(void)
 {
 	struct strbuf new_path = STRBUF_INIT;
 	const char *old_path = getenv("MANPATH");
@@ -455,6 +455,16 @@ static void setup_man_path(void)
 	strbuf_release(&new_path);
 }
 
+static void setup_man_path(void)
+{
+	const char *test_manpath = getenv("GIT_TEST_MANPATH");
+
+	if (test_manpath)
+		xsetenv("MANPATH", test_manpath, 1);
+	else
+		setup_real_man_path();
+}
+
 static void exec_viewer(const char *name, const char *page)
 {
 	const char *info = get_man_viewer_info(name);
diff --git a/check-builtins.sh b/check-builtins.sh
deleted file mode 100755
index a0aaf3a3473..00000000000
--- a/check-builtins.sh
+++ /dev/null
@@ -1,34 +0,0 @@
-#!/bin/sh
-
-{
-	cat <<\EOF
-sayIt:
-	$(foreach b,$(BUILT_INS),echo XXX $(b:$X=) YYY;)
-EOF
-	cat Makefile
-} |
-make -f - sayIt 2>/dev/null |
-sed -n -e 's/.*XXX \(.*\) YYY.*/\1/p' |
-sort |
-{
-    bad=0
-    while read builtin
-    do
-	base=$(expr "$builtin" : 'git-\(.*\)')
-	x=$(sed -ne 's/.*{ "'$base'", \(cmd_[^, ]*\).*/'$base'	\1/p' git.c)
-	if test -z "$x"
-	then
-		echo "$base is builtin but not listed in git.c command list"
-		bad=1
-	fi
-	for sfx in sh perl py
-	do
-		if test -f "$builtin.$sfx"
-		then
-			echo "$base is builtin but $builtin.$sfx still exists"
-			bad=1
-		fi
-	done
-    done
-    exit $bad
-}
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index dbcebad2fb2..1df7c908db0 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -71,6 +71,9 @@ Documentation)
 
 	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
 	sudo gem install --version 1.5.8 asciidoctor
+
+	sudo apt-get -q -y install libcurl4-openssl-dev libssl-dev \
+		libexpat-dev gettext make
 	;;
 linux-gcc-default)
 	sudo apt-get -q update
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 6c3e1f7159d..522eb1b492d 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -234,7 +234,9 @@ test_expect_success "'git help -g' section spacing" '
 
 test_expect_success 'generate builtin list' '
 	mkdir -p sub &&
-	git --list-cmds=builtins >builtins
+	git --list-cmds=builtins >builtins &&
+	uniq builtins >builtins.uniq &&
+	test_cmp builtins builtins.uniq
 '
 
 while read builtin
@@ -249,4 +251,86 @@ do
 	'
 done <builtins
 
+
+symlink_test_manpath () {
+	local doc="$GIT_BUILD_DIR"/Documentation &&
+	test_path_exists "$doc"/git-add.1 &&
+	test_when_finished "rm -f man1" &&
+	ln -s "$doc" man1
+}
+
+# Use "git-add" as a guinea pig, and check the basic sanity of the
+# output.
+test_lazy_prereq HAVE_BUILT_DOCS '
+	symlink_test_manpath &&
+	test_when_finished "rm -f man.txt" &&
+	GIT_TEST_MANPATH="$PWD" git add --help >man.txt &&
+	grep GIT-ADD man.txt &&
+	grep ^SYNOPSIS man.txt
+'
+
+is_documented () {
+	cat >undocumented <<-\EOF
+	add--interactive
+	bisect--helper
+	checkout--worker
+	difftool--helper
+	env--helper
+	merge-octopus
+	merge-ours
+	merge-recursive
+	merge-recursive-ours
+	merge-recursive-theirs
+	merge-resolve
+	merge-subtree
+	pickaxe
+	remote-ftp
+	remote-ftps
+	remote-http
+	remote-https
+	submodule--helper
+	upload-archive--writer
+	EOF
+	! grep -q "^$1$" undocumented
+}
+
+test_undocumented () {
+	cmd=$1 &&
+	test_expect_success HAVE_BUILT_DOCS "$cmd does not have --help documentation" '
+		symlink_test_manpath &&
+		test_when_finished "rm -f man.txt" &&
+		test_must_fail env GIT_TEST_MANPATH="$PWD" git $cmd --help >man.txt
+	'
+}
+
+test_documented () {
+	cmd=$1 &&
+	test_expect_success HAVE_BUILT_DOCS "$cmd can handle --help" '
+		symlink_test_manpath &&
+		test_when_finished "rm -f man.txt" &&
+		GIT_TEST_MANPATH="$PWD" git $cmd --help >man.txt &&
+		grep "git-$cmd" man.txt
+	'
+}
+
+test_expect_success 'generate main list' '
+	mkdir -p sub &&
+	git --list-cmds=main >main
+'
+
+while read cmd
+do
+	case "$cmd" in
+	*.sh|*.perl|*.py)
+		    continue
+		    ;;
+	esac &&
+	if is_documented "$cmd"
+	then
+		test_documented "$cmd"
+	else
+		test_undocumented "$cmd"
+	fi
+done <main
+
 test_done
-- 
2.35.1.1535.gc79a163ff4c


      reply	other threads:[~2022-03-27 13:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-24  3:13 What's cooking in git.git (Mar 2022, #05; Wed, 23) Junio C Hamano
2022-03-25 13:38 ` ab/commit-plug-leaks (was: What's cooking in git.git (Mar 2022, #05; Wed, 23)) Ævar Arnfjörð Bjarmason
2022-03-26 16:05 ` fr/vimdiff-layout " Ævar Arnfjörð Bjarmason
2022-03-26 22:55   ` Fernando Ramos
2022-03-27  0:29     ` fr/vimdiff-layout Junio C Hamano
2022-03-27 13:18       ` Ævar Arnfjörð Bjarmason [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-1.1-4a5d219dfe7-20220327T131751Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=greenfoo@u92.eu \
    --cc=larsxschneider@gmail.com \
    --cc=mha1993@live.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.