Re: [PATCH 08/11] builtin/maintenance: let tasks do maintenance before and after detach

From: Patrick Steinhardt <ps@pks.im>
Date: May 30, 2025, 14:05 UTC
Message-ID: <aDm7JeVg9vqsq-09@pks.im>
In-Reply-To: <6E33013D-8336-4BE1-9DBB-CAFF835A49C3@gmail.com>

On Fri, May 30, 2025 at 08:56:38AM -0400, Ben Knoble wrote:

Show 9 quoted lines

> > @@ -1599,20 +1600,25 @@ static const struct maintenance_task tasks[] = {
> > static int maybe_run_task(const struct maintenance_task *task,
> >             struct repository *repo,
> >             struct maintenance_run_opts *opts,
> > -              struct gc_config *cfg)
> > +              struct gc_config *cfg,
> > +              int before)
> 
> Perhaps we can use a small enum…

Show 22 quoted lines

> > @@ -1641,6 +1647,10 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
> >   }
> >   free(lock_path);
> > 
> > +    for (size_t i = 0; i < opts->tasks_nr; i++)
> > +        if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg, 1))
> 
> So that is clear what « 1 » (« BEFORE ») does here…
> 
> > +            result = 1;
> > +
> >   /* Failure to daemonize is ok, we'll continue in foreground. */
> >   if (opts->detach > 0) {
> >       trace2_region_enter("maintenance", "detach", the_repository);
> > @@ -1649,7 +1659,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
> >   }
> > 
> >   for (size_t i = 0; i < opts->tasks_nr; i++)
> > -        if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg))
> > +        if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg, 0))
> 
> And « 0 » (« AFTER ») does here?

Yeah, I was very close to adding one, but then decided against it to not produce additional churn. But agreed, it would be way easier to read this way, so let me add one.

Patrick