Re: [PATCH 2/3] send-pack: fix push nego. when remote has refs

On Wed, Jun 23, 2021 at 03:30:52PM -0700, Jonathan Tan wrote:

quoted

Commit 477673d6f3 ("send-pack: support push negotiation", 2021-05-05)
did not test the case in which a remote advertises at least one ref. In
such a case, "remote_refs" in get_commons_through_negotiation() in
send-pack.c would also contain those refs with a zero ref->new_oid (in
addition to the refs being pushed with a nonzero ref->new_oid). Passing
them as negotiation tips to "git fetch" causes an error, so filter them
out.

So here we are filtering those redundant refs on the client side?

Yes, but I don't know what you mean by "redundant".

quoted

@@ -210,6 +211,7 @@ test_expect_success 'push with negotiation' '
 	rm event &&
 	mk_empty testrepo &&
 	git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
+	test_commit -C testrepo unrelated_commit &&

So now we are asking 'testrepo' to initially advertise that it also has
unrelated_commit, which we don't care about, and expect to work fine
anyway. Ok.

Yes.

Seems reasonable enough to me.
Reviewed-by: Emily Shaffer <redacted>

Thanks.