Re: [PATCH] get_sha1: improve ambiguity warning regarding SHA-1 and ref names
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 7, 2013, 16:09 UTC
- Message-ID
- <7v38tylqkp.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1367639113-17342-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 20 quoted lines
> > 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.
> ...
> 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);The warning is issued at the right spot from the codeflow's point of view, but it is very likely that the user did not even mean to use the str in question as a 'refname'. The warning message we see above is not appropriate for this case, is it?
Show 15 quoted lines
> + 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);Ditto for the case in which (refs_found <= 1) and get_short_sha1() finds str as a short object name.
Show 27 quoted lines
> 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