Am 13.04.22 um 16:34 schrieb Ævar Arnfjörð Bjarmason: > > On Tue, Apr 12 2022, Bryan Turner wrote: > >> On Tue, Apr 12, 2022 at 2:20 AM Bryan Turner <bturner@xxxxxxxxxxxxx> wrote: >>> >>> It looks like Git 2.36.0-rc1 includes the changes to eliminate/pare >>> back reference transactions being raised independently for loose and >>> packed refs. It's a great improvement, and one we're very grateful >>> for. >>> >>> It looks like there's a regression, though. When deleting a ref that >>> _only_ exists in packed-refs, by the time the "prepared" callback is >>> raised the ref has already been deleted completely. Normally when >>> "prepared" is raised the ref is still present. The ref still being >>> present is important to us, since the reference-transaction hook is >>> frequently not passed any previous hash; we resolve the ref during >>> "prepared", if the previous hash is the null SHA1, so that we can >>> correctly note what the tip commit was when the ref was deleted. >> >> I've re-tested this with 2.36.0-rc2 and it has the same regression (as >> expected). However, in playing with it more, the regression is more >> serious than I had initially considered. It goes beyond just losing >> access to the SHA of the tip commit for deleted refs. If a ref only >> exists packed, this regression means vetoing the "prepared" callback >> _cannot prevent its deletion_, which violates the contract for the >> reference-transaction as I understand it. >> >> Here's a slightly modified reproduction: >> git init ref-tx >> cd ref-tx >> git commit -m "Initial commit" --allow-empty >> git branch to-delete >> git pack-refs --all >> echo 'exit 1;' > .git/hooks/reference-transaction >> chmod +x .git/hooks/reference-transaction >> git branch -d to-delete >> >> Running this reproduction ends with: >> $ git branch -d to-delete >> fatal: ref updates aborted by hook >> $ git rev-parse to-delete -- >> fatal: bad revision 'to-delete' >> >> Even though the reference-transaction vetoed "prepared", the ref was >> still deleted. >> >> In Bitbucket, we use the reference-transaction to perform replication. >> When we get the "prepared" callback on one machine, we dispatch the >> same change(s) to other replicas. Those replicas process the changes >> and arrive at their own "prepared" callbacks (or don't), at which >> point they vote to commit or rollback. The votes are tallied and the >> majority decision wins. >> >> With this regression, that sort of setup no longer works reliably for >> ref deletions. If the ref only exists packed, it's deleted (and >> _visibly_ deleted) before we ever get the "prepared" callback. So if >> coordination fails (i.e. the majority votes to rollback), even if we >> try to abort the change it's already too late. > > This does look lik a series regression. > > I haven't had time to bisect this, but I suspect that it'll come down to > something in the 991b4d47f0a (Merge branch > 'ps/avoid-unnecessary-hook-invocation-with-packed-refs', 2022-02-18) > series. Indeed, it bisects to 2ed1b64ebd (refs: skip hooks when deleting uncovered packed refs, 2022-01-17). > I happen to know that Patrick is OoO until after the final v2.36.0 is > scheduled (but I don't know for sure that we won't spot this thread & > act on it before then). > > Is this something you think you'll be able to dig into further and > possibly come up with a patch? It looks like you're way ahead of at > least myself in knowing how this *should* work :)
