{"thread":{"id":"60482","subject":"[PATCH 0/2] revision: exclude all packed objects with `--unpacked`","startedAt":"2023-11-06T22:56:31Z","lastAt":"2023-11-07T09:43:13Z","messageCount":10,"participants":["Taylor Blau","Junio C Hamano","Jeff King","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"484476","messageId":"cover.1699311386.git.me@ttaylorr.com","threadId":"60482","inReplyTo":null,"subject":"[PATCH 0/2] revision: exclude all packed objects with `--unpacked`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-11-06T22:56:27Z","receivedAt":"2023-11-06T22:56:31Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"While working on my longer series to enable verbatim pack reuse across\nmultiple packs[^1], I noticed a couple of oddities with the `--unpacked`\nrev-walk flag.\n\nWhile it does exclude packed commits, it does not exclude (all) packed\ntrees/blobs/annotated tags. This problem exists in the pack-bitmap\nmachinery, too, which will over-count queries like:\n\n    $ git rev-list --use-bitmap-index --all --unpacked --objects\n\n, etc.\n\nThe fix is relatively straightforward, split across two patches that\nPeff and I worked on together earlier today.\n\nThis is technically a backwards-incompatible change, but the existing\nbehavior is broken and does not match the documented behavior, so I\nthink in this case we are OK to change --unpacked to faithfully\nimplement its documentation.\n\n[^1]: Which, I'm very excited to say, is working :-).\n\nTaylor Blau (2):\n  list-objects: drop --unpacked non-commit objects from results\n  pack-bitmap: drop --unpacked non-commit objects from results\n\n list-objects.c                     |  3 +++\n pack-bitmap.c                      | 27 +++++++++++++++++++++++++++\n t/t6000-rev-list-misc.sh           | 13 +++++++++++++\n t/t6113-rev-list-bitmap-filters.sh | 13 +++++++++++++\n t/t6115-rev-list-du.sh             |  7 +++++++\n 5 files changed, 63 insertions(+)\n\n\nbase-commit: bc5204569f7db44d22477485afd52ea410d83743\n-- \n2.43.0.rc0.2.gef6b2154a3\n"},{"id":"484477","messageId":"d3992c98aaa54c3635c249a15d919aa1177324d8.1699311386.git.me@ttaylorr.com","threadId":"60482","inReplyTo":"cover.1699311386.git.me@ttaylorr.com","subject":"[PATCH 1/2] list-objects: drop --unpacked non-commit objects from results","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-11-06T22:56:30Z","receivedAt":"2023-11-06T22:56:34Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In git-rev-list(1), we describe the `--unpacked` option as:\n\n    Only useful with `--objects`; print the object IDs that are not in\n    packs.\n\nThis is true of commits, which we discard via get_commit_action(), but\nnot of the objects they reach. So if we ask for an --objects traversal\nwith --unpacked, we may get arbitrarily many objects which are indeed\npacked.\n\nI am nearly certain this behavior dates back to the introduction of\n`--unpacked` via 12d2a18780 (\"git rev-list --unpacked\" shows only\nunpacked commits, 2005-07-03), but I couldn't get that revision of Git\nto compile for me. At least as early as v2.0.0 this has been subtly\nbroken:\n\n    $ git.compile --version\n    git version 2.0.0\n\n    $ git.compile rev-list --objects --all --unpacked\n    72791fe96c93f9ec5c311b8bc966ab349b3b5bbe\n    05713d991c18bbeef7e154f99660005311b5004d v1.0\n    153ed8b7719c6f5a68ce7ffc43133e95a6ac0fdb\n    8e4020bb5a8d8c873b25de15933e75cc0fc275df one\n    9200b628cf9dc883a85a7abc8d6e6730baee589c two\n    3e6b46e1b7e3b91acce99f6a823104c28aae0b58 unpacked.t\n\nThere, only the first, third, and sixth entries are loose, with the\nremaining set of objects belonging to at least one pack.\n\nThe implications for this are relatively benign: bare 'git repack'\ninvocations which invoke pack-objects with --unpacked are impacted, and\nat worst we'll store a few extra objects that should have been excluded.\n\nArguably changing this behavior is a backwards-incompatible change,\nsince it alters the set of objects emitted from rev-list queries with\n`--objects` and `--unpacked`. But I argue that this change is still\nsensible, since the existing implementation deviates from\nclearly-written documentation.\n\nThe fix here is straightforward: avoid showing any non-commit objects\nwhich are contained in packs by discarding them within list-objects.c,\nbefore they are shown to the user. Note that similar treatment for\n`list-objects.c::show_commit()` is not needed, since that case is\nalready handled by `revision.c::get_commit_action()`.\n\nCo-authored-by: Jeff King <peff@peff.net>\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n list-objects.c           |  3 +++\n t/t6000-rev-list-misc.sh | 13 +++++++++++++\n 2 files changed, 16 insertions(+)\n\ndiff --git a/list-objects.c b/list-objects.c\nindex c25c72b32c..c8a5fb998e 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -39,6 +39,9 @@ static void show_object(struct traversal_context *ctx,\n {\n \tif (!ctx->show_object)\n \t\treturn;\n+\tif (ctx->revs->unpacked && has_object_pack(&object->oid))\n+\t\treturn;\n+\n \tctx->show_object(object, name, ctx->show_data);\n }\n \ndiff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\nindex 12def7bcbf..6289a2e8b0 100755\n--- a/t/t6000-rev-list-misc.sh\n+++ b/t/t6000-rev-list-misc.sh\n@@ -169,4 +169,17 @@ test_expect_success 'rev-list --count --objects' '\n \ttest_line_count = $count actual\n '\n \n+test_expect_success 'rev-list --unpacked' '\n+\tgit repack -ad &&\n+\ttest_commit unpacked &&\n+\n+\tgit rev-list --objects --no-object-names unpacked^.. >expect.raw &&\n+\tsort expect.raw >expect &&\n+\n+\tgit rev-list --all --objects --unpacked --no-object-names >actual.raw &&\n+\tsort actual.raw >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.43.0.rc0.2.gef6b2154a3\n\n"},{"id":"484478","messageId":"7492dc699052a392d2fb394e1dcfabebac82ded0.1699311386.git.me@ttaylorr.com","threadId":"60482","inReplyTo":"cover.1699311386.git.me@ttaylorr.com","subject":"[PATCH 2/2] pack-bitmap: drop --unpacked non-commit objects from results","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-11-06T22:56:33Z","receivedAt":"2023-11-06T22:56:36Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"When performing revision queries with `--objects` and\n`--use-bitmap-index`, the output may incorrectly contain objects which\nare packed, even when the `--unpacked` option is given. This affects\ntraversals, but also other querying operations, like `--count`,\n`--disk-usage`, etc.\n\nLike in the previous commit, the fix is to exclude those objects from\nthe result set before they are shown to the user (or, in this case,\nbefore the bitmap containing the result of the traversal is enumerated\nand its objects listed).\n\nThis is performed by a new function in pack-bitmap.c, called\n`filter_packed_objects_from_bitmap()`. Note that we do not have to\ninspect individual bits in the result bitmap, since we know that the\nfirst N (where N is the number of objects in the bitmap's pack/MIDX)\nbits correspond to objects which packed by definition.\n\nIn other words, for an object to have a bitmap position (not in the\nextended index), it must appear in either the bitmap's pack or one of\nthe packs in its MIDX.\n\nThis presents an appealing optimization to us, which is that we can\nsimply memset() the corresponding number of `eword_t`'s to zero,\nprovided that we handle any objects which spill into the next word (but\ndon't occupy all 64 bits of the word itself).\n\nWe only have to handle objects in the bitmap's extended index. These\nobjects may (or may not) appear in one or more pack(s). Since these\nobjects are known to not appear in either the bitmap's MIDX or pack,\nthey may be stored as loose, appear in other pack(s), or both.\n\nBefore returning a bitmap containing the result of the traversal back to\nthe caller, drop any bits from the extended index which appear in one or\nmore packs. This implements the correct behavior for rev-list operations\nwhich use the bitmap index to compute their result.\n\nCo-authored-by: Jeff King <peff@peff.net>\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n pack-bitmap.c                      | 27 +++++++++++++++++++++++++++\n t/t6113-rev-list-bitmap-filters.sh | 13 +++++++++++++\n t/t6115-rev-list-du.sh             |  7 +++++++\n 3 files changed, 47 insertions(+)\n\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex ca8319b87c..0260890341 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -1666,6 +1666,30 @@ static int can_filter_bitmap(struct list_objects_filter_options *filter)\n \treturn !filter_bitmap(NULL, NULL, NULL, filter);\n }\n \n+\n+static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,\n+\t\t\t\t\t      struct bitmap *result)\n+{\n+\tstruct eindex *eindex = &bitmap_git->ext_index;\n+\tuint32_t objects_nr;\n+\tsize_t i, pos;\n+\n+\tobjects_nr = bitmap_num_objects(bitmap_git);\n+\tpos = objects_nr / BITS_IN_EWORD;\n+\n+\tif (pos > result->word_alloc)\n+\t\tpos = result->word_alloc;\n+\n+\tmemset(result->words, 0x00, sizeof(eword_t) * pos);\n+\tfor (i = pos * BITS_IN_EWORD; i < objects_nr; i++)\n+\t\tbitmap_unset(result, i);\n+\n+\tfor (i = 0; i < eindex->count; ++i) {\n+\t\tif (has_object_pack(&eindex->objects[i]->oid))\n+\t\t\tbitmap_unset(result, objects_nr + i);\n+\t}\n+}\n+\n struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,\n \t\t\t\t\t int filter_provided_objects)\n {\n@@ -1788,6 +1812,9 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,\n \t\t      wants_bitmap,\n \t\t      &revs->filter);\n \n+\tif (revs->unpacked)\n+\t\tfilter_packed_objects_from_bitmap(bitmap_git, wants_bitmap);\n+\n \tbitmap_git->result = wants_bitmap;\n \tbitmap_git->haves = haves_bitmap;\n \ndiff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh\nindex 4d8e09167e..86c70521f1 100755\n--- a/t/t6113-rev-list-bitmap-filters.sh\n+++ b/t/t6113-rev-list-bitmap-filters.sh\n@@ -141,4 +141,17 @@ test_expect_success 'combine filter with --filter-provided-objects' '\n \tdone <objects\n '\n \n+test_expect_success 'bitmap traversal with --unpacked' '\n+\tgit repack -adb &&\n+\ttest_commit unpacked &&\n+\n+\tgit rev-list --objects --no-object-names unpacked^.. >expect.raw &&\n+\tsort expect.raw >expect &&\n+\n+\tgit rev-list --use-bitmap-index --objects --all --unpacked >actual.raw &&\n+\tsort actual.raw >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/t/t6115-rev-list-du.sh b/t/t6115-rev-list-du.sh\nindex d59111dede..c0cfda62fa 100755\n--- a/t/t6115-rev-list-du.sh\n+++ b/t/t6115-rev-list-du.sh\n@@ -48,6 +48,13 @@ check_du HEAD\n check_du --objects HEAD\n check_du --objects HEAD^..HEAD\n \n+test_expect_success 'setup for --unpacked tests' '\n+\tgit repack -adb &&\n+\ttest_commit unpacked\n+'\n+\n+check_du --all --objects --unpacked\n+\n # As mentioned above, don't use hardcode sizes as actual size, but use the\n # output from git cat-file.\n test_expect_success 'rev-list --disk-usage=human' '\n-- \n2.43.0.rc0.2.gef6b2154a3\n"},{"id":"484496","messageId":"xmqqy1fabapu.fsf@gitster.g","threadId":"60482","inReplyTo":"cover.1699311386.git.me@ttaylorr.com","subject":"Re: [PATCH 0/2] revision: exclude all packed objects with `--unpacked`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-07T01:42:21Z","receivedAt":"2023-11-07T01:42:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> While working on my longer series to enable verbatim pack reuse across\n> multiple packs[^1], I noticed a couple of oddities with the `--unpacked`\n> rev-walk flag.\n>\n> While it does exclude packed commits, it does not exclude (all) packed\n> trees/blobs/annotated tags. This problem exists in the pack-bitmap\n> machinery, too, which will over-count queries like:\n>\n>     $ git rev-list --use-bitmap-index --all --unpacked --objects\n>\n> , etc.\n>\n> The fix is relatively straightforward, split across two patches that\n> Peff and I worked on together earlier today.\n>\n> This is technically a backwards-incompatible change, but the existing\n> behavior is broken and does not match the documented behavior, so I\n> think in this case we are OK to change --unpacked to faithfully\n> implement its documentation.\n\nYeah, it does sound like a straight bugfix to me.\n\n>\n> [^1]: Which, I'm very excited to say, is working :-).\n>\n> Taylor Blau (2):\n>   list-objects: drop --unpacked non-commit objects from results\n>   pack-bitmap: drop --unpacked non-commit objects from results\n>\n>  list-objects.c                     |  3 +++\n>  pack-bitmap.c                      | 27 +++++++++++++++++++++++++++\n>  t/t6000-rev-list-misc.sh           | 13 +++++++++++++\n>  t/t6113-rev-list-bitmap-filters.sh | 13 +++++++++++++\n>  t/t6115-rev-list-du.sh             |  7 +++++++\n>  5 files changed, 63 insertions(+)\n>\n>\n> base-commit: bc5204569f7db44d22477485afd52ea410d83743\n"},{"id":"484497","messageId":"xmqqfs1ib8xu.fsf@gitster.g","threadId":"60482","inReplyTo":"7492dc699052a392d2fb394e1dcfabebac82ded0.1699311386.git.me@ttaylorr.com","subject":"Re: [PATCH 2/2] pack-bitmap: drop --unpacked non-commit objects from results","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-07T02:20:45Z","receivedAt":"2023-11-07T02:20:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\nThe same comment to the commit title applies here.\n\n> .... Note that we do not have to\n> inspect individual bits in the result bitmap, since we know that the\n> first N (where N is the number of objects in the bitmap's pack/MIDX)\n> bits correspond to objects which packed by definition.\n\n;-)\n\nVery nice.  Since we are omitting any object that appears in some\npackfile, this produces an expected result even when some of these\nobjects also appear in loose form.\n\n> +test_expect_success 'bitmap traversal with --unpacked' '\n> +\tgit repack -adb &&\n> +\ttest_commit unpacked &&\n> +\n> +\tgit rev-list --objects --no-object-names unpacked^.. >expect.raw &&\n> +\tsort expect.raw >expect &&\n> +\n> +\tgit rev-list --use-bitmap-index --objects --all --unpacked >actual.raw &&\n> +\tsort actual.raw >actual &&\n> +\n> +\ttest_cmp expect actual\n> +'\n\nOK.\n\n"},{"id":"484499","messageId":"xmqq7cmub8wm.fsf@gitster.g","threadId":"60482","inReplyTo":"d3992c98aaa54c3635c249a15d919aa1177324d8.1699311386.git.me@ttaylorr.com","subject":"Re: [PATCH 1/2] list-objects: drop --unpacked non-commit objects from results","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-07T02:21:29Z","receivedAt":"2023-11-07T02:21:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> Subject: Re: [PATCH 1/2] list-objects: drop --unpacked non-commit objects from results\n\nI thought the purpose of this change is to make sure that we drop\npacked objects from results when --unpacked is given?  This makes\nit sound as if we are dropping unpacked ones instead?  I dunno.\n\n> In git-rev-list(1), we describe the `--unpacked` option as:\n>\n>     Only useful with `--objects`; print the object IDs that are not in\n>     packs.\n>\n> This is true of commits, which we discard via get_commit_action(), but\n> not of the objects they reach. So if we ask for an --objects traversal\n> with --unpacked, we may get arbitrarily many objects which are indeed\n> packed.\n\nStrictly speaking, as long as all the objects that are not in packs\nare shown, \"print the object IDs that are not in packs\" is satisfied.\nWith this fix, perhaps we would want to tighten the explanation a\nlittle bit while we are at it.  Perhaps\n\n\tprint the object names but exclude those that are in packs\n\nor something along that line?\n\n> diff --git a/list-objects.c b/list-objects.c\n> index c25c72b32c..c8a5fb998e 100644\n> --- a/list-objects.c\n> +++ b/list-objects.c\n> @@ -39,6 +39,9 @@ static void show_object(struct traversal_context *ctx,\n>  {\n>  \tif (!ctx->show_object)\n>  \t\treturn;\n> +\tif (ctx->revs->unpacked && has_object_pack(&object->oid))\n> +\t\treturn;\n> +\n>  \tctx->show_object(object, name, ctx->show_data);\n>  }\n\nThe implementation is as straight-forward as it can get.  Very\npleasing.\n\n> diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\n> index 12def7bcbf..6289a2e8b0 100755\n> --- a/t/t6000-rev-list-misc.sh\n> +++ b/t/t6000-rev-list-misc.sh\n> @@ -169,4 +169,17 @@ test_expect_success 'rev-list --count --objects' '\n>  \ttest_line_count = $count actual\n>  '\n>  \n> +test_expect_success 'rev-list --unpacked' '\n> +\tgit repack -ad &&\n> +\ttest_commit unpacked &&\n> +\n> +\tgit rev-list --objects --no-object-names unpacked^.. >expect.raw &&\n> +\tsort expect.raw >expect &&\n> +\n> +\tgit rev-list --all --objects --unpacked --no-object-names >actual.raw &&\n> +\tsort actual.raw >actual &&\n> +\n> +\ttest_cmp expect actual\n> +'\n> +\n>  test_done\n"},{"id":"484508","messageId":"20231107035252.GB873619@coredump.intra.peff.net","threadId":"60482","inReplyTo":"7492dc699052a392d2fb394e1dcfabebac82ded0.1699311386.git.me@ttaylorr.com","subject":"Re: [PATCH 2/2] pack-bitmap: drop --unpacked non-commit objects from results","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-07T03:52:52Z","receivedAt":"2023-11-07T03:52:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 06, 2023 at 05:56:33PM -0500, Taylor Blau wrote:\n\n> Before returning a bitmap containing the result of the traversal back to\n> the caller, drop any bits from the extended index which appear in one or\n> more packs. This implements the correct behavior for rev-list operations\n> which use the bitmap index to compute their result.\n\nNice. I was very happy to see the extra test for \"rev-list --disk-usage\"\nto demonstrate this. The same should apply for \"pack-objects --unpacked\n--use-bitmap-index\" (though in practice I don't think we'd do that, as\n\"--unpacked\" implies on-disk repacking, which we do not generally use\nwith bitmaps).\n\n-Peff\n"},{"id":"484509","messageId":"20231107040200.GC873619@coredump.intra.peff.net","threadId":"60482","inReplyTo":"xmqq7cmub8wm.fsf@gitster.g","subject":"Re: [PATCH 1/2] list-objects: drop --unpacked non-commit objects from results","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-07T04:02:00Z","receivedAt":"2023-11-07T04:02:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 07, 2023 at 11:21:29AM +0900, Junio C Hamano wrote:\n\n> > In git-rev-list(1), we describe the `--unpacked` option as:\n> >\n> >     Only useful with `--objects`; print the object IDs that are not in\n> >     packs.\n> >\n> > This is true of commits, which we discard via get_commit_action(), but\n> > not of the objects they reach. So if we ask for an --objects traversal\n> > with --unpacked, we may get arbitrarily many objects which are indeed\n> > packed.\n> \n> Strictly speaking, as long as all the objects that are not in packs\n> are shown, \"print the object IDs that are not in packs\" is satisfied.\n> With this fix, perhaps we would want to tighten the explanation a\n> little bit while we are at it.  Perhaps\n> \n> \tprint the object names but exclude those that are in packs\n> \n> or something along that line?\n\nI think using the word \"exclude\" is a good idea, as it makes it clear\nthat we are omitting objects that otherwise would be traversed (as\nopposed to just showing unpacked objects, reachable or not).\n\nBut I wanted to point out one other subtlety here. The existing code\n(before this patch) checks for already-packed commits, and avoids adding\nthem to the traversal. The problem this patch is fixing is that we may\nsee objects they point to via other non-packed commits. But the opposite\nproblem exists, too: we have unpacked objects that are reachable from\nthose packed commits.\n\nIt's probably reasonably rare, since we _tend_ to make packs by rolling\nup reachable chunks of history. But that's not a guarantee. One way I\ncan think of for it to happen in practice is that somebody pushes (or\nfetches) a thin pack with commit C as a delta against an unpacked C'. In\nthat case \"index-pack --fix-thin\" will create a duplicate of C' in the\nnew pack, but its trees and blobs may remain unpacked.\n\nI think with the patch in this series we could actually drop that \"do\nnot traverse commits that are unpacked\" line of code, and end up \"more\ncorrect\". But I suspect performance of an incremental \"git repack -d\"\nwould suffer. This is kind of analagous to the \"we do not traverse every\nUNINTERESTING commit just to mark its trees/blobs as UNINTERESTING\"\noptimization. We know that it is not a true set difference, but it is OK\nin practice and it buys us a lot of performance. And just like that\ncase, bitmaps do let us cheaply compute the true set difference. ;)\n\n-Peff\n"},{"id":"484510","messageId":"20231107040235.GD873619@coredump.intra.peff.net","threadId":"60482","inReplyTo":"cover.1699311386.git.me@ttaylorr.com","subject":"Re: [PATCH 0/2] revision: exclude all packed objects with `--unpacked`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-11-07T04:02:35Z","receivedAt":"2023-11-07T04:02:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 06, 2023 at 05:56:27PM -0500, Taylor Blau wrote:\n\n> While working on my longer series to enable verbatim pack reuse across\n> multiple packs[^1], I noticed a couple of oddities with the `--unpacked`\n> rev-walk flag.\n> \n> While it does exclude packed commits, it does not exclude (all) packed\n> trees/blobs/annotated tags. This problem exists in the pack-bitmap\n> machinery, too, which will over-count queries like:\n> \n>     $ git rev-list --use-bitmap-index --all --unpacked --objects\n> \n> , etc.\n> \n> The fix is relatively straightforward, split across two patches that\n> Peff and I worked on together earlier today.\n\nI'm not sure my review is worth anything, but this looks good to me. ;)\nI do think it might be worth tightening up the docs as Junio suggested,\nbut I would be fine to see that as a patch on top.\n\n-Peff\n"},{"id":"484516","messageId":"ZUoGoYpJFvGSll-u@tanuki","threadId":"60482","inReplyTo":"20231107040235.GD873619@coredump.intra.peff.net","subject":"Re: [PATCH 0/2] revision: exclude all packed objects with `--unpacked`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-07T09:43:06Z","receivedAt":"2023-11-07T09:43:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Nov 06, 2023 at 11:02:35PM -0500, Jeff King wrote:\n> On Mon, Nov 06, 2023 at 05:56:27PM -0500, Taylor Blau wrote:\n> \n> > While working on my longer series to enable verbatim pack reuse across\n> > multiple packs[^1], I noticed a couple of oddities with the `--unpacked`\n> > rev-walk flag.\n> > \n> > While it does exclude packed commits, it does not exclude (all) packed\n> > trees/blobs/annotated tags. This problem exists in the pack-bitmap\n> > machinery, too, which will over-count queries like:\n> > \n> >     $ git rev-list --use-bitmap-index --all --unpacked --objects\n> > \n> > , etc.\n> > \n> > The fix is relatively straightforward, split across two patches that\n> > Peff and I worked on together earlier today.\n> \n> I'm not sure my review is worth anything, but this looks good to me. ;)\n> I do think it might be worth tightening up the docs as Junio suggested,\n> but I would be fine to see that as a patch on top.\n> \n> -Peff\n\nI also read through the patches and agree, this looks good to me.\nThanks!\n\nPatrick\n"}]}