remote.c: fix "git push" weak match disambiguation

When "git push A:B" is given, and A (or B) is not a full refname
that begins with refs/, we require an unambiguous match with an
existing ref.  For this purpose, a match with a local branch or
a tag (i.e. refs/heads/A and refs/tags/A) is called a "strong
match", and any other match is called a "weak match".  A partial
refname is unambiguous when there is only one strong match with
any number of weak matches, or when there is only one weak match
and no other match.

However, as reported by Sparse with Ramsay Jones recently,
count_refspec_match() function had a bug where a variable in an
inner block masked a different variable of the same name, which
caused the weak matches to be ignored.

This fixes it, and adds tests for the fix.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/remote.c b/remote.c
index 30abdbb4d9fdeeb39241a88754a7e511cc2e94eb..120df36be002b543e609060b8f318fe7b38d437c 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -333,7 +333,6 @@ static int count_refspec_match(const char *pattern,
        for (weak_match = match = 0; refs; refs = refs->next) {
                char *name = refs->name;
                int namelen = strlen(name);
        for (weak_match = match = 0; refs; refs = refs->next) {
                char *name = refs->name;
                int namelen = strlen(name);

-               int weak_match;

 
                if (namelen < patlen ||
                    memcmp(name + namelen - patlen, pattern, patlen))
 
                if (namelen < patlen ||
                    memcmp(name + namelen - patlen, pattern, patlen))

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index dba018f667048d7bd5233136dfe70ee0f1305a9a..b3b57faf9c4548aef5431b3862f9991348fe473f 100755 (executable)
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -15,12 +15,58 @@ mk_empty () {
        )
 }
 
        )
 }
 

+mk_test () {
+       mk_empty &&
+       (
+               for ref in "$@"
+               do
+                       git push testrepo $the_first_commit:refs/$ref || {
+                               echo "Oops, push refs/$ref failure"
+                               exit 1
+                       }
+               done &&
+               cd testrepo &&
+               for ref in "$@"
+               do
+                       r=$(git show-ref -s --verify refs/$ref) &&
+                       test "z$r" = "z$the_first_commit" || {
+                               echo "Oops, refs/$ref is wrong"
+                               exit 1
+                       }
+               done &&
+               git fsck --full
+       )
+}
+
+check_push_result () {
+       (
+               cd testrepo &&
+               it="$1" &&
+               shift
+               for ref in "$@"
+               do
+                       r=$(git show-ref -s --verify refs/$ref) &&
+                       test "z$r" = "z$it" || {
+                               echo "Oops, refs/$ref is wrong"
+                               exit 1
+                       }
+               done &&
+               git fsck --full
+       )
+}
+

 test_expect_success setup '
 
        : >path1 &&
        git add path1 &&
        test_tick &&
        git commit -a -m repo &&
 test_expect_success setup '
 
        : >path1 &&
        git add path1 &&
        test_tick &&
        git commit -a -m repo &&

+       the_first_commit=$(git show-ref -s --verify refs/heads/master) &&
+
+       : >path2 &&
+       git add path2 &&
+       test_tick &&
+       git commit -a -m second &&

        the_commit=$(git show-ref -s --verify refs/heads/master)
 
 '
        the_commit=$(git show-ref -s --verify refs/heads/master)
 
 '


@@ -79,4 +125,70 @@ test_expect_success 'push with wildcard' '
        )
 '
 
        )
 '
 

+test_expect_success 'push with matching heads' '
+
+       mk_test heads/master &&
+       git push testrepo &&
+       check_push_result $the_commit heads/master
+
+'
+
+test_expect_success 'push with no ambiguity (1)' '
+
+       mk_test heads/master &&
+       git push testrepo master:master &&
+       check_push_result $the_commit heads/master
+
+'
+
+test_expect_success 'push with no ambiguity (2)' '
+
+       mk_test remotes/origin/master &&
+       git push testrepo master:master &&
+       check_push_result $the_commit remotes/origin/master
+
+'
+
+test_expect_success 'push with weak ambiguity (1)' '
+
+       mk_test heads/master remotes/origin/master &&
+       git push testrepo master:master &&
+       check_push_result $the_commit heads/master &&
+       check_push_result $the_first_commit remotes/origin/master
+
+'
+
+test_expect_success 'push with weak ambiguity (2)' '
+
+       mk_test heads/master remotes/origin/master remotes/another/master &&
+       git push testrepo master:master &&
+       check_push_result $the_commit heads/master &&
+       check_push_result $the_first_commit remotes/origin/master remotes/another/master
+
+'
+
+test_expect_success 'push with ambiguity (1)' '
+
+       mk_test remotes/origin/master remotes/frotz/master &&
+       if git push testrepo master:master
+       then
+               echo "Oops, should have failed"
+               false
+       else
+               check_push_result $the_first_commit remotes/origin/master remotes/frotz/master
+       fi
+'
+
+test_expect_success 'push with ambiguity (2)' '
+
+       mk_test heads/frotz tags/frotz &&
+       if git push testrepo master:frotz
+       then
+               echo "Oops, should have failed"
+               false
+       else
+               check_push_result $the_first_commit heads/frotz tags/frotz
+       fi
+'
+

 test_done
 test_done