Re: [PATCH 3/3] builtin/maintenance: fix loose objects task emitting pack hash

Jeff King
2024-08-19T09:17:17Z

On Mon, Aug 19, 2024 at 11:07:51AM +0200, Patrick Steinhardt wrote:

> > I have not paid much attention to the "maintenance" stuff. It is a
> > little weird to me that it is not building on "git repack", which
> > already handles this, but perhaps there are reasons. Anyway, totally
> > unrelated to your patch (which looks good to me).
>
> git-repack(1) is way less efficient than running git-pack-objects(1)
> directly. I've also noticed that at one point in time when revamping how
> we do housekeeping in Git.
>
> It mostly boils down to git-repack(1) doing a connectivity check,
> whereas git-pack-objects(1) doesn't. We just soak up every single loose
> object, and then eventually we expire them via git-multi-pack-index(1)'s
> "expire" subcommand.

Hmph. I'd have suggested that we should teach git-repack to do the more
efficient thing. I'm a bit worried about having parallel universes of
how maintenance works making it harder to reason about when or how
things happen, and how various concurrent / racy behaviors work.

But it's probably a bit late to re-open that (and certainly it's not
part of your series).

> > I wondered if you needed --no-detach here to avoid a race, but I guess
> > as a non-auto run, it would never background?
>
> Even the `--auto` run does not background. That was the case for
> git-gc(1), but is not the case for git-maintenance(1). You now have to
> pass `--detach` explicitly to cause it to background, which I think is
> the saner way to do this anyway.

Am I misreading the documentation? The entry for maintenance.autoDetach
on 'next' says:

If unset, the value of `gc.autoDetach` is used as a fallback. Defaults
to true if both are unset, meaning that the maintenance process will
detach.

-Peff