Re: [PATCH v4] refs: implement reference transaction hook

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 23, 2020, 03:59 UTC
Message-ID: <xmqqpn59k2xw.fsf@gitster.c.googlers.com>
In-Reply-To: <20201023010315.GA1542721@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 41 quoted lines

> So I wonder if:
>
> diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
> index f6e741c6c0..8155522a1a 100755
> --- a/t/t1416-ref-transaction-hooks.sh
> +++ b/t/t1416-ref-transaction-hooks.sh
> @@ -9,6 +9,7 @@ test_expect_success setup '
>  	test_commit PRE &&
>  	PRE_OID=$(git rev-parse PRE) &&
>  	test_commit POST &&
> +	PRE_OID=$(git rev-parse PRE) &&
>  	POST_OID=$(git rev-parse POST)
>  '
>  
> @@ -52,10 +53,10 @@ test_expect_success 'hook gets all queued updates in prepared state' '
>  		fi
>  	EOF
>  	cat >expect <<-EOF &&
> -		$ZERO_OID $POST_OID HEAD
> -		$ZERO_OID $POST_OID refs/heads/master
> +		$PRE_OID $POST_OID HEAD
> +		$PRE_OID $POST_OID refs/heads/master
>  	EOF
> -	git update-ref HEAD POST <<-EOF &&
> +	git update-ref HEAD POST POST <<-EOF &&
>  		update HEAD $ZERO_OID $POST_OID
>  		update refs/heads/master $ZERO_OID $POST_OID
>  	EOF
> @@ -75,10 +76,10 @@ test_expect_success 'hook gets all queued updates in committed state' '
>  		fi
>  	EOF
>  	cat >expect <<-EOF &&
> -		$ZERO_OID $POST_OID HEAD
> -		$ZERO_OID $POST_OID refs/heads/master
> +		$PRE_OID $POST_OID HEAD
> +		$PRE_OID $POST_OID refs/heads/master
>  	EOF
> -	git update-ref HEAD POST &&
> +	git update-ref HEAD POST PRE &&
>  	test_cmp expect actual
>  '

I think that makes a lot of sense.  In addition,

> ...  But we'd possibly want to actually change the behavior
> to always send the actual ref value to the hook.

I offhand do not see a reason why we shouldn't do that.

Thanks for carefully thinking things through.