On Mon, Aug 19, 2024 at 04:55:22AM -0400, Jeff King wrote:
> On Mon, Aug 19, 2024 at 09:48:05AM +0200, Patrick Steinhardt wrote:
>
> > The "loose-objects" maintenance tasks executes git-pack-objects(1) to
> > pack all loose objects into a new packfile. This command ends up
> > printing the hash of the packfile to stdout though, which clutters the
> > output of `git maintenance run`.
> >
> > Fix this issue by disabling stdout of the child process.
>
> Ah, I wondered where that output was coming from.
>
> > diff --git a/builtin/gc.c b/builtin/gc.c
> > index 13bc0572a3..be75efa17a 100644
> > --- a/builtin/gc.c
> > +++ b/builtin/gc.c
> > @@ -1159,6 +1159,12 @@ static int pack_loose(struct maintenance_run_opts *opts)
> >
> > pack_proc.in = -1;
> >
> > + /*
> > + * git-pack-objects(1) ends up writing the pack hash to stdout, which
> > + * we do not care for.
> > + */
> > + pack_proc.out = -1;
> > +
> > if (start_command(&pack_proc)) {
> > error(_("failed to start 'git pack-objects' process"));
> > return 1;
>
> 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.
> > +++ b/t/t7900-maintenance.sh
> > @@ -978,4 +978,20 @@ test_expect_success '--detach causes maintenance to run in background' '
> > )
> > '
> >
> > +test_expect_success 'repacking loose objects is quiet' '
> > + test_when_finished "rm -rf repo" &&
> > + git init repo &&
> > + (
> > + cd repo &&
> > +
> > + test_commit something &&
> > + git config set maintenance.gc.enabled false &&
> > + git config set maintenance.loose-objects.enabled true &&
> > + git config set maintenance.loose-objects.auto 1 &&
> > +
> > + git maintenance run --quiet >out 2>&1 &&
> > + test_must_be_empty out
> > + )
> > +'
>
> 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.
Patrick
