Re: [PATCH 1/2] list-objects: drop --unpacked non-commit objects from results

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 7, 2023, 02:21 UTC
Message-ID: <xmqq7cmub8wm.fsf@gitster.g>
In-Reply-To: <d3992c98aaa54c3635c249a15d919aa1177324d8.1699311386.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

> Subject: Re: [PATCH 1/2] list-objects: drop --unpacked non-commit objects from results

I thought the purpose of this change is to make sure that we drop packed objects from results when --unpacked is given? This makes it sound as if we are dropping unpacked ones instead? I dunno.

Show 9 quoted lines

> In git-rev-list(1), we describe the `--unpacked` option as:
>
>     Only useful with `--objects`; print the object IDs that are not in
>     packs.
>
> This is true of commits, which we discard via get_commit_action(), but
> not of the objects they reach. So if we ask for an --objects traversal
> with --unpacked, we may get arbitrarily many objects which are indeed
> packed.

Strictly speaking, as long as all the objects that are not in packs are shown, "print the object IDs that are not in packs" is satisfied. With this fix, perhaps we would want to tighten the explanation a little bit while we are at it. Perhaps

	print the object names but exclude those that are in packs

or something along that line?

Show 13 quoted lines

> diff --git a/list-objects.c b/list-objects.c
> index c25c72b32c..c8a5fb998e 100644
> --- a/list-objects.c
> +++ b/list-objects.c
> @@ -39,6 +39,9 @@ static void show_object(struct traversal_context *ctx,
>  {
>  	if (!ctx->show_object)
>  		return;
> +	if (ctx->revs->unpacked && has_object_pack(&object->oid))
> +		return;
> +
>  	ctx->show_object(object, name, ctx->show_data);
>  }

The implementation is as straight-forward as it can get. Very pleasing.

Show 22 quoted lines

> diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
> index 12def7bcbf..6289a2e8b0 100755
> --- a/t/t6000-rev-list-misc.sh
> +++ b/t/t6000-rev-list-misc.sh
> @@ -169,4 +169,17 @@ test_expect_success 'rev-list --count --objects' '
>  	test_line_count = $count actual
>  '
>  
> +test_expect_success 'rev-list --unpacked' '
> +	git repack -ad &&
> +	test_commit unpacked &&
> +
> +	git rev-list --objects --no-object-names unpacked^.. >expect.raw &&
> +	sort expect.raw >expect &&
> +
> +	git rev-list --all --objects --unpacked --no-object-names >actual.raw &&
> +	sort actual.raw >actual &&
> +
> +	test_cmp expect actual
> +'
> +
>  test_done