On Thu, Oct 22, 2020 at 08:59:23PM -0700, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > > 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. Thanks for having a look! I agree, changing this seems sensible to me. In the end, the intention of the hook is to have a single script which is able to verify all reference updates, no matter where they come from. And behaving differently based on whether the user passed the zero OID or not doesn't seem useful to me in that context. We should also a better job than I did and describe what the old OID actually is in the documentation. @Taylor, given that you've already dug into the code: do you already have plans to post a patch for this? Patrick
Attachment:
signature.asc
Description: PGP signature
