receive-pack: avoid duplicates between our refs and alternates

We de-duplicate ".have" refs among themselves, but never check if they are duplicates of our local refs. It's not unreasonable that they would be if we are a "--shared" or "--reference" clone of a similar repository; we'd have all the same tags. We can handle this by inserting our local refs into the oidset, but obviously not suppressing duplicates (since the refnames are important). Note that this also switches the order in which we advertise refs, processing ours first and then any alternates. The order shouldn't matter (and arguably showing our refs first makes more sense). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 8, 2017 at 15:53 UTC 63d428e656edcd670fa87e74136726096ff3de6f
2 files changed +41 -1
builtin/receive-pack.c
+3 -1
@@ -268,6 +268,8 @@ static int show_ref_cb(const char *path_full, const struct object_id *oid,
268 if (oidset_insert(seen, oid))
269 return 0;
270 path = ".have";
271 + } else {
272 + oidset_insert(seen, oid);
273 }
274 show_ref(path, oid->hash);
275 return 0;
@@ -289,9 +291,9 @@ static void write_head_info(void)
291 {
292 static struct oidset seen = OIDSET_INIT;
293
294 + for_each_ref(show_ref_cb, &seen);
295 for_each_alternate_ref(show_one_alternate_ref, &seen);
296 oidset_clear(&seen);
294 - for_each_ref(show_ref_cb, &seen);
297 if (!sent_capabilities)
298 show_ref("capabilities^{}", null_sha1);
299
t/t5400-send-pack.sh
+38
@@ -255,4 +255,42 @@ test_expect_success 'deny pushing to delete current branch' '
255 )
256 '
257
258 +extract_ref_advertisement () {
259 + perl -lne '
260 + # \\ is there to skip capabilities after \0
261 + /push< ([^\\]+)/ or next;
262 + exit 0 if $1 eq "0000";
263 + print $1;
264 + '
265 +}
266 +
267 +test_expect_success 'receive-pack de-dupes .have lines' '
268 + git init shared &&
269 + git -C shared commit --allow-empty -m both &&
270 + git clone -s shared fork &&
271 + (
272 + cd shared &&
273 + git checkout -b only-shared &&
274 + git commit --allow-empty -m only-shared &&
275 + git update-ref refs/heads/foo HEAD
276 + ) &&
277 +
278 + # Notable things in this expectation:
279 + # - local refs are not de-duped
280 + # - .have does not duplicate locals
281 + # - .have does not duplicate itself
282 + local=$(git -C fork rev-parse HEAD) &&
283 + shared=$(git -C shared rev-parse only-shared) &&
284 + cat >expect <<-EOF &&
285 + $local refs/heads/master
286 + $local refs/remotes/origin/HEAD
287 + $local refs/remotes/origin/master
288 + $shared .have
289 + EOF
290 +
291 + GIT_TRACE_PACKET=$(pwd)/trace git push fork HEAD:foo &&
292 + extract_ref_advertisement <trace >refs &&
293 + test_cmp expect refs
294 +'
295 +
296 test_done