[PATCH] get_sha1: improve ambiguity warning regarding SHA-1 and ref names

["Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>]

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 <pclouds@gmail.com>
---
 On Thu, May 2, 2013 at 1:43 AM, Jonathan Nieder <jrnieder@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