refactor fetch's ref matching to use refname_match()

The old rules used by fetch were coded as a series of ifs.  The old
rules are:
1) match full refname if it starts with "refs/" or matches "HEAD"
2) verify that full refname starts with "refs/"
3) match abbreviated name in "refs/" if it starts with "heads/",
"tags/", or "remotes/".
4) match abbreviated name in "refs/heads/"

This is replaced by the new rules
a) match full refname
b) match abbreviated name prefixed with "refs/"
c) match abbreviated name prefixed with "refs/heads/"

The details of the new rules are different from the old rules.  We no
longer verify that the full refname starts with "refs/".  The new rule
(a) matches any full string.  The old rules (1) and (2) were stricter.
Now, the caller is responsible for using sensible full refnames.  This
should be the case for the current code.  The new rule (b) is less
strict than old rule (3).  The new rule accepts abbreviated names that
start with a non-standard prefix below "refs/".

Despite this modifications the new rules should handle all cases as
expected.  Two tests are added to verify that fetch does not resolve
short tags or HEAD in remotes.

We may even think about loosening the rules a bit more and unify them
with the rev-parse rules.  This would be done by replacing
ref_ref_fetch_rules with ref_ref_parse_rules.  Note, the two new test
would break.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/cache.h b/cache.h
index 38d9a285e01418015cb94e77afdd5047244e81af..cb8f3cabbbdc70170249e96bb213c178b29389cc 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -417,6 +417,7 @@ extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
 
 extern int refname_match(const char *abbrev_name, const char *full_name, const char **rules);
 extern const char *ref_rev_parse_rules[];
+extern const char *ref_fetch_rules[];
 
 extern int create_symref(const char *ref, const char *refs_heads_master, const char *logmsg);
 extern int validate_headref(const char *ref);

diff --git a/refs.c b/refs.c
index fc26a93cbadfe4795d06c99409f0b6e33aa22a20..6a04a667bbb84b13f895ea95f1fadc5e2cb1bb33 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -653,6 +653,13 @@ const char *ref_rev_parse_rules[] = {
        NULL
 };
 
+const char *ref_fetch_rules[] = {
+       "%.*s",
+       "refs/%.*s",
+       "refs/heads/%.*s",
+       NULL
+};
+
 int refname_match(const char *abbrev_name, const char *full_name, const char **rules)
 {
        const char **p;

diff --git a/remote.c b/remote.c
index 4085c517e4de40474738794ce3c5d0dc1901134d..48812a713e56bbc74418d2d484ee345b4dc5aa32 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -417,25 +417,6 @@ int remote_has_url(struct remote *remote, const char *url)
        return 0;
 }
 
-/*
- * Returns true if, under the matching rules for fetching, name is the
- * same as the given full name.
- */
-static int ref_matches_abbrev(const char *name, const char *full)
-{
-       if (!prefixcmp(name, "refs/") || !strcmp(name, "HEAD"))
-               return !strcmp(name, full);
-       if (prefixcmp(full, "refs/"))
-               return 0;
-       if (!prefixcmp(name, "heads/") ||
-           !prefixcmp(name, "tags/") ||
-           !prefixcmp(name, "remotes/"))
-               return !strcmp(name, full + 5);
-       if (prefixcmp(full + 5, "heads/"))
-               return 0;
-       return !strcmp(full + 11, name);
-}
-
 int remote_find_tracking(struct remote *remote, struct refspec *refspec)
 {
        int find_src = refspec->src == NULL;
@@ -804,7 +785,7 @@ int branch_merge_matches(struct branch *branch,
 {
        if (!branch || i < 0 || i >= branch->merge_nr)
                return 0;
-       return ref_matches_abbrev(branch->merge[i]->src, refname);
+       return refname_match(branch->merge[i]->src, refname, ref_fetch_rules);
 }
 
 static struct ref *get_expanded_map(struct ref *remote_refs,
@@ -843,7 +824,7 @@ static struct ref *find_ref_by_name_abbrev(struct ref *refs, const char *name)
 {
        struct ref *ref;
        for (ref = refs; ref; ref = ref->next) {
-               if (ref_matches_abbrev(name, ref->name))
+               if (refname_match(name, ref->name, ref_fetch_rules))
                        return ref;
        }
        return NULL;

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index aad863db7ad74ed217b3bcc76de43556f9fe7a07..20257428eb0a98f639e7d373f1d00ad91d7c16ad 100755 (executable)
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -95,6 +95,31 @@ test_expect_success 'fetch following tags' '
 
 '
 
+test_expect_failure 'fetch must not resolve short tag name' '
+
+       cd "$D" &&
+
+       mkdir five &&
+       cd five &&
+       git init &&
+
+       git fetch .. anno:five
+
+'
+
+test_expect_failure 'fetch must not resolve short remote name' '
+
+       cd "$D" &&
+       git-update-ref refs/remotes/six/HEAD HEAD
+
+       mkdir six &&
+       cd six &&
+       git init &&
+
+       git fetch .. six:six
+
+'
+
 test_expect_success 'create bundle 1' '
        cd "$D" &&
        echo >file updated again by origin &&