fetch-pack: do not filter out one-level refs

Currently fetching a one-level ref like "refs/foo" does not
work consistently. The outer "git fetch" program filters the
list of refs, checking each against check_refname_format.
Then it feeds the result to do_fetch_pack to actually
negotiate the haves/wants and get the pack. The fetch-pack
code does its own filter, and it behaves differently.

The fetch-pack filter looks for refs in "refs/", and then
feeds everything _after_ the slash (i.e., just "foo") into
check_refname_format.  But check_refname_format is not
designed to look at a partial refname. It complains that the
ref has only one component, thinking it is at the root
(i.e., alongside "HEAD"), when in reality we just fed it a
partial refname.

As a result, we omit a ref like "refs/foo" from the pack
request, even though "git fetch" then tries to store the
resulting ref.  If we happen to get the object anyway (e.g.,
because the ref is contained in another ref we are
fetching), then the fetch succeeds. But if it is a unique
object, we fail when trying to update "refs/foo".

We can fix this by just passing the whole refname into
check_refname_format; we know the part we were omitting is
"refs/", which is acceptable in a refname. This at least
makes the checks consistent with each other.

This problem happens most commonly with "refs/stash", which
is the only one-level ref in wide use. However, our test
does not use "refs/stash", as we may later want to restrict
it specifically (not because it is one-level, but because
of the semantics of stashes).

We may also want to do away with the multiple levels of
filtering (which can cause problems when they are out of
sync), or even forbid one-level refs entirely. However,
those decisions can come later; this fixes the most
immediate problem, which is the mismatch between the two.

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 aff4f5abab667d054c9ab1b925e4877a4fd8d243..4637eb13287ca253ef008b9df2848cacbf6f9dc6 100644 (file)
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -506,7 +506,7 @@ static void filter_refs(struct fetch_pack_args *args,
                next = ref->next;
 
                if (!memcmp(ref->name, "refs/", 5) &&
-                   check_refname_format(ref->name + 5, 0))
+                   check_refname_format(ref->name, 0))
                        ; /* trash */
                else {
                        while (i < nr_sought) {

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index fde689166a5dcb419e0519580d628f91f1f29f83..d52ef7fe80683c455e9024c565938a5bda88b827 100755 (executable)
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -512,4 +512,15 @@ test_expect_success 'all boundary commits are excluded' '
        test_bundle_object_count .git/objects/pack/pack-${pack##pack    }.pack 3
 '
 
+test_expect_success 'fetching a one-level ref works' '
+       test_commit extra &&
+       git reset --hard HEAD^ &&
+       git update-ref refs/foo extra &&
+       git init one-level &&
+       (
+               cd one-level &&
+               git fetch .. HEAD refs/foo
+       )
+'
+
 test_done