When we get 40 hex digits, we immediately assume it's an SHA-1. Warn
about ambiguity if there's also refs/heads/$sha1 (or similar) on system.

When we successfully resolve a ref like "1234abc" and "1234abc"
happens to be valid abbreviated SHA-1 on system, warn also.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
---
 On Thu, May 2, 2013 at 1:43 AM, Jonathan Nieder <jrnie...@gmail.com> wrote:
 > Nguyễn Thái Ngọc Duy wrote:
 >
 >>                                              "git rev-parse 1234" will
 >> resolve refs/heads/1234 if exists even if there is an unambiguous
 >> SHA-1 starting with 1234. However if it's full SHA-1, the SHA-1 takes
 >> precedence and refs with the same name are ignored.
 >
 > That's an important feature for safety.  When a script has created an
 > object or learned about it some other way, as long as it doesn't
 > abbreviate its name it can be sure that git commands will not
 > misunderstand it.
 >
 > So I think this is a bad change.  Maybe check-ref-format should
 > reject 40-hexdigit refnames?

 We can't, at least not in a simple way, because there are 40-hex
 digit refs in refs/replace. I think warning only is a simpler
 approach to this minor issue.

 sha1_name.c                         | 12 ++++++++++--
 t/t1512-rev-parse-disambiguation.sh | 18 ++++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 3820f28..04a9fbe 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -435,12 +435,18 @@ static int get_sha1_1(const char *name, int len, unsigned 
char *sha1, unsigned l
 static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 {
        static const char *warn_msg = "refname '%.*s' is ambiguous.";
+       unsigned char tmp_sha1[20];
        char *real_ref = NULL;
        int refs_found = 0;
        int at, reflog_len;
 
-       if (len == 40 && !get_sha1_hex(str, sha1))
+       if (len == 40 && !get_sha1_hex(str, sha1)) {
+               refs_found = dwim_ref(str, len, tmp_sha1, &real_ref);
+               if (refs_found > 0 && warn_ambiguous_refs)
+                       warning(warn_msg, len, str);
+               free(real_ref);
                return 0;
+       }
 
        /* basic@{time or number or -number} format to query ref-log */
        reflog_len = at = 0;
@@ -481,7 +487,9 @@ static int get_sha1_basic(const char *str, int len, 
unsigned char *sha1)
        if (!refs_found)
                return -1;
 
-       if (warn_ambiguous_refs && refs_found > 1)
+       if (warn_ambiguous_refs &&
+           (refs_found > 1 ||
+            !get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY)))
                warning(warn_msg, len, str);
 
        if (reflog_len) {
diff --git a/t/t1512-rev-parse-disambiguation.sh 
b/t/t1512-rev-parse-disambiguation.sh
index 6b3d797..db22808 100755
--- a/t/t1512-rev-parse-disambiguation.sh
+++ b/t/t1512-rev-parse-disambiguation.sh
@@ -261,4 +261,22 @@ test_expect_success 'rev-parse --disambiguate' '
        test "$(sed -e "s/^\(.........\).*/\1/" actual | sort -u)" = 000000000
 '
 
+test_expect_success 'ambiguous 40-hex ref' '
+       TREE=$(git mktree </dev/null) &&
+       REF=`git rev-parse HEAD` &&
+       VAL=$(git commit-tree $TREE </dev/null) &&
+       git update-ref refs/heads/$REF $VAL &&
+       test `git rev-parse $REF 2>err` = $REF &&
+       grep "refname.*${REF}.*ambiguous" err
+'
+
+test_expect_success 'ambiguous short sha1 ref' '
+       TREE=$(git mktree </dev/null) &&
+       REF=`git rev-parse --short HEAD` &&
+       VAL=$(git commit-tree $TREE </dev/null) &&
+       git update-ref refs/heads/$REF $VAL &&
+       test `git rev-parse $REF 2>err` = $VAL &&
+       grep "refname.*${REF}.*ambiguous" err
+'
+
 test_done
-- 
1.8.2.83.gc99314b

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to