Re: [PATCH RESEND] refs: Always pass old object name to reftx hook
From: Junio C Hamano <hidden>
Date: 2021-01-22 19:17:39
Patrick Steinhardt [off-list ref] writes:
quoted
Yes, it can mean both, but when you pretend to be that hook, wouldn't you check if the ref exists? If not, the user is trying to create it, and otherwise, the user does not know or care what the original value is, no?As long as you're aware as the script author, yes.
As you said downbelow, I agree that clear documentation may be necessary.
There is one gotcha though: you can verify the state when the reference-transaction hook gets invoked in the "prepared" state, as it means that all references have been locked and thus cannot be changed by any other well-behaved process accessing the git repository. In "committed" or "aborted" that's not true anymore, given that the state has changed already, so any locks have been released and it's impossible to find out what happened now.
True, but isn't the situation the same if we replaced the 0{40} old
side with (one version of) original value of the ref?
different from the user-provided change. E.g.
0{40} <new> <ref>
^<old>
or
0{40}^<old> <new> <ref>
That can be considered as backwards-incompatible though.
Yes, it is an incompatible change. I thought of somehow annotating
the old side, e.g. "<old> <new> <ref>" vs "<OLD> <new> <ref>", to
show the distinction between "this is the original value of ref the
user wanted to see when updating <ref>" and "the user does not care
what value the <ref> gets updated from, but by the way, here is the
original value of the ref as Git sees it" [*], but I cannot think of
a way to do so without breaking existing readers.
Side note: here, I am exploring the approach to replace 0{40}
that is given when "do not care" into an actual original object
name taken from the current state, like your patch did, but
trying to find a way to make non-NULL object name distinguishable
between the two cases (i.e. user-supplied vs system-filled).
That raises another question. How much trust should the hook place
on the value of the <old> given to it? When a non-NULL <old> value
is given by the end-user, does the hook get the value as-is, or do
we read the current value of the ref and send that as <old>? Does
the transaction get rejected if the two are different and such a
record is not even given to the hook?
Yup. Whatever we agree on, what is clear is that the documentation needs to be more specific here.
Yes, agreed. Thanks.