Re: [PATCH v3 2/8] branch: consider refs under 'update-refs'
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Jun 30, 2022, 09:47 UTC
- Message-ID
- <4907af96-cb62-1d7f-d9ea-18d0e646f4ea@gmail.com>
- In-Reply-To
- <9d188e11-7bb4-cae7-b38a-f9f38c8c2fe1@github.com>
On 29/06/2022 13:58, Derrick Stolee wrote:
Show 51 quoted lines
> 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
Show 9 quoted lines
> 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