Re: [PATCH v5 2/3] rerere: add "gc --auto" that skips a held lock
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Sep 30, 2026, 15:00 UTC
- Message-ID
- <ar0kJPdY1WSsWvP8@pks.im>
- In-Reply-To
- <27673137aae961105a3d3b6ea615879e0686cc65.1790596702.git.gitgitgadget@gmail.com>
On Mon, Sep 28, 2026 at 11:58:21AM +0000, Thomas Bachem via GitGitGadget wrote:
Show 21 quoted lines
> diff --git a/Documentation/git-rerere.adoc b/Documentation/git-rerere.adoc > index 4e6ab9a27c..da7a1d093e 100644 > --- a/Documentation/git-rerere.adoc > +++ b/Documentation/git-rerere.adoc > @@ -63,14 +63,17 @@ Print paths with conflicts that have not been autoresolved by rerere. > This includes paths whose resolutions cannot be tracked by rerere, > such as conflicting submodules. > > -'gc':: > +'gc' [--auto]:: > > Prune records of conflicted merges that > occurred a long time ago. By default, unresolved conflicts older > than 15 days and resolved conflicts older than 60 > days are pruned. These defaults are controlled via the > `gc.rerereUnresolved` and `gc.rerereResolved` configuration > -variables respectively. > +variables respectively. With `--auto`, which `git maintenance run > +--auto` and `git gc --auto` pass, `gc` does nothing while another > +process holds the rerere lock. Without it, `gc` waits for the lock > +as long as `rerere.lockTimeout` allows and then fails.
It's a bit weird to have git-rerere(1) document who calls it. We may want to document why specifically this is useful though.
Show 18 quoted lines
> diff --git a/builtin/rerere.c b/builtin/rerere.c
> index a056cb791b..2a8871df41 100644
> --- a/builtin/rerere.c
> +++ b/builtin/rerere.c
> @@ -56,16 +57,21 @@ int cmd_rerere(int argc,
> struct repository *repo UNUSED)
> {
> struct string_list merge_rr = STRING_LIST_INIT_DUP;
> - int autoupdate = -1, flags = 0;
> + int autoupdate = -1, auto_flag = 0, flags = 0;
>
> struct option options[] = {
> OPT_SET_INT(0, "rerere-autoupdate", &autoupdate,
> N_("register clean resolutions in index"), 1),
> + OPT_BOOL(0, "auto", &auto_flag,
> + N_("skip gc while another process holds the lock")),
> OPT_END(),
> };Thinking about this a bit... I know it was my suggestion, but I wonder whether "auto" is misnamed. We don't let any heuristics kick in like we typically do for other commands like `git pack-refs --auto`, we only know to skip garbage collection if the lock is taken. So there is a bit of a mismatch here.
How about we instead call this "--skip-locked"? We could even mark it as a hidden option and not even document it, as it feels very specific to how git-maintenance(1) wants to invoke it. If so, we could maybe remove it again at a later point.
An alternative could be to instead call `rerere_gc()` directly, and if so we wouldn't have to add this flag at all. But that may result in some bigger changes, so I'll leave it up to you to decide.
Show 8 quoted lines
> argc = parse_options(argc, argv, prefix, options, rerere_usage, 0);
>
> + if (auto_flag && (argc < 1 || strcmp(argv[0], "gc")))
> + die(_("the option '%s' requires '%s'"), "--auto", "gc");
> +
> repo_config(the_repository, git_xmerge_config, NULL);
>
> if (autoupdate == 1)Oh dear, this is a mess. The file could really use a refactoring to use proper subcommands.
But anyway, that's certainly outside the scope of this patch series.
Patrick