Re: [PATCH v3 2/8] branch: consider refs under 'update-refs'

[Phillip Wood <phillip.wood123@gmail.com>]

On 29/06/2022 13:58, Derrick Stolee wrote:
> On 6/28/22 4:48 PM, Junio C Hamano wrote:
>> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>>> From: Derrick Stolee <derrickstolee@github.com>
>>>
>>> The branch_checked_out() helper helps commands like 'git branch' and
>>> 'git fetch' from overwriting refs that are currently checked out in
>>> other worktrees.
>>>
>>> A future update to 'git rebase' will introduce a new '--update-refs'
>>> option which will update the local refs that point to commits that are
>>> being rebased. To avoid collisions as the rebase completes, we want to
>>> make the future data store for these refs to be considered by
>>> branch_checked_out().
>>>
>>> The data store is a plaintext file inside the 'rebase-merge' directory
>>> for that worktree. The file alternates refnames and OIDs. The OIDs will
>>> be used to store the to-be-written values as the rebase progresses, but
>>> can be ignored at the moment.
>>>
>>> Create a new sequencer_get_update_refs_state() method that parses this
>>> file and populates a struct string_list with the ref-OID pairs. We can
>>> then use this list to add to the current_checked_out_branches strmap
>>> used by branch_checked_out().
>>>
>>> To properly navigate to the rebase directory for a given worktree,
>>> extract the static strbuf_worktree_gitdir() method to a public API
>>> method.
>>>
>>> We can test that this works without having Git write this file by
>>> artificially creating one in our test script.
>>
>> Hmph, I am not thrilled to see an ad-hoc text file for things like
>> this.  Do the objects that appear in this file otherwise already
>> anchored by existing refs, or can some of them be "floating" without
>> any refs pointing at them, i.e. making the "connected" and "fsck"
>> machinery also being aware of them to protect them from collected as
>> garbage and reported as dangling/unreachable?
> 
> You're right that we could have this sequence of events in a todo
> file:
> 
>    pick deadbeef Here is a commit that will become unreachable
>    update-ref will-be-lost
> 
>    squash 1234567 make will-be-lost unreachable
>    ...
> 
> So if a GC runs after this 'update-ref' step but before the rebase
> completes, then that commit can be lost. The ref-update at the end
> of the rebase process would fail.

The commit is protected by HEAD's reflog for a while because we update 
HEAD after each pick but it is not permanently safe.

I've taken a look at the first few patches and they're looking good, I 
hope to look at the rest tomorrow

Best Wishes

Phillip

> I wonder how important such a situation is, though. But I'm willing
> to add the extra lookups in 'git gc' and 'git fsck' to make this a
> non-issue.
> 
> I'll take a look to see where the other refs in rebase-<backend>/*
> are handled by these processes.
> 
> Thanks,
> -Stolee