Re: [PATCH] maintenance: trace when auto maintenance skips due to lock

[Patrick Steinhardt <ps@pks.im>]

On Sun, Nov 01, 2026 at 10:59:00AM +0000, Pushkar Singh wrote:
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 81cecf6b6c..e73d1f1d49 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1409,6 +1409,8 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
>  		 * recursive process stack. Do not report an error in
>  		 * that case.
>  		 */
> +		if (opts->auto_flag && errno == EEXIST)
> +			trace2_data_string("maintenance", r, "lock/skip", "exists");

So we now know to report failure when the lock exists already, but in
any other case we don't log anything at all. But wouldn't the caller
want to know when we failed because of for example EPERM, too, or
because of any other error?

>  		if (!opts->auto_flag && !opts->quiet)
>  			warning(_("lock file '%s' exists, skipping maintenance"),
>  				lock_path);

It's quite curious that we only print a warning when `git maintenance
run` has failed to acquire the lock instead of bubbling up an error. I
would have expected that we:

  - Exit successfully but print a warning in the case where we are
    executed with "--auto".

  - Exit with an error code and print an error in the case where we
    execute without "--auto".

Silently swallowing the fact that we didn't do anything in the latter
case certainly feels wrong to me. And git-gc(1) for example dies right
away when the lock exists, so that to me is another indicator that we
should follow and do the same for git-maintenance(1).

This is of course outside the scope of your patch series, but if you
feel like it I think it could be easily fixed while at it in a second
patch.

Thanks!

Patrick