Re: [PATCH v2] refs: run copy and rename through transactions

On Fri, Oct 02, 2026 at 04:16:08PM +0200, Maciej Ciemborowicz wrote:

On Fri, Oct 2, 2026 at 12:56 PM Patrick Steinhardt [off-list ref] wrote:

quoted

why can't we make this whole mechanism completely agnostic of the
backend and implement this via pure transactions?

The part I was trying to preserve is the existing reflog semantics. A
normal ref transaction can express the logical ref updates. In example
deleting the old ref and creating/updating the destination. But branch
rename/copy also moves or copies the existing reflog history. For the
files backend that currently involves filesystem-level reflog
rename/copy and D/F handling, while reftable represents the same
operation differently. So my assumption was that the logical ref
updates could go through the generic transaction API, while the
reflog-history operation would remain backend-specific.

Yes, the reflog semantics should of course stay the same. But nowadays,
this would also be achievable with only backend-agnostic logic as the
reference transactions have learned to write many reflog entries for a
single reference. This was added back when we introduced the migration
logic to convert between two different backends.

Now there's potentially two caveats:

  - I don't think we have a way to delete many old reflog entries yet.

  - There may be a significant impact on performance.

The question thus is whether we can avoid or fix those caveats somehow
and thus arrive at a more future-proof mechanism.

quoted

Is this new behaviour? Is this retaining old behaviour?

The source/destination revalidation is new validation required by
introducing the preparing hook before the backend locks are taken. The
hook can itself change one of the refs. Without revalidation, the hook
payload could describe one state while the rename/copy later operates
on another state. The intention is therefore to reject an operation
when the state observed by the preparing hook is no longer the state
being committed.

I don't feel like that's sensible. The "preparing" hook is explicitly
run before we perform locking and is documented as such. So it is fully
expected that the on-disk state may still change between executing this
and the "prepared" phase. It is the responsibility of the hook author to
handle such cases, we shouldn't do this ourselves as we're now starting
to assume semantics of the hook itself.

quoted

Sorry, but I'm going to stop reading here. This is not in a state that
is reviewable and has way too much stuff that is obviously generated by
an AI without much thought being put into it by the author. I don't want
to invest my time into a topic where the author has obviously not spent
their time thinking about it, either.

I'm really sorry to hear that. Yes, the patches I prepared were
AI-assisted, but I do feel that I understand what I am doing. I would
appreciate some understanding, though, as I do not work with C on a
daily basis. The bug report and my attempt to fix it came from the
fact that I am working on a Ruby gem for per-branch and per-worktree
containerization. That is why I had to write git-hooks-ext, which is
how I ended up running into this bug in the first place.

I am not insisting that my patch should be merged. I simply thought
that submitting a patch might help get the bug fixed faster, and
getting the bug fixed is what I care about most. Karthik Nayak offered
to help fix it, so perhaps it would be better for someone who works
with C on a daily basis to take it over.

I can, of course, also prepare a v3, split it into more commits, and
explain my reasoning more clearly. But I cannot guarantee that it will
meet your standards, simply because I am not yet familiar with them.

I'd suggest to iterate then. In the current version this patch is not in
a shape that is ready for review. The patch needs to be split up, and
there are a lot of gaps in the commit message. Taken together that gives
the signal that you don't really understand what you are doing.

That doesn't mean that you cannot fix that with another iteration
though. But I'd suggest to take your time prepping the next iteration to
read through the code, understand the concepts and doubt what AI spits
out.

Thanks!

Patrick