Subject: [PATCH v2 0/3] Push negotiation fixes

From: Jonathan Tan <[email-masked]>
Date: Thu, 15 Jul 2021 10:44:29 -0700

Thanks, Emily and Ævar for your reviews. There are no changes to C code
in this version of the patch set, but the tests have been updated
following both your comments.

I have ended up splitting up the HTTP tests away from t5516 into its own
file. I had to use the number 5549 because it was the next available
one. There is one Bash function that is duplicated - I don't foresee
many such functions being duplicated, but if that happens, I guess we
can always created a helper script somewhere else.

Jonathan Tan (3):
  send-pack: fix push.negotiate with remote helper
  send-pack: fix push nego. when remote has refs
  fetch: die on invalid --negotiation-tip hash

 builtin/fetch.c            |  4 ++-
 builtin/send-pack.c        |  1 +
 send-pack.c                |  6 ++--
 t/t5510-fetch.sh           | 13 +++++++
 t/t5516-fetch-push.sh      |  4 ++-
 t/t5549-fetch-push-http.sh | 72 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 96 insertions(+), 4 deletions(-)
 create mode 100755 t/t5549-fetch-push-http.sh

Range-diff against v1:
1:  eb0dce9f49 < -:  ---------- send-pack: fix push.negotiate with remote helper
-:  ---------- > 1:  af40bee611 send-pack: fix push.negotiate with remote helper
2:  3bf6921d96 ! 2:  c841693303 send-pack: fix push nego. when remote has refs
    @@ t/t5516-fetch-push.sh: test_expect_success 'push with negotiation proceeds anywa
      	git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
      	GIT_TEST_PROTOCOL_VERSION=0 GIT_TRACE2_EVENT="$(pwd)/event" \
      		git -c push.negotiate=1 push testrepo refs/heads/main:refs/remotes/origin/main 2>err &&
    -@@ t/t5516-fetch-push.sh: test_expect_success 'http push with negotiation' '
    - 	# Without negotiation
    - 	test_create_repo "$SERVER" &&
    - 	test_config -C "$SERVER" http.receivepack true &&
    -+	test_commit -C "$SERVER" unrelated_commit &&
    - 	git -C client push "$URI" first_commit:refs/remotes/origin/first_commit &&
    - 	git -C "$SERVER" config receive.hideRefs refs/remotes/origin/first_commit &&
    - 	GIT_TRACE2_EVENT="$(pwd)/event" git -C client -c protocol.version=2 \
    -@@ t/t5516-fetch-push.sh: test_expect_success 'http push with negotiation' '
    - 	rm -rf "$SERVER" &&
    - 	test_create_repo "$SERVER" &&
    +@@ t/t5516-fetch-push.sh: test_expect_success 'denyCurrentBranch and worktrees' '
    + 	git -C cloned push origin HEAD:new-wt &&
    + 	test_must_fail git -C cloned push --delete origin new-wt
    + '
    +-
    + test_done
    +
    + ## t/t5549-fetch-push-http.sh ##
    +@@ t/t5549-fetch-push-http.sh: setup_client_and_server () {
    + 	git init "$SERVER" &&
    + 	test_when_finished 'rm -rf "$SERVER"' &&
      	test_config -C "$SERVER" http.receivepack true &&
     +	test_commit -C "$SERVER" unrelated_commit &&
      	git -C client push "$URI" first_commit:refs/remotes/origin/first_commit &&
    - 	git -C "$SERVER" config receive.hideRefs refs/remotes/origin/first_commit &&
    - 	GIT_TRACE2_EVENT="$(pwd)/event" git -C client -c protocol.version=2 -c push.negotiate=1 \
    + 	git -C "$SERVER" config receive.hideRefs refs/remotes/origin/first_commit
    + }
3:  1453a1b4e5 ! 3:  476da85859 fetch: die on invalid --negotiation-tip hash
    @@ builtin/fetch.c: static void add_negotiation_tips(struct git_transport_options *
     -				die("%s is not a valid object", s);
     +				die(_("%s is not a valid object"), s);
     +			if (!has_object(the_repository, &oid, 0))
    -+				die(_("%s is not a valid object"), s);
    ++				die(_("the object %s does not exist"), s);
      			oid_array_append(oids, &oid);
      			continue;
      		}
    @@ t/t5510-fetch.sh: test_expect_success '--negotiation-tip understands abbreviated
     +		--negotiation-tip=alpha_1 \
     +		--negotiation-tip=$(test_oid zero) \
     +		origin alpha_s beta_s 2>err &&
    -+	test_i18ngrep "is not a valid object" err
    ++	cat >fatal-expect <<-EOF &&
    ++	fatal: the object $(test_oid zero) does not exist
    ++EOF
    ++	grep fatal: err >fatal-actual &&
    ++	test_cmp fatal-expect fatal-actual
     +'
     +
      . "$TEST_DIRECTORY"/lib-httpd.sh
-- 
2.32.0.93.g670b81a890-goog