Re: [PATCH v5 2/3] rerere: add "gc --auto" that skips a held lock

[Patrick Steinhardt <ps@pks.im>]

On Mon, Sep 28, 2026 at 11:58:21AM +0000, Thomas Bachem via GitGitGadget wrote:
> 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.

> 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.

>  	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