reftable: wrong HEAD reflog when its target is updated in the same transaction

Hi,

With the reftable backend, if one transaction updates a branch and points HEAD at it, the new HEAD reflog entry records the branch's old value. If the branch is created in the same transaction, no HEAD reflog entry is written at all. The files backend records the new value in both cases.

To reproduce on current master (fb255012ac):

  git init -q --ref-format=reftable repo && cd repo
  git commit -q --allow-empty -m one
  git commit -q --allow-empty -m two
  git branch y HEAD~
  printf '%s\n' start "update refs/heads/y $(git rev-parse HEAD)" \
          'option no-deref' 'symref-update HEAD refs/heads/y' commit |
  git update-ref -m test --stdin >/dev/null
  git log -1 --format='%h %s' HEAD
  git reflog -1 --format='%h %gs' HEAD

HEAD is at "two", but the "test" reflog entry points at "one". With --ref-format=files it points at "two".

Using "create refs/heads/z <oid>" instead of updating y leaves HEAD's reflog without a new entry.

I think the cause is write_transaction_table() in refs/reftable-backend.c. For a symref update it gets the new oid for the log record with refs_resolve_ref_unsafe() on new_target, which reads the refs as they were before the transaction. So it sees the old value of y, or no y at all, in which case the update is treated as a dangling symref and the reflog entry is skipped.

I ran into this while looking at updating a new branch and HEAD in one transaction for "git switch -c". Would taking the new oid from the target's update in the same transaction, when there is one, be the right fix? I can try to write a patch.

With the files backend, this isn't an issue only because the files backend doesn't support atomicity. If it did, we'd have the same bug there too ;)

I wonder if we can move move reflog logic to the generic 'refs.c' layer.

Quoted 19 lines · 井上司

Hi,

With the reftable backend, if one transaction updates a branch and points HEAD at it, the new HEAD reflog entry records the branch's old value. If the branch is created in the same transaction, no HEAD reflog entry is written at all. The files backend records the new value in both cases.

To reproduce on current master (fb255012ac):

  git init -q --ref-format=reftable repo && cd repo
  git commit -q --allow-empty -m one
  git commit -q --allow-empty -m two
  git branch y HEAD~
  printf '%s\n' start "update refs/heads/y $(git rev-parse HEAD)" \
          'option no-deref' 'symref-update HEAD refs/heads/y' commit |
  git update-ref -m test --stdin >/dev/null
  git log -1 --format='%h %s' HEAD
  git reflog -1 --format='%h %gs' HEAD

Quoted 6 lines

HEAD is at "two", but the "test" reflog entry points at "one". With --ref-format=files it points at "two".

Using "create refs/heads/z <oid>" instead of updating y leaves HEAD's reflog without a new entry.

With the files backend, this isn't an issue only because the files backend doesn't support atomicity. If it did, we'd have the same bug there too ;)

Quoted 7 lines

I think the cause is write_transaction_table() in refs/reftable-backend.c. For a symref update it gets the new oid for the log record with refs_resolve_ref_unsafe() on new_target, which reads the refs as they were before the transaction. So it sees the old value of y, or no y at all, in which case the update is treated as a dangling symref and the reflog entry is skipped.

I haven't looked at the code, but your inference seems to make sense to me.

Quoted 5 lines

I ran into this while looking at updating a new branch and HEAD in one transaction for "git switch -c". Would taking the new oid from the target's update in the same transaction, when there is one, be the right fix? I can try to write a patch.

That would definitely work, but this same bug would exist with any new backend that is introduced and supports atomic transactions.

I wonder if we can move move reflog logic to the generic 'refs.c' layer. So for a given transaction, 'refs.c' can already parse all the updates and also set the reflog values. Then the individual backends would simply receive the reflog entry to create and wouldn't have to worry about solving inter-transaction conflict resoultion.

But that might be a complex undertaking. I prefer the generic approach, but your approach would also definitely fix the issue and would be a very welcome fix.

Thanks,
Karthik

Quoted 3 lines · Karthik Nayak

With the files backend, this isn't an issue only because the files backend doesn't support atomicity. If it did, we'd have the same bug there too ;)

Thanks for looking at this so quickly. You're right, and it shows up with files already. files commits the updates one by one in the order they were queued, so the reflog is right only when the branch update comes before HEAD. With "symref-update HEAD" first, files also records the old oid, or no entry when the branch is created. I was wrong to say files gets it right in both cases.

Quoted 1 line

I wonder if we can move move reflog logic to the generic 'refs.c' layer.

I like that direction. As a first step toward it, I'd like to add a helper in refs.c that returns the new oid of a ref updated in the same transaction, and have both backends use it before falling back to refs_resolve_ref_unsafe(). That fixes both backends whatever the order, and one test can cover both. Moving the rest of the reflog logic to refs.c could then come on top of that.

Does that sound reasonable?

Thanks,
Tsukasa

2026年11月9日(月) 7:18 Karthik Nayak <[email protected]>: