Skip to contentSkip to conversation

Assorted Documentation-related fixes

PATCH Superseded Johannes Schindelin via GitGitGadget 6 participants 41 messages Apr 12, 2019 – Apr 29, 2019

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover Assorted Documentation-related fixes

While working on better support for make check-docs on Windows, I noticed a couple more things, e.g. some "commands" were reported as being listed but not built, e.g. gitcli (i.e. the parts of command-list.txt that are marked as "guide").

This patch series cleans up those loose ends: after this, make check-docs reports no issues on Windows.

Johannes Schindelin (7):
remote-testgit: move it into the support directory for t5801 help -a: do not list commands that are excluded from the build
check-docs: do not pretend that commands are listed which are excluded
docs: exclude documentation for commands that have been excluded
check-docs: do not bother checking for legacy scripts' documentation
test-tool: handle the -C <directory> option just like git Turn git serve into a test helper

 .gitignore                                    |  1 -
 Documentation/.gitignore                      |  1 +
 Documentation/Makefile                        |  3 +
 Makefile                                      | 58 +++++++++++--------
 builtin.h                                     |  1 -
 generate-cmdlist.sh                           | 10 +++-
 git.c                                         |  1 -
 builtin/serve.c => t/helper/test-serve-v2.c   |  7 ++-
 t/helper/test-tool.c                          | 20 +++++++
 t/helper/test-tool.h                          |  1 +
 t/t5701-git-serve.sh                          | 32 +++++-----
 t/t5702-protocol-v2.sh                        |  5 +-
 t/t5703-upload-pack-ref-in-want.sh            | 16 ++---
 t/t5801-remote-helpers.sh                     |  2 +
 .../t5801/git-remote-testgit                  |  0
 15 files changed, 104 insertions(+), 54 deletions(-)
 rename builtin/serve.c => t/helper/test-serve-v2.c (81%)
 rename git-remote-testgit.sh => t/t5801/git-remote-testgit (100%)

base-commit: 5ee42463399ca3cc75b7e6e4368a3a5df5b010f2
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-168%2Fdscho%2Fdocs-misc-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-168/dscho/docs-misc-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/168

Signature · 1 line
gitgitgadget
#1 lore raw

PATCH 1/7 remote-testgit: move it into the support directory for t5801

From: Johannes Schindelin <[email protected]>

The git-remote-testgit script is really only used in t5801-remote-helpers.sh. It does not even contain any @@<MAGIC>@@ placeholders that would need to be interpolated via make git-remote-testgit.

Let's just move it to a new home, decluttering the top-level directory and clarifying that this is just a test helper, not an official Git command that we would want to ever support.

Suggested by Ævar Arnfjörð Bjarmason.

---
 .gitignore                                          | 1 -
 Makefile                                            | 3 ---
 t/t5801-remote-helpers.sh                           | 2 ++
 git-remote-testgit.sh => t/t5801/git-remote-testgit | 0
 4 files changed, 2 insertions(+), 4 deletions(-)
 rename git-remote-testgit.sh => t/t5801/git-remote-testgit (100%)
.gitignore +0 -1
index 7374587f9d..de8fc2f5b1 100644
@@ -135,7 +135,6 @@
 /git-remote-ftps
 /git-remote-fd
 /git-remote-ext
-/git-remote-testgit
 /git-remote-testpy
 /git-remote-testsvn
 /git-repack
Makefile +0 -3
index 8654c130f8..26f8ed2228 100644
@@ -633,7 +633,6 @@ SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
 SCRIPT_SH += git-quiltimport.sh
 SCRIPT_SH += git-legacy-rebase.sh
-SCRIPT_SH += git-remote-testgit.sh
 SCRIPT_SH += git-request-pull.sh
 SCRIPT_SH += git-stash.sh
 SCRIPT_SH += git-submodule.sh
@@ -657,8 +656,6 @@ SCRIPT_PERL += git-svn.perl
 
 SCRIPT_PYTHON += git-p4.py
 
-NO_INSTALL += git-remote-testgit
-
 # Generated files for scripts
 SCRIPT_SH_GEN = $(patsubst %.sh,%,$(SCRIPT_SH))
 SCRIPT_PERL_GEN = $(patsubst %.perl,%,$(SCRIPT_PERL))
t/t5801-remote-helpers.sh +2 -0
index aaaa722cca..d04f8007e0 100755
@@ -8,6 +8,8 @@ test_description='Test remote-helper import and export commands'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-gpg.sh
 
+PATH="$TEST_DIRECTORY/t5801:$PATH"
+
 compare_refs() {
 	git --git-dir="$1/.git" rev-parse --verify $2 >expect &&
 	git --git-dir="$3/.git" rev-parse --verify $4 >actual &&
t/t5801/git-remote-testgit renamed from git-remote-testgit.sh +0 -0
similarity index 100%
rename from git-remote-testgit.sh
rename to t/t5801/git-remote-testgit
Signature · 1 line
gitgitgadget
#2 lore raw

Re: [PATCH 1/7] remote-testgit: move it into the support directory for t5801

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 10 lines · writes:

From: Johannes Schindelin <[email protected]>

The git-remote-testgit script is really only used in t5801-remote-helpers.sh. It does not even contain any @@<MAGIC>@@ placeholders that would need to be interpolated via make git-remote-testgit.

Let's just move it to a new home, decluttering the top-level directory and clarifying that this is just a test helper, not an official Git command that we would want to ever support.

Makes sense.

Makefile SCRIPT_PERL += git-svn.perl · line 661

Quoted 6 lines
Makefile +0 -2
@@ -657,8 +656,6 @@ SCRIPT_PERL += git-svn.perl
 
 SCRIPT_PYTHON += git-p4.py
 
-NO_INSTALL += git-remote-testgit
-

The line lost here was the last one that updated the value of NO_INSTALL, so we should be able to lose all the mentions of the make variable now.

t/t5801-remote-helpers.sh test_description='Test remote-helper import and export commands' · line 12

Quoted 10 lines
t/t5801-remote-helpers.sh +2 -0
index aaaa722cca..d04f8007e0 100755
@@ -8,6 +8,8 @@ test_description='Test remote-helper import and export commands'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-gpg.sh
 
+PATH="$TEST_DIRECTORY/t5801:$PATH"
+

I guess this makes much more sense than having it next to other test helpers, as t5801 is the home for the remote-helper tests.

Thanks, will queue.

#3 lore raw

Re: [PATCH 1/7] remote-testgit: move it into the support directory for t5801

Hi Junio,

Quoted 5 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 2 lines · writes:
(quoted diff) +0 -0
@@ -657,8 +656,6 @@ SCRIPT_PERL += git-svn.perl
 
Quoted 8 lines

Makefile SCRIPT_PERL += git-svn.perl · line 661

Quoted 4 lines
Makefile +0 -2
 SCRIPT_PYTHON += git-p4.py
 
-NO_INSTALL += git-remote-testgit
-

The line lost here was the last one that updated the value of NO_INSTALL, so we should be able to lose all the mentions of the make variable now.

Sure, I added a separate commit to make it so.

Thanks,
Dscho

#4 lore raw

PATCH 2/7 help -a: do not list commands that are excluded from the build

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

When built with NO_CURL or with NO_UNIX_SOCKETS, some commands are skipped from the build. It does not make sense to list them in the output of git help -a, so let's just not.

---
 Makefile            | 14 ++++++++++++--
 generate-cmdlist.sh | 10 +++++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)
Makefile +12 -2
index 26f8ed2228..8f3c477ab3 100644
@@ -611,6 +611,7 @@ FUZZ_PROGRAMS =
 LIB_OBJS =
 PROGRAM_OBJS =
 PROGRAMS =
+EXCLUDED_PROGRAMS =
 SCRIPT_PERL =
 SCRIPT_PYTHON =
 SCRIPT_SH =
@@ -1323,6 +1324,7 @@ ifdef NO_CURL
 	REMOTE_CURL_PRIMARY =
 	REMOTE_CURL_ALIASES =
 	REMOTE_CURL_NAMES =
+	EXCLUDED_PROGRAMS += git-http-fetch git-http-push
 else
 	ifdef CURLDIR
 		# Try "-Wl,-rpath=$(CURLDIR)/$(lib)" in such a case.
@@ -1347,7 +1349,11 @@ endif
 	ifeq "$(curl_check)" "070908"
 		ifndef NO_EXPAT
 			PROGRAM_OBJS += http-push.o
+		else
+			EXCLUDED_PROGRAMS += git-http-push
 		endif
+	else
+		EXCLUDED_PROGRAMS += git-http-push
 	endif
 	curl_check := $(shell (echo 072200; $(CURL_CONFIG) --vernum | sed -e '/^70[BC]/s/^/0/') 2>/dev/null | sort -r | sed -ne 2p)
 	ifeq "$(curl_check)" "072200"
@@ -1593,7 +1599,9 @@ ifdef NO_INET_PTON
 	LIB_OBJS += compat/inet_pton.o
 	BASIC_CFLAGS += -DNO_INET_PTON
 endif
-ifndef NO_UNIX_SOCKETS
+ifdef NO_UNIX_SOCKETS
+	EXCLUDED_PROGRAMS += git-credential-cache git-credential-cache--daemon
+else
 	LIB_OBJS += unix-socket.o
 	PROGRAM_OBJS += credential-cache.o
 	PROGRAM_OBJS += credential-cache--daemon.o
@@ -2112,7 +2120,9 @@ $(BUILT_INS): git$X
 command-list.h: generate-cmdlist.sh command-list.txt
 
 command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt Documentation/config/*.txt
-	$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh command-list.txt >$@+ && mv $@+ $@
+	$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh \
+		$(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \
+		command-list.txt >$@+ && mv $@+ $@
 
 SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
 	$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
generate-cmdlist.sh +9 -1
index 709d67405b..7867b99d19 100755
@@ -6,7 +6,7 @@ die () {
 }
 
 command_list () {
-	grep -v '^#' "$1"
+	eval grep -ve '^#' $exclude_programs "$1"
 }
 
 get_categories () {
@@ -93,6 +93,14 @@ EOF
 EOF
 }
 
+exclude_programs=
+while test "a$1" = "a--exclude-program"
+do
+	shift
+	exclude_programs="$exclude_programs -e \"^$1 \""
+	shift
+done
+
 echo "/* Automatically generated by generate-cmdlist.sh */
 struct cmdname_help {
 	const char *name;
Signature · 1 line
gitgitgadget
#5 lore raw

Re: [PATCH 2/7] help -a: do not list commands that are excluded from the build

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 5 lines · writes:

From: Johannes Schindelin <[email protected]>

When built with NO_CURL or with NO_UNIX_SOCKETS, some commands are skipped from the build. It does not make sense to list them in the output of git help -a, so let's just not.

The objective makes sense quite a lot.

Makefile $(BUILT_INS): git$X · line 2124

Quoted 4 lines

command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt Documentation/config/*.txt

Makefile +2 -1
-	$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh command-list.txt >$@+ && mv $@+ $@
+	$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh \
+		$(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \

Each whitespace separated token on $(EXCLUDED_PROGRAMS) is the name of a program to be excluded and we know there is no funny characters in it, hence quoing in "^$1 " (without protecting us against funnies) is sufficient.

Quoted 9 lines
Makefile +1 -0
+		command-list.txt >$@+ && mv $@+ $@
 
 SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
 	$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
generate-cmdlist.sh +0 -0
index 709d67405b..7867b99d19 100755

generate-cmdlist.sh die () { · line 9

Quoted 5 lines
generate-cmdlist.sh +1 -1
@@ -6,7 +6,7 @@ die () {
 }
 
 command_list () {
-	grep -v '^#' "$1"
+	eval grep -ve '^#' $exclude_programs "$1"

The original protects against $IFS in the filename given as $1, but with the eval that is no longer true. We have been feeding the constant "command-list.txt" to the program since its inception, and I do not expect it to change, so this regression in defensiveness does not matter in practice. Also because # is prefixed with ^, the unintended loss of quotes around it when the eval makes the shell re-parse the generated command line would not make the remainder into a comment.

But it does look brittle, and smells like a trap for less experienced shell programmers to blindly copy & paste & tweak without understanding what is going on, leading to bugs.

	eval "grep -v -e '^#' $exclude_programs" <"$1"

or something like that, perhaps?

generate-cmdlist.sh EOF · line 97

Quoted 6 lines
generate-cmdlist.sh +2 -0
@@ -93,6 +93,14 @@ EOF
 EOF
 }
 
+exclude_programs=
+while test "a$1" = "a--exclude-program"

s/a/z/g; if you want to pretend to be old-fashioned, but s/a//g; should be sufficient in the modern world.

generate-cmdlist.sh EOF · line 102

Quoted 5 lines
generate-cmdlist.sh +5 -0
+do
+	shift
+	exclude_programs="$exclude_programs -e \"^$1 \""
+	shift
+done

As I said, this part looks good enough given the things we feed as parameters to --exclude-program option.

Thanks.

#6 lore raw

Re: [PATCH 2/7] help -a: do not list commands that are excluded from the build

Hi Junio,

Quoted 25 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

generate-cmdlist.sh die () { · line 9

Quoted 12 lines · writes:

SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\

Makefile +0 -0
 	$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
generate-cmdlist.sh +1 -1
index 709d67405b..7867b99d19 100755
@@ -6,7 +6,7 @@ die () {
 }
 
 command_list () {
-	grep -v '^#' "$1"
+	eval grep -ve '^#' $exclude_programs "$1"

The original protects against $IFS in the filename given as $1, but with the eval that is no longer true. We have been feeding the constant "command-list.txt" to the program since its inception, and I do not expect it to change, so this regression in defensiveness does not matter in practice. Also because # is prefixed with ^, the unintended loss of quotes around it when the eval makes the shell re-parse the generated command line would not make the remainder into a comment.

Quoted 7 lines

But it does look brittle, and smells like a trap for less experienced shell programmers to blindly copy & paste & tweak without understanding what is going on, leading to bugs.

	eval "grep -v -e '^#' $exclude_programs" <"$1"

or something like that, perhaps?

Yes! Thank you.

Quoted 9 lines

generate-cmdlist.sh EOF · line 97

Quoted 6 lines
generate-cmdlist.sh +2 -0
@@ -93,6 +93,14 @@ EOF
 EOF
 }
 
+exclude_programs=
+while test "a$1" = "a--exclude-program"

s/a/z/g; if you want to pretend to be old-fashioned, but s/a//g; should be sufficient in the modern world.

Don't you know, I always used "a" without realizing. You can see my misdeed even in git-rebase--preserve-merges.sh. I won't fix it there, though, as I hope that it'll be gone soon enough.

Will be fixed in the next iteration I send.

Thanks,
Dscho

#7 lore raw

PATCH 3/7 check-docs: do not pretend that commands are listed which are excluded

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

In the previous commit, we taught git help -a to stop listing commands that are excluded from the build.

In this commit, we stop check-docs from claiming that those commands are listed.

---
 Makefile | 1 +
 1 file changed, 1 insertion(+)
Makefile +1 -0
index 8f3c477ab3..5880d4d3a9 100644
@@ -3085,6 +3085,7 @@ check-docs::
 	( \
 		sed -e '1,/^### command list/d' \
 		    -e '/^#/d' \
+		    $(patsubst %,-e '/^% /d',$(EXCLUDED_PROGRAMS)) \
 		    -e '/guide$$/d' \
 		    -e 's/[ 	].*//' \
 		    -e 's/^/listed /' command-list.txt; \
Signature · 1 line
gitgitgadget
#8 lore raw

Re: [PATCH 3/7] check-docs: do not pretend that commands are listed which are excluded

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 2 lines · writes:

From: Johannes Schindelin <[email protected]>
Subject: Re: [PATCH 3/7] check-docs: do not pretend that commands are listed which are excluded

Sorry, but I cannot quite parse the title. I am guessing that you meant "do not pretend that commands, which are excluded, are listed", which is a mouthful but at least can be parseable X-<.

Quoted 5 lines

In the previous commit, we taught git help -a to stop listing commands that are excluded from the build.

In this commit, we stop check-docs from claiming that those commands are listed.

I think the result would become more readable (and I suspect that it would at the same time make the title parseable, but I am not sure as I do not quite know what the title wants to say in the first place) if you clarified the verb "list" here perhaps with "listed in command-list.txt".

The output from the sed script that filters the contents of the command-list.txt file is compared with the ALL_COMMANDS list, and we complain if an entry in command-list.txt is no longer in the ALL_COMMANDS list (i.e. a developer removed a stale command but forgot to remove it from the command-list.txt) [*1*].

The step 2/7 marked the subcommands that ought to be on ALL_COMMANDS for the purpose of this check but that are excluded from the build, as a preparation for this step, and this step 3/7 uses the list of excluded ones to avoid complaining them being in the categorized list of subcommands (i.e. command-list.txt).

Makes sense.

Side note *1*. Another thing we would want to catch is a developer adding a new subcommand to ALL_COMMANDS while forgetting to give it a category in the command-list.txt file. That is done in the "other" loop before this part, and the patch is correct that it does not touch that loop to filter the command-list using the EXCLUDED_PROGRAMS list.

We can read that "stop listing them" is done as means to some end, but it is unclear what the end-user/developer visible effect this step intends to achieve. After thinking about what the patch does a bit more, here is what I came up with.

check-docs: allow excluded subcommands to be in the command-list file

One of the things this build target makes sure is that all entries in the command-list.txt are about subcommands that still exist in the system (i.e. if a developer removes a subcommand and forgets to remove it from the command list file, it needs to be flagged as an incomplete change), and it is done by comparing the entries in the file against the list of subprograms in $(ALL_COMMANDS) variable.

However, the latter list is dynamic---not all build contains all the Git subcommands categorized in the command-list.txt file. And such a partial build would falsely trigger the check, complaining that some subcommands are removed but still are listed in the command list file.

Fix this by teaching the logic to use the EXCLUDED_PROGRAMS list prepared in the previous step.

or something like that, perhaps?

The same logic to warn about "removed but listed" commands that are not built (hence missing from ALL_COMMANDS list) is also fed the list of all documented subcommands by looking at "make print-man1" in the Documentation directory, so that it can issue "removed but documented" when a developer removes a subcommand but forgets to remove its documentation. Don't we need to teach a similar treatment on that side of the coin?

I am wondering if it makes that easier to instead add EXCLUDED ones back to ALL_COMMANDS on the receiving end of the pipe, rather than filtering them out in the sed script that reads from command-list, i.e.

	# revert what this patch did to the upstream of the pipe
	(
		sed ... filters comments ... \
			-e 's/^/listed /' command-list.txt;
		$(MAKE) -C Documentation print-man1 |
		sed ... -e 's/^/documented /'
	) | \
	while read how cmd; \
	do
		# instead add them back here
		case " $(patsubst %$X,%,$(ALL_COMMANDS) $(EXCLUDED)) " in
		*" $$cmd "*) : ok ;;
		...

That way, the documentation side can be fixed at the same time as the command-list side of the check that incorrectly reports "removed but listed".

If we go that route, the earlier rewrite of the proposed log message I suggested may want to be further updated. Perhaps by replacing "use the EXCLUDED_PROGRAMS list prepared in the previous step" with "add the EXCLUDED_PROGRAMS list prepared in the previous step back to ALL_COMMANDS list when checking entries from the list of documented and categorized subcommands." or something like that.

Thanks.

#9 lore raw

Re: [PATCH 3/7] check-docs: do not pretend that commands are listed which are excluded

Hi Junio,

Quoted 9 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 2 lines · writes:

From: Johannes Schindelin <[email protected]>
Subject: Re: [PATCH 3/7] check-docs: do not pretend that commands are listed which are excluded

Sorry, but I cannot quite parse the title. I am guessing that you meant "do not pretend that commands, which are excluded, are listed", which is a mouthful but at least can be parseable X-<.

Well, what can I say, you're right.

Quoted 11 lines
Quoted 5 lines

In the previous commit, we taught git help -a to stop listing commands that are excluded from the build.

In this commit, we stop check-docs from claiming that those commands are listed.

I think the result would become more readable (and I suspect that it would at the same time make the title parseable, but I am not sure as I do not quite know what the title wants to say in the first place) if you clarified the verb "list" here perhaps with "listed in command-list.txt".

I was actually coming from the help -a side. But that is not obvious from the commit message, nor does that Makefile target handle that help -a output in the first place.

Quoted 11 lines

The output from the sed script that filters the contents of the command-list.txt file is compared with the ALL_COMMANDS list, and we complain if an entry in command-list.txt is no longer in the ALL_COMMANDS list (i.e. a developer removed a stale command but forgot to remove it from the command-list.txt) [*1*].

The step 2/7 marked the subcommands that ought to be on ALL_COMMANDS for the purpose of this check but that are excluded from the build, as a preparation for this step, and this step 3/7 uses the list of excluded ones to avoid complaining them being in the categorized list of subcommands (i.e. command-list.txt).

Right.

Quoted 21 lines

We can read that "stop listing them" is done as means to some end, but it is unclear what the end-user/developer visible effect this step intends to achieve. After thinking about what the patch does a bit more, here is what I came up with.

check-docs: allow excluded subcommands to be in the command-list file

One of the things this build target makes sure is that all entries in the command-list.txt are about subcommands that still exist in the system (i.e. if a developer removes a subcommand and forgets to remove it from the command list file, it needs to be flagged as an incomplete change), and it is done by comparing the entries in the file against the list of subprograms in $(ALL_COMMANDS) variable.

However, the latter list is dynamic---not all build contains all the Git subcommands categorized in the command-list.txt file. And such a partial build would falsely trigger the check, complaining that some subcommands are removed but still are listed in the command list file.

Quoted 4 lines

Fix this by teaching the logic to use the EXCLUDED_PROGRAMS list prepared in the previous step.

or something like that, perhaps?

I rephrased this yet again (to reflect the way I speak):

check-docs: allow command-list.txt to contain excluded commands

Among other things, the check-docs target ensures that command-list.txt no longer contains commands that were dropped (or that were never added in the first place).

To do so, it compares the list of commands from that file to the commands listed in $(ALL_COMMANDS).

However, some build options exclude commands from the latter. Fix the target to handle this situation correctly by taking the just-introduced $(EXCLUDED_PROGRAMS) into account.

Quoted 7 lines

The same logic to warn about "removed but listed" commands that are not built (hence missing from ALL_COMMANDS list) is also fed the list of all documented subcommands by looking at "make print-man1" in the Documentation directory, so that it can issue "removed but documented" when a developer removes a subcommand but forgets to remove its documentation. Don't we need to teach a similar treatment on that side of the coin?

Yep, that's what the later patch "docs: exclude documentation for commands that have been excluded" is all about.

Quoted 19 lines

I am wondering if it makes that easier to instead add EXCLUDED ones back to ALL_COMMANDS on the receiving end of the pipe, rather than filtering them out in the sed script that reads from command-list, i.e.

	# revert what this patch did to the upstream of the pipe
	(
		sed ... filters comments ... \
			-e 's/^/listed /' command-list.txt;
		$(MAKE) -C Documentation print-man1 |
		sed ... -e 's/^/documented /'
	) | \
	while read how cmd; \
	do
		# instead add them back here
		case " $(patsubst %$X,%,$(ALL_COMMANDS) $(EXCLUDED)) " in
		*" $$cmd "*) : ok ;;
		...
Quoted 3 lines

That way, the documentation side can be fixed at the same time as the command-list side of the check that incorrectly reports "removed but listed".

I did exactly that, thank you for that suggestion.

Thanks,
Dscho

#10 lore raw

PATCH 4/7 docs: exclude documentation for commands that have been excluded

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

When building with certain build options, some commands are excluded from the build. For example, git-credential-cache is skipped when building with NO_UNIX_SOCKETS.

Let's not build or package documentation for those excluded commands.

This issue was pointed out rightfully when running make check-docs on Windows, where we do not yet have Unix sockets, and therefore the credential-cache command is excluded (yet its documentation was built and shipped).

Note: building the documentation via make -C Documentation leaves the build system with no way to determine which commands have been excluded. If called thusly, we gracefully fail to exclude their documentation. Only when building the documentation via the top-level Makefile will it get excluded properly, or after building Documentation/GIT-EXCLUDED-PROGRAMS manually.

---
 Documentation/.gitignore |  1 +
 Documentation/Makefile   |  3 +++
 Makefile                 | 36 ++++++++++++++++++++----------------
 3 files changed, 24 insertions(+), 16 deletions(-)
Documentation/.gitignore +1 -0
index 3ef54e0adb..ea27148c59 100644
@@ -13,3 +13,4 @@ mergetools-*.txt
 manpage-base-url.xsl
 SubmittingPatches.txt
 tmp-doc-diff/
+/GIT-EXCLUDED-PROGRAMS
Documentation/Makefile +3 -0
index af0e2cf11a..e22ea2f57c 100644
@@ -7,7 +7,10 @@ ARTICLES =
 SP_ARTICLES =
 OBSOLETE_HTML =
 
+-include GIT-EXCLUDED-PROGRAMS
+
 MAN1_TXT += $(filter-out \
+		$(patsubst %,%.txt,$(EXCLUDED_PROGRAMS)) \
 		$(addsuffix .txt, $(ARTICLES) $(SP_ARTICLES)), \
 		$(wildcard git-*.txt))
 MAN1_TXT += git.txt
Makefile +20 -16
index 5880d4d3a9..a5212c64bf 100644
@@ -2455,22 +2455,25 @@ $(VCSSVN_LIB): $(VCSSVN_OBJS)
 
 export DEFAULT_EDITOR DEFAULT_PAGER
 
+Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname
+	$(QUIET_GEN)echo "EXCLUDED_PROGRAMS := $(EXCLUDED_PROGRAMS)" >$@
+
 .PHONY: doc man man-perl html info pdf
-doc: man-perl
+doc: man-perl Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation all
 
-man: man-perl
+man: man-perl Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation man
 
 man-perl: perl/build/man/man3/Git.3pm
 
-html:
+html: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation html
 
-info:
+info: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation info
 
-pdf:
+pdf: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation pdf
 
 XGETTEXT_FLAGS = \
@@ -2907,33 +2910,33 @@ endif
 install-gitweb:
 	$(MAKE) -C gitweb install
 
-install-doc: install-man-perl
+install-doc: install-man-perl Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation install
 
-install-man: install-man-perl
+install-man: install-man-perl Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation install-man
 
-install-man-perl: man-perl
+install-man-perl: man-perl Documentation/GIT-EXCLUDED-PROGRAMS
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mandir_SQ)/man3'
 	(cd perl/build/man/man3 && $(TAR) cf - .) | \
 	(cd '$(DESTDIR_SQ)$(mandir_SQ)/man3' && umask 022 && $(TAR) xof -)
 
-install-html:
+install-html: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation install-html
 
-install-info:
+install-info: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation install-info
 
-install-pdf:
+install-pdf: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation install-pdf
 
-quick-install-doc:
+quick-install-doc: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation quick-install
 
-quick-install-man:
+quick-install-man: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation quick-install-man
 
-quick-install-html:
+quick-install-html: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation quick-install-html
 
 
@@ -2988,7 +2991,7 @@ artifacts-tar:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) \
 htmldocs = git-htmldocs-$(GIT_VERSION)
 manpages = git-manpages-$(GIT_VERSION)
 .PHONY: dist-doc distclean
-dist-doc:
+dist-doc: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(RM) -r .doc-tmp-dir
 	mkdir .doc-tmp-dir
 	$(MAKE) -C Documentation WEBDOC_DEST=../.doc-tmp-dir install-webdoc
@@ -3035,6 +3038,7 @@ clean: profile-clean coverage-clean cocciclean
 	$(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
 	$(RM) $(htmldocs).tar.gz $(manpages).tar.gz
 	$(MAKE) -C Documentation/ clean
+	$(RM) Documentation/GIT-EXCLUDED-PROGRAMS
 ifndef NO_PERL
 	$(MAKE) -C gitweb clean
 	$(RM) -r perl/build/
@@ -3062,7 +3066,7 @@ ALL_COMMANDS += gitweb
 ALL_COMMANDS += git-gui git-citool
 
 .PHONY: check-docs
-check-docs::
+check-docs:: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation lint-docs
 	@(for v in $(patsubst %$X,%,$(ALL_COMMANDS)); \
 	do \
Signature · 1 line
gitgitgadget
#11 lore raw

Re: [PATCH 4/7] docs: exclude documentation for commands that have been excluded

Makefile $(VCSSVN_LIB): $(VCSSVN_OBJS) · line 2456

Quoted 12 lines · Johannes Schindelin via GitGitGadget

When building with certain build options, some commands are excluded from the build. For example, git-credential-cache is skipped when building with NO_UNIX_SOCKETS.

Let's not build or package documentation for those excluded commands.

---

Makefile +2 -0
@@ -2455,22 +2455,25 @@ $(VCSSVN_LIB): $(VCSSVN_OBJS)
+Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname
+       $(QUIET_GEN)echo "EXCLUDED_PROGRAMS := $(EXCLUDED_PROGRAMS)" >$@

Should this rule also have a dependency upon "config.mak.autogen"? Perhaps like this:

Documentation/GIT-EXCLUDED-PROGRAMS: Makefile $(wildcard config.mak*)

#12 lore raw

Re: [PATCH 4/7] docs: exclude documentation for commands that have been excluded

Quoted 6 lines · Eric Sunshine

Makefile $(VCSSVN_LIB): $(VCSSVN_OBJS) · line 2456

Quoted 4 lines
Makefile +2 -0
@@ -2455,22 +2455,25 @@ $(VCSSVN_LIB): $(VCSSVN_OBJS)
+Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname
+       $(QUIET_GEN)echo "EXCLUDED_PROGRAMS := $(EXCLUDED_PROGRAMS)" >$@

Should this rule also have a dependency upon "config.mak.autogen"?

That is probably a good point.

Quoted 3 lines

Perhaps like this:

Documentation/GIT-EXCLUDED-PROGRAMS: Makefile $(wildcard config.mak*)

I'd rather not let changes to "config.mak-", which I keep in my working tree (untracked and disabled copy of config.mak, that can be readily activated by renaming), be part of dependency rules.

If we know 'autogen' is the only dependency that optionally can exist, then depending explicitly on $(wildcard config.mak.autogen) would be a better alternative. To those who do not use the autoconf, the macro will expand to an empty string, and to those who do have the path on the filesystem, the macro parrots the constant string it was given, which is exactly what we want.

Thanks.

#13 lore raw

Re: [PATCH 4/7] docs: exclude documentation for commands that have been excluded

Quoted 12 lines · Junio C Hamano
Quoted 4 lines · Eric Sunshine

Makefile $(VCSSVN_LIB): $(VCSSVN_OBJS) · line 2459

Quoted 2 lines
Makefile +2 -0
+Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname
+       $(QUIET_GEN)echo "EXCLUDED_PROGRAMS := $(EXCLUDED_PROGRAMS)" >$@

Should this rule also have a dependency upon "config.mak.autogen"?

That is probably a good point.

Quoted 3 lines

Perhaps like this:

Documentation/GIT-EXCLUDED-PROGRAMS: Makefile $(wildcard config.mak*)

Quoted 7 lines

I'd rather not let changes to "config.mak-", which I keep in my working tree (untracked and disabled copy of config.mak, that can be readily activated by renaming), be part of dependency rules.

If we know 'autogen' is the only dependency that optionally can exist, then depending explicitly on $(wildcard config.mak.autogen) would be a better alternative.

When composing that email, I originally wrote $(wildcard config.mak.autogen) as the suggestion but changed it to the looser $(wildcard config.mak*) when I realized that the developer's own config.mak probably ought to be a dependency, as well. Taking your objection into consideration, we could mention both explicitly:

    Documentation/GIT-EXCLUDED-PROGRAMS: Makefile \
        $(wildcard config.mak) $(wildcard config.mak.autogen)
#14 lore raw

Re: [PATCH 4/7] docs: exclude documentation for commands that have been excluded

Quoted 8 lines · Eric Sunshine

When composing that email, I originally wrote $(wildcard config.mak.autogen) as the suggestion but changed it to the looser $(wildcard config.mak*) when I realized that the developer's own config.mak probably ought to be a dependency, as well. Taking your objection into consideration, we could mention both explicitly:

    Documentation/GIT-EXCLUDED-PROGRAMS: Makefile \
        $(wildcard config.mak) $(wildcard config.mak.autogen)

Bleh, I forgot config.mak.uname from Dscho's original patch.

    Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname \
        $(wildcard config.mak) $(wildcard config.mak.autogen)
#15 lore raw

Re: [PATCH 4/7] docs: exclude documentation for commands that have been excluded

Hi Eric & Junio,

Quoted 7 lines · Eric Sunshine
Quoted 6 lines · Eric Sunshine

When composing that email, I originally wrote $(wildcard config.mak.autogen) as the suggestion but changed it to the looser $(wildcard config.mak*) when I realized that the developer's own config.mak probably ought to be a dependency, as well. Taking your objection into consideration, we could mention both explicitly:

Quoted 7 lines
Quoted 2 lines
    Documentation/GIT-EXCLUDED-PROGRAMS: Makefile \
        $(wildcard config.mak) $(wildcard config.mak.autogen)

Bleh, I forgot config.mak.uname from Dscho's original patch.

    Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname \
        $(wildcard config.mak) $(wildcard config.mak.autogen)

Right.

I had that locally in the meantime, but based on Peff's suggestion went for a GIT-CFLAGS like approach instead.

Ciao,
Dscho

#16 lore raw

Re: [PATCH 4/7] docs: exclude documentation for commands that have been excluded

In reply to Eric Sunshine
Quoted 21 lines · Eric Sunshine
Quoted 12 lines · Junio C Hamano
Quoted 4 lines · Eric Sunshine

Makefile $(VCSSVN_LIB): $(VCSSVN_OBJS) · line 2459

Quoted 2 lines
Makefile +2 -0
+Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname
+       $(QUIET_GEN)echo "EXCLUDED_PROGRAMS := $(EXCLUDED_PROGRAMS)" >$@

Should this rule also have a dependency upon "config.mak.autogen"?

That is probably a good point.

Quoted 3 lines

Perhaps like this:

Documentation/GIT-EXCLUDED-PROGRAMS: Makefile $(wildcard config.mak*)

Quoted 7 lines

I'd rather not let changes to "config.mak-", which I keep in my working tree (untracked and disabled copy of config.mak, that can be readily activated by renaming), be part of dependency rules.

If we know 'autogen' is the only dependency that optionally can exist, then depending explicitly on $(wildcard config.mak.autogen) would be a better alternative.

Quoted 8 lines

When composing that email, I originally wrote $(wildcard config.mak.autogen) as the suggestion but changed it to the looser $(wildcard config.mak*) when I realized that the developer's own config.mak probably ought to be a dependency, as well. Taking your objection into consideration, we could mention both explicitly:

    Documentation/GIT-EXCLUDED-PROGRAMS: Makefile \
        $(wildcard config.mak) $(wildcard config.mak.autogen)

What about command-line options that influence the outcome? It sounds like this is the same problem we have in lots of other places (like say, compiler flags being updated), that we solve by generating the proposed file output unconditionally and comparing it to what's on disk. E.g., see the way GIT-CFLAGS or GIT-BUILD-OPTIONS is generated.

-Peff

#17 lore raw

Re: [PATCH 4/7] docs: exclude documentation for commands that have been excluded

Quoted 5 lines · Jeff King

What about command-line options that influence the outcome? It sounds like this is the same problem we have in lots of other places (like say, compiler flags being updated), that we solve by generating the proposed file output unconditionally and comparing it to what's on disk. E.g., see the way GIT-CFLAGS or GIT-BUILD-OPTIONS is generated.

;-).

#18 lore raw

Re: [PATCH 4/7] docs: exclude documentation for commands that have been excluded

In reply to Jeff King

Hi Peff,

Quoted 29 lines · Jeff King
Quoted 27 lines · Eric Sunshine
Quoted 12 lines · Junio C Hamano
Quoted 4 lines · Eric Sunshine

Makefile $(VCSSVN_LIB): $(VCSSVN_OBJS) · line 2459

Quoted 2 lines
Makefile +2 -0
+Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname
+       $(QUIET_GEN)echo "EXCLUDED_PROGRAMS := $(EXCLUDED_PROGRAMS)" >$@

Should this rule also have a dependency upon "config.mak.autogen"?

That is probably a good point.

Quoted 3 lines

Perhaps like this:

Documentation/GIT-EXCLUDED-PROGRAMS: Makefile $(wildcard config.mak*)

Quoted 7 lines

I'd rather not let changes to "config.mak-", which I keep in my working tree (untracked and disabled copy of config.mak, that can be readily activated by renaming), be part of dependency rules.

If we know 'autogen' is the only dependency that optionally can exist, then depending explicitly on $(wildcard config.mak.autogen) would be a better alternative.

When composing that email, I originally wrote $(wildcard config.mak.autogen) as the suggestion but changed it to the looser $(wildcard config.mak*) when I realized that the developer's own config.mak probably ought to be a dependency, as well. Taking your objection into consideration, we could mention both explicitly:

Quoted 8 lines
Quoted 2 lines
    Documentation/GIT-EXCLUDED-PROGRAMS: Makefile \
        $(wildcard config.mak) $(wildcard config.mak.autogen)

What about command-line options that influence the outcome? It sounds like this is the same problem we have in lots of other places (like say, compiler flags being updated), that we solve by generating the proposed file output unconditionally and comparing it to what's on disk. E.g., see the way GIT-CFLAGS or GIT-BUILD-OPTIONS is generated.

*Sigh*

I really did not want to do that because I thought it would be tedious and more complicated and result in a longer patch.

Well, don't you know. The patch is actually *a lot* shorter now. So much so that range-diff thinks it is a different commit ;-)

Thanks for the reality check/prod,
Dscho

#19 lore raw

Re: [PATCH 4/7] docs: exclude documentation for commands that have been excluded

Quoted 6 lines · Johannes Schindelin
Quoted 5 lines

What about command-line options that influence the outcome? It sounds like this is the same problem we have in lots of other places (like say, compiler flags being updated), that we solve by generating the proposed file output unconditionally and comparing it to what's on disk. E.g., see the way GIT-CFLAGS or GIT-BUILD-OPTIONS is generated.

Quoted 7 lines

*Sigh*

I really did not want to do that because I thought it would be tedious and more complicated and result in a longer patch.

Well, don't you know. The patch is actually *a lot* shorter now. So much so that range-diff thinks it is a different commit ;-)

:) Great. I looked at the result in your v2 and it looks good to me.

-Peff

#20 lore raw

PATCH 5/7 check-docs: do not bother checking for legacy scripts' documentation

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

In the recent years, there has been a big push to convert more and more of Git's commands that are implemented as scripts to built-ins written in pure, portable C, for robustness, speed and portability.

One strategy that served us well is to convert those scripts incrementally, starting by renaming the scripts to git-legacy-<command>, then introducing a built-in that does nothing else at first than checking the config setting <command>.useBuiltin (which defaults to false at the outset) and handing off to the legacy script if so asked.

Obviously, those git-legacy-<command> commands share the documentation with the built-in git-<command>, and are not intended to be called directly anyway. So let's not try to ensure that they are documented separately from their built-in versions.

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Makefile +1 -1
index a5212c64bf..caa20923a4 100644
@@ -3074,7 +3074,7 @@ check-docs:: Documentation/GIT-EXCLUDED-PROGRAMS
 		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-remote-* | git-stage | git-legacy-* | \
 		git-?*--?* ) continue ;; \
 		esac ; \
 		test -f "Documentation/$$v.txt" || \
Signature · 1 line
gitgitgadget
#21 lore raw

Re: [PATCH 5/7] check-docs: do not bother checking for legacy scripts' documentation

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 4 lines · writes:

Obviously, those git-legacy-<command> commands share the documentation with the built-in git-<command>, and are not intended to be called directly anyway. So let's not try to ensure that they are documented separately from their built-in versions.

Yup.

This is the "other" loop that goes over the list of ALL_COMMANDS and makes sure each of them has its own documentation page.

Looking good.

Quoted 19 lines
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Makefile +1 -1
index a5212c64bf..caa20923a4 100644
@@ -3074,7 +3074,7 @@ check-docs:: Documentation/GIT-EXCLUDED-PROGRAMS
 		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-remote-* | git-stage | git-legacy-* | \
 		git-?*--?* ) continue ;; \
 		esac ; \
 		test -f "Documentation/$$v.txt" || \
#22 lore raw

PATCH 6/7 test-tool: handle the `-C <directory>` option just like `git`

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

In preparation for moving git serve into test-tool (because it really is only used by the test suite), we teach the test-tool the useful trick to change the working directory before running the test command, which will avoid introducing subshells in the test code.

---
 t/helper/test-tool.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
t/helper/test-tool.c +19 -0
index 99db7409b8..2b21943f93 100644
@@ -1,5 +1,11 @@
 #include "git-compat-util.h"
 #include "test-tool.h"
+#include "parse-options.h"
+
+static const char * const test_tool_usage[] = {
+	"test-tool [-C <directory>] <command [<arguments>...]]",
+	NULL
+};
 
 struct test_cmd {
 	const char *name;
@@ -73,11 +79,24 @@ static NORETURN void die_usage(void)
 int cmd_main(int argc, const char **argv)
 {
 	int i;
+	const char *working_directory = NULL;
+	struct option options[] = {
+		OPT_STRING('C', NULL, &working_directory, "directory",
+			   "change the working directory"),
+		OPT_END()
+	};
 
 	BUG_exit_code = 99;
+	argc = parse_options(argc, argv, NULL, options, test_tool_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION |
+			     PARSE_OPT_KEEP_ARGV0);
+
 	if (argc < 2)
 		die_usage();
 
+	if (working_directory && chdir(working_directory) < 0)
+		die("Could not cd to '%s'", working_directory);
+
 	for (i = 0; i < ARRAY_SIZE(cmds); i++) {
 		if (!strcmp(cmds[i].name, argv[1])) {
 			argv++;
Signature · 1 line
gitgitgadget
#23 lore raw

PATCH 7/7 Turn `git serve` into a test helper

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

The git serve built-in was introduced in ed10cb952d31 (serve:
introduce git-serve, 2018-03-15) as a backend to serve Git protocol v2, probably originally intended to be spawned by git upload-pack.

However, in the version that the protocol v2 patches made it into core Git, git upload-pack calls the serve() function directly instead of spawning git serve; The only reason in life for git serve to survive as a built-in command is to provide a way to test the protocol v2 functionality.

Meaning that it does not even have to be a built-in that is installed with end-user facing Git installations, but it can be a test helper instead.

Let's make it so.

---
 Makefile                                    |  2 +-
 builtin.h                                   |  1 -
 git.c                                       |  1 -
 builtin/serve.c => t/helper/test-serve-v2.c |  7 +++--
 t/helper/test-tool.c                        |  1 +
 t/helper/test-tool.h                        |  1 +
 t/t5701-git-serve.sh                        | 32 ++++++++++++---------
 t/t5702-protocol-v2.sh                      |  5 ++--
 t/t5703-upload-pack-ref-in-want.sh          | 16 +++++------
 9 files changed, 36 insertions(+), 30 deletions(-)
 rename builtin/serve.c => t/helper/test-serve-v2.c (81%)
Makefile +1 -1
index caa20923a4..d7656cdc4f 100644
@@ -762,6 +762,7 @@ TEST_BUILTINS_OBJS += test-repository.o
 TEST_BUILTINS_OBJS += test-revision-walking.o
 TEST_BUILTINS_OBJS += test-run-command.o
 TEST_BUILTINS_OBJS += test-scrap-cache-tree.o
+TEST_BUILTINS_OBJS += test-serve-v2.o
 TEST_BUILTINS_OBJS += test-sha1.o
 TEST_BUILTINS_OBJS += test-sha1-array.o
 TEST_BUILTINS_OBJS += test-sha256.o
@@ -1131,7 +1132,6 @@ BUILTIN_OBJS += builtin/rev-parse.o
 BUILTIN_OBJS += builtin/revert.o
 BUILTIN_OBJS += builtin/rm.o
 BUILTIN_OBJS += builtin/send-pack.o
-BUILTIN_OBJS += builtin/serve.o
 BUILTIN_OBJS += builtin/shortlog.o
 BUILTIN_OBJS += builtin/show-branch.o
 BUILTIN_OBJS += builtin/show-index.o
builtin.h +0 -1
index 6538932e99..c48636e244 100644
@@ -219,7 +219,6 @@ extern int cmd_rev_parse(int argc, const char **argv, const char *prefix);
 extern int cmd_revert(int argc, const char **argv, const char *prefix);
 extern int cmd_rm(int argc, const char **argv, const char *prefix);
 extern int cmd_send_pack(int argc, const char **argv, const char *prefix);
-extern int cmd_serve(int argc, const char **argv, const char *prefix);
 extern int cmd_shortlog(int argc, const char **argv, const char *prefix);
 extern int cmd_show(int argc, const char **argv, const char *prefix);
 extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
git.c +0 -1
index 2dd588674f..c58b067771 100644
@@ -548,7 +548,6 @@ static struct cmd_struct commands[] = {
 	{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
 	{ "rm", cmd_rm, RUN_SETUP },
 	{ "send-pack", cmd_send_pack, RUN_SETUP },
-	{ "serve", cmd_serve, RUN_SETUP },
 	{ "shortlog", cmd_shortlog, RUN_SETUP_GENTLY | USE_PAGER },
 	{ "show", cmd_show, RUN_SETUP },
 	{ "show-branch", cmd_show_branch, RUN_SETUP },
t/helper/test-serve-v2.c renamed from builtin/serve.c +4 -3
similarity index 81%
rename from builtin/serve.c
rename to t/helper/test-serve-v2.c
index d3fd240bb3..aee35e5aef 100644
@@ -1,14 +1,14 @@
+#include "test-tool.h"
 #include "cache.h"
-#include "builtin.h"
 #include "parse-options.h"
 #include "serve.h"
 
 static char const * const serve_usage[] = {
-	N_("git serve [<options>]"),
+	N_("test-tool serve-v2 [<options>]"),
 	NULL
 };
 
-int cmd_serve(int argc, const char **argv, const char *prefix)
+int cmd__serve_v2(int argc, const char **argv)
 {
 	struct serve_options opts = SERVE_OPTIONS_INIT;
 
@@ -19,6 +19,7 @@ int cmd_serve(int argc, const char **argv, const char *prefix)
 			 N_("exit immediately after advertising capabilities")),
 		OPT_END()
 	};
+	const char *prefix = setup_git_directory();
 
 	/* ignore all unknown cmdline switches for now */
 	argc = parse_options(argc, argv, prefix, options, serve_usage,
t/helper/test-tool.c +1 -0
index 2b21943f93..4bf3666b43 100644
@@ -48,6 +48,7 @@ static struct test_cmd cmds[] = {
 	{ "revision-walking", cmd__revision_walking },
 	{ "run-command", cmd__run_command },
 	{ "scrap-cache-tree", cmd__scrap_cache_tree },
+	{ "serve-v2", cmd__serve_v2 },
 	{ "sha1", cmd__sha1 },
 	{ "sha1-array", cmd__sha1_array },
 	{ "sha256", cmd__sha256 },
t/helper/test-tool.h +1 -0
index 25abed1cf2..bc72370916 100644
@@ -39,6 +39,7 @@ int cmd__repository(int argc, const char **argv);
 int cmd__revision_walking(int argc, const char **argv);
 int cmd__run_command(int argc, const char **argv);
 int cmd__scrap_cache_tree(int argc, const char **argv);
+int cmd__serve_v2(int argc, const char **argv);
 int cmd__sha1(int argc, const char **argv);
 int cmd__sha1_array(int argc, const char **argv);
 int cmd__sha256(int argc, const char **argv);
t/t5701-git-serve.sh +18 -14
index fe45bf828d..ffb9613885 100755
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='test git-serve and server commands'
+test_description='test protocol v2 server commands'
 
 . ./test-lib.sh
 
@@ -14,7 +14,8 @@ test_expect_success 'test capability advertisement' '
 	0000
 	EOF
 
-	GIT_TEST_SIDEBAND_ALL=0 git serve --advertise-capabilities >out &&
+	GIT_TEST_SIDEBAND_ALL=0 test-tool serve-v2 \
+		--advertise-capabilities >out &&
 	test-tool pkt-line unpack <out >actual &&
 	test_cmp expect actual
 '
@@ -24,11 +25,11 @@ test_expect_success 'stateless-rpc flag does not list capabilities' '
 	test-tool pkt-line pack >in <<-EOF &&
 	0000
 	EOF
-	git serve --stateless-rpc >out <in &&
+	test-tool serve-v2 --stateless-rpc >out <in &&
 	test_must_be_empty out &&
 
 	# EOF
-	git serve --stateless-rpc >out &&
+	test-tool serve-v2 --stateless-rpc >out &&
 	test_must_be_empty out
 '
 
@@ -37,7 +38,7 @@ test_expect_success 'request invalid capability' '
 	foobar
 	0000
 	EOF
-	test_must_fail git serve --stateless-rpc 2>err <in &&
+	test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
 	test_i18ngrep "unknown capability" err
 '
 
@@ -46,7 +47,7 @@ test_expect_success 'request with no command' '
 	agent=git/test
 	0000
 	EOF
-	test_must_fail git serve --stateless-rpc 2>err <in &&
+	test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
 	test_i18ngrep "no command requested" err
 '
 
@@ -56,7 +57,7 @@ test_expect_success 'request invalid command' '
 	agent=git/test
 	0000
 	EOF
-	test_must_fail git serve --stateless-rpc 2>err <in &&
+	test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
 	test_i18ngrep "invalid command" err
 '
 
@@ -87,7 +88,7 @@ test_expect_success 'basics of ls-refs' '
 	0000
 	EOF
 
-	git serve --stateless-rpc <in >out &&
+	test-tool serve-v2 --stateless-rpc <in >out &&
 	test-tool pkt-line unpack <out >actual &&
 	test_cmp expect actual
 '
@@ -107,7 +108,7 @@ test_expect_success 'basic ref-prefixes' '
 	0000
 	EOF
 
-	git serve --stateless-rpc <in >out &&
+	test-tool serve-v2 --stateless-rpc <in >out &&
 	test-tool pkt-line unpack <out >actual &&
 	test_cmp expect actual
 '
@@ -127,7 +128,7 @@ test_expect_success 'refs/heads prefix' '
 	0000
 	EOF
 
-	git serve --stateless-rpc <in >out &&
+	test-tool serve-v2 --stateless-rpc <in >out &&
 	test-tool pkt-line unpack <out >actual &&
 	test_cmp expect actual
 '
@@ -148,7 +149,7 @@ test_expect_success 'peel parameter' '
 	0000
 	EOF
 
-	git serve --stateless-rpc <in >out &&
+	test-tool serve-v2 --stateless-rpc <in >out &&
 	test-tool pkt-line unpack <out >actual &&
 	test_cmp expect actual
 '
@@ -169,7 +170,7 @@ test_expect_success 'symrefs parameter' '
 	0000
 	EOF
 
-	git serve --stateless-rpc <in >out &&
+	test-tool serve-v2 --stateless-rpc <in >out &&
 	test-tool pkt-line unpack <out >actual &&
 	test_cmp expect actual
 '
@@ -189,7 +190,7 @@ test_expect_success 'sending server-options' '
 	0000
 	EOF
 
-	git serve --stateless-rpc <in >out &&
+	test-tool serve-v2 --stateless-rpc <in >out &&
 	test-tool pkt-line unpack <out >actual &&
 	test_cmp expect actual
 '
@@ -204,7 +205,10 @@ test_expect_success 'unexpected lines are not allowed in fetch request' '
 	0000
 	EOF
 
-	test_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err &&
+	(
+		cd server &&
+		test_must_fail test-tool serve-v2 --stateless-rpc
+	) <in >/dev/null 2>err &&
 	grep "unexpected line: .this-is-not-a-command." err
 '
 
t/t5702-protocol-v2.sh +3 -2
index db4ae09f2f..8691bfc52d 100755
@@ -359,12 +359,13 @@ test_expect_success 'even with handcrafted request, filter does not work if not
 	0000
 	EOF
 
-	test_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err &&
+	test_must_fail test-tool -C server serve-v2 --stateless-rpc \
+		<in >/dev/null 2>err &&
 	grep "unexpected line: .filter blob:none." err &&
 
 	# Exercise to ensure that if advertised, filter works
 	git -C server config uploadpack.allowfilter 1 &&
-	git -C server serve --stateless-rpc <in >/dev/null
+	test-tool -C server serve-v2 --stateless-rpc <in >/dev/null
 '
 
 test_expect_success 'default refspec is used to filter ref when fetchcing' '
t/t5703-upload-pack-ref-in-want.sh +8 -8
index f87b2f6df3..dd1cbd0dd6 100755
@@ -48,15 +48,15 @@ test_expect_success 'setup repository' '
 '
 
 test_expect_success 'config controls ref-in-want advertisement' '
-	git serve --advertise-capabilities >out &&
+	test-tool serve-v2 --advertise-capabilities >out &&
 	! grep -a ref-in-want out &&
 
 	git config uploadpack.allowRefInWant false &&
-	git serve --advertise-capabilities >out &&
+	test-tool serve-v2 --advertise-capabilities >out &&
 	! grep -a ref-in-want out &&
 
 	git config uploadpack.allowRefInWant true &&
-	git serve --advertise-capabilities >out &&
+	test-tool serve-v2 --advertise-capabilities >out &&
 	grep -a ref-in-want out
 '
 
@@ -70,7 +70,7 @@ test_expect_success 'invalid want-ref line' '
 	0000
 	EOF
 
-	test_must_fail git serve --stateless-rpc 2>out <in &&
+	test_must_fail test-tool serve-v2 --stateless-rpc 2>out <in &&
 	grep "unknown ref" out
 '
 
@@ -90,7 +90,7 @@ test_expect_success 'basic want-ref' '
 	0000
 	EOF
 
-	git serve --stateless-rpc >out <in &&
+	test-tool serve-v2 --stateless-rpc >out <in &&
 	check_output
 '
 
@@ -112,7 +112,7 @@ test_expect_success 'multiple want-ref lines' '
 	0000
 	EOF
 
-	git serve --stateless-rpc >out <in &&
+	test-tool serve-v2 --stateless-rpc >out <in &&
 	check_output
 '
 
@@ -133,7 +133,7 @@ test_expect_success 'mix want and want-ref' '
 	0000
 	EOF
 
-	git serve --stateless-rpc >out <in &&
+	test-tool serve-v2 --stateless-rpc >out <in &&
 	check_output
 '
 
@@ -153,7 +153,7 @@ test_expect_success 'want-ref with ref we already have commit for' '
 	0000
 	EOF
 
-	git serve --stateless-rpc >out <in &&
+	test-tool serve-v2 --stateless-rpc >out <in &&
 	check_output
 '
 
Signature · 1 line
gitgitgadget
#24 lore raw

Re: [PATCH 7/7] Turn `git serve` into a test helper

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 12 lines · writes:

From: Johannes Schindelin <[email protected]>

The git serve built-in was introduced in ed10cb952d31 (serve:
introduce git-serve, 2018-03-15) as a backend to serve Git protocol v2, probably originally intended to be spawned by git upload-pack.

However, in the version that the protocol v2 patches made it into core Git, git upload-pack calls the serve() function directly instead of spawning git serve; The only reason in life for git serve to survive as a built-in command is to provide a way to test the protocol v2 functionality.

Quoted 7 lines

Meaning that it does not even have to be a built-in that is installed with end-user facing Git installations, but it can be a test helper instead.

Let's make it so.

I've excluded this step from tonight's pushout, as I would want to hear from the people on the other side who have (once) thought that this was an addition we would want to have, before we remove/demote it.

I do not personally think, as the design of v2 stands, a standalone "serve" server that "can serve anything as long as it goes over protocol v2" makes much sense, but perhaps those who have been doing the v2 work may have different ideas, in which case let's hear what their plans are.

Thanks.

#25 lore raw

Re: [PATCH 7/7] Turn `git serve` into a test helper

Quoted 28 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 12 lines · writes:

From: Johannes Schindelin <[email protected]>

The git serve built-in was introduced in ed10cb952d31 (serve:
introduce git-serve, 2018-03-15) as a backend to serve Git protocol v2, probably originally intended to be spawned by git upload-pack.

However, in the version that the protocol v2 patches made it into core Git, git upload-pack calls the serve() function directly instead of spawning git serve; The only reason in life for git serve to survive as a built-in command is to provide a way to test the protocol v2 functionality.

Quoted 7 lines

Meaning that it does not even have to be a built-in that is installed with end-user facing Git installations, but it can be a test helper instead.

Let's make it so.

I've excluded this step from tonight's pushout, as I would want to hear from the people on the other side who have (once) thought that this was an addition we would want to have, before we remove/demote it.

Quoted 5 lines

I do not personally think, as the design of v2 stands, a standalone "serve" server that "can serve anything as long as it goes over protocol v2" makes much sense, but perhaps those who have been doing the v2 work may have different ideas, in which case let's hear what their plans are.

I too would like to hear more definite comments from people who think git-serve is worth keeping. In the meantime, there's some discussion from this thread in December:

https://public-inbox.org/git/[email protected]/

especially this sub-thread:

https://public-inbox.org/git/[email protected]/

(In case you do not feel like reading the whole thing, my opinion there is that git-serve is probably not the right direction, and we would do well to demote it as Dscho's patch does).

-Peff

#26 lore raw

Re: [PATCH 7/7] Turn `git serve` into a test helper

Quoted 12 lines · Jeff King
Quoted 5 lines

I do not personally think, as the design of v2 stands, a standalone "serve" server that "can serve anything as long as it goes over protocol v2" makes much sense, but perhaps those who have been doing the v2 work may have different ideas, in which case let's hear what their plans are.

I too would like to hear more definite comments from people who think git-serve is worth keeping. In the meantime, there's some discussion from this thread in December:
...
https://public-inbox.org/git/[email protected]/

Quoted 3 lines

(In case you do not feel like reading the whole thing, my opinion there is that git-serve is probably not the right direction, and we would do well to demote it as Dscho's patch does).

I guess we are more or less on the same page, then. I'll let others chime in by waiting for a bit more but I won't wait forever ;-).

Thanks.

#27 lore raw

Re: [PATCH 7/7] Turn `git serve` into a test helper

Quoted 14 lines · Junio C Hamano
Quoted 12 lines · Jeff King
Quoted 5 lines

I do not personally think, as the design of v2 stands, a standalone "serve" server that "can serve anything as long as it goes over protocol v2" makes much sense, but perhaps those who have been doing the v2 work may have different ideas, in which case let's hear what their plans are.

I too would like to hear more definite comments from people who think git-serve is worth keeping. In the meantime, there's some discussion from this thread in December:
...
https://public-inbox.org/git/[email protected]/

Quoted 8 lines
Quoted 3 lines

(In case you do not feel like reading the whole thing, my opinion there is that git-serve is probably not the right direction, and we would do well to demote it as Dscho's patch does).

I guess we are more or less on the same page, then. I'll let others chime in by waiting for a bit more but I won't wait forever ;-).

Thanks.

FWIW I used git-serve a fair amount while working on V2 support for archiving, and everything I did with it would have been just as easy with a test helper as with a builtin. So I have no objections to this change.

#28 lore raw

Re: [PATCH 7/7] Turn `git serve` into a test helper

Quoted 6 lines · Josh Steadmon
Quoted 5 lines · Junio C Hamano

Jeff King <[email protected]> writes:
...
I guess we are more or less on the same page, then. I'll let others chime in by waiting for a bit more but I won't wait forever ;-).

Quoted 6 lines
Quoted 1 line

Thanks.

FWIW I used git-serve a fair amount while working on V2 support for archiving, and everything I did with it would have been just as easy with a test helper as with a builtin. So I have no objections to this change.

Well, let's queue the final step together with the others, than.

Thanks, all.

#29 lore raw

Re: [PATCH 7/7] Turn `git serve` into a test helper

In reply to Junio C Hamano

Hi Junio,

Quoted 19 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 16 lines · writes:

From: Johannes Schindelin <[email protected]>

The git serve built-in was introduced in ed10cb952d31 (serve:
introduce git-serve, 2018-03-15) as a backend to serve Git protocol v2, probably originally intended to be spawned by git upload-pack.

However, in the version that the protocol v2 patches made it into core Git, git upload-pack calls the serve() function directly instead of spawning git serve; The only reason in life for git serve to survive as a built-in command is to provide a way to test the protocol v2 functionality.

Meaning that it does not even have to be a built-in that is installed with end-user facing Git installations, but it can be a test helper instead.

Quoted 5 lines
Quoted 3 lines

Let's make it so.

I've excluded this step from tonight's pushout, as I would want to

                                         ^^^^^^^

Would you be surprised that I first read this as "pu shout"?

Quoted 9 lines

hear from the people on the other side who have (once) thought that this was an addition we would want to have, before we remove/demote it.

I do not personally think, as the design of v2 stands, a standalone "serve" server that "can serve anything as long as it goes over protocol v2" makes much sense, but perhaps those who have been doing the v2 work may have different ideas, in which case let's hear what their plans are.

As this has been resolved in the meantime, I'll just leave it as-is.

Ciao,
Dscho

#30 lore raw

v2 cover Assorted Documentation-related fixes

In reply to Johannes Schindelin via GitGitGadget

While working on better support for make check-docs on Windows, I noticed a couple more things, e.g. some "commands" were reported as being listed but not built, e.g. gitcli (i.e. the parts of command-list.txt that are marked as "guide").

This patch series cleans up those loose ends: after this, make check-docs reports no issues on Windows.

Changes since v1:

* There is now an extra patch that gets rid of the NO_INSTALL variable in the Makefile altogether.
* The generate-cmdlist.sh patch now results in more robust code (thanks, Junio!).
* Patch 2/7 has a much better commit message now, and instead of filtering out excluded commands from the command-list.txt, it expects excluded commands by looking not only at $(ALL_COMMANDS) but also at $(EXCLUDED_PROGRAMS).
* Instead of the fragile logic to generate Documentation/GIT-EXCLUDED-PROGRAMS that I hoped would let me get away with less work, I now imitate the logic of GIT-CFLAGS (and the resulting patch is actually a lot easier to read).

Johannes Schindelin (8):
remote-testgit: move it into the support directory for t5801
Makefile: drop the NO_INSTALL variable
help -a: do not list commands that are excluded from the build
check-docs: allow command-list.txt to contain excluded commands
docs: exclude documentation for commands that have been excluded
check-docs: do not bother checking for legacy scripts' documentation
test-tool: handle the -C <directory> option just like git Turn git serve into a test helper

 .gitignore                                    |  1 -
 Documentation/.gitignore                      |  1 +
 Documentation/Makefile                        |  3 ++
 Makefile                                      | 53 +++++++++++--------
 builtin.h                                     |  1 -
 generate-cmdlist.sh                           | 10 +++-
 git.c                                         |  1 -
 builtin/serve.c => t/helper/test-serve-v2.c   |  7 +--
 t/helper/test-tool.c                          | 20 +++++++
 t/helper/test-tool.h                          |  1 +
 t/t5701-git-serve.sh                          | 32 ++++++-----
 t/t5702-protocol-v2.sh                        |  5 +-
 t/t5703-upload-pack-ref-in-want.sh            | 16 +++---
 t/t5801-remote-helpers.sh                     |  2 +
 .../t5801/git-remote-testgit                  |  0
 15 files changed, 101 insertions(+), 52 deletions(-)
 rename builtin/serve.c => t/helper/test-serve-v2.c (81%)
 rename git-remote-testgit.sh => t/t5801/git-remote-testgit (100%)

base-commit: 5ee42463399ca3cc75b7e6e4368a3a5df5b010f2
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-168%2Fdscho%2Fdocs-misc-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-168/dscho/docs-misc-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/168

Range-diff vs v1:

 1:  81c08b178b = 1:  81c08b178b remote-testgit: move it into the support directory for t5801
 -:  ---------- > 2:  fda0b10c84 Makefile: drop the NO_INSTALL variable
 2:  7dc5293e9e ! 3:  9b498a6f21 help -a: do not list commands that are excluded from the build
     @@ -70,7 +70,7 @@
       
       command_list () {
      -	grep -v '^#' "$1"
     -+	eval grep -ve '^#' $exclude_programs "$1"
     ++	eval "grep -ve '^#' $exclude_programs" <"$1"
       }
       
       get_categories () {
     @@ -79,7 +79,7 @@
       }
       
      +exclude_programs=
     -+while test "a$1" = "a--exclude-program"
     ++while test "--exclude-program" = "$1"
      +do
      +	shift
      +	exclude_programs="$exclude_programs -e \"^$1 \""
 3:  96ced7e17c < -:  ---------- check-docs: do not pretend that commands are listed which are excluded
 4:  31d8e43cbf < -:  ---------- docs: exclude documentation for commands that have been excluded
 -:  ---------- > 4:  ac3670a805 check-docs: allow command-list.txt to contain excluded commands
 -:  ---------- > 5:  f8d133c597 docs: exclude documentation for commands that have been excluded
 5:  fb3daa6427 = 6:  05d4ad62d6 check-docs: do not bother checking for legacy scripts' documentation
 6:  2e19f538bc = 7:  cf73021574 test-tool: handle the `-C <directory>` option just like `git`
 7:  411587e4b8 = 8:  88a5ab2332 Turn `git serve` into a test helper
Signature · 1 line
gitgitgadget
#31 lore raw

PATCH v2 1/8 remote-testgit: move it into the support directory for t5801

From: Johannes Schindelin <[email protected]>

The git-remote-testgit script is really only used in t5801-remote-helpers.sh. It does not even contain any @@<MAGIC>@@ placeholders that would need to be interpolated via make git-remote-testgit.

Let's just move it to a new home, decluttering the top-level directory and clarifying that this is just a test helper, not an official Git command that we would want to ever support.

Suggested by Ævar Arnfjörð Bjarmason.

---
 .gitignore                                          | 1 -
 Makefile                                            | 3 ---
 t/t5801-remote-helpers.sh                           | 2 ++
 git-remote-testgit.sh => t/t5801/git-remote-testgit | 0
 4 files changed, 2 insertions(+), 4 deletions(-)
 rename git-remote-testgit.sh => t/t5801/git-remote-testgit (100%)
.gitignore +0 -1
index 7374587f9d..de8fc2f5b1 100644
@@ -135,7 +135,6 @@
 /git-remote-ftps
 /git-remote-fd
 /git-remote-ext
-/git-remote-testgit
 /git-remote-testpy
 /git-remote-testsvn
 /git-repack
Makefile +0 -3
index 8654c130f8..26f8ed2228 100644
@@ -633,7 +633,6 @@ SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
 SCRIPT_SH += git-quiltimport.sh
 SCRIPT_SH += git-legacy-rebase.sh
-SCRIPT_SH += git-remote-testgit.sh
 SCRIPT_SH += git-request-pull.sh
 SCRIPT_SH += git-stash.sh
 SCRIPT_SH += git-submodule.sh
@@ -657,8 +656,6 @@ SCRIPT_PERL += git-svn.perl
 
 SCRIPT_PYTHON += git-p4.py
 
-NO_INSTALL += git-remote-testgit
-
 # Generated files for scripts
 SCRIPT_SH_GEN = $(patsubst %.sh,%,$(SCRIPT_SH))
 SCRIPT_PERL_GEN = $(patsubst %.perl,%,$(SCRIPT_PERL))
t/t5801-remote-helpers.sh +2 -0
index aaaa722cca..d04f8007e0 100755
@@ -8,6 +8,8 @@ test_description='Test remote-helper import and export commands'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-gpg.sh
 
+PATH="$TEST_DIRECTORY/t5801:$PATH"
+
 compare_refs() {
 	git --git-dir="$1/.git" rev-parse --verify $2 >expect &&
 	git --git-dir="$3/.git" rev-parse --verify $4 >actual &&
t/t5801/git-remote-testgit renamed from git-remote-testgit.sh +0 -0
similarity index 100%
rename from git-remote-testgit.sh
rename to t/t5801/git-remote-testgit
Signature · 1 line
gitgitgadget
#32 lore raw

PATCH v2 2/8 Makefile: drop the NO_INSTALL variable

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

The last user was just removed; There is no longer any need to carry it around. Should we ever run into a need for it again, it is easy enough to revert this commit.

It is unlikely, though, that we need NO_INSTALL again: as we saw with the just-removed item, git-remote-testgit, we have better locations to put executables and scripts that we do not want to install, e.g. a subdirectory in t/, or contrib/.

---
 Makefile | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)
Makefile +8 -13
index 26f8ed2228..f5be2f633e 100644
@@ -661,10 +661,6 @@ SCRIPT_SH_GEN = $(patsubst %.sh,%,$(SCRIPT_SH))
 SCRIPT_PERL_GEN = $(patsubst %.perl,%,$(SCRIPT_PERL))
 SCRIPT_PYTHON_GEN = $(patsubst %.py,%,$(SCRIPT_PYTHON))
 
-SCRIPT_SH_INS = $(filter-out $(NO_INSTALL),$(SCRIPT_SH_GEN))
-SCRIPT_PERL_INS = $(filter-out $(NO_INSTALL),$(SCRIPT_PERL_GEN))
-SCRIPT_PYTHON_INS = $(filter-out $(NO_INSTALL),$(SCRIPT_PYTHON_GEN))
-
 # Individual rules to allow e.g.
 # "make -C ../.. SCRIPT_PERL=contrib/foo/bar.perl build-perl-script"
 # from subdirectories like contrib/*/
@@ -674,11 +670,11 @@ build-sh-script: $(SCRIPT_SH_GEN)
 build-python-script: $(SCRIPT_PYTHON_GEN)
 
 .PHONY: install-perl-script install-sh-script install-python-script
-install-sh-script: $(SCRIPT_SH_INS)
+install-sh-script: $(SCRIPT_SH_GEN)
 	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
-install-perl-script: $(SCRIPT_PERL_INS)
+install-perl-script: $(SCRIPT_PERL_GEN)
 	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
-install-python-script: $(SCRIPT_PYTHON_INS)
+install-python-script: $(SCRIPT_PYTHON_GEN)
 	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 
 .PHONY: clean-perl-script clean-sh-script clean-python-script
@@ -689,9 +685,9 @@ clean-perl-script:
 clean-python-script:
 	$(RM) $(SCRIPT_PYTHON_GEN)
 
-SCRIPTS = $(SCRIPT_SH_INS) \
-	  $(SCRIPT_PERL_INS) \
-	  $(SCRIPT_PYTHON_INS) \
+SCRIPTS = $(SCRIPT_SH_GEN) \
+	  $(SCRIPT_PERL_GEN) \
+	  $(SCRIPT_PYTHON_GEN) \
 	  git-instaweb
 
 ETAGS_TARGET = TAGS
@@ -2683,7 +2679,6 @@ endif
 test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
 
 all:: $(TEST_PROGRAMS) $(test_bindir_programs)
-all:: $(NO_INSTALL)
 
 bin-wrappers/%: wrap-for-bin.sh
 	@mkdir -p bin-wrappers
@@ -2967,7 +2962,7 @@ rpm::
 
 artifacts-tar:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) \
 		GIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \
-		$(NO_INSTALL) $(MOFILES)
+		$(MOFILES)
 	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) \
 		SHELL_PATH='$(SHELL_PATH_SQ)' PERL_PATH='$(PERL_PATH_SQ)'
 	test -n "$(ARTIFACTS_DIRECTORY)"
@@ -3016,7 +3011,7 @@ clean: profile-clean coverage-clean cocciclean
 	$(RM) $(OBJECTS)
 	$(RM) $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB)
 	$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X
-	$(RM) $(TEST_PROGRAMS) $(NO_INSTALL)
+	$(RM) $(TEST_PROGRAMS)
 	$(RM) $(FUZZ_PROGRAMS)
 	$(RM) -r bin-wrappers $(dep_dirs)
 	$(RM) -r po/build/
Signature · 1 line
gitgitgadget
#33 lore raw

PATCH v2 3/8 help -a: do not list commands that are excluded from the build

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

When built with NO_CURL or with NO_UNIX_SOCKETS, some commands are skipped from the build. It does not make sense to list them in the output of git help -a, so let's just not.

---
 Makefile            | 14 ++++++++++++--
 generate-cmdlist.sh | 10 +++++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)
Makefile +12 -2
index f5be2f633e..e7bcfcaae9 100644
@@ -611,6 +611,7 @@ FUZZ_PROGRAMS =
 LIB_OBJS =
 PROGRAM_OBJS =
 PROGRAMS =
+EXCLUDED_PROGRAMS =
 SCRIPT_PERL =
 SCRIPT_PYTHON =
 SCRIPT_SH =
@@ -1319,6 +1320,7 @@ ifdef NO_CURL
 	REMOTE_CURL_PRIMARY =
 	REMOTE_CURL_ALIASES =
 	REMOTE_CURL_NAMES =
+	EXCLUDED_PROGRAMS += git-http-fetch git-http-push
 else
 	ifdef CURLDIR
 		# Try "-Wl,-rpath=$(CURLDIR)/$(lib)" in such a case.
@@ -1343,7 +1345,11 @@ endif
 	ifeq "$(curl_check)" "070908"
 		ifndef NO_EXPAT
 			PROGRAM_OBJS += http-push.o
+		else
+			EXCLUDED_PROGRAMS += git-http-push
 		endif
+	else
+		EXCLUDED_PROGRAMS += git-http-push
 	endif
 	curl_check := $(shell (echo 072200; $(CURL_CONFIG) --vernum | sed -e '/^70[BC]/s/^/0/') 2>/dev/null | sort -r | sed -ne 2p)
 	ifeq "$(curl_check)" "072200"
@@ -1589,7 +1595,9 @@ ifdef NO_INET_PTON
 	LIB_OBJS += compat/inet_pton.o
 	BASIC_CFLAGS += -DNO_INET_PTON
 endif
-ifndef NO_UNIX_SOCKETS
+ifdef NO_UNIX_SOCKETS
+	EXCLUDED_PROGRAMS += git-credential-cache git-credential-cache--daemon
+else
 	LIB_OBJS += unix-socket.o
 	PROGRAM_OBJS += credential-cache.o
 	PROGRAM_OBJS += credential-cache--daemon.o
@@ -2108,7 +2116,9 @@ $(BUILT_INS): git$X
 command-list.h: generate-cmdlist.sh command-list.txt
 
 command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt Documentation/config/*.txt
-	$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh command-list.txt >$@+ && mv $@+ $@
+	$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh \
+		$(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \
+		command-list.txt >$@+ && mv $@+ $@
 
 SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
 	$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
generate-cmdlist.sh +9 -1
index 709d67405b..71158f7d8b 100755
@@ -6,7 +6,7 @@ die () {
 }
 
 command_list () {
-	grep -v '^#' "$1"
+	eval "grep -ve '^#' $exclude_programs" <"$1"
 }
 
 get_categories () {
@@ -93,6 +93,14 @@ EOF
 EOF
 }
 
+exclude_programs=
+while test "--exclude-program" = "$1"
+do
+	shift
+	exclude_programs="$exclude_programs -e \"^$1 \""
+	shift
+done
+
 echo "/* Automatically generated by generate-cmdlist.sh */
 struct cmdname_help {
 	const char *name;
Signature · 1 line
gitgitgadget
#34 lore raw

PATCH v2 4/8 check-docs: allow command-list.txt to contain excluded commands

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

Among other things, the check-docs target ensures that command-list.txt no longer contains commands that were dropped (or that were never added in the first place).

To do so, it compares the list of commands from that file to the commands listed in $(ALL_COMMANDS).

However, some build options exclude commands from the latter. Fix the target to handle this situation correctly by taking the just-introduced $(EXCLUDED_PROGRAMS) into account.

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Makefile +1 -1
index e7bcfcaae9..d83104d884 100644
@@ -3089,7 +3089,7 @@ check-docs::
 		    -e 's/\.txt//'; \
 	) | while read how cmd; \
 	do \
-		case " $(patsubst %$X,%,$(ALL_COMMANDS)) " in \
+		case " $(patsubst %$X,%,$(ALL_COMMANDS) $(EXCLUDED_PROGRAMS)) " in \
 		*" $$cmd "*)	;; \
 		*) echo "removed but $$how: $$cmd" ;; \
 		esac; \
Signature · 1 line
gitgitgadget
#35 lore raw

PATCH v2 5/8 docs: exclude documentation for commands that have been excluded

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

When building with certain build options, some commands are excluded from the build. For example, git-credential-cache is skipped when building with NO_UNIX_SOCKETS.

Let's not build or package documentation for those excluded commands.

This issue was pointed out rightfully when running make check-docs on Windows, where we do not yet have Unix sockets, and therefore the credential-cache command is excluded (yet its documentation was built and shipped).

Note: building the documentation via make -C Documentation leaves the build system with no way to determine which commands have been excluded. If called thusly, we gracefully fail to exclude their documentation. Only when building the documentation via the top-level Makefile will it get excluded properly, or after building Documentation/GIT-EXCLUDED-PROGRAMS manually.

---
 Documentation/.gitignore | 1 +
 Documentation/Makefile   | 3 +++
 Makefile                 | 9 +++++++++
 3 files changed, 13 insertions(+)
Documentation/.gitignore +1 -0
index 3ef54e0adb..ea27148c59 100644
@@ -13,3 +13,4 @@ mergetools-*.txt
 manpage-base-url.xsl
 SubmittingPatches.txt
 tmp-doc-diff/
+/GIT-EXCLUDED-PROGRAMS
Documentation/Makefile +3 -0
index af0e2cf11a..e22ea2f57c 100644
@@ -7,7 +7,10 @@ ARTICLES =
 SP_ARTICLES =
 OBSOLETE_HTML =
 
+-include GIT-EXCLUDED-PROGRAMS
+
 MAN1_TXT += $(filter-out \
+		$(patsubst %,%.txt,$(EXCLUDED_PROGRAMS)) \
 		$(addsuffix .txt, $(ARTICLES) $(SP_ARTICLES)), \
 		$(wildcard git-*.txt))
 MAN1_TXT += git.txt
Makefile +9 -0
index d83104d884..6f90cec590 100644
@@ -2451,6 +2451,14 @@ $(VCSSVN_LIB): $(VCSSVN_OBJS)
 
 export DEFAULT_EDITOR DEFAULT_PAGER
 
+Documentation/GIT-EXCLUDED-PROGRAMS: FORCE
+	@EXCLUDED='EXCLUDED_PROGRAMS := $(EXCLUDED_PROGRAMS)'; \
+	    if test x"$$EXCLUDED" != \
+		x"`cat Documentation/GIT-EXCLUDED-PROGRAMS 2>/dev/null`" ; then \
+		echo >&2 "    * new documentation flags"; \
+		echo "$$EXCLUDED" >Documentation/GIT-EXCLUDED-PROGRAMS; \
+            fi
+
 .PHONY: doc man man-perl html info pdf
 doc: man-perl
 	$(MAKE) -C Documentation all
@@ -3030,6 +3038,7 @@ clean: profile-clean coverage-clean cocciclean
 	$(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
 	$(RM) $(htmldocs).tar.gz $(manpages).tar.gz
 	$(MAKE) -C Documentation/ clean
+	$(RM) Documentation/GIT-EXCLUDED-PROGRAMS
 ifndef NO_PERL
 	$(MAKE) -C gitweb clean
 	$(RM) -r perl/build/
Signature · 1 line
gitgitgadget
#36 lore raw

Re: [PATCH v2 5/8] docs: exclude documentation for commands that have been excluded

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 13 lines · writes:

From: Johannes Schindelin <[email protected]>

When building with certain build options, some commands are excluded from the build. For example, git-credential-cache is skipped when building with NO_UNIX_SOCKETS.

Let's not build or package documentation for those excluded commands.

This issue was pointed out rightfully when running make check-docs on Windows, where we do not yet have Unix sockets, and therefore the credential-cache command is excluded (yet its documentation was built and shipped).

Quoted 6 lines

Note: building the documentation via make -C Documentation leaves the build system with no way to determine which commands have been excluded. If called thusly, we gracefully fail to exclude their documentation. Only when building the documentation via the top-level Makefile will it get excluded properly, or after building Documentation/GIT-EXCLUDED-PROGRAMS manually.

I certainly know where you come from, but I am on the fence. Being able to omit documentation, without thinking, for what you do not ship is surely handy, but at the same time, being able to format everything you have the necessary material for is also valuable.

#37 lore raw

Re: [PATCH v2 5/8] docs: exclude documentation for commands that have been excluded

Hi Junio,

Quoted 23 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 13 lines · writes:

From: Johannes Schindelin <[email protected]>

When building with certain build options, some commands are excluded from the build. For example, git-credential-cache is skipped when building with NO_UNIX_SOCKETS.

Let's not build or package documentation for those excluded commands.

This issue was pointed out rightfully when running make check-docs on Windows, where we do not yet have Unix sockets, and therefore the credential-cache command is excluded (yet its documentation was built and shipped).

Quoted 6 lines

Note: building the documentation via make -C Documentation leaves the build system with no way to determine which commands have been excluded. If called thusly, we gracefully fail to exclude their documentation. Only when building the documentation via the top-level Makefile will it get excluded properly, or after building Documentation/GIT-EXCLUDED-PROGRAMS manually.

Quoted 4 lines

I certainly know where you come from, but I am on the fence. Being able to omit documentation, without thinking, for what you do not ship is surely handy, but at the same time, being able to format everything you have the necessary material for is also valuable.

I guess you need a maintainer's mode ;-)

Ciao,
Dscho

#38 lore raw

PATCH v2 6/8 check-docs: do not bother checking for legacy scripts' documentation

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

In the recent years, there has been a big push to convert more and more of Git's commands that are implemented as scripts to built-ins written in pure, portable C, for robustness, speed and portability.

One strategy that served us well is to convert those scripts incrementally, starting by renaming the scripts to git-legacy-<command>, then introducing a built-in that does nothing else at first than checking the config setting <command>.useBuiltin (which defaults to false at the outset) and handing off to the legacy script if so asked.

Obviously, those git-legacy-<command> commands share the documentation with the built-in git-<command>, and are not intended to be called directly anyway. So let's not try to ensure that they are documented separately from their built-in versions.

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Makefile +1 -1
index 6f90cec590..f5fc977ee8 100644
@@ -3074,7 +3074,7 @@ check-docs::
 		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-remote-* | git-stage | git-legacy-* | \
 		git-?*--?* ) continue ;; \
 		esac ; \
 		test -f "Documentation/$$v.txt" || \
Signature · 1 line
gitgitgadget
#39 lore raw

PATCH v2 7/8 test-tool: handle the `-C <directory>` option just like `git`

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

In preparation for moving git serve into test-tool (because it really is only used by the test suite), we teach the test-tool the useful trick to change the working directory before running the test command, which will avoid introducing subshells in the test code.

---
 t/helper/test-tool.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
t/helper/test-tool.c +19 -0
index 99db7409b8..2b21943f93 100644
@@ -1,5 +1,11 @@
 #include "git-compat-util.h"
 #include "test-tool.h"
+#include "parse-options.h"
+
+static const char * const test_tool_usage[] = {
+	"test-tool [-C <directory>] <command [<arguments>...]]",
+	NULL
+};
 
 struct test_cmd {
 	const char *name;
@@ -73,11 +79,24 @@ static NORETURN void die_usage(void)
 int cmd_main(int argc, const char **argv)
 {
 	int i;
+	const char *working_directory = NULL;
+	struct option options[] = {
+		OPT_STRING('C', NULL, &working_directory, "directory",
+			   "change the working directory"),
+		OPT_END()
+	};
 
 	BUG_exit_code = 99;
+	argc = parse_options(argc, argv, NULL, options, test_tool_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION |
+			     PARSE_OPT_KEEP_ARGV0);
+
 	if (argc < 2)
 		die_usage();
 
+	if (working_directory && chdir(working_directory) < 0)
+		die("Could not cd to '%s'", working_directory);
+
 	for (i = 0; i < ARRAY_SIZE(cmds); i++) {
 		if (!strcmp(cmds[i].name, argv[1])) {
 			argv++;
Signature · 1 line
gitgitgadget
#40 lore raw

PATCH v2 8/8 Turn `git serve` into a test helper

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

The git serve built-in was introduced in ed10cb952d31 (serve:
introduce git-serve, 2018-03-15) as a backend to serve Git protocol v2, probably originally intended to be spawned by git upload-pack.

However, in the version that the protocol v2 patches made it into core Git, git upload-pack calls the serve() function directly instead of spawning git serve; The only reason in life for git serve to survive as a built-in command is to provide a way to test the protocol v2 functionality.

Meaning that it does not even have to be a built-in that is installed with end-user facing Git installations, but it can be a test helper instead.

Let's make it so.

---
 Makefile                                    |  2 +-
 builtin.h                                   |  1 -
 git.c                                       |  1 -
 builtin/serve.c => t/helper/test-serve-v2.c |  7 +++--
 t/helper/test-tool.c                        |  1 +
 t/helper/test-tool.h                        |  1 +
 t/t5701-git-serve.sh                        | 32 ++++++++++++---------
 t/t5702-protocol-v2.sh                      |  5 ++--
 t/t5703-upload-pack-ref-in-want.sh          | 16 +++++------
 9 files changed, 36 insertions(+), 30 deletions(-)
 rename builtin/serve.c => t/helper/test-serve-v2.c (81%)
Makefile +1 -1
index f5fc977ee8..9ce62fa295 100644
@@ -758,6 +758,7 @@ TEST_BUILTINS_OBJS += test-repository.o
 TEST_BUILTINS_OBJS += test-revision-walking.o
 TEST_BUILTINS_OBJS += test-run-command.o
 TEST_BUILTINS_OBJS += test-scrap-cache-tree.o
+TEST_BUILTINS_OBJS += test-serve-v2.o
 TEST_BUILTINS_OBJS += test-sha1.o
 TEST_BUILTINS_OBJS += test-sha1-array.o
 TEST_BUILTINS_OBJS += test-sha256.o
@@ -1127,7 +1128,6 @@ BUILTIN_OBJS += builtin/rev-parse.o
 BUILTIN_OBJS += builtin/revert.o
 BUILTIN_OBJS += builtin/rm.o
 BUILTIN_OBJS += builtin/send-pack.o
-BUILTIN_OBJS += builtin/serve.o
 BUILTIN_OBJS += builtin/shortlog.o
 BUILTIN_OBJS += builtin/show-branch.o
 BUILTIN_OBJS += builtin/show-index.o
builtin.h +0 -1
index 6538932e99..c48636e244 100644
@@ -219,7 +219,6 @@ extern int cmd_rev_parse(int argc, const char **argv, const char *prefix);
 extern int cmd_revert(int argc, const char **argv, const char *prefix);
 extern int cmd_rm(int argc, const char **argv, const char *prefix);
 extern int cmd_send_pack(int argc, const char **argv, const char *prefix);
-extern int cmd_serve(int argc, const char **argv, const char *prefix);
 extern int cmd_shortlog(int argc, const char **argv, const char *prefix);
 extern int cmd_show(int argc, const char **argv, const char *prefix);
 extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
git.c +0 -1
index 2dd588674f..c58b067771 100644
@@ -548,7 +548,6 @@ static struct cmd_struct commands[] = {
 	{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
 	{ "rm", cmd_rm, RUN_SETUP },
 	{ "send-pack", cmd_send_pack, RUN_SETUP },
-	{ "serve", cmd_serve, RUN_SETUP },
 	{ "shortlog", cmd_shortlog, RUN_SETUP_GENTLY | USE_PAGER },
 	{ "show", cmd_show, RUN_SETUP },
 	{ "show-branch", cmd_show_branch, RUN_SETUP },
t/helper/test-serve-v2.c renamed from builtin/serve.c +4 -3
similarity index 81%
rename from builtin/serve.c
rename to t/helper/test-serve-v2.c
index d3fd240bb3..aee35e5aef 100644
@@ -1,14 +1,14 @@
+#include "test-tool.h"
 #include "cache.h"
-#include "builtin.h"
 #include "parse-options.h"
 #include "serve.h"
 
 static char const * const serve_usage[] = {
-	N_("git serve [<options>]"),
+	N_("test-tool serve-v2 [<options>]"),
 	NULL
 };
 
-int cmd_serve(int argc, const char **argv, const char *prefix)
+int cmd__serve_v2(int argc, const char **argv)
 {
 	struct serve_options opts = SERVE_OPTIONS_INIT;
 
@@ -19,6 +19,7 @@ int cmd_serve(int argc, const char **argv, const char *prefix)
 			 N_("exit immediately after advertising capabilities")),
 		OPT_END()
 	};
+	const char *prefix = setup_git_directory();
 
 	/* ignore all unknown cmdline switches for now */
 	argc = parse_options(argc, argv, prefix, options, serve_usage,
t/helper/test-tool.c +1 -0
index 2b21943f93..4bf3666b43 100644
@@ -48,6 +48,7 @@ static struct test_cmd cmds[] = {
 	{ "revision-walking", cmd__revision_walking },
 	{ "run-command", cmd__run_command },
 	{ "scrap-cache-tree", cmd__scrap_cache_tree },
+	{ "serve-v2", cmd__serve_v2 },
 	{ "sha1", cmd__sha1 },
 	{ "sha1-array", cmd__sha1_array },
 	{ "sha256", cmd__sha256 },
t/helper/test-tool.h +1 -0
index 25abed1cf2..bc72370916 100644
@@ -39,6 +39,7 @@ int cmd__repository(int argc, const char **argv);
 int cmd__revision_walking(int argc, const char **argv);
 int cmd__run_command(int argc, const char **argv);
 int cmd__scrap_cache_tree(int argc, const char **argv);
+int cmd__serve_v2(int argc, const char **argv);
 int cmd__sha1(int argc, const char **argv);
 int cmd__sha1_array(int argc, const char **argv);
 int cmd__sha256(int argc, const char **argv);
t/t5701-git-serve.sh +18 -14
index fe45bf828d..ffb9613885 100755
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='test git-serve and server commands'
+test_description='test protocol v2 server commands'
 
 . ./test-lib.sh
 
@@ -14,7 +14,8 @@ test_expect_success 'test capability advertisement' '
 	0000
 	EOF
 
-	GIT_TEST_SIDEBAND_ALL=0 git serve --advertise-capabilities >out &&
+	GIT_TEST_SIDEBAND_ALL=0 test-tool serve-v2 \
+		--advertise-capabilities >out &&
 	test-tool pkt-line unpack <out >actual &&
 	test_cmp expect actual
 '
@@ -24,11 +25,11 @@ test_expect_success 'stateless-rpc flag does not list capabilities' '
 	test-tool pkt-line pack >in <<-EOF &&
 	0000
 	EOF
-	git serve --stateless-rpc >out <in &&
+	test-tool serve-v2 --stateless-rpc >out <in &&
 	test_must_be_empty out &&
 
 	# EOF
-	git serve --stateless-rpc >out &&
+	test-tool serve-v2 --stateless-rpc >out &&
 	test_must_be_empty out
 '
 
@@ -37,7 +38,7 @@ test_expect_success 'request invalid capability' '
 	foobar
 	0000
 	EOF
-	test_must_fail git serve --stateless-rpc 2>err <in &&
+	test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
 	test_i18ngrep "unknown capability" err
 '
 
@@ -46,7 +47,7 @@ test_expect_success 'request with no command' '
 	agent=git/test
 	0000
 	EOF
-	test_must_fail git serve --stateless-rpc 2>err <in &&
+	test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
 	test_i18ngrep "no command requested" err
 '
 
@@ -56,7 +57,7 @@ test_expect_success 'request invalid command' '
 	agent=git/test
 	0000
 	EOF
-	test_must_fail git serve --stateless-rpc 2>err <in &&
+	test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
 	test_i18ngrep "invalid command" err
 '
 
@@ -87,7 +88,7 @@ test_expect_success 'basics of ls-refs' '
 	0000
 	EOF
 
-	git serve --stateless-rpc <in >out &&
+	test-tool serve-v2 --stateless-rpc <in >out &&
 	test-tool pkt-line unpack <out >actual &&
 	test_cmp expect actual
 '
@@ -107,7 +108,7 @@ test_expect_success 'basic ref-prefixes' '
 	0000
 	EOF
 
-	git serve --stateless-rpc <in >out &&
+	test-tool serve-v2 --stateless-rpc <in >out &&
 	test-tool pkt-line unpack <out >actual &&
 	test_cmp expect actual
 '
@@ -127,7 +128,7 @@ test_expect_success 'refs/heads prefix' '
 	0000
 	EOF
 
-	git serve --stateless-rpc <in >out &&
+	test-tool serve-v2 --stateless-rpc <in >out &&
 	test-tool pkt-line unpack <out >actual &&
 	test_cmp expect actual
 '
@@ -148,7 +149,7 @@ test_expect_success 'peel parameter' '
 	0000
 	EOF
 
-	git serve --stateless-rpc <in >out &&
+	test-tool serve-v2 --stateless-rpc <in >out &&
 	test-tool pkt-line unpack <out >actual &&
 	test_cmp expect actual
 '
@@ -169,7 +170,7 @@ test_expect_success 'symrefs parameter' '
 	0000
 	EOF
 
-	git serve --stateless-rpc <in >out &&
+	test-tool serve-v2 --stateless-rpc <in >out &&
 	test-tool pkt-line unpack <out >actual &&
 	test_cmp expect actual
 '
@@ -189,7 +190,7 @@ test_expect_success 'sending server-options' '
 	0000
 	EOF
 
-	git serve --stateless-rpc <in >out &&
+	test-tool serve-v2 --stateless-rpc <in >out &&
 	test-tool pkt-line unpack <out >actual &&
 	test_cmp expect actual
 '
@@ -204,7 +205,10 @@ test_expect_success 'unexpected lines are not allowed in fetch request' '
 	0000
 	EOF
 
-	test_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err &&
+	(
+		cd server &&
+		test_must_fail test-tool serve-v2 --stateless-rpc
+	) <in >/dev/null 2>err &&
 	grep "unexpected line: .this-is-not-a-command." err
 '
 
t/t5702-protocol-v2.sh +3 -2
index db4ae09f2f..8691bfc52d 100755
@@ -359,12 +359,13 @@ test_expect_success 'even with handcrafted request, filter does not work if not
 	0000
 	EOF
 
-	test_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err &&
+	test_must_fail test-tool -C server serve-v2 --stateless-rpc \
+		<in >/dev/null 2>err &&
 	grep "unexpected line: .filter blob:none." err &&
 
 	# Exercise to ensure that if advertised, filter works
 	git -C server config uploadpack.allowfilter 1 &&
-	git -C server serve --stateless-rpc <in >/dev/null
+	test-tool -C server serve-v2 --stateless-rpc <in >/dev/null
 '
 
 test_expect_success 'default refspec is used to filter ref when fetchcing' '
t/t5703-upload-pack-ref-in-want.sh +8 -8
index f87b2f6df3..dd1cbd0dd6 100755
@@ -48,15 +48,15 @@ test_expect_success 'setup repository' '
 '
 
 test_expect_success 'config controls ref-in-want advertisement' '
-	git serve --advertise-capabilities >out &&
+	test-tool serve-v2 --advertise-capabilities >out &&
 	! grep -a ref-in-want out &&
 
 	git config uploadpack.allowRefInWant false &&
-	git serve --advertise-capabilities >out &&
+	test-tool serve-v2 --advertise-capabilities >out &&
 	! grep -a ref-in-want out &&
 
 	git config uploadpack.allowRefInWant true &&
-	git serve --advertise-capabilities >out &&
+	test-tool serve-v2 --advertise-capabilities >out &&
 	grep -a ref-in-want out
 '
 
@@ -70,7 +70,7 @@ test_expect_success 'invalid want-ref line' '
 	0000
 	EOF
 
-	test_must_fail git serve --stateless-rpc 2>out <in &&
+	test_must_fail test-tool serve-v2 --stateless-rpc 2>out <in &&
 	grep "unknown ref" out
 '
 
@@ -90,7 +90,7 @@ test_expect_success 'basic want-ref' '
 	0000
 	EOF
 
-	git serve --stateless-rpc >out <in &&
+	test-tool serve-v2 --stateless-rpc >out <in &&
 	check_output
 '
 
@@ -112,7 +112,7 @@ test_expect_success 'multiple want-ref lines' '
 	0000
 	EOF
 
-	git serve --stateless-rpc >out <in &&
+	test-tool serve-v2 --stateless-rpc >out <in &&
 	check_output
 '
 
@@ -133,7 +133,7 @@ test_expect_success 'mix want and want-ref' '
 	0000
 	EOF
 
-	git serve --stateless-rpc >out <in &&
+	test-tool serve-v2 --stateless-rpc >out <in &&
 	check_output
 '
 
@@ -153,7 +153,7 @@ test_expect_success 'want-ref with ref we already have commit for' '
 	0000
 	EOF
 
-	git serve --stateless-rpc >out <in &&
+	test-tool serve-v2 --stateless-rpc >out <in &&
 	check_output
 '
 
Signature · 1 line
gitgitgadget