Re: [PATCH v4 01/14] t: teach test_cmp_rev to accept ! for not-equals

[Junio C Hamano <gitster@pobox.com>]

Denton Liu <liu.denton@gmail.com> writes:

> Currently, in the case where we are using test_cmp_rev() to report
> not-equals, we write `! test_cmp_rev`. However, since test_cmp_rev()
> contains
>
> 	r1=$(git rev-parse --verify "$1") &&
> 	r2=$(git rev-parse --verify "$2") &&
>
> In the case where `git rev-parse` segfaults and dies unexpectedly, the
> failure will be ignored.

Good justification.  The last two lines are continuation of the
sentence that begins the proposed log message, so downcase "In" at
the beginning of the line.  Also, when we present the problem to be
solved at the beginning, it is customary to describe the status quo,
and "Currently, " is a noiseword that does not add much information,
so drop it.

> Rewrite test_cmp_rev() to optionally accept "!" as the first argument to
> do a not-equals comparison. Rewrite `! test_cmp_rev` to `test_cmp_rev !`
> in all tests to take advantage of this new functionality.

Good.

> -# Tests that its two parameters refer to the same revision
> +# Tests that its two parameters refer to the same revision, or if '!' is
> +# provided first, that its other two parameters refer to different
> +# revisions.

OK.

>  test_cmp_rev () {
> +	local inverted_op
> +	inverted_op='!='
> +	if test $# -ge 1 && test "x$1" = 'x!'
> +	then
> +	    inverted_op='='
> +	    shift
> +	fi

I'd rather avoid having to keep track of negation to reduce mental
burden.  How about using = by default and != when '!' was given
(which would be more natural to readers) and call it $op, and say
"if ! test $r1 $op $r2" where it is used?


>  	if test $# != 2
>  	then
>  		error "bug in the test script: test_cmp_rev requires two revisions, but got $#"
>  	else
>  		local r1 r2
>  		r1=$(git rev-parse --verify "$1") &&
>  		r2=$(git rev-parse --verify "$2") &&

If either of the calls fail, the assignment itself would fail, and
the &&-cascade would stop without executing the if statment below.

I see the "!" feature, but where is the promised "fix" for
segfaulting rev-parse?

Puzzled.


> -		if test "$r1" != "$r2"
> +		if test "$r1" "$inverted_op" "$r2"
>  		then
> +			local comp_out
> +			if "x$inverted_op" = 'x='
> +			then
> +				comp_out='the same'
> +			else
> +				comp_out='different'
> +			fi
>  			cat >&4 <<-EOF
> -			error: two revisions point to different objects:
> +			error: two revisions point to $comp_out objects:
>  			  '$1': $r1
>  			  '$2': $r2
>  			EOF
>  			return 1
>  		fi
>  	fi
>  }