git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org,  Jeff King <peff@peff.net>
Subject: Re: [PATCH 1/2] list-objects: drop --unpacked non-commit objects from results
Date: Tue, 07 Nov 2023 11:21:29 +0900	[thread overview]
Message-ID: <xmqq7cmub8wm.fsf@gitster.g> (raw)
In-Reply-To: <d3992c98aaa54c3635c249a15d919aa1177324d8.1699311386.git.me@ttaylorr.com> (Taylor Blau's message of "Mon, 6 Nov 2023 17:56:30 -0500")

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.

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

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

> 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

  reply	other threads:[~2023-11-07  2:21 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-06 22:56 [PATCH 0/2] revision: exclude all packed objects with `--unpacked` Taylor Blau
2023-11-06 22:56 ` [PATCH 1/2] list-objects: drop --unpacked non-commit objects from results Taylor Blau
2023-11-07  2:21   ` Junio C Hamano [this message]
2023-11-07  4:02     ` Jeff King
2023-11-06 22:56 ` [PATCH 2/2] pack-bitmap: " Taylor Blau
2023-11-07  2:20   ` Junio C Hamano
2023-11-07  3:52   ` Jeff King
2023-11-07  1:42 ` [PATCH 0/2] revision: exclude all packed objects with `--unpacked` Junio C Hamano
2023-11-07  4:02 ` Jeff King
2023-11-07  9:43   ` Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq7cmub8wm.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).