[PATCH] get_sha1: improve ambiguity warning regarding SHA-1 and ref names
- From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
- Date
- May 4, 2013, 03:45 UTC
- Message-ID
- <1367639113-17342-1-git-send-email-pclouds@gmail.com>
- In-Reply-To
- <20130501184327.GM24467@google.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