井上司 [off-list ref] writes:
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.
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 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.
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