filter_ref: make a copy of extra "sought" entries

If the server supports allow_tip_sha1_in_want, we add any
unmatched raw-sha1 entries in our "sought" list of refs to
the list of refs we will ask the other side for. We do so by
inserting the original "struct ref" directly into our list,
rather than making a copy. This has several problems.

The most minor problem is that one cannot ever free the
resulting list; it contains structs that are copies of the
remote refs (made earlier by fetch_pack) along with sought
refs that are referenced elsewhere.

But more importantly that we set the ref->next pointer to
NULL, chopping off the remainder of any existing list that
the ref was a part of. We get the set of "sought" refs in
an array rather than a linked list, but that array is often
in turn generated from a list.  The test modification in
t5516 demonstrates this. Rather than fetching just an exact
sha1, we fetch that sha1 plus another ref:

  - we build a linked list of refs to fetch when do_fetch
    calls get_ref_map; the exact sha1 is first, followed by
    the named ref ("refs/heads/extra" in this case).

  - we pass that linked list to transport_fetch_ref, which
    squashes it into an array of pointers

  - that array goes to fetch_pack, which calls filter_ref.
    There we generate the want list from a mix of what the
    remote side has advertised, and the "sought" entry for
    the exact sha1. We set the sought entry's "next" pointer
    to NULL.

  - after we return from transport_fetch_refs, we then try
    to update the refs by following the linked list. But our
    list is now truncated, and we do not update
    refs/heads/extra at all.

We can fix this by making a copy of the ref. There's nothing
that fetch_pack does to it that must be reflected in the
original "sought" list (and indeed, if that were the case we
would have a serious bug, because it is only exact-sha1
entries which are treated this way).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/fetch-pack.c b/fetch-pack.c
index 058c25837d8ba633de4619049eec6b9823db62a6..84d5a66967a608c279b114a1b3957d668010f995 100644 (file)
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -555,9 +555,8 @@ static void filter_refs(struct fetch_pack_args *args,
                                continue;
 
                        ref->matched = 1;
-                       *newtail = ref;
-                       ref->next = NULL;
-                       newtail = &ref->next;
+                       *newtail = copy_ref(ref);
+                       newtail = &(*newtail)->next;
                }
        }
        *refs = newlist;

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index f4da20aa9bf7df1b2ab1fda207b70e1913277603..f7947315ba0fda1060930932afe5e6a0c065d0da 100755 (executable)
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1107,9 +1107,16 @@ test_expect_success 'fetch exact SHA1' '
                        git config uploadpack.allowtipsha1inwant true
                ) &&
 
-               git fetch -v ../testrepo $the_commit:refs/heads/copy &&
-               result=$(git rev-parse --verify refs/heads/copy) &&
-               test "$the_commit" = "$result"
+               git fetch -v ../testrepo $the_commit:refs/heads/copy master:refs/heads/extra &&
+               cat >expect <<-EOF &&
+               $the_commit
+               $the_first_commit
+               EOF
+               {
+                       git rev-parse --verify refs/heads/copy &&
+                       git rev-parse --verify refs/heads/extra
+               } >actual &&
+               test_cmp expect actual
        )
 '