On Fri, Nov 15, 2024 at 08:48:12AM +0900, Junio C Hamano wrote: > Karthik Nayak <karthik.188@xxxxxxxxx> writes: > > > The reference-transaction hook is invoked whenever there is a reference > > update being performed. For each state of the transaction, we iterate > > over the updates present and pass this information to the hook. > > > > The `ref_update` structure is used to hold these updates within a > > `transaction`. We use the same structure for holding reflog updates too. > > Which means that the reference transaction hook is also obtaining > > information about a reflog update. This is a bug, since: > > Yeah, the transaction hook is deciding how the values of refs should > (or should not) change, and its decisions should be sufficient to > determine what should happen to corresponding reflog updates. If an > update to the 'main' branch is let through, that update should result > in a new reflog record for that branch. If such an update is blocked, > there is no update to the branch, and a reflog record would not be > created for such an update that did not happen. > > One thing that the above argument does not capture is "stash", > especially "stash drop". The way the subsystem abuses reflog > disconnects ref updates from reflog updates, so there _is_ a use > case for hooks to interfere with reflog updates. > > However, the existing ref update transaction hook does not have to > be the mechanism to vet "git stash" operation. If we really needed > to, we could add reflog transaction hook for that later, outside the > scope of this fix. Indeed, I'm also happy to declare this a bug and change the behaviour retroactively to skip over reflogs. I highly doubt that anybody uses this in a sensible way to handle reflog updates: they have no way to distinguish a ref update from a reflog update, so they would have to essentially guess what is what. So this patch looks good to me, thanks! Patrick
