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

Jeff King
2024-08-19T08:55:23Z

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

> +++ 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?

-Peff