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>

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c23b0cce8607e8361a603cc23803eed6562b85cd..9ed8fbbfad71daf45381dbccf0aebe4a5b9af03d 100644 (file)
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -268,6 +268,8 @@ static int show_ref_cb(const char *path_full, const struct object_id *oid,
                if (oidset_insert(seen, oid))
                        return 0;
                path = ".have";
+       } else {
+               oidset_insert(seen, oid);
        }
        show_ref(path, oid->hash);
        return 0;
@@ -289,9 +291,9 @@ static void write_head_info(void)
 {
        static struct oidset seen = OIDSET_INIT;
 
+       for_each_ref(show_ref_cb, &seen);
        for_each_alternate_ref(show_one_alternate_ref, &seen);
        oidset_clear(&seen);
-       for_each_ref(show_ref_cb, &seen);
        if (!sent_capabilities)
                show_ref("capabilities^{}", null_sha1);
 

diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 305ca7a9308685c7d040ac254fd34cebbbc86cc8..3331e0f53443f9a8566d01ac08dd0cbc5aa2d33b 100755 (executable)
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -255,4 +255,42 @@ test_expect_success 'deny pushing to delete current branch' '
        )
 '
 
+extract_ref_advertisement () {
+       perl -lne '
+               # \\ is there to skip capabilities after \0
+               /push< ([^\\]+)/ or next;
+               exit 0 if $1 eq "0000";
+               print $1;
+       '
+}
+
+test_expect_success 'receive-pack de-dupes .have lines' '
+       git init shared &&
+       git -C shared commit --allow-empty -m both &&
+       git clone -s shared fork &&
+       (
+               cd shared &&
+               git checkout -b only-shared &&
+               git commit --allow-empty -m only-shared &&
+               git update-ref refs/heads/foo HEAD
+       ) &&
+
+       # Notable things in this expectation:
+       #  - local refs are not de-duped
+       #  - .have does not duplicate locals
+       #  - .have does not duplicate itself
+       local=$(git -C fork rev-parse HEAD) &&
+       shared=$(git -C shared rev-parse only-shared) &&
+       cat >expect <<-EOF &&
+       $local refs/heads/master
+       $local refs/remotes/origin/HEAD
+       $local refs/remotes/origin/master
+       $shared .have
+       EOF
+
+       GIT_TRACE_PACKET=$(pwd)/trace git push fork HEAD:foo &&
+       extract_ref_advertisement <trace >refs &&
+       test_cmp expect refs
+'
+
 test_done