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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Nguyễn Thái Ngọc Duy  <pclouds@xxxxxxxxx> writes:

>  > 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?

> +		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.


> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]