Re: [PATCH v2] rerere: keep a background gc from killing a rebase

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

Hi Thomas

On 04/09/2026 16:55, Thomas Bachem wrote:
> On 04/09/2026 16:21, Phillip Wood wrote:
>> With Patricks patches that's no-longer true I think. I think a better
>> motivation, as the cache is per-repository, rather than per-worktree, is
>> concurrent writers running in different worktrees.
> 
> MERGE_RR is per worktree, though, and so is its lock:
> 
>      $ git -C linked rev-parse --git-path MERGE_RR
>      /path/to/main/.git/worktrees/linked/MERGE_RR
> 
> so writers in different worktrees never meet on it. What they share is
> rr-cache, which a gc in one worktree prunes under its own worktree's
> lock only. That is a gap of its own, and not one this patch closes.

Oh, I didn't realize the lock was per-worktree. So the lock "rerere gc" 
takes does not actually stop another process running in a different 
worktree from altering the rerere cache.

> What remains after Patrick's series is any "git rerere gc" that runs
> while a command records a conflict, from "git gc", from a maintenance
> run, or from auto maintenance once enough entries are stale. The v3
> message says it that way.
> 
>> Overall, this commit message is rather long and it would be helpful if
>> you could distill it to remove unnecessary and unrelated details.
> 
> Done, it is a quarter of the size now.
> 
>> Why do those commands fail rather than wait?
> 
> They wait like everything else, and once the time is up they fail
> instead of going on without rerere, which is all they are for. That
> way a stale lock gets the usual advice to remove it. The config text
> said otherwise, fixed.

That's good, I think I'd maybe misunderstood what the original patch was 
trying to say.

>> It might be worth adding a check above here that BUG()s out if the
>> caller passes an incompatible set of flags.
> 
> Added, for RERERE_NOWAIT with RERERE_LOCK_OR_DIE and for
> RERERE_READONLY with either.
> 
>> A background job that the user did not explicitly start printing to the
>> terminal is rather confusing as it is likely to get mixed in with the
>> output of whatever is running in the foreground.
> 
> The detached maintenance run has no terminal: daemonize() closes the
> standard descriptors and reopens them on /dev/null, so the gc's
> warning goes nowhere when it loses the lock. Where it cannot detach,
> on Windows, it runs in the foreground of the commit that started it
> and there is no race to lose. The warning the user does see is the
> foreground command's own, when it gives up waiting.

Thanks for clarifying that

Phillip

> 
> Thanks,
> Thomas