[v2,1/3] send-pack: fix push.negotiate with remote helper

Commit Message

Jonathan Tan July 15, 2021, 5:44 p.m. UTC

Commit 477673d6f3 ("send-pack: support push negotiation", 2021-05-05)
introduced the push.negotiate config variable and included a test. The
test only covered pushing without a remote helper, so the fact that
pushing with a remote helper doesn't work went unnoticed.

This is ultimately caused by the "url" field not being set in the args
struct. This field being unset probably went unnoticed because besides
push negotiation, this field is only used to generate a "pushee" line in
a push cert (and if not given, no such line is generated). Therefore,
set this field.

Signed-off-by: Jonathan Tan <[e-mail]>
Signed-off-by: Junio C Hamano <[e-mail]>
---
 builtin/send-pack.c        |  1 +
 t/t5549-fetch-push-http.sh | 71 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)
 create mode 100755 t/t5549-fetch-push-http.sh

Comments

Ævar Arnfjörð Bjarmason July 27, 2021, 7:56 a.m. UTC | #1

On Thu, Jul 15 2021, Jonathan Tan wrote:


> +	git init client &&
> +	test_when_finished 'rm -rf client' &&

Nit (don't think this needs a re-roll): Better to do test_when_finished
before whatever creates the thing, in case the command has a bug/errors
out etc. during development. Ditto the below.

Patch

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index a7e01667b0..729dea1d25 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -230,6 +230,7 @@  int cmd_send_pack(int argc, const char **argv, const char *prefix)
 	args.atomic = atomic;
 	args.stateless_rpc = stateless_rpc;
 	args.push_options = push_options.nr ? &push_options : NULL;
+	args.url = dest;
 
 	if (from_stdin) {
 		if (args.stateless_rpc) {
diff --git a/t/t5549-fetch-push-http.sh b/t/t5549-fetch-push-http.sh
new file mode 100755
index 0000000000..f50d584881
--- /dev/null
+++ b/t/t5549-fetch-push-http.sh
@@ -0,0 +1,71 @@ 
+#!/bin/sh
+
+test_description='fetch/push functionality using the HTTP protocol'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+SERVER="$HTTPD_DOCUMENT_ROOT_PATH/server"
+URI="$HTTPD_URL/smart/server"
+
+grep_wrote () {
+	object_count=$1
+	file_name=$2
+	grep 'write_pack_file/wrote.*"value":"'$1'"' $2
+}
+
+setup_client_and_server () {
+	git init client &&
+	test_when_finished 'rm -rf client' &&
+	test_commit -C client first_commit &&
+	test_commit -C client second_commit &&
+
+	git init "$SERVER" &&
+	test_when_finished 'rm -rf "$SERVER"' &&
+	test_config -C "$SERVER" http.receivepack true &&
+	git -C client push "$URI" first_commit:refs/remotes/origin/first_commit &&
+	git -C "$SERVER" config receive.hideRefs refs/remotes/origin/first_commit
+}
+
+test_expect_success 'push without negotiation (for comparing object counts with the next test)' '
+	setup_client_and_server &&
+
+	GIT_TRACE2_EVENT="$(pwd)/event" git -C client -c protocol.version=2 \
+		push "$URI" refs/heads/main:refs/remotes/origin/main &&
+	test_when_finished "rm -f event" &&
+	grep_wrote 6 event # 2 commits, 2 trees, 2 blobs
+'
+
+test_expect_success 'push with negotiation' '
+	setup_client_and_server &&
+
+	GIT_TRACE2_EVENT="$(pwd)/event" git -C client -c protocol.version=2 -c push.negotiate=1 \
+		push "$URI" refs/heads/main:refs/remotes/origin/main &&
+	test_when_finished "rm -f event" &&
+	grep_wrote 3 event # 1 commit, 1 tree, 1 blob
+'
+
+test_expect_success 'push with negotiation proceeds anyway even if negotiation fails' '
+	setup_client_and_server &&
+
+	# Use protocol v0 to make negotiation fail (because protocol v0 does
+	# not support the "wait-for-done" capability, which is required for
+	# push negotiation)
+	GIT_TEST_PROTOCOL_VERSION=0 GIT_TRACE2_EVENT="$(pwd)/event" git -C client -c push.negotiate=1 \
+		push "$URI" refs/heads/main:refs/remotes/origin/main 2>err &&
+	test_when_finished "rm -f event" &&
+	grep_wrote 6 event && # 2 commits, 2 trees, 2 blobs
+
+	cat >warning-expect <<-EOF &&
+	warning: --negotiate-only requires protocol v2
+	warning: push negotiation failed; proceeding anyway with push
+EOF
+	grep warning: err >warning-actual &&
+	test_cmp warning-expect warning-actual
+'
+
+test_done