{"thread":{"id":"63203","subject":"[PATCH v2 01/10] builtin/cat-file: rename variable that tracks usage","startedAt":"2025-03-27T09:44:08Z","lastAt":"2025-04-07T20:25:31Z","messageCount":28,"participants":["Patrick Steinhardt","Karthik Nayak","Toon Claes","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":10},"messages":[{"id":"515135","messageId":"20250327-pks-cat-file-object-type-filter-v2-0-4bbc7085d7c5@pks.im","threadId":"63203","inReplyTo":"20250221-pks-cat-file-object-type-filter-v1-0-0852530888e2@pks.im","subject":"[PATCH v2 00/10] builtin/cat-file: allow filtering objects in batch mode","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-27T09:43:55Z","receivedAt":"2025-03-27T09:44:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nat GitLab, we sometimes have the need to list all objects regardless of\ntheir reachability. We use git-cat-file(1) with `--batch-all-objects` to\ndo this, and typically this is quite a good fit. In some cases though,\nwe only want to list objects of a specific type, where we then basically\nhave the following pipeline:\n\n    git cat-file --batch-all-objects --batch-check='%(objecttype) %(objectname)' |\n    grep '^commit ' |\n    cut -d' ' -f2 |\n    git cat-file --batch\n\nThis works okayish in medium-sized repositories, but once you reach a\ncertain size this isn't really an option anymore. In the Chromium\nrepository for example [1] simply listing all objects in the first\ninvocation of git-cat-file(1) takes around 80 to 100 seconds. The\nworkload is completely I/O-bottlenecked: my machine reads at ~500MB/s,\nand the packfile is 50GB in size, which matches the 100 seconds that I\nobserve.\n\nThis series addresses the issue by introducing object filters into\ngit-cat-file(1). These object filters use the exact same syntax as the\nfilters we have in git-rev-list(1), but only a subset of them is\nsupported because not all filters can be computed by git-cat-file(1).\nSupported are \"blob:none\", \"blob:limit=\" as well as \"object:type=\".\n\nThe filters alone don't really help though: we still have to scan\nthrough the whole packfile in order to compute the packfiles. While we\nare able to shed a bit of CPU time because we can stop emitting some of\nthe objects, we're still I/O-bottlenecked.\n\nThe second part of the series thus expands the filters so that they can\nmake use of bitmap indices for some of the filters, if available. This\nallows us to efficiently answer the question where to find all objects\nof a specific type, and thus we can avoid scanning through the packfile\nand instead directly look up relevant objects, leading to a significant\nspeedup:\n\n    Benchmark 1: cat-file with filter=object:type=commit (revision = HEAD~)\n      Time (mean ± σ):     86.444 s ±  4.081 s    [User: 36.830 s, System: 11.312 s]\n      Range (min … max):   80.305 s … 93.104 s    10 runs\n\n    Benchmark 2: cat-file with filter=object:type=commit (revision = HEAD)\n      Time (mean ± σ):      2.089 s ±  0.015 s    [User: 1.872 s, System: 0.207 s]\n      Range (min … max):    2.073 s …  2.119 s    10 runs\n\n    Summary\n      cat-file with filter=object:type=commit (revision = HEAD) ran\n       41.38 ± 1.98 times faster than cat-file with filter=object:type=commit (revision = HEAD~)\n\nWe now directly scale with the number of objects of a specific type\ncontained in the packfile instead of scaling with the overall number of\nobjects. It's quite fun to see how the math plays out: if you sum up the\ntimes for each of the types you arrive at the time for the unfiltered\ncase.\n\nChanges in v2:\n  - The series is now built on top of \"master\" at 683c54c999c (Git 2.49,\n    2025-03-14) with \"tb/incremental-midx-part-2\" at 27afc272c49 (midx:\n    implement writing incremental MIDX bitmaps, 2025-03-20) merged into\n    it.\n  - Rename the filter options to \"--filter=\" to match\n    git-pack-objects(1).\n  - The bitmap-filtering is now reusing existing mechanisms that we\n    already have in \"pack-bitmap.c\", as proposed by Taylor.\n  - Link to v1: https://lore.kernel.org/r/20250221-pks-cat-file-object-type-filter-v1-0-0852530888e2@pks.im\n\nThanks!\n\nPatrick\n\n[1]: https://github.com/chromium/chromium.git\n\n---\nPatrick Steinhardt (10):\n      builtin/cat-file: rename variable that tracks usage\n      builtin/cat-file: wire up an option to filter objects\n      builtin/cat-file: support \"blob:none\" objects filter\n      builtin/cat-file: support \"blob:limit=\" objects filter\n      builtin/cat-file: support \"object:type=\" objects filter\n      pack-bitmap: allow passing payloads to `show_reachable_fn()`\n      pack-bitmap: add function to iterate over filtered bitmapped objects\n      pack-bitmap: introduce function to check whether a pack is bitmapped\n      builtin/cat-file: deduplicate logic to iterate over all objects\n      builtin/cat-file: use bitmaps to efficiently filter by object type\n\n Documentation/git-cat-file.adoc |  16 +++\n builtin/cat-file.c              | 212 +++++++++++++++++++++++++++++-----------\n builtin/pack-objects.c          |   3 +-\n builtin/rev-list.c              |   3 +-\n pack-bitmap.c                   |  81 +++++++++++++--\n pack-bitmap.h                   |  22 ++++-\n reachable.c                     |   3 +-\n t/t1006-cat-file.sh             |  77 +++++++++++++++\n 8 files changed, 346 insertions(+), 71 deletions(-)\n\nRange-diff versus v1:\n\n 1:  108b50d8a66 =  1:  d16b84702fd builtin/cat-file: rename variable that tracks usage\n 2:  4a4a22ac465 !  2:  d3259ff034c builtin/cat-file: wire up an option to filter objects\n    @@ Commit message\n         been passed by the user, but they will be wired up in subsequent\n         commits.\n     \n    -    Note that we don't use the same `--filter=` name fo the option as we use\n    -    in git-rev-list(1). We already have `--filters`, and having both\n    -    `--filter=` and `--filters` would be quite confusing. Instead, the new\n    -    option is called `--objects-filter`.\n    -\n         Further note that the filters that we are about to introduce don't\n         significantly speed up the runtime of git-cat-file(1). While we can skip\n         emitting a lot of objects in case they are uninteresting to us, the\n    @@ Documentation/git-cat-file.adoc: OPTIONS\n      \tend-of-line conversion, etc). In this case, `<object>` has to be of\n      \tthe form `<tree-ish>:<path>`, or `:<path>`.\n      \n    -+--objects-filter=<filter-spec>::\n    -+--no-objects-filter::\n    ++--filter=<filter-spec>::\n    ++--no-filter::\n     +\tOmit objects from the list of printed objects. This can only be used in\n     +\tcombination with one of the batched modes. The '<filter-spec>' may be\n     +\tone of the following:\n    @@ builtin/cat-file.c: int cmd_cat_file(int argc,\n      \t\t\t    N_(\"run filters on object's content\"), 'w'),\n      \t\tOPT_STRING(0, \"path\", &force_path, N_(\"blob|tree\"),\n      \t\t\t   N_(\"use a <path> for (--textconv | --filters); Not with 'batch'\")),\n    -+\t\tOPT_CALLBACK(0, \"objects-filter\", &batch.objects_filter, N_(\"args\"),\n    ++\t\tOPT_CALLBACK(0, \"filter\", &batch.objects_filter, N_(\"args\"),\n     +\t\t\t     N_(\"object filtering\"), opt_parse_list_objects_filter),\n      \t\tOPT_END()\n      \t};\n    @@ t/t1006-cat-file.sh: test_expect_success PERL '--batch-command info is unbuffere\n     +\tcat >expect <<-EOF &&\n     +\tfatal: invalid filter-spec ${SQ}unknown${SQ}\n     +\tEOF\n    -+\ttest_must_fail git -C repo cat-file --objects-filter=unknown 2>err &&\n    ++\ttest_must_fail git -C repo cat-file --filter=unknown 2>err &&\n     +\ttest_cmp expect err\n     +'\n     +\n    @@ t/t1006-cat-file.sh: test_expect_success PERL '--batch-command info is unbuffere\n     +\t\t\tprintf \"usage: objects filter not supported: ${SQ}%s${SQ}\\n\" \"$option_name\" >expect\n     +\t\t\t;;\n     +\t\tesac &&\n    -+\t\ttest_must_fail git -C repo cat-file --objects-filter=$option 2>err &&\n    ++\t\ttest_must_fail git -C repo cat-file --filter=$option 2>err &&\n     +\t\ttest_cmp expect err\n     +\t'\n     +done\n 3:  baddbca6de6 !  3:  02c7fc38986 builtin/cat-file: support \"blob:none\" objects filter\n    @@ t/t1006-cat-file.sh: do\n     +\tfilter=\"$1\"\n     +\n     +\ttest_expect_success \"objects filter: $filter\" '\n    -+\t\tgit -C repo cat-file --batch-check=\"%(objectname)\" --batch-all-objects --objects-filter=\"$filter\" >actual &&\n    ++\t\tgit -C repo cat-file --batch-check=\"%(objectname)\" --batch-all-objects --filter=\"$filter\" >actual &&\n     +\t\tsort actual >actual.sorted &&\n     +\t\tgit -C repo rev-list --objects --no-object-names --all --filter=\"$filter\" --filter-provided-objects >expect &&\n     +\t\tsort expect >expect.sorted &&\n 4:  e55fa01810d !  4:  33c5ea58fdc builtin/cat-file: support \"blob:limit=\" objects filter\n    @@ Documentation/git-cat-file.adoc: OPTIONS\n      The form '--filter=blob:none' omits all blobs.\n     ++\n     +The form '--filter=blob:limit=<n>[kmg]' omits blobs of size at least n\n    -+bytes or units.  n may be zero.  The suffixes k, m, and g can be used\n    -+to name units in KiB, MiB, or GiB.  For example, 'blob:limit=1k'\n    -+is the same as 'blob:limit=1024'.\n    ++bytes or units.  n may be zero.  The suffixes k, m, and g can be used to name\n    ++units in KiB, MiB, or GiB.  For example, 'blob:limit=1k' is the same as\n    ++'blob:limit=1024'.\n      \n      --path=<path>::\n      \tFor use with `--textconv` or `--filters`, to allow specifying an object\n 5:  1882ac07c9b !  5:  40e2f7f2f82 builtin/cat-file: support \"object:type=\" objects filter\n    @@ Commit message\n     \n      ## Documentation/git-cat-file.adoc ##\n     @@ Documentation/git-cat-file.adoc: The form '--filter=blob:limit=<n>[kmg]' omits blobs of size at least n\n    - bytes or units.  n may be zero.  The suffixes k, m, and g can be used\n    - to name units in KiB, MiB, or GiB.  For example, 'blob:limit=1k'\n    - is the same as 'blob:limit=1024'.\n    + bytes or units.  n may be zero.  The suffixes k, m, and g can be used to name\n    + units in KiB, MiB, or GiB.  For example, 'blob:limit=1k' is the same as\n    + 'blob:limit=1024'.\n     ++\n    -+The form '--filter=object:type=(tag|commit|tree|blob)' omits all objects\n    -+which are not of the requested type.\n    ++The form '--filter=object:type=(tag|commit|tree|blob)' omits all objects which\n    ++are not of the requested type.\n      \n      --path=<path>::\n      \tFor use with `--textconv` or `--filters`, to allow specifying an object\n 6:  c8ac9481b39 !  6:  5affa992909 pack-bitmap: expose function to iterate over bitmapped objects\n    @@ Metadata\n     Author: Patrick Steinhardt <ps@pks.im>\n     \n      ## Commit message ##\n    -    pack-bitmap: expose function to iterate over bitmapped objects\n    +    pack-bitmap: allow passing payloads to `show_reachable_fn()`\n     \n    -    Expose a function that allows the caller to iterate over all bitmapped\n    -    objects of a specific type. This mechanism allows us to use the object\n    -    type-specific bitmaps to enumerate all objects of that type without\n    -    having to scan through a complete packfile.\n    +    The `show_reachable_fn` callback is used by a couple of functions to\n    +    present reachable objects to the caller. The function does not provide a\n    +    way for the caller to pass a payload though, which is functionality that\n    +    we'll require in a subsequent commit.\n     \n    -    This functionality will be used in a subsequent commit.\n    +    Change the callback type to accept a payload and adapt all callsites\n    +    accordingly.\n     \n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n    @@ pack-bitmap.c: static void show_extended_objects(struct bitmap_index *bitmap_git\n      \t}\n      }\n      \n    --static void init_type_iterator(struct ewah_iterator *it,\n    --\t\t\t       struct bitmap_index *bitmap_git,\n    --\t\t\t       enum object_type type)\n    -+static struct ewah_bitmap *ewah_for_type(struct bitmap_index *bitmap_git,\n    -+\t\t\t\t\t enum object_type type)\n    - {\n    - \tswitch (type) {\n    - \tcase OBJ_COMMIT:\n    --\t\tewah_iterator_init(it, bitmap_git->commits);\n    --\t\tbreak;\n    --\n    -+\t\treturn bitmap_git->commits;\n    - \tcase OBJ_TREE:\n    --\t\tewah_iterator_init(it, bitmap_git->trees);\n    --\t\tbreak;\n    --\n    -+\t\treturn bitmap_git->trees;\n    - \tcase OBJ_BLOB:\n    --\t\tewah_iterator_init(it, bitmap_git->blobs);\n    --\t\tbreak;\n    --\n    -+\t\treturn bitmap_git->blobs;\n    - \tcase OBJ_TAG:\n    --\t\tewah_iterator_init(it, bitmap_git->tags);\n    --\t\tbreak;\n    --\n    -+\t\treturn bitmap_git->tags;\n    - \tdefault:\n    - \t\tBUG(\"object type %d not stored by bitmap type index\", type);\n    --\t\tbreak;\n    - \t}\n    - }\n    - \n    --static void show_objects_for_type(\n    --\tstruct bitmap_index *bitmap_git,\n    --\tenum object_type object_type,\n    +@@ pack-bitmap.c: static void init_type_iterator(struct ewah_or_iterator *it,\n    + static void show_objects_for_type(\n    + \tstruct bitmap_index *bitmap_git,\n    + \tenum object_type object_type,\n     -\tshow_reachable_fn show_reach)\n    -+static void init_type_iterator(struct ewah_iterator *it,\n    -+\t\t\t       struct bitmap_index *bitmap_git,\n    -+\t\t\t       enum object_type type)\n    -+{\n    -+\tewah_iterator_init(it, ewah_for_type(bitmap_git, type));\n    -+}\n    -+\n    -+static void for_each_bitmapped_object_internal(struct bitmap_index *bitmap_git,\n    -+\t\t\t\t\t       struct bitmap *objects,\n    -+\t\t\t\t\t       enum object_type object_type,\n    -+\t\t\t\t\t       show_reachable_fn show_reach,\n    -+\t\t\t\t\t       void *payload)\n    ++\tshow_reachable_fn show_reach,\n    ++\tvoid *payload)\n      {\n      \tsize_t i = 0;\n      \tuint32_t offset;\n    --\n    - \tstruct ewah_iterator it;\n    - \teword_t filter;\n    - \n    --\tstruct bitmap *objects = bitmap_git->result;\n    --\n    - \tinit_type_iterator(&it, bitmap_git, object_type);\n    - \n    - \tfor (i = 0; i < objects->word_alloc &&\n     @@ pack-bitmap.c: static void show_objects_for_type(\n      \t\t\tif (bitmap_git->hashes)\n      \t\t\t\thash = get_be32(bitmap_git->hashes + index_pos);\n    @@ pack-bitmap.c: static void show_objects_for_type(\n     +\t\t\tshow_reach(&oid, object_type, 0, hash, pack, ofs, payload);\n      \t\t}\n      \t}\n    - }\n      \n    -+static void show_objects_for_type(\n    -+\tstruct bitmap_index *bitmap_git,\n    -+\tenum object_type object_type,\n    -+\tshow_reachable_fn show_reach)\n    -+{\n    -+\tfor_each_bitmapped_object_internal(bitmap_git, bitmap_git->result,\n    -+\t\t\t\t\t   object_type, show_reach, NULL);\n    -+}\n    -+\n    -+void for_each_bitmapped_object(struct bitmap_index *bitmap_git,\n    -+\t\t\t       enum object_type object_type,\n    -+\t\t\t       show_reachable_fn show_reach,\n    -+\t\t\t       void *payload)\n    -+{\n    -+\tstruct bitmap *bitmap = ewah_to_bitmap(ewah_for_type(bitmap_git, object_type));\n    -+\tfor_each_bitmapped_object_internal(bitmap_git, bitmap,\n    -+\t\t\t\t\t   object_type, show_reach, payload);\n    -+\tbitmap_free(bitmap);\n    -+}\n    -+\n    - static int in_bitmapped_pack(struct bitmap_index *bitmap_git,\n    - \t\t\t     struct object_list *roots)\n    +@@ pack-bitmap.c: void traverse_bitmap_commit_list(struct bitmap_index *bitmap_git,\n      {\n    + \tassert(bitmap_git->result);\n    + \n    +-\tshow_objects_for_type(bitmap_git, OBJ_COMMIT, show_reachable);\n    ++\tshow_objects_for_type(bitmap_git, OBJ_COMMIT, show_reachable, NULL);\n    + \tif (revs->tree_objects)\n    +-\t\tshow_objects_for_type(bitmap_git, OBJ_TREE, show_reachable);\n    ++\t\tshow_objects_for_type(bitmap_git, OBJ_TREE, show_reachable, NULL);\n    + \tif (revs->blob_objects)\n    +-\t\tshow_objects_for_type(bitmap_git, OBJ_BLOB, show_reachable);\n    ++\t\tshow_objects_for_type(bitmap_git, OBJ_BLOB, show_reachable, NULL);\n    + \tif (revs->tag_objects)\n    +-\t\tshow_objects_for_type(bitmap_git, OBJ_TAG, show_reachable);\n    ++\t\tshow_objects_for_type(bitmap_git, OBJ_TAG, show_reachable, NULL);\n    + \n    + \tshow_extended_objects(bitmap_git, revs, show_reachable);\n    + }\n     \n      ## pack-bitmap.h ##\n     @@ pack-bitmap.h: typedef int (*show_reachable_fn)(\n    @@ pack-bitmap.h: typedef int (*show_reachable_fn)(\n      \n      struct bitmap_index;\n      \n    -@@ pack-bitmap.h: int test_bitmap_pseudo_merges(struct repository *r);\n    - int test_bitmap_pseudo_merge_commits(struct repository *r, uint32_t n);\n    - int test_bitmap_pseudo_merge_objects(struct repository *r, uint32_t n);\n    - \n    -+/*\n    -+ * Iterate through all bitmapped objects of the given type and execute the\n    -+ * `show_reach` for each of them.\n    -+ */\n    -+ void for_each_bitmapped_object(struct bitmap_index *bitmap_git,\n    -+\t\t\t       enum object_type object_type,\n    -+\t\t\t       show_reachable_fn show_reach,\n    -+\t\t\t       void *payload);\n    -+\n    - #define GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL \\\n    - \t\"GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL\"\n    - \n     \n      ## reachable.c ##\n     @@ reachable.c: static int mark_object_seen(const struct object_id *oid,\n -:  ----------- >  7:  9de8dff849c pack-bitmap: add function to iterate over filtered bitmapped objects\n 7:  86a520477f5 !  8:  67a3cdf5fb9 pack-bitmap: introduce function to check whether a pack is bitmapped\n    @@ Commit message\n         bitmapped or not. This functionality will be used in a subsequent\n         commit.\n     \n    +    Helped-by: Taylor Blau <me@ttaylorr.com>\n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n      ## pack-bitmap.c ##\n    @@ pack-bitmap.c: struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_in\n      \n     +int bitmap_index_contains_pack(struct bitmap_index *bitmap, struct packed_git *pack)\n     +{\n    -+\tif (bitmap->pack)\n    -+\t\treturn bitmap->pack == pack;\n    -+\n    -+\tif (!bitmap->midx->chunk_bitmapped_packs)\n    -+\t\treturn 0;\n    -+\n    -+\tfor (size_t i = 0; i < bitmap->midx->num_packs; i++)\n    -+\t\tif (bitmap->midx->packs[i] == pack)\n    ++\tfor (; bitmap; bitmap = bitmap->base) {\n    ++\t\tif (bitmap_is_midx(bitmap)) {\n    ++\t\t\tfor (size_t i = 0; i < bitmap->midx->num_packs; i++)\n    ++\t\t\t\tif (bitmap->midx->packs[i] == pack)\n    ++\t\t\t\t\treturn 1;\n    ++\t\t} else if (bitmap->pack == pack) {\n     +\t\t\treturn 1;\n    ++\t\t}\n    ++\t}\n     +\n     +\treturn 0;\n     +}\n 8:  56168d29a7c =  9:  c4dc2fe1de2 builtin/cat-file: deduplicate logic to iterate over all objects\n 9:  36d88811991 ! 10:  61904db7d35 builtin/cat-file: use bitmaps to efficiently filter by object type\n    @@ Commit message\n         we can use these bitmaps to improve the runtime.\n     \n         While bitmaps are typically used to compute reachability of objects,\n    -    they also contain one bitmap per object type encodes which object has\n    -    what type. So instead of reading through the whole packfile(s), we can\n    -    use the bitmaps and iterate through the type-specific bitmap. Typically,\n    -    only a subset of packfiles will have a bitmap. But this isn't really\n    -    much of a problem: we can use bitmaps when available, and then use the\n    -    non-bitmap walk for every packfile that isn't covered by one.\n    +    they also contain one bitmap per object type that encodes which object\n    +    has what type. So instead of reading through the whole packfile(s), we\n    +    can use the bitmaps and iterate through the type-specific bitmap.\n    +    Typically, only a subset of packfiles will have a bitmap. But this isn't\n    +    really much of a problem: we can use bitmaps when available, and then\n    +    use the non-bitmap walk for every packfile that isn't covered by one.\n     \n         Overall, this leads to quite a significant speedup depending on how many\n         objects of a certain type exist. The following benchmarks have been\n         executed in the Chromium repository, which has a 50GB packfile with\n    -    almost 25 million objects:\n    +    almost 25 million objects. As expected, there isn't really much of a\n    +    change in performance without an object filter:\n     \n    -        Benchmark 1: git cat-file --batch-check --batch-all-objects --unordered --buffer --no-objects-filter\n    -          Time (mean ± σ):     82.806 s ±  6.363 s    [User: 30.956 s, System: 8.264 s]\n    -          Range (min … max):   73.936 s … 89.690 s    10 runs\n    +        Benchmark 1: cat-file with no-filter (revision = HEAD~)\n    +          Time (mean ± σ):     89.675 s ±  4.527 s    [User: 40.807 s, System: 10.782 s]\n    +          Range (min … max):   83.052 s … 96.084 s    10 runs\n     \n    -        Benchmark 2: git cat-file --batch-check --batch-all-objects --unordered --buffer --objects-filter=object:type=tag\n    -          Time (mean ± σ):      20.8 ms ±   1.3 ms    [User: 6.1 ms, System: 14.5 ms]\n    -          Range (min … max):    18.2 ms …  23.6 ms    127 runs\n    +        Benchmark 2: cat-file with no-filter (revision = HEAD)\n    +          Time (mean ± σ):     88.991 s ±  2.488 s    [User: 42.278 s, System: 10.305 s]\n    +          Range (min … max):   82.843 s … 91.271 s    10 runs\n     \n    -        Benchmark 3: git cat-file --batch-check --batch-all-objects --unordered --buffer --objects-filter=object:type=commit\n    -          Time (mean ± σ):      1.551 s ±  0.008 s    [User: 1.401 s, System: 0.147 s]\n    -          Range (min … max):    1.541 s …  1.566 s    10 runs\n    +        Summary\n    +          cat-file with no-filter (revision = HEAD) ran\n    +            1.01 ± 0.06 times faster than cat-file with no-filter (revision = HEAD~)\n    +\n    +    We still have to scan through all objects as we yield all of them, so\n    +    using the bitmap in this case doesn't really buy us anything. What is\n    +    noticeable in this benchmark is that we're I/O-bound, not CPU-bound, as\n    +    can be seen from the user/system runtimes, which combined are way lower\n    +    than the overall benchmarked runtime.\n     \n    -        Benchmark 4: git cat-file --batch-check --batch-all-objects --unordered --buffer --objects-filter=object:type=tree\n    -          Time (mean ± σ):     11.169 s ±  0.046 s    [User: 10.076 s, System: 1.063 s]\n    -          Range (min … max):   11.114 s … 11.245 s    10 runs\n    +    But when we do use a filter we can see a significant improvement:\n     \n    -        Benchmark 5: git cat-file --batch-check --batch-all-objects --unordered --buffer --objects-filter=object:type=blob\n    -          Time (mean ± σ):     67.342 s ±  3.368 s    [User: 20.318 s, System: 7.787 s]\n    -          Range (min … max):   62.836 s … 73.618 s    10 runs\n    +        Benchmark 1: cat-file with filter=object:type=commit (revision = HEAD~)\n    +          Time (mean ± σ):     86.444 s ±  4.081 s    [User: 36.830 s, System: 11.312 s]\n    +          Range (min … max):   80.305 s … 93.104 s    10 runs\n     \n    -        Benchmark 6: git cat-file --batch-check --batch-all-objects --unordered --buffer --objects-filter=blob:none\n    -          Time (mean ± σ):     13.032 s ±  0.072 s    [User: 11.638 s, System: 1.368 s]\n    -          Range (min … max):   12.960 s … 13.199 s    10 runs\n    +        Benchmark 2: cat-file with filter=object:type=commit (revision = HEAD)\n    +          Time (mean ± σ):      2.089 s ±  0.015 s    [User: 1.872 s, System: 0.207 s]\n    +          Range (min … max):    2.073 s …  2.119 s    10 runs\n     \n             Summary\n    -          git cat-file --batch-check --batch-all-objects --unordered --buffer --objects-filter=object:type=tag\n    -           74.75 ± 4.61 times faster than git cat-file --batch-check --batch-all-objects --unordered --buffer --objects-filter=object:type=commit\n    -          538.17 ± 33.17 times faster than git cat-file --batch-check --batch-all-objects --unordered --buffer --objects-filter=object:type=tree\n    -          627.98 ± 38.77 times faster than git cat-file --batch-check --batch-all-objects --unordered --buffer --objects-filter=blob:none\n    -         3244.93 ± 257.23 times faster than git cat-file --batch-check --batch-all-objects --unordered --buffer --objects-filter=object:type=blob\n    -         3990.07 ± 392.72 times faster than git cat-file --batch-check --batch-all-objects --unordered --buffer --no-objects-filter\n    -\n    -    The first benchmark is mostly equivalent in runtime compared to all the\n    -    others without the bitmap-optimization introduced in this commit. What\n    -    is noticeable in the benchmarks is that we're I/O-bound, not CPU-bound,\n    -    as can be seen from the user/system runtimes, which is often way lower\n    -    than the overall benchmarked runtime.\n    +          cat-file with filter=object:type=commit (revision = HEAD) ran\n    +           41.38 ± 1.98 times faster than cat-file with filter=object:type=commit (revision = HEAD~)\n    +\n    +    This is because we don't have to scan through all packfiles anymore, but\n    +    can instead directly look up relevant objects.\n     \n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n    @@ builtin/cat-file.c: static void batch_each_object(for_each_object_fn callback,\n     -\tfor_each_packed_object(the_repository, batch_one_object_packed,\n     -\t\t\t       &payload, flags);\n     +\n    -+\tif (bitmap &&\n    -+\t    (opt->objects_filter.choice == LOFC_OBJECT_TYPE ||\n    -+\t     opt->objects_filter.choice == LOFC_BLOB_NONE)) {\n    ++\tif (bitmap && !for_each_bitmapped_object(bitmap, &opt->objects_filter,\n    ++\t\t\t\t\t\t batch_one_object_bitmapped, &payload)) {\n     +\t\tstruct packed_git *pack;\n     +\n    -+\t\tif (opt->objects_filter.choice == LOFC_OBJECT_TYPE) {\n    -+\t\t\tfor_each_bitmapped_object(bitmap, opt->objects_filter.object_type,\n    -+\t\t\t\t\t\t  batch_one_object_bitmapped, &payload);\n    -+\t\t} else {\n    -+\t\t\tfor_each_bitmapped_object(bitmap, OBJ_COMMIT,\n    -+\t\t\t\t\t\t  batch_one_object_bitmapped, &payload);\n    -+\t\t\tfor_each_bitmapped_object(bitmap, OBJ_TAG,\n    -+\t\t\t\t\t\t  batch_one_object_bitmapped, &payload);\n    -+\t\t\tfor_each_bitmapped_object(bitmap, OBJ_TREE,\n    -+\t\t\t\t\t\t  batch_one_object_bitmapped, &payload);\n    -+\t\t}\n    -+\n     +\t\tfor (pack = get_all_packs(the_repository); pack; pack = pack->next) {\n     +\t\t\tif (bitmap_index_contains_pack(bitmap, pack) ||\n     +\t\t\t    open_pack_index(pack))\n\n---\nbase-commit: 003c5f45b8447877015b2a23ceab2297638fe1f1\nchange-id: 20250220-pks-cat-file-object-type-filter-9140c0ed5ee1\n\n"},{"id":"515134","messageId":"20250327-pks-cat-file-object-type-filter-v2-1-4bbc7085d7c5@pks.im","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-0-4bbc7085d7c5@pks.im","subject":"[PATCH v2 01/10] builtin/cat-file: rename variable that tracks usage","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-27T09:43:56Z","receivedAt":"2025-03-27T09:44:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The usage strings for git-cat-file(1) that we pass to `parse_options()`\nand `usage_msg_optf()` are stored in a variable called `usage`. This\nvariable shadows the declaration of `usage()`, which we'll want to use\nin a subsequent commit.\n\nRename the variable to `builtin_catfile_usage`, which is in line with\nhow the variable is typically called in other builtins.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/cat-file.c | 32 ++++++++++++++++----------------\n 1 file changed, 16 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex b13561cf73b..8e40016dd24 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -941,7 +941,7 @@ int cmd_cat_file(int argc,\n \tint input_nul_terminated = 0;\n \tint nul_terminated = 0;\n \n-\tconst char * const usage[] = {\n+\tconst char * const builtin_catfile_usage[] = {\n \t\tN_(\"git cat-file <type> <object>\"),\n \t\tN_(\"git cat-file (-e | -p) <object>\"),\n \t\tN_(\"git cat-file (-t | -s) [--allow-unknown-type] <object>\"),\n@@ -1007,7 +1007,7 @@ int cmd_cat_file(int argc,\n \n \tbatch.buffer_output = -1;\n \n-\targc = parse_options(argc, argv, prefix, options, usage, 0);\n+\targc = parse_options(argc, argv, prefix, options, builtin_catfile_usage, 0);\n \topt_cw = (opt == 'c' || opt == 'w');\n \topt_epts = (opt == 'e' || opt == 'p' || opt == 't' || opt == 's');\n \n@@ -1021,7 +1021,7 @@ int cmd_cat_file(int argc,\n \t/* Option compatibility */\n \tif (force_path && !opt_cw)\n \t\tusage_msg_optf(_(\"'%s=<%s>' needs '%s' or '%s'\"),\n-\t\t\t       usage, options,\n+\t\t\t       builtin_catfile_usage, options,\n \t\t\t       \"--path\", _(\"path|tree-ish\"), \"--filters\",\n \t\t\t       \"--textconv\");\n \n@@ -1029,19 +1029,19 @@ int cmd_cat_file(int argc,\n \tif (batch.enabled)\n \t\t;\n \telse if (batch.follow_symlinks)\n-\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), usage, options,\n+\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), builtin_catfile_usage, options,\n \t\t\t       \"--follow-symlinks\");\n \telse if (batch.buffer_output >= 0)\n-\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), usage, options,\n+\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), builtin_catfile_usage, options,\n \t\t\t       \"--buffer\");\n \telse if (batch.all_objects)\n-\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), usage, options,\n+\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), builtin_catfile_usage, options,\n \t\t\t       \"--batch-all-objects\");\n \telse if (input_nul_terminated)\n-\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), usage, options,\n+\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), builtin_catfile_usage, options,\n \t\t\t       \"-z\");\n \telse if (nul_terminated)\n-\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), usage, options,\n+\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), builtin_catfile_usage, options,\n \t\t\t       \"-Z\");\n \n \tbatch.input_delim = batch.output_delim = '\\n';\n@@ -1063,9 +1063,9 @@ int cmd_cat_file(int argc,\n \t\t\tbatch.transform_mode = opt;\n \t\telse if (opt && opt != 'b')\n \t\t\tusage_msg_optf(_(\"'-%c' is incompatible with batch mode\"),\n-\t\t\t\t       usage, options, opt);\n+\t\t\t\t       builtin_catfile_usage, options, opt);\n \t\telse if (argc)\n-\t\t\tusage_msg_opt(_(\"batch modes take no arguments\"), usage,\n+\t\t\tusage_msg_opt(_(\"batch modes take no arguments\"), builtin_catfile_usage,\n \t\t\t\t      options);\n \n \t\treturn batch_objects(&batch);\n@@ -1074,22 +1074,22 @@ int cmd_cat_file(int argc,\n \tif (opt) {\n \t\tif (!argc && opt == 'c')\n \t\t\tusage_msg_optf(_(\"<rev> required with '%s'\"),\n-\t\t\t\t       usage, options, \"--textconv\");\n+\t\t\t\t       builtin_catfile_usage, options, \"--textconv\");\n \t\telse if (!argc && opt == 'w')\n \t\t\tusage_msg_optf(_(\"<rev> required with '%s'\"),\n-\t\t\t\t       usage, options, \"--filters\");\n+\t\t\t\t       builtin_catfile_usage, options, \"--filters\");\n \t\telse if (!argc && opt_epts)\n \t\t\tusage_msg_optf(_(\"<object> required with '-%c'\"),\n-\t\t\t\t       usage, options, opt);\n+\t\t\t\t       builtin_catfile_usage, options, opt);\n \t\telse if (argc == 1)\n \t\t\tobj_name = argv[0];\n \t\telse\n-\t\t\tusage_msg_opt(_(\"too many arguments\"), usage, options);\n+\t\t\tusage_msg_opt(_(\"too many arguments\"), builtin_catfile_usage, options);\n \t} else if (!argc) {\n-\t\tusage_with_options(usage, options);\n+\t\tusage_with_options(builtin_catfile_usage, options);\n \t} else if (argc != 2) {\n \t\tusage_msg_optf(_(\"only two arguments allowed in <type> <object> mode, not %d\"),\n-\t\t\t      usage, options, argc);\n+\t\t\t      builtin_catfile_usage, options, argc);\n \t} else if (argc) {\n \t\texp_type = argv[0];\n \t\tobj_name = argv[1];\n\n-- \n2.49.0.472.ge94155a9ec.dirty\n\n"},{"id":"515138","messageId":"20250327-pks-cat-file-object-type-filter-v2-2-4bbc7085d7c5@pks.im","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-0-4bbc7085d7c5@pks.im","subject":"[PATCH v2 02/10] builtin/cat-file: wire up an option to filter objects","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-27T09:43:57Z","receivedAt":"2025-03-27T09:44:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In batch mode, git-cat-file(1) enumerates all objects and prints them\nby iterating through both loose and packed objects. This works without\nconsidering their reachability at all, and consequently most options to\nfilter objects as they exist in e.g. git-rev-list(1) are not applicable.\nIn some situations it may still be useful though to filter objects based\non properties that are inherent to them. This includes the object size\nas well as its type.\n\nSuch a filter already exists in git-rev-list(1) with the `--filter=`\ncommand line option. While this option supports a couple of filters that\nare not applicable to our usecase, some of them are quite a neat fit.\n\nWire up the filter as an option for git-cat-file(1). This allows us to\nreuse the same syntax as in git-rev-list(1) so that we don't have to\nreinvent the wheel. For now, we die when any of the filter options has\nbeen passed by the user, but they will be wired up in subsequent\ncommits.\n\nFurther note that the filters that we are about to introduce don't\nsignificantly speed up the runtime of git-cat-file(1). While we can skip\nemitting a lot of objects in case they are uninteresting to us, the\nmajority of time is spent reading the packfile, which is bottlenecked by\nI/O and not the processor. This will change though once we start to make\nuse of bitmaps, which will allow us to skip reading the whole packfile.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/git-cat-file.adoc |  6 ++++++\n builtin/cat-file.c              | 37 +++++++++++++++++++++++++++++++++----\n t/t1006-cat-file.sh             | 32 ++++++++++++++++++++++++++++++++\n 3 files changed, 71 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc\nindex d5890ae3686..f7f57b7f538 100644\n--- a/Documentation/git-cat-file.adoc\n+++ b/Documentation/git-cat-file.adoc\n@@ -81,6 +81,12 @@ OPTIONS\n \tend-of-line conversion, etc). In this case, `<object>` has to be of\n \tthe form `<tree-ish>:<path>`, or `:<path>`.\n \n+--filter=<filter-spec>::\n+--no-filter::\n+\tOmit objects from the list of printed objects. This can only be used in\n+\tcombination with one of the batched modes. The '<filter-spec>' may be\n+\tone of the following:\n+\n --path=<path>::\n \tFor use with `--textconv` or `--filters`, to allow specifying an object\n \tname and a path separately, e.g. when it is difficult to figure out\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 8e40016dd24..940900d92ad 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -15,6 +15,7 @@\n #include \"gettext.h\"\n #include \"hex.h\"\n #include \"ident.h\"\n+#include \"list-objects-filter-options.h\"\n #include \"parse-options.h\"\n #include \"userdiff.h\"\n #include \"streaming.h\"\n@@ -35,6 +36,7 @@ enum batch_mode {\n };\n \n struct batch_options {\n+\tstruct list_objects_filter_options objects_filter;\n \tint enabled;\n \tint follow_symlinks;\n \tenum batch_mode batch_mode;\n@@ -487,6 +489,13 @@ static void batch_object_write(const char *obj_name,\n \t\t\treturn;\n \t\t}\n \n+\t\tswitch (opt->objects_filter.choice) {\n+\t\tcase LOFC_DISABLED:\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tBUG(\"unsupported objects filter\");\n+\t\t}\n+\n \t\tif (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {\n \t\t\tsize_t s = data->size;\n \t\t\tchar *buf = NULL;\n@@ -812,7 +821,8 @@ static int batch_objects(struct batch_options *opt)\n \t\tstruct object_cb_data cb;\n \t\tstruct object_info empty = OBJECT_INFO_INIT;\n \n-\t\tif (!memcmp(&data.info, &empty, sizeof(empty)))\n+\t\tif (!memcmp(&data.info, &empty, sizeof(empty)) &&\n+\t\t    opt->objects_filter.choice == LOFC_DISABLED)\n \t\t\tdata.skip_object_info = 1;\n \n \t\tif (repo_has_promisor_remote(the_repository))\n@@ -936,10 +946,13 @@ int cmd_cat_file(int argc,\n \tint opt_cw = 0;\n \tint opt_epts = 0;\n \tconst char *exp_type = NULL, *obj_name = NULL;\n-\tstruct batch_options batch = {0};\n+\tstruct batch_options batch = {\n+\t\t.objects_filter = LIST_OBJECTS_FILTER_INIT,\n+\t};\n \tint unknown_type = 0;\n \tint input_nul_terminated = 0;\n \tint nul_terminated = 0;\n+\tint ret;\n \n \tconst char * const builtin_catfile_usage[] = {\n \t\tN_(\"git cat-file <type> <object>\"),\n@@ -1000,6 +1013,8 @@ int cmd_cat_file(int argc,\n \t\t\t    N_(\"run filters on object's content\"), 'w'),\n \t\tOPT_STRING(0, \"path\", &force_path, N_(\"blob|tree\"),\n \t\t\t   N_(\"use a <path> for (--textconv | --filters); Not with 'batch'\")),\n+\t\tOPT_CALLBACK(0, \"filter\", &batch.objects_filter, N_(\"args\"),\n+\t\t\t     N_(\"object filtering\"), opt_parse_list_objects_filter),\n \t\tOPT_END()\n \t};\n \n@@ -1014,6 +1029,14 @@ int cmd_cat_file(int argc,\n \tif (use_mailmap)\n \t\tread_mailmap(&mailmap);\n \n+\tswitch (batch.objects_filter.choice) {\n+\tcase LOFC_DISABLED:\n+\t\tbreak;\n+\tdefault:\n+\t\tusagef(_(\"objects filter not supported: '%s'\"),\n+\t\t       list_object_filter_config_name(batch.objects_filter.choice));\n+\t}\n+\n \t/* --batch-all-objects? */\n \tif (opt == 'b')\n \t\tbatch.all_objects = 1;\n@@ -1068,7 +1091,8 @@ int cmd_cat_file(int argc,\n \t\t\tusage_msg_opt(_(\"batch modes take no arguments\"), builtin_catfile_usage,\n \t\t\t\t      options);\n \n-\t\treturn batch_objects(&batch);\n+\t\tret = batch_objects(&batch);\n+\t\tgoto out;\n \t}\n \n \tif (opt) {\n@@ -1097,5 +1121,10 @@ int cmd_cat_file(int argc,\n \n \tif (unknown_type && opt != 't' && opt != 's')\n \t\tdie(\"git cat-file --allow-unknown-type: use with -s or -t\");\n-\treturn cat_one_file(opt, exp_type, obj_name, unknown_type);\n+\n+\tret = cat_one_file(opt, exp_type, obj_name, unknown_type);\n+\n+out:\n+\tlist_objects_filter_release(&batch.objects_filter);\n+\treturn ret;\n }\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex 398865d6ebe..1246d3119f8 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -1353,4 +1353,36 @@ test_expect_success PERL '--batch-command info is unbuffered by default' '\n \tperl -e \"$script\" -- --batch-command $hello_oid \"$expect\" \"info \"\n '\n \n+test_expect_success 'setup for objects filter' '\n+\tgit init repo\n+'\n+\n+test_expect_success 'objects filter with unknown option' '\n+\tcat >expect <<-EOF &&\n+\tfatal: invalid filter-spec ${SQ}unknown${SQ}\n+\tEOF\n+\ttest_must_fail git -C repo cat-file --filter=unknown 2>err &&\n+\ttest_cmp expect err\n+'\n+\n+for option in blob:none blob:limit=1 object:type=tag sparse:oid=1234 tree:1 sparse:path=x\n+do\n+\ttest_expect_success \"objects filter with unsupported option $option\" '\n+\t\tcase \"$option\" in\n+\t\ttree:1)\n+\t\t\techo \"usage: objects filter not supported: ${SQ}tree${SQ}\" >expect\n+\t\t\t;;\n+\t\tsparse:path=x)\n+\t\t\techo \"fatal: sparse:path filters support has been dropped\" >expect\n+\t\t\t;;\n+\t\t*)\n+\t\t\toption_name=$(echo \"$option\" | cut -d= -f1) &&\n+\t\t\tprintf \"usage: objects filter not supported: ${SQ}%s${SQ}\\n\" \"$option_name\" >expect\n+\t\t\t;;\n+\t\tesac &&\n+\t\ttest_must_fail git -C repo cat-file --filter=$option 2>err &&\n+\t\ttest_cmp expect err\n+\t'\n+done\n+\n test_done\n\n-- \n2.49.0.472.ge94155a9ec.dirty\n\n"},{"id":"515136","messageId":"20250327-pks-cat-file-object-type-filter-v2-4-4bbc7085d7c5@pks.im","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-0-4bbc7085d7c5@pks.im","subject":"[PATCH v2 04/10] builtin/cat-file: support \"blob:limit=\" objects filter","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-27T09:43:59Z","receivedAt":"2025-03-27T09:44:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Implement support for the \"blob:limit=\" filter in git-cat-file(1), which\ncauses us to omit all blobs that are bigger than a certain size.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/git-cat-file.adoc |  5 +++++\n builtin/cat-file.c              | 11 ++++++++++-\n t/t1006-cat-file.sh             | 18 +++++++++++++++---\n 3 files changed, 30 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc\nindex bb32f715944..62bfb00f4b1 100644\n--- a/Documentation/git-cat-file.adoc\n+++ b/Documentation/git-cat-file.adoc\n@@ -88,6 +88,11 @@ OPTIONS\n \tone of the following:\n +\n The form '--filter=blob:none' omits all blobs.\n++\n+The form '--filter=blob:limit=<n>[kmg]' omits blobs of size at least n\n+bytes or units.  n may be zero.  The suffixes k, m, and g can be used to name\n+units in KiB, MiB, or GiB.  For example, 'blob:limit=1k' is the same as\n+'blob:limit=1024'.\n \n --path=<path>::\n \tFor use with `--textconv` or `--filters`, to allow specifying an object\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex e783dbbad58..55755a461bc 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -473,8 +473,11 @@ static void batch_object_write(const char *obj_name,\n \t\tint ret;\n \n \t\tif (use_mailmap ||\n-\t\t    opt->objects_filter.choice == LOFC_BLOB_NONE)\n+\t\t    opt->objects_filter.choice == LOFC_BLOB_NONE ||\n+\t\t    opt->objects_filter.choice == LOFC_BLOB_LIMIT)\n \t\t\tdata->info.typep = &data->type;\n+\t\tif (opt->objects_filter.choice == LOFC_BLOB_LIMIT)\n+\t\t\tdata->info.sizep = &data->size;\n \n \t\tif (pack)\n \t\t\tret = packed_object_info(the_repository, pack, offset,\n@@ -497,6 +500,11 @@ static void batch_object_write(const char *obj_name,\n \t\t\tif (data->type == OBJ_BLOB)\n \t\t\t\treturn;\n \t\t\tbreak;\n+\t\tcase LOFC_BLOB_LIMIT:\n+\t\t\tif (data->type == OBJ_BLOB &&\n+\t\t\t    data->size >= opt->objects_filter.blob_limit_value)\n+\t\t\t\treturn;\n+\t\t\tbreak;\n \t\tdefault:\n \t\t\tBUG(\"unsupported objects filter\");\n \t\t}\n@@ -1038,6 +1046,7 @@ int cmd_cat_file(int argc,\n \tcase LOFC_DISABLED:\n \t\tbreak;\n \tcase LOFC_BLOB_NONE:\n+\tcase LOFC_BLOB_LIMIT:\n \t\tif (!batch.enabled)\n \t\t\tusage(_(\"objects filter only supported in batch mode\"));\n \t\tbreak;\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex d00073f8add..1a0931bd2ca 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -1356,11 +1356,12 @@ test_expect_success PERL '--batch-command info is unbuffered by default' '\n test_expect_success 'setup for objects filter' '\n \tgit init repo &&\n \t(\n-\t\t# Seed the repository with three different sets of objects:\n+\t\t# Seed the repository with four different sets of objects:\n \t\t#\n \t\t#   - The first set is fully packed and has a bitmap.\n \t\t#   - The second set is packed, but has no bitmap.\n \t\t#   - The third set is loose.\n+\t\t#   - The fourth set is loose and contains big objects.\n \t\t#\n \t\t# This ensures that we cover all these types as expected.\n \t\tcd repo &&\n@@ -1368,7 +1369,14 @@ test_expect_success 'setup for objects filter' '\n \t\tgit repack -Adb &&\n \t\ttest_commit second &&\n \t\tgit repack -d &&\n-\t\ttest_commit third\n+\t\ttest_commit third &&\n+\n+\t\tfor n in 1000 10000\n+\t\tdo\n+\t\t\tprintf \"%\"$n\"s\" X >large.$n || return 1\n+\t\tdone &&\n+\t\tgit add large.* &&\n+\t\tgit commit -m fourth\n \t)\n '\n \n@@ -1380,7 +1388,7 @@ test_expect_success 'objects filter with unknown option' '\n \ttest_cmp expect err\n '\n \n-for option in blob:limit=1 object:type=tag sparse:oid=1234 tree:1 sparse:path=x\n+for option in object:type=tag sparse:oid=1234 tree:1 sparse:path=x\n do\n \ttest_expect_success \"objects filter with unsupported option $option\" '\n \t\tcase \"$option\" in\n@@ -1413,5 +1421,9 @@ test_objects_filter () {\n }\n \n test_objects_filter \"blob:none\"\n+test_objects_filter \"blob:limit=1\"\n+test_objects_filter \"blob:limit=500\"\n+test_objects_filter \"blob:limit=1000\"\n+test_objects_filter \"blob:limit=1g\"\n \n test_done\n\n-- \n2.49.0.472.ge94155a9ec.dirty\n\n"},{"id":"515137","messageId":"20250327-pks-cat-file-object-type-filter-v2-3-4bbc7085d7c5@pks.im","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-0-4bbc7085d7c5@pks.im","subject":"[PATCH v2 03/10] builtin/cat-file: support \"blob:none\" objects filter","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-27T09:43:58Z","receivedAt":"2025-03-27T09:44:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Implement support for the \"blob:none\" filter in git-cat-file(1), which\ncauses us to omit all blobs.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/git-cat-file.adoc |  2 ++\n builtin/cat-file.c              | 11 ++++++++++-\n t/t1006-cat-file.sh             | 33 +++++++++++++++++++++++++++++++--\n 3 files changed, 43 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc\nindex f7f57b7f538..bb32f715944 100644\n--- a/Documentation/git-cat-file.adoc\n+++ b/Documentation/git-cat-file.adoc\n@@ -86,6 +86,8 @@ OPTIONS\n \tOmit objects from the list of printed objects. This can only be used in\n \tcombination with one of the batched modes. The '<filter-spec>' may be\n \tone of the following:\n++\n+The form '--filter=blob:none' omits all blobs.\n \n --path=<path>::\n \tFor use with `--textconv` or `--filters`, to allow specifying an object\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 940900d92ad..e783dbbad58 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -472,7 +472,8 @@ static void batch_object_write(const char *obj_name,\n \tif (!data->skip_object_info) {\n \t\tint ret;\n \n-\t\tif (use_mailmap)\n+\t\tif (use_mailmap ||\n+\t\t    opt->objects_filter.choice == LOFC_BLOB_NONE)\n \t\t\tdata->info.typep = &data->type;\n \n \t\tif (pack)\n@@ -492,6 +493,10 @@ static void batch_object_write(const char *obj_name,\n \t\tswitch (opt->objects_filter.choice) {\n \t\tcase LOFC_DISABLED:\n \t\t\tbreak;\n+\t\tcase LOFC_BLOB_NONE:\n+\t\t\tif (data->type == OBJ_BLOB)\n+\t\t\t\treturn;\n+\t\t\tbreak;\n \t\tdefault:\n \t\t\tBUG(\"unsupported objects filter\");\n \t\t}\n@@ -1032,6 +1037,10 @@ int cmd_cat_file(int argc,\n \tswitch (batch.objects_filter.choice) {\n \tcase LOFC_DISABLED:\n \t\tbreak;\n+\tcase LOFC_BLOB_NONE:\n+\t\tif (!batch.enabled)\n+\t\t\tusage(_(\"objects filter only supported in batch mode\"));\n+\t\tbreak;\n \tdefault:\n \t\tusagef(_(\"objects filter not supported: '%s'\"),\n \t\t       list_object_filter_config_name(batch.objects_filter.choice));\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex 1246d3119f8..d00073f8add 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -1354,7 +1354,22 @@ test_expect_success PERL '--batch-command info is unbuffered by default' '\n '\n \n test_expect_success 'setup for objects filter' '\n-\tgit init repo\n+\tgit init repo &&\n+\t(\n+\t\t# Seed the repository with three different sets of objects:\n+\t\t#\n+\t\t#   - The first set is fully packed and has a bitmap.\n+\t\t#   - The second set is packed, but has no bitmap.\n+\t\t#   - The third set is loose.\n+\t\t#\n+\t\t# This ensures that we cover all these types as expected.\n+\t\tcd repo &&\n+\t\ttest_commit first &&\n+\t\tgit repack -Adb &&\n+\t\ttest_commit second &&\n+\t\tgit repack -d &&\n+\t\ttest_commit third\n+\t)\n '\n \n test_expect_success 'objects filter with unknown option' '\n@@ -1365,7 +1380,7 @@ test_expect_success 'objects filter with unknown option' '\n \ttest_cmp expect err\n '\n \n-for option in blob:none blob:limit=1 object:type=tag sparse:oid=1234 tree:1 sparse:path=x\n+for option in blob:limit=1 object:type=tag sparse:oid=1234 tree:1 sparse:path=x\n do\n \ttest_expect_success \"objects filter with unsupported option $option\" '\n \t\tcase \"$option\" in\n@@ -1385,4 +1400,18 @@ do\n \t'\n done\n \n+test_objects_filter () {\n+\tfilter=\"$1\"\n+\n+\ttest_expect_success \"objects filter: $filter\" '\n+\t\tgit -C repo cat-file --batch-check=\"%(objectname)\" --batch-all-objects --filter=\"$filter\" >actual &&\n+\t\tsort actual >actual.sorted &&\n+\t\tgit -C repo rev-list --objects --no-object-names --all --filter=\"$filter\" --filter-provided-objects >expect &&\n+\t\tsort expect >expect.sorted &&\n+\t\ttest_cmp expect.sorted actual.sorted\n+\t'\n+}\n+\n+test_objects_filter \"blob:none\"\n+\n test_done\n\n-- \n2.49.0.472.ge94155a9ec.dirty\n\n"},{"id":"515139","messageId":"20250327-pks-cat-file-object-type-filter-v2-5-4bbc7085d7c5@pks.im","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-0-4bbc7085d7c5@pks.im","subject":"[PATCH v2 05/10] builtin/cat-file: support \"object:type=\" objects filter","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-27T09:44:00Z","receivedAt":"2025-03-27T09:44:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Implement support for the \"object:type=\" filter in git-cat-file(1),\nwhich causes us to omit all objects that don't match the provided object\ntype.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/git-cat-file.adoc | 3 +++\n builtin/cat-file.c              | 8 +++++++-\n t/t1006-cat-file.sh             | 6 +++++-\n 3 files changed, 15 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc\nindex 62bfb00f4b1..9931840567b 100644\n--- a/Documentation/git-cat-file.adoc\n+++ b/Documentation/git-cat-file.adoc\n@@ -93,6 +93,9 @@ The form '--filter=blob:limit=<n>[kmg]' omits blobs of size at least n\n bytes or units.  n may be zero.  The suffixes k, m, and g can be used to name\n units in KiB, MiB, or GiB.  For example, 'blob:limit=1k' is the same as\n 'blob:limit=1024'.\n++\n+The form '--filter=object:type=(tag|commit|tree|blob)' omits all objects which\n+are not of the requested type.\n \n --path=<path>::\n \tFor use with `--textconv` or `--filters`, to allow specifying an object\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 55755a461bc..430320adfe9 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -474,7 +474,8 @@ static void batch_object_write(const char *obj_name,\n \n \t\tif (use_mailmap ||\n \t\t    opt->objects_filter.choice == LOFC_BLOB_NONE ||\n-\t\t    opt->objects_filter.choice == LOFC_BLOB_LIMIT)\n+\t\t    opt->objects_filter.choice == LOFC_BLOB_LIMIT ||\n+\t\t    opt->objects_filter.choice == LOFC_OBJECT_TYPE)\n \t\t\tdata->info.typep = &data->type;\n \t\tif (opt->objects_filter.choice == LOFC_BLOB_LIMIT)\n \t\t\tdata->info.sizep = &data->size;\n@@ -505,6 +506,10 @@ static void batch_object_write(const char *obj_name,\n \t\t\t    data->size >= opt->objects_filter.blob_limit_value)\n \t\t\t\treturn;\n \t\t\tbreak;\n+\t\tcase LOFC_OBJECT_TYPE:\n+\t\t\tif (data->type != opt->objects_filter.object_type)\n+\t\t\t\treturn;\n+\t\t\tbreak;\n \t\tdefault:\n \t\t\tBUG(\"unsupported objects filter\");\n \t\t}\n@@ -1047,6 +1052,7 @@ int cmd_cat_file(int argc,\n \t\tbreak;\n \tcase LOFC_BLOB_NONE:\n \tcase LOFC_BLOB_LIMIT:\n+\tcase LOFC_OBJECT_TYPE:\n \t\tif (!batch.enabled)\n \t\t\tusage(_(\"objects filter only supported in batch mode\"));\n \t\tbreak;\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex 1a0931bd2ca..9edd3d0c048 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -1388,7 +1388,7 @@ test_expect_success 'objects filter with unknown option' '\n \ttest_cmp expect err\n '\n \n-for option in object:type=tag sparse:oid=1234 tree:1 sparse:path=x\n+for option in sparse:oid=1234 tree:1 sparse:path=x\n do\n \ttest_expect_success \"objects filter with unsupported option $option\" '\n \t\tcase \"$option\" in\n@@ -1425,5 +1425,9 @@ test_objects_filter \"blob:limit=1\"\n test_objects_filter \"blob:limit=500\"\n test_objects_filter \"blob:limit=1000\"\n test_objects_filter \"blob:limit=1g\"\n+test_objects_filter \"object:type=blob\"\n+test_objects_filter \"object:type=commit\"\n+test_objects_filter \"object:type=tag\"\n+test_objects_filter \"object:type=tree\"\n \n test_done\n\n-- \n2.49.0.472.ge94155a9ec.dirty\n\n"},{"id":"515140","messageId":"20250327-pks-cat-file-object-type-filter-v2-6-4bbc7085d7c5@pks.im","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-0-4bbc7085d7c5@pks.im","subject":"[PATCH v2 06/10] pack-bitmap: allow passing payloads to `show_reachable_fn()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-27T09:44:01Z","receivedAt":"2025-03-27T09:44:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `show_reachable_fn` callback is used by a couple of functions to\npresent reachable objects to the caller. The function does not provide a\nway for the caller to pass a payload though, which is functionality that\nwe'll require in a subsequent commit.\n\nChange the callback type to accept a payload and adapt all callsites\naccordingly.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/pack-objects.c |  3 ++-\n builtin/rev-list.c     |  3 ++-\n pack-bitmap.c          | 15 ++++++++-------\n pack-bitmap.h          |  3 ++-\n reachable.c            |  3 ++-\n 5 files changed, 16 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex a7e4bb79049..38784613fc0 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1736,7 +1736,8 @@ static int add_object_entry(const struct object_id *oid, enum object_type type,\n static int add_object_entry_from_bitmap(const struct object_id *oid,\n \t\t\t\t\tenum object_type type,\n \t\t\t\t\tint flags UNUSED, uint32_t name_hash,\n-\t\t\t\t\tstruct packed_git *pack, off_t offset)\n+\t\t\t\t\tstruct packed_git *pack, off_t offset,\n+\t\t\t\t\tvoid *payload UNUSED)\n {\n \tdisplay_progress(progress_state, ++nr_seen);\n \ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex bb26bee0d45..1100dd2abe7 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -429,7 +429,8 @@ static int show_object_fast(\n \tint exclude UNUSED,\n \tuint32_t name_hash UNUSED,\n \tstruct packed_git *found_pack UNUSED,\n-\toff_t found_offset UNUSED)\n+\toff_t found_offset UNUSED,\n+\tvoid *payload UNUSED)\n {\n \tfprintf(stdout, \"%s\\n\", oid_to_hex(oid));\n \treturn 1;\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 6f7fd94c36f..d192fb87da9 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -1625,7 +1625,7 @@ static void show_extended_objects(struct bitmap_index *bitmap_git,\n \t\t    (obj->type == OBJ_TAG && !revs->tag_objects))\n \t\t\tcontinue;\n \n-\t\tshow_reach(&obj->oid, obj->type, 0, eindex->hashes[i], NULL, 0);\n+\t\tshow_reach(&obj->oid, obj->type, 0, eindex->hashes[i], NULL, 0, NULL);\n \t}\n }\n \n@@ -1663,7 +1663,8 @@ static void init_type_iterator(struct ewah_or_iterator *it,\n static void show_objects_for_type(\n \tstruct bitmap_index *bitmap_git,\n \tenum object_type object_type,\n-\tshow_reachable_fn show_reach)\n+\tshow_reachable_fn show_reach,\n+\tvoid *payload)\n {\n \tsize_t i = 0;\n \tuint32_t offset;\n@@ -1715,7 +1716,7 @@ static void show_objects_for_type(\n \t\t\tif (bitmap_git->hashes)\n \t\t\t\thash = get_be32(bitmap_git->hashes + index_pos);\n \n-\t\t\tshow_reach(&oid, object_type, 0, hash, pack, ofs);\n+\t\t\tshow_reach(&oid, object_type, 0, hash, pack, ofs, payload);\n \t\t}\n \t}\n \n@@ -2518,13 +2519,13 @@ void traverse_bitmap_commit_list(struct bitmap_index *bitmap_git,\n {\n \tassert(bitmap_git->result);\n \n-\tshow_objects_for_type(bitmap_git, OBJ_COMMIT, show_reachable);\n+\tshow_objects_for_type(bitmap_git, OBJ_COMMIT, show_reachable, NULL);\n \tif (revs->tree_objects)\n-\t\tshow_objects_for_type(bitmap_git, OBJ_TREE, show_reachable);\n+\t\tshow_objects_for_type(bitmap_git, OBJ_TREE, show_reachable, NULL);\n \tif (revs->blob_objects)\n-\t\tshow_objects_for_type(bitmap_git, OBJ_BLOB, show_reachable);\n+\t\tshow_objects_for_type(bitmap_git, OBJ_BLOB, show_reachable, NULL);\n \tif (revs->tag_objects)\n-\t\tshow_objects_for_type(bitmap_git, OBJ_TAG, show_reachable);\n+\t\tshow_objects_for_type(bitmap_git, OBJ_TAG, show_reachable, NULL);\n \n \tshow_extended_objects(bitmap_git, revs, show_reachable);\n }\ndiff --git a/pack-bitmap.h b/pack-bitmap.h\nindex dd0951088f6..de6bf534fef 100644\n--- a/pack-bitmap.h\n+++ b/pack-bitmap.h\n@@ -50,7 +50,8 @@ typedef int (*show_reachable_fn)(\n \tint flags,\n \tuint32_t hash,\n \tstruct packed_git *found_pack,\n-\toff_t found_offset);\n+\toff_t found_offset,\n+\tvoid *payload);\n \n struct bitmap_index;\n \ndiff --git a/reachable.c b/reachable.c\nindex 9ee04c89ec6..421d354d3b5 100644\n--- a/reachable.c\n+++ b/reachable.c\n@@ -341,7 +341,8 @@ static int mark_object_seen(const struct object_id *oid,\n \t\t\t     int exclude UNUSED,\n \t\t\t     uint32_t name_hash UNUSED,\n \t\t\t     struct packed_git *found_pack UNUSED,\n-\t\t\t     off_t found_offset UNUSED)\n+\t\t\t     off_t found_offset UNUSED,\n+\t\t\t     void *payload UNUSED)\n {\n \tstruct object *obj = lookup_object_by_type(the_repository, oid, type);\n \tif (!obj)\n\n-- \n2.49.0.472.ge94155a9ec.dirty\n\n"},{"id":"515141","messageId":"20250327-pks-cat-file-object-type-filter-v2-7-4bbc7085d7c5@pks.im","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-0-4bbc7085d7c5@pks.im","subject":"[PATCH v2 07/10] pack-bitmap: add function to iterate over filtered bitmapped objects","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-27T09:44:02Z","receivedAt":"2025-03-27T09:44:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Introduce a function that allows the caller to iterate over all\nbitmapped objects that match a given filter. This mechanism will be used\nin a subsequent commit to optimize object filters in git-cat-file(1).\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n pack-bitmap.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++------\n pack-bitmap.h | 12 ++++++++++++\n 2 files changed, 65 insertions(+), 6 deletions(-)\n\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex d192fb87da9..6adb8aaa1c2 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -1662,6 +1662,7 @@ static void init_type_iterator(struct ewah_or_iterator *it,\n \n static void show_objects_for_type(\n \tstruct bitmap_index *bitmap_git,\n+\tstruct bitmap *objects,\n \tenum object_type object_type,\n \tshow_reachable_fn show_reach,\n \tvoid *payload)\n@@ -1672,8 +1673,6 @@ static void show_objects_for_type(\n \tstruct ewah_or_iterator it;\n \teword_t filter;\n \n-\tstruct bitmap *objects = bitmap_git->result;\n-\n \tinit_type_iterator(&it, bitmap_git, object_type);\n \n \tfor (i = 0; i < objects->word_alloc &&\n@@ -2025,6 +2024,50 @@ static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,\n \t}\n }\n \n+int for_each_bitmapped_object(struct bitmap_index *bitmap_git,\n+\t\t\t      struct list_objects_filter_options *filter,\n+\t\t\t      show_reachable_fn show_reach,\n+\t\t\t      void *payload)\n+{\n+\tstruct bitmap *filtered_bitmap = NULL;\n+\tuint32_t objects_nr;\n+\tsize_t full_word_count;\n+\tint ret;\n+\n+\tif (!can_filter_bitmap(filter)) {\n+\t\tret = -1;\n+\t\tgoto out;\n+\t}\n+\n+\tobjects_nr = bitmap_num_objects(bitmap_git);\n+\tfull_word_count = objects_nr / BITS_IN_EWORD;\n+\n+\t/* We start from the all-1 bitmap and then filter down from there. */\n+\tfiltered_bitmap = bitmap_word_alloc(full_word_count + !!(objects_nr % BITS_IN_EWORD));\n+\tmemset(filtered_bitmap->words, 0xff, full_word_count * sizeof(*filtered_bitmap->words));\n+\tfor (size_t i = full_word_count * BITS_IN_EWORD; i < objects_nr; i++)\n+\t\tbitmap_set(filtered_bitmap, i);\n+\n+\tif (filter_bitmap(bitmap_git, NULL, filtered_bitmap, filter) < 0) {\n+\t\tret = -1;\n+\t\tgoto out;\n+\t}\n+\n+\tshow_objects_for_type(bitmap_git, filtered_bitmap,\n+\t\t\t      OBJ_COMMIT, show_reach, payload);\n+\tshow_objects_for_type(bitmap_git, filtered_bitmap,\n+\t\t\t      OBJ_TREE, show_reach, payload);\n+\tshow_objects_for_type(bitmap_git, filtered_bitmap,\n+\t\t\t      OBJ_BLOB, show_reach, payload);\n+\tshow_objects_for_type(bitmap_git, filtered_bitmap,\n+\t\t\t      OBJ_TAG, show_reach, payload);\n+\n+\tret = 0;\n+out:\n+\tbitmap_free(filtered_bitmap);\n+\treturn ret;\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@@ -2519,13 +2562,17 @@ void traverse_bitmap_commit_list(struct bitmap_index *bitmap_git,\n {\n \tassert(bitmap_git->result);\n \n-\tshow_objects_for_type(bitmap_git, OBJ_COMMIT, show_reachable, NULL);\n+\tshow_objects_for_type(bitmap_git, bitmap_git->result,\n+\t\t\t      OBJ_COMMIT, show_reachable, NULL);\n \tif (revs->tree_objects)\n-\t\tshow_objects_for_type(bitmap_git, OBJ_TREE, show_reachable, NULL);\n+\t\tshow_objects_for_type(bitmap_git, bitmap_git->result,\n+\t\t\t\t      OBJ_TREE, show_reachable, NULL);\n \tif (revs->blob_objects)\n-\t\tshow_objects_for_type(bitmap_git, OBJ_BLOB, show_reachable, NULL);\n+\t\tshow_objects_for_type(bitmap_git, bitmap_git->result,\n+\t\t\t\t      OBJ_BLOB, show_reachable, NULL);\n \tif (revs->tag_objects)\n-\t\tshow_objects_for_type(bitmap_git, OBJ_TAG, show_reachable, NULL);\n+\t\tshow_objects_for_type(bitmap_git, bitmap_git->result,\n+\t\t\t\t      OBJ_TAG, show_reachable, NULL);\n \n \tshow_extended_objects(bitmap_git, revs, show_reachable);\n }\ndiff --git a/pack-bitmap.h b/pack-bitmap.h\nindex de6bf534fef..079bae32466 100644\n--- a/pack-bitmap.h\n+++ b/pack-bitmap.h\n@@ -79,6 +79,18 @@ int test_bitmap_pseudo_merges(struct repository *r);\n int test_bitmap_pseudo_merge_commits(struct repository *r, uint32_t n);\n int test_bitmap_pseudo_merge_objects(struct repository *r, uint32_t n);\n \n+struct list_objects_filter_options;\n+\n+/*\n+ * Filter bitmapped objects and iterate through all resulting objects,\n+ * executing `show_reach` for each of them. Returns `-1` in case the filter is\n+ * not supported, `0` otherwise.\n+ */\n+int for_each_bitmapped_object(struct bitmap_index *bitmap_git,\n+\t\t\t      struct list_objects_filter_options *filter,\n+\t\t\t      show_reachable_fn show_reach,\n+\t\t\t      void *payload);\n+\n #define GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL \\\n \t\"GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL\"\n \n\n-- \n2.49.0.472.ge94155a9ec.dirty\n\n"},{"id":"515142","messageId":"20250327-pks-cat-file-object-type-filter-v2-8-4bbc7085d7c5@pks.im","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-0-4bbc7085d7c5@pks.im","subject":"[PATCH v2 08/10] pack-bitmap: introduce function to check whether a pack is bitmapped","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-27T09:44:03Z","receivedAt":"2025-03-27T09:44:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Introduce a function that allows us to verify whether a pack is\nbitmapped or not. This functionality will be used in a subsequent\ncommit.\n\nHelped-by: Taylor Blau <me@ttaylorr.com>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n pack-bitmap.c | 15 +++++++++++++++\n pack-bitmap.h |  7 +++++++\n 2 files changed, 22 insertions(+)\n\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 6adb8aaa1c2..edc8f42122d 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -745,6 +745,21 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)\n \treturn NULL;\n }\n \n+int bitmap_index_contains_pack(struct bitmap_index *bitmap, struct packed_git *pack)\n+{\n+\tfor (; bitmap; bitmap = bitmap->base) {\n+\t\tif (bitmap_is_midx(bitmap)) {\n+\t\t\tfor (size_t i = 0; i < bitmap->midx->num_packs; i++)\n+\t\t\t\tif (bitmap->midx->packs[i] == pack)\n+\t\t\t\t\treturn 1;\n+\t\t} else if (bitmap->pack == pack) {\n+\t\t\treturn 1;\n+\t\t}\n+\t}\n+\n+\treturn 0;\n+}\n+\n struct include_data {\n \tstruct bitmap_index *bitmap_git;\n \tstruct bitmap *base;\ndiff --git a/pack-bitmap.h b/pack-bitmap.h\nindex 079bae32466..55df1b3af5a 100644\n--- a/pack-bitmap.h\n+++ b/pack-bitmap.h\n@@ -67,6 +67,13 @@ struct bitmapped_pack {\n \n struct bitmap_index *prepare_bitmap_git(struct repository *r);\n struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx);\n+\n+/*\n+ * Given a bitmap index, determine whether it contains the pack either directly\n+ * or via the multi-pack-index.\n+ */\n+int bitmap_index_contains_pack(struct bitmap_index *bitmap, struct packed_git *pack);\n+\n void count_bitmap_commit_list(struct bitmap_index *, uint32_t *commits,\n \t\t\t      uint32_t *trees, uint32_t *blobs, uint32_t *tags);\n void traverse_bitmap_commit_list(struct bitmap_index *,\n\n-- \n2.49.0.472.ge94155a9ec.dirty\n\n"},{"id":"515143","messageId":"20250327-pks-cat-file-object-type-filter-v2-10-4bbc7085d7c5@pks.im","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-0-4bbc7085d7c5@pks.im","subject":"[PATCH v2 10/10] builtin/cat-file: use bitmaps to efficiently filter by object type","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-27T09:44:05Z","receivedAt":"2025-03-27T09:44:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While it is now possible to filter objects by type, this mechanism is\nfor now mostly a convenience. Most importantly, we still have to iterate\nthrough the whole packfile to find all objects of a specific type. This\ncan be prohibitively expensive depending on the size of the packfiles.\n\nIt isn't really possible to do better than this when only considering a\npackfile itself, as the order of objects is not fixed. But when we have\na packfile with a corresponding bitmap, either because the packfile\nitself has one or because the multi-pack index has a bitmap for it, then\nwe can use these bitmaps to improve the runtime.\n\nWhile bitmaps are typically used to compute reachability of objects,\nthey also contain one bitmap per object type that encodes which object\nhas what type. So instead of reading through the whole packfile(s), we\ncan use the bitmaps and iterate through the type-specific bitmap.\nTypically, only a subset of packfiles will have a bitmap. But this isn't\nreally much of a problem: we can use bitmaps when available, and then\nuse the non-bitmap walk for every packfile that isn't covered by one.\n\nOverall, this leads to quite a significant speedup depending on how many\nobjects of a certain type exist. The following benchmarks have been\nexecuted in the Chromium repository, which has a 50GB packfile with\nalmost 25 million objects. As expected, there isn't really much of a\nchange in performance without an object filter:\n\n    Benchmark 1: cat-file with no-filter (revision = HEAD~)\n      Time (mean ± σ):     89.675 s ±  4.527 s    [User: 40.807 s, System: 10.782 s]\n      Range (min … max):   83.052 s … 96.084 s    10 runs\n\n    Benchmark 2: cat-file with no-filter (revision = HEAD)\n      Time (mean ± σ):     88.991 s ±  2.488 s    [User: 42.278 s, System: 10.305 s]\n      Range (min … max):   82.843 s … 91.271 s    10 runs\n\n    Summary\n      cat-file with no-filter (revision = HEAD) ran\n        1.01 ± 0.06 times faster than cat-file with no-filter (revision = HEAD~)\n\nWe still have to scan through all objects as we yield all of them, so\nusing the bitmap in this case doesn't really buy us anything. What is\nnoticeable in this benchmark is that we're I/O-bound, not CPU-bound, as\ncan be seen from the user/system runtimes, which combined are way lower\nthan the overall benchmarked runtime.\n\nBut when we do use a filter we can see a significant improvement:\n\n    Benchmark 1: cat-file with filter=object:type=commit (revision = HEAD~)\n      Time (mean ± σ):     86.444 s ±  4.081 s    [User: 36.830 s, System: 11.312 s]\n      Range (min … max):   80.305 s … 93.104 s    10 runs\n\n    Benchmark 2: cat-file with filter=object:type=commit (revision = HEAD)\n      Time (mean ± σ):      2.089 s ±  0.015 s    [User: 1.872 s, System: 0.207 s]\n      Range (min … max):    2.073 s …  2.119 s    10 runs\n\n    Summary\n      cat-file with filter=object:type=commit (revision = HEAD) ran\n       41.38 ± 1.98 times faster than cat-file with filter=object:type=commit (revision = HEAD~)\n\nThis is because we don't have to scan through all packfiles anymore, but\ncan instead directly look up relevant objects.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/cat-file.c | 42 +++++++++++++++++++++++++++++++++++++-----\n 1 file changed, 37 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 6f5dbc821a2..eb6f0536c9e 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -21,6 +21,7 @@\n #include \"streaming.h\"\n #include \"oid-array.h\"\n #include \"packfile.h\"\n+#include \"pack-bitmap.h\"\n #include \"object-file.h\"\n #include \"object-name.h\"\n #include \"object-store-ll.h\"\n@@ -805,7 +806,20 @@ static int batch_one_object_packed(const struct object_id *oid,\n \t\t\t\t payload->payload);\n }\n \n-static void batch_each_object(for_each_object_fn callback,\n+static int batch_one_object_bitmapped(const struct object_id *oid,\n+\t\t\t\t      enum object_type type UNUSED,\n+\t\t\t\t      int flags UNUSED,\n+\t\t\t\t      uint32_t hash UNUSED,\n+\t\t\t\t      struct packed_git *pack,\n+\t\t\t\t      off_t offset,\n+\t\t\t\t      void *_payload)\n+{\n+\tstruct for_each_object_payload *payload = _payload;\n+\treturn payload->callback(oid, pack, offset, payload->payload);\n+}\n+\n+static void batch_each_object(struct batch_options *opt,\n+\t\t\t      for_each_object_fn callback,\n \t\t\t      unsigned flags,\n \t\t\t      void *_payload)\n {\n@@ -813,9 +827,27 @@ static void batch_each_object(for_each_object_fn callback,\n \t\t.callback = callback,\n \t\t.payload = _payload,\n \t};\n+\tstruct bitmap_index *bitmap = prepare_bitmap_git(the_repository);\n+\n \tfor_each_loose_object(batch_one_object_loose, &payload, 0);\n-\tfor_each_packed_object(the_repository, batch_one_object_packed,\n-\t\t\t       &payload, flags);\n+\n+\tif (bitmap && !for_each_bitmapped_object(bitmap, &opt->objects_filter,\n+\t\t\t\t\t\t batch_one_object_bitmapped, &payload)) {\n+\t\tstruct packed_git *pack;\n+\n+\t\tfor (pack = get_all_packs(the_repository); pack; pack = pack->next) {\n+\t\t\tif (bitmap_index_contains_pack(bitmap, pack) ||\n+\t\t\t    open_pack_index(pack))\n+\t\t\t\tcontinue;\n+\t\t\tfor_each_object_in_pack(pack, batch_one_object_packed,\n+\t\t\t\t\t\t&payload, flags);\n+\t\t}\n+\t} else {\n+\t\tfor_each_packed_object(the_repository, batch_one_object_packed,\n+\t\t\t\t       &payload, flags);\n+\t}\n+\n+\tfree_bitmap_index(bitmap);\n }\n \n static int batch_objects(struct batch_options *opt)\n@@ -872,14 +904,14 @@ static int batch_objects(struct batch_options *opt)\n \n \t\t\tcb.seen = &seen;\n \n-\t\t\tbatch_each_object(batch_unordered_object,\n+\t\t\tbatch_each_object(opt, batch_unordered_object,\n \t\t\t\t\t  FOR_EACH_OBJECT_PACK_ORDER, &cb);\n \n \t\t\toidset_clear(&seen);\n \t\t} else {\n \t\t\tstruct oid_array sa = OID_ARRAY_INIT;\n \n-\t\t\tbatch_each_object(collect_object, 0, &sa);\n+\t\t\tbatch_each_object(opt, collect_object, 0, &sa);\n \t\t\toid_array_for_each_unique(&sa, batch_object_cb, &cb);\n \n \t\t\toid_array_clear(&sa);\n\n-- \n2.49.0.472.ge94155a9ec.dirty\n\n"},{"id":"515144","messageId":"20250327-pks-cat-file-object-type-filter-v2-9-4bbc7085d7c5@pks.im","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-0-4bbc7085d7c5@pks.im","subject":"[PATCH v2 09/10] builtin/cat-file: deduplicate logic to iterate over all objects","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-27T09:44:04Z","receivedAt":"2025-03-27T09:44:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Pull out a common function that allows us to iterate over all objects in\na repository. Right now the logic is trivial and would only require two\nfunction calls, making this refactoring a bit pointless. But in the next\ncommit we will iterate on this logic to make use of bitmaps, so this is\nabout to become a bit more complex.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/cat-file.c | 85 ++++++++++++++++++++++++++++++------------------------\n 1 file changed, 48 insertions(+), 37 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 430320adfe9..6f5dbc821a2 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -622,25 +622,18 @@ static int batch_object_cb(const struct object_id *oid, void *vdata)\n \treturn 0;\n }\n \n-static int collect_loose_object(const struct object_id *oid,\n-\t\t\t\tconst char *path UNUSED,\n-\t\t\t\tvoid *data)\n-{\n-\toid_array_append(data, oid);\n-\treturn 0;\n-}\n-\n-static int collect_packed_object(const struct object_id *oid,\n-\t\t\t\t struct packed_git *pack UNUSED,\n-\t\t\t\t uint32_t pos UNUSED,\n-\t\t\t\t void *data)\n+static int collect_object(const struct object_id *oid,\n+\t\t\t  struct packed_git *pack UNUSED,\n+\t\t\t  off_t offset UNUSED,\n+\t\t\t  void *data)\n {\n \toid_array_append(data, oid);\n \treturn 0;\n }\n \n static int batch_unordered_object(const struct object_id *oid,\n-\t\t\t\t  struct packed_git *pack, off_t offset,\n+\t\t\t\t  struct packed_git *pack,\n+\t\t\t\t  off_t offset,\n \t\t\t\t  void *vdata)\n {\n \tstruct object_cb_data *data = vdata;\n@@ -654,23 +647,6 @@ static int batch_unordered_object(const struct object_id *oid,\n \treturn 0;\n }\n \n-static int batch_unordered_loose(const struct object_id *oid,\n-\t\t\t\t const char *path UNUSED,\n-\t\t\t\t void *data)\n-{\n-\treturn batch_unordered_object(oid, NULL, 0, data);\n-}\n-\n-static int batch_unordered_packed(const struct object_id *oid,\n-\t\t\t\t  struct packed_git *pack,\n-\t\t\t\t  uint32_t pos,\n-\t\t\t\t  void *data)\n-{\n-\treturn batch_unordered_object(oid, pack,\n-\t\t\t\t      nth_packed_object_offset(pack, pos),\n-\t\t\t\t      data);\n-}\n-\n typedef void (*parse_cmd_fn_t)(struct batch_options *, const char *,\n \t\t\t       struct strbuf *, struct expand_data *);\n \n@@ -803,6 +779,45 @@ static void batch_objects_command(struct batch_options *opt,\n \n #define DEFAULT_FORMAT \"%(objectname) %(objecttype) %(objectsize)\"\n \n+typedef int (*for_each_object_fn)(const struct object_id *oid, struct packed_git *pack,\n+\t\t\t\t  off_t offset, void *data);\n+\n+struct for_each_object_payload {\n+\tfor_each_object_fn callback;\n+\tvoid *payload;\n+};\n+\n+static int batch_one_object_loose(const struct object_id *oid,\n+\t\t\t\t  const char *path UNUSED,\n+\t\t\t\t  void *_payload)\n+{\n+\tstruct for_each_object_payload *payload = _payload;\n+\treturn payload->callback(oid, NULL, 0, payload->payload);\n+}\n+\n+static int batch_one_object_packed(const struct object_id *oid,\n+\t\t\t\t   struct packed_git *pack,\n+\t\t\t\t   uint32_t pos,\n+\t\t\t\t   void *_payload)\n+{\n+\tstruct for_each_object_payload *payload = _payload;\n+\treturn payload->callback(oid, pack, nth_packed_object_offset(pack, pos),\n+\t\t\t\t payload->payload);\n+}\n+\n+static void batch_each_object(for_each_object_fn callback,\n+\t\t\t      unsigned flags,\n+\t\t\t      void *_payload)\n+{\n+\tstruct for_each_object_payload payload = {\n+\t\t.callback = callback,\n+\t\t.payload = _payload,\n+\t};\n+\tfor_each_loose_object(batch_one_object_loose, &payload, 0);\n+\tfor_each_packed_object(the_repository, batch_one_object_packed,\n+\t\t\t       &payload, flags);\n+}\n+\n static int batch_objects(struct batch_options *opt)\n {\n \tstruct strbuf input = STRBUF_INIT;\n@@ -857,18 +872,14 @@ static int batch_objects(struct batch_options *opt)\n \n \t\t\tcb.seen = &seen;\n \n-\t\t\tfor_each_loose_object(batch_unordered_loose, &cb, 0);\n-\t\t\tfor_each_packed_object(the_repository, batch_unordered_packed,\n-\t\t\t\t\t       &cb, FOR_EACH_OBJECT_PACK_ORDER);\n+\t\t\tbatch_each_object(batch_unordered_object,\n+\t\t\t\t\t  FOR_EACH_OBJECT_PACK_ORDER, &cb);\n \n \t\t\toidset_clear(&seen);\n \t\t} else {\n \t\t\tstruct oid_array sa = OID_ARRAY_INIT;\n \n-\t\t\tfor_each_loose_object(collect_loose_object, &sa, 0);\n-\t\t\tfor_each_packed_object(the_repository, collect_packed_object,\n-\t\t\t\t\t       &sa, 0);\n-\n+\t\t\tbatch_each_object(collect_object, 0, &sa);\n \t\t\toid_array_for_each_unique(&sa, batch_object_cb, &cb);\n \n \t\t\toid_array_clear(&sa);\n\n-- \n2.49.0.472.ge94155a9ec.dirty\n\n"},{"id":"515418","messageId":"CAOLa=ZTukdPiciDSyH1oGwYN_bdCw+vYjsHaV9AwHEpxJ5hBgQ@mail.gmail.com","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-1-4bbc7085d7c5@pks.im","subject":"Re: [PATCH v2 01/10] builtin/cat-file: rename variable that tracks usage","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-04-01T09:51:01Z","receivedAt":"2025-04-01T09:51:03Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> The usage strings for git-cat-file(1) that we pass to `parse_options()`\n> and `usage_msg_optf()` are stored in a variable called `usage`. This\n> variable shadows the declaration of `usage()`, which we'll want to use\n> in a subsequent commit.\n>\n> Rename the variable to `builtin_catfile_usage`, which is in line with\n> how the variable is typically called in other builtins.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  builtin/cat-file.c | 32 ++++++++++++++++----------------\n>  1 file changed, 16 insertions(+), 16 deletions(-)\n>\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index b13561cf73b..8e40016dd24 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -941,7 +941,7 @@ int cmd_cat_file(int argc,\n>  \tint input_nul_terminated = 0;\n>  \tint nul_terminated = 0;\n>\n> -\tconst char * const usage[] = {\n> +\tconst char * const builtin_catfile_usage[] = {\n\nNit: Style: we use a right pointer alignment, while it is not part of\nyour code change, would be nice to fix.\n\n>  \t\tN_(\"git cat-file <type> <object>\"),\n>  \t\tN_(\"git cat-file (-e | -p) <object>\"),\n>  \t\tN_(\"git cat-file (-t | -s) [--allow-unknown-type] <object>\"),\n> @@ -1007,7 +1007,7 @@ int cmd_cat_file(int argc,\n>\n>  \tbatch.buffer_output = -1;\n>\n> -\targc = parse_options(argc, argv, prefix, options, usage, 0);\n> +\targc = parse_options(argc, argv, prefix, options, builtin_catfile_usage, 0);\n>  \topt_cw = (opt == 'c' || opt == 'w');\n>  \topt_epts = (opt == 'e' || opt == 'p' || opt == 't' || opt == 's');\n>\n> @@ -1021,7 +1021,7 @@ int cmd_cat_file(int argc,\n>  \t/* Option compatibility */\n>  \tif (force_path && !opt_cw)\n>  \t\tusage_msg_optf(_(\"'%s=<%s>' needs '%s' or '%s'\"),\n> -\t\t\t       usage, options,\n> +\t\t\t       builtin_catfile_usage, options,\n>  \t\t\t       \"--path\", _(\"path|tree-ish\"), \"--filters\",\n>  \t\t\t       \"--textconv\");\n>\n> @@ -1029,19 +1029,19 @@ int cmd_cat_file(int argc,\n>  \tif (batch.enabled)\n>  \t\t;\n>  \telse if (batch.follow_symlinks)\n> -\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), usage, options,\n> +\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), builtin_catfile_usage, options,\n>  \t\t\t       \"--follow-symlinks\");\n>  \telse if (batch.buffer_output >= 0)\n> -\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), usage, options,\n> +\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), builtin_catfile_usage, options,\n>  \t\t\t       \"--buffer\");\n>  \telse if (batch.all_objects)\n> -\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), usage, options,\n> +\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), builtin_catfile_usage, options,\n>  \t\t\t       \"--batch-all-objects\");\n>  \telse if (input_nul_terminated)\n> -\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), usage, options,\n> +\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), builtin_catfile_usage, options,\n>  \t\t\t       \"-z\");\n>  \telse if (nul_terminated)\n> -\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), usage, options,\n> +\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), builtin_catfile_usage, options,\n>  \t\t\t       \"-Z\");\n>\n>  \tbatch.input_delim = batch.output_delim = '\\n';\n> @@ -1063,9 +1063,9 @@ int cmd_cat_file(int argc,\n>  \t\t\tbatch.transform_mode = opt;\n>  \t\telse if (opt && opt != 'b')\n>  \t\t\tusage_msg_optf(_(\"'-%c' is incompatible with batch mode\"),\n> -\t\t\t\t       usage, options, opt);\n> +\t\t\t\t       builtin_catfile_usage, options, opt);\n>  \t\telse if (argc)\n> -\t\t\tusage_msg_opt(_(\"batch modes take no arguments\"), usage,\n> +\t\t\tusage_msg_opt(_(\"batch modes take no arguments\"), builtin_catfile_usage,\n>  \t\t\t\t      options);\n>\n>  \t\treturn batch_objects(&batch);\n> @@ -1074,22 +1074,22 @@ int cmd_cat_file(int argc,\n>  \tif (opt) {\n>  \t\tif (!argc && opt == 'c')\n>  \t\t\tusage_msg_optf(_(\"<rev> required with '%s'\"),\n> -\t\t\t\t       usage, options, \"--textconv\");\n> +\t\t\t\t       builtin_catfile_usage, options, \"--textconv\");\n>  \t\telse if (!argc && opt == 'w')\n>  \t\t\tusage_msg_optf(_(\"<rev> required with '%s'\"),\n> -\t\t\t\t       usage, options, \"--filters\");\n> +\t\t\t\t       builtin_catfile_usage, options, \"--filters\");\n>  \t\telse if (!argc && opt_epts)\n>  \t\t\tusage_msg_optf(_(\"<object> required with '-%c'\"),\n> -\t\t\t\t       usage, options, opt);\n> +\t\t\t\t       builtin_catfile_usage, options, opt);\n>  \t\telse if (argc == 1)\n>  \t\t\tobj_name = argv[0];\n>  \t\telse\n> -\t\t\tusage_msg_opt(_(\"too many arguments\"), usage, options);\n> +\t\t\tusage_msg_opt(_(\"too many arguments\"), builtin_catfile_usage, options);\n>  \t} else if (!argc) {\n> -\t\tusage_with_options(usage, options);\n> +\t\tusage_with_options(builtin_catfile_usage, options);\n>  \t} else if (argc != 2) {\n>  \t\tusage_msg_optf(_(\"only two arguments allowed in <type> <object> mode, not %d\"),\n> -\t\t\t      usage, options, argc);\n> +\t\t\t      builtin_catfile_usage, options, argc);\n>  \t} else if (argc) {\n>  \t\texp_type = argv[0];\n>  \t\tobj_name = argv[1];\n>\n> --\n> 2.49.0.472.ge94155a9ec.dirty\n\nNit: Some of these lines could potentially be wrapped. But I think are\nwrapping rules are a bit too strict. So I'd let it be as is.\n\nThe changes look good.\n"},{"id":"515424","messageId":"87r02cf6l1.fsf@iotcl.com","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-2-4bbc7085d7c5@pks.im","subject":"Re: [PATCH v2 02/10] builtin/cat-file: wire up an option to filter objects","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-04-01T11:45:46Z","receivedAt":"2025-04-01T11:46:07Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> In batch mode, git-cat-file(1) enumerates all objects and prints them\n> by iterating through both loose and packed objects. This works without\n> considering their reachability at all, and consequently most options to\n> filter objects as they exist in e.g. git-rev-list(1) are not applicable.\n> In some situations it may still be useful though to filter objects based\n> on properties that are inherent to them. This includes the object size\n> as well as its type.\n>\n> Such a filter already exists in git-rev-list(1) with the `--filter=`\n> command line option. While this option supports a couple of filters that\n> are not applicable to our usecase, some of them are quite a neat fit.\n>\n> Wire up the filter as an option for git-cat-file(1). This allows us to\n> reuse the same syntax as in git-rev-list(1) so that we don't have to\n> reinvent the wheel. For now, we die when any of the filter options has\n> been passed by the user, but they will be wired up in subsequent\n> commits.\n>\n> Further note that the filters that we are about to introduce don't\n> significantly speed up the runtime of git-cat-file(1). While we can skip\n> emitting a lot of objects in case they are uninteresting to us, the\n> majority of time is spent reading the packfile, which is bottlenecked by\n> I/O and not the processor. This will change though once we start to make\n> use of bitmaps, which will allow us to skip reading the whole packfile.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  Documentation/git-cat-file.adoc |  6 ++++++\n>  builtin/cat-file.c              | 37 +++++++++++++++++++++++++++++++++----\n>  t/t1006-cat-file.sh             | 32 ++++++++++++++++++++++++++++++++\n>  3 files changed, 71 insertions(+), 4 deletions(-)\n>\n> diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc\n> index d5890ae3686..f7f57b7f538 100644\n> --- a/Documentation/git-cat-file.adoc\n> +++ b/Documentation/git-cat-file.adoc\n> @@ -81,6 +81,12 @@ OPTIONS\n>  \tend-of-line conversion, etc). In this case, `<object>` has to be of\n>  \tthe form `<tree-ish>:<path>`, or `:<path>`.\n>  \n> +--filter=<filter-spec>::\n> +--no-filter::\n> +\tOmit objects from the list of printed objects. This can only be used in\n> +\tcombination with one of the batched modes. The '<filter-spec>' may be\n> +\tone of the following:\n> +\n>  --path=<path>::\n>  \tFor use with `--textconv` or `--filters`, to allow specifying an object\n>  \tname and a path separately, e.g. when it is difficult to figure out\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index 8e40016dd24..940900d92ad 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -936,10 +946,13 @@ int cmd_cat_file(int argc,\n>  \tint opt_cw = 0;\n>  \tint opt_epts = 0;\n>  \tconst char *exp_type = NULL, *obj_name = NULL;\n> -\tstruct batch_options batch = {0};\n> +\tstruct batch_options batch = {\n> +\t\t.objects_filter = LIST_OBJECTS_FILTER_INIT,\n> +\t};\n>  \tint unknown_type = 0;\n>  \tint input_nul_terminated = 0;\n>  \tint nul_terminated = 0;\n> +\tint ret;\n>  \n>  \tconst char * const builtin_catfile_usage[] = {\n>  \t\tN_(\"git cat-file <type> <object>\"),\n> @@ -1000,6 +1013,8 @@ int cmd_cat_file(int argc,\n>  \t\t\t    N_(\"run filters on object's content\"), 'w'),\n>  \t\tOPT_STRING(0, \"path\", &force_path, N_(\"blob|tree\"),\n>  \t\t\t   N_(\"use a <path> for (--textconv | --filters); Not with 'batch'\")),\n> +\t\tOPT_CALLBACK(0, \"filter\", &batch.objects_filter, N_(\"args\"),\n> +\t\t\t     N_(\"object filtering\"), opt_parse_list_objects_filter),\n\nBecause we've decided on `--filter` we can use\n`OPT_PARSE_LIST_OBJECTS_FILTER` here now.\n\n--\nToon\n"},{"id":"515425","messageId":"87plhwf6ke.fsf@iotcl.com","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-8-4bbc7085d7c5@pks.im","subject":"Re: [PATCH v2 08/10] pack-bitmap: introduce function to check whether a pack is bitmapped","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-04-01T11:46:09Z","receivedAt":"2025-04-01T11:46:20Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Introduce a function that allows us to verify whether a pack is\n> bitmapped or not. This functionality will be used in a subsequent\n> commit.\n>\n> Helped-by: Taylor Blau <me@ttaylorr.com>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  pack-bitmap.c | 15 +++++++++++++++\n>  pack-bitmap.h |  7 +++++++\n>  2 files changed, 22 insertions(+)\n>\n> diff --git a/pack-bitmap.c b/pack-bitmap.c\n> index 6adb8aaa1c2..edc8f42122d 100644\n> --- a/pack-bitmap.c\n> +++ b/pack-bitmap.c\n> @@ -745,6 +745,21 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)\n>  \treturn NULL;\n>  }\n>  \n> +int bitmap_index_contains_pack(struct bitmap_index *bitmap, struct packed_git *pack)\n> +{\n> +\tfor (; bitmap; bitmap = bitmap->base) {\n> +\t\tif (bitmap_is_midx(bitmap)) {\n> +\t\t\tfor (size_t i = 0; i < bitmap->midx->num_packs; i++)\n> +\t\t\t\tif (bitmap->midx->packs[i] == pack)\n> +\t\t\t\t\treturn 1;\n> +\t\t} else if (bitmap->pack == pack) {\n\nHere, and two lines above, we compare packs by their pointer address,\nthis doesn't seem to be common practice to me. Or is it in the Git\ncodebase? Do we expect any problems with this, for example when we stop\nusing `the_repository`?\n\n-- \nToon\n"},{"id":"515426","messageId":"CAOLa=ZTgU+E3Y6DRCA0EOg9uOvNoggCRPBNYjDZTOinaDVj95Q@mail.gmail.com","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-2-4bbc7085d7c5@pks.im","subject":"Re: [PATCH v2 02/10] builtin/cat-file: wire up an option to filter objects","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-04-01T12:05:17Z","receivedAt":"2025-04-01T12:05:18Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> In batch mode, git-cat-file(1) enumerates all objects and prints them\n> by iterating through both loose and packed objects. This works without\n\nNit: I assume you're referring to the `--batch-all-objects` mode. So\nwould be nice to specify here perhaps?\n\n> considering their reachability at all, and consequently most options to\n> filter objects as they exist in e.g. git-rev-list(1) are not applicable.\n> In some situations it may still be useful though to filter objects based\n> on properties that are inherent to them. This includes the object size\n> as well as its type.\n>\n> Such a filter already exists in git-rev-list(1) with the `--filter=`\n> command line option. While this option supports a couple of filters that\n> are not applicable to our usecase, some of them are quite a neat fit.\n>\n> Wire up the filter as an option for git-cat-file(1). This allows us to\n> reuse the same syntax as in git-rev-list(1) so that we don't have to\n> reinvent the wheel. For now, we die when any of the filter options has\n> been passed by the user, but they will be wired up in subsequent\n> commits.\n>\n> Further note that the filters that we are about to introduce don't\n> significantly speed up the runtime of git-cat-file(1). While we can skip\n> emitting a lot of objects in case they are uninteresting to us, the\n> majority of time is spent reading the packfile, which is bottlenecked by\n> I/O and not the processor. This will change though once we start to make\n> use of bitmaps, which will allow us to skip reading the whole packfile.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  Documentation/git-cat-file.adoc |  6 ++++++\n>  builtin/cat-file.c              | 37 +++++++++++++++++++++++++++++++++----\n>  t/t1006-cat-file.sh             | 32 ++++++++++++++++++++++++++++++++\n>  3 files changed, 71 insertions(+), 4 deletions(-)\n>\n> diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc\n> index d5890ae3686..f7f57b7f538 100644\n> --- a/Documentation/git-cat-file.adoc\n> +++ b/Documentation/git-cat-file.adoc\n> @@ -81,6 +81,12 @@ OPTIONS\n>  \tend-of-line conversion, etc). In this case, `<object>` has to be of\n>  \tthe form `<tree-ish>:<path>`, or `:<path>`.\n>\n> +--filter=<filter-spec>::\n> +--no-filter::\n> +\tOmit objects from the list of printed objects. This can only be used in\n> +\tcombination with one of the batched modes. The '<filter-spec>' may be\n> +\tone of the following:\n> +\n\nShouldn't we say this is specific to `--batch-all-objects`?\n\n>  --path=<path>::\n>  \tFor use with `--textconv` or `--filters`, to allow specifying an object\n>  \tname and a path separately, e.g. when it is difficult to figure out\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index 8e40016dd24..940900d92ad 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -15,6 +15,7 @@\n>  #include \"gettext.h\"\n>  #include \"hex.h\"\n>  #include \"ident.h\"\n> +#include \"list-objects-filter-options.h\"\n>  #include \"parse-options.h\"\n>  #include \"userdiff.h\"\n>  #include \"streaming.h\"\n> @@ -35,6 +36,7 @@ enum batch_mode {\n>  };\n>\n>  struct batch_options {\n> +\tstruct list_objects_filter_options objects_filter;\n>  \tint enabled;\n>  \tint follow_symlinks;\n>  \tenum batch_mode batch_mode;\n> @@ -487,6 +489,13 @@ static void batch_object_write(const char *obj_name,\n>  \t\t\treturn;\n>  \t\t}\n>\n> +\t\tswitch (opt->objects_filter.choice) {\n> +\t\tcase LOFC_DISABLED:\n> +\t\t\tbreak;\n> +\t\tdefault:\n> +\t\t\tBUG(\"unsupported objects filter\");\n> +\t\t}\n> +\n\nOkay here it seems like it also applies to other batch modes. So it\nwould be nice to perhaps clarify how this works when not used with\n`--batch-all-objects`?\n\n>  \t\tif (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {\n>  \t\t\tsize_t s = data->size;\n>  \t\t\tchar *buf = NULL;\n> @@ -812,7 +821,8 @@ static int batch_objects(struct batch_options *opt)\n>  \t\tstruct object_cb_data cb;\n>  \t\tstruct object_info empty = OBJECT_INFO_INIT;\n>\n> -\t\tif (!memcmp(&data.info, &empty, sizeof(empty)))\n> +\t\tif (!memcmp(&data.info, &empty, sizeof(empty)) &&\n> +\t\t    opt->objects_filter.choice == LOFC_DISABLED)\n>  \t\t\tdata.skip_object_info = 1;\n>\n>  \t\tif (repo_has_promisor_remote(the_repository))\n> @@ -936,10 +946,13 @@ int cmd_cat_file(int argc,\n>  \tint opt_cw = 0;\n>  \tint opt_epts = 0;\n>  \tconst char *exp_type = NULL, *obj_name = NULL;\n> -\tstruct batch_options batch = {0};\n> +\tstruct batch_options batch = {\n> +\t\t.objects_filter = LIST_OBJECTS_FILTER_INIT,\n> +\t};\n>  \tint unknown_type = 0;\n>  \tint input_nul_terminated = 0;\n>  \tint nul_terminated = 0;\n> +\tint ret;\n>\n>  \tconst char * const builtin_catfile_usage[] = {\n>  \t\tN_(\"git cat-file <type> <object>\"),\n> @@ -1000,6 +1013,8 @@ int cmd_cat_file(int argc,\n>  \t\t\t    N_(\"run filters on object's content\"), 'w'),\n>  \t\tOPT_STRING(0, \"path\", &force_path, N_(\"blob|tree\"),\n>  \t\t\t   N_(\"use a <path> for (--textconv | --filters); Not with 'batch'\")),\n> +\t\tOPT_CALLBACK(0, \"filter\", &batch.objects_filter, N_(\"args\"),\n> +\t\t\t     N_(\"object filtering\"), opt_parse_list_objects_filter),\n>  \t\tOPT_END()\n>  \t};\n>\n> @@ -1014,6 +1029,14 @@ int cmd_cat_file(int argc,\n>  \tif (use_mailmap)\n>  \t\tread_mailmap(&mailmap);\n>\n> +\tswitch (batch.objects_filter.choice) {\n> +\tcase LOFC_DISABLED:\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tusagef(_(\"objects filter not supported: '%s'\"),\n> +\t\t       list_object_filter_config_name(batch.objects_filter.choice));\n> +\t}\n> +\n>  \t/* --batch-all-objects? */\n>  \tif (opt == 'b')\n>  \t\tbatch.all_objects = 1;\n> @@ -1068,7 +1091,8 @@ int cmd_cat_file(int argc,\n>  \t\t\tusage_msg_opt(_(\"batch modes take no arguments\"), builtin_catfile_usage,\n>  \t\t\t\t      options);\n>\n> -\t\treturn batch_objects(&batch);\n> +\t\tret = batch_objects(&batch);\n> +\t\tgoto out;\n>  \t}\n>\n>  \tif (opt) {\n> @@ -1097,5 +1121,10 @@ int cmd_cat_file(int argc,\n>\n>  \tif (unknown_type && opt != 't' && opt != 's')\n>  \t\tdie(\"git cat-file --allow-unknown-type: use with -s or -t\");\n> -\treturn cat_one_file(opt, exp_type, obj_name, unknown_type);\n> +\n> +\tret = cat_one_file(opt, exp_type, obj_name, unknown_type);\n> +\n> +out:\n> +\tlist_objects_filter_release(&batch.objects_filter);\n> +\treturn ret;\n>  }\n> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n> index 398865d6ebe..1246d3119f8 100755\n> --- a/t/t1006-cat-file.sh\n> +++ b/t/t1006-cat-file.sh\n> @@ -1353,4 +1353,36 @@ test_expect_success PERL '--batch-command info is unbuffered by default' '\n>  \tperl -e \"$script\" -- --batch-command $hello_oid \"$expect\" \"info \"\n>  '\n>\n> +test_expect_success 'setup for objects filter' '\n> +\tgit init repo\n> +'\n> +\n> +test_expect_success 'objects filter with unknown option' '\n> +\tcat >expect <<-EOF &&\n> +\tfatal: invalid filter-spec ${SQ}unknown${SQ}\n> +\tEOF\n> +\ttest_must_fail git -C repo cat-file --filter=unknown 2>err &&\n> +\ttest_cmp expect err\n> +'\n> +\n\nWould it be also worthwhile to test the `--no-filter` option?\n\n> +for option in blob:none blob:limit=1 object:type=tag sparse:oid=1234 tree:1 sparse:path=x\n> +do\n> +\ttest_expect_success \"objects filter with unsupported option $option\" '\n> +\t\tcase \"$option\" in\n> +\t\ttree:1)\n> +\t\t\techo \"usage: objects filter not supported: ${SQ}tree${SQ}\" >expect\n> +\t\t\t;;\n> +\t\tsparse:path=x)\n> +\t\t\techo \"fatal: sparse:path filters support has been dropped\" >expect\n> +\t\t\t;;\n> +\t\t*)\n> +\t\t\toption_name=$(echo \"$option\" | cut -d= -f1) &&\n> +\t\t\tprintf \"usage: objects filter not supported: ${SQ}%s${SQ}\\n\" \"$option_name\" >expect\n> +\t\t\t;;\n> +\t\tesac &&\n> +\t\ttest_must_fail git -C repo cat-file --filter=$option 2>err &&\n> +\t\ttest_cmp expect err\n> +\t'\n> +done\n> +\n>  test_done\n>\n> --\n> 2.49.0.472.ge94155a9ec.dirty\n"},{"id":"515427","messageId":"87jz84f5a2.fsf@iotcl.com","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-9-4bbc7085d7c5@pks.im","subject":"Re: [PATCH v2 09/10] builtin/cat-file: deduplicate logic to iterate over all objects","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-04-01T12:13:57Z","receivedAt":"2025-04-01T12:14:11Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Pull out a common function that allows us to iterate over all objects in\n> a repository. Right now the logic is trivial and would only require two\n> function calls, making this refactoring a bit pointless. But in the next\n> commit we will iterate on this logic to make use of bitmaps, so this is\n> about to become a bit more complex.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  builtin/cat-file.c | 85 ++++++++++++++++++++++++++++++------------------------\n>  1 file changed, 48 insertions(+), 37 deletions(-)\n>\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index 430320adfe9..6f5dbc821a2 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -622,25 +622,18 @@ static int batch_object_cb(const struct object_id *oid, void *vdata)\n>  \treturn 0;\n>  }\n>  \n> -static int collect_loose_object(const struct object_id *oid,\n> -\t\t\t\tconst char *path UNUSED,\n> -\t\t\t\tvoid *data)\n> -{\n> -\toid_array_append(data, oid);\n> -\treturn 0;\n> -}\n> -\n> -static int collect_packed_object(const struct object_id *oid,\n> -\t\t\t\t struct packed_git *pack UNUSED,\n> -\t\t\t\t uint32_t pos UNUSED,\n> -\t\t\t\t void *data)\n> +static int collect_object(const struct object_id *oid,\n> +\t\t\t  struct packed_git *pack UNUSED,\n> +\t\t\t  off_t offset UNUSED,\n> +\t\t\t  void *data)\n>  {\n>  \toid_array_append(data, oid);\n>  \treturn 0;\n>  }\n>  \n>  static int batch_unordered_object(const struct object_id *oid,\n> -\t\t\t\t  struct packed_git *pack, off_t offset,\n> +\t\t\t\t  struct packed_git *pack,\n> +\t\t\t\t  off_t offset,\n>  \t\t\t\t  void *vdata)\n>  {\n>  \tstruct object_cb_data *data = vdata;\n> @@ -654,23 +647,6 @@ static int batch_unordered_object(const struct object_id *oid,\n>  \treturn 0;\n>  }\n>  \n> -static int batch_unordered_loose(const struct object_id *oid,\n> -\t\t\t\t const char *path UNUSED,\n> -\t\t\t\t void *data)\n> -{\n> -\treturn batch_unordered_object(oid, NULL, 0, data);\n> -}\n> -\n> -static int batch_unordered_packed(const struct object_id *oid,\n> -\t\t\t\t  struct packed_git *pack,\n> -\t\t\t\t  uint32_t pos,\n> -\t\t\t\t  void *data)\n> -{\n> -\treturn batch_unordered_object(oid, pack,\n> -\t\t\t\t      nth_packed_object_offset(pack, pos),\n> -\t\t\t\t      data);\n> -}\n> -\n>  typedef void (*parse_cmd_fn_t)(struct batch_options *, const char *,\n>  \t\t\t       struct strbuf *, struct expand_data *);\n>  \n> @@ -803,6 +779,45 @@ static void batch_objects_command(struct batch_options *opt,\n>  \n>  #define DEFAULT_FORMAT \"%(objectname) %(objecttype) %(objectsize)\"\n>  \n> +typedef int (*for_each_object_fn)(const struct object_id *oid, struct packed_git *pack,\n> +\t\t\t\t  off_t offset, void *data);\n> +\n> +struct for_each_object_payload {\n> +\tfor_each_object_fn callback;\n> +\tvoid *payload;\n> +};\n> +\n> +static int batch_one_object_loose(const struct object_id *oid,\n> +\t\t\t\t  const char *path UNUSED,\n> +\t\t\t\t  void *_payload)\n> +{\n> +\tstruct for_each_object_payload *payload = _payload;\n> +\treturn payload->callback(oid, NULL, 0, payload->payload);\n> +}\n> +\n> +static int batch_one_object_packed(const struct object_id *oid,\n> +\t\t\t\t   struct packed_git *pack,\n> +\t\t\t\t   uint32_t pos,\n> +\t\t\t\t   void *_payload)\n> +{\n> +\tstruct for_each_object_payload *payload = _payload;\n> +\treturn payload->callback(oid, pack, nth_packed_object_offset(pack, pos),\n> +\t\t\t\t payload->payload);\n> +}\n> +\n> +static void batch_each_object(for_each_object_fn callback,\n> +\t\t\t      unsigned flags,\n> +\t\t\t      void *_payload)\n\nWhy is this `_payload` typeless? I see it only getting passed in\n`struct object_cb_data`, is there a reason to hide this type? With\npayload being wrapped in payload I think it's beneficial to keep type\ninfo where possible.\n\n-- \nToon\n"},{"id":"515428","messageId":"87h638f54w.fsf@iotcl.com","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-6-4bbc7085d7c5@pks.im","subject":"Re: [PATCH v2 06/10] pack-bitmap: allow passing payloads to `show_reachable_fn()`","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-04-01T12:17:03Z","receivedAt":"2025-04-01T12:17:14Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> The `show_reachable_fn` callback is used by a couple of functions to\n> present reachable objects to the caller. The function does not provide a\n> way for the caller to pass a payload though, which is functionality that\n> we'll require in a subsequent commit.\n>\n> Change the callback type to accept a payload and adapt all callsites\n> accordingly.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  builtin/pack-objects.c |  3 ++-\n>  builtin/rev-list.c     |  3 ++-\n>  pack-bitmap.c          | 15 ++++++++-------\n>  pack-bitmap.h          |  3 ++-\n>  reachable.c            |  3 ++-\n>  5 files changed, 16 insertions(+), 11 deletions(-)\n>\n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index a7e4bb79049..38784613fc0 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -1736,7 +1736,8 @@ static int add_object_entry(const struct object_id *oid, enum object_type type,\n>  static int add_object_entry_from_bitmap(const struct object_id *oid,\n>  \t\t\t\t\tenum object_type type,\n>  \t\t\t\t\tint flags UNUSED, uint32_t name_hash,\n> -\t\t\t\t\tstruct packed_git *pack, off_t offset)\n> +\t\t\t\t\tstruct packed_git *pack, off_t offset,\n> +\t\t\t\t\tvoid *payload UNUSED)\n>  {\n>  \tdisplay_progress(progress_state, ++nr_seen);\n>  \n> diff --git a/builtin/rev-list.c b/builtin/rev-list.c\n> index bb26bee0d45..1100dd2abe7 100644\n> --- a/builtin/rev-list.c\n> +++ b/builtin/rev-list.c\n> @@ -429,7 +429,8 @@ static int show_object_fast(\n>  \tint exclude UNUSED,\n>  \tuint32_t name_hash UNUSED,\n>  \tstruct packed_git *found_pack UNUSED,\n> -\toff_t found_offset UNUSED)\n> +\toff_t found_offset UNUSED,\n> +\tvoid *payload UNUSED)\n>  {\n>  \tfprintf(stdout, \"%s\\n\", oid_to_hex(oid));\n>  \treturn 1;\n> diff --git a/pack-bitmap.c b/pack-bitmap.c\n> index 6f7fd94c36f..d192fb87da9 100644\n> --- a/pack-bitmap.c\n> +++ b/pack-bitmap.c\n> @@ -1625,7 +1625,7 @@ static void show_extended_objects(struct bitmap_index *bitmap_git,\n>  \t\t    (obj->type == OBJ_TAG && !revs->tag_objects))\n>  \t\t\tcontinue;\n>  \n> -\t\tshow_reach(&obj->oid, obj->type, 0, eindex->hashes[i], NULL, 0);\n> +\t\tshow_reach(&obj->oid, obj->type, 0, eindex->hashes[i], NULL, 0, NULL);\n>  \t}\n>  }\n>  \n> @@ -1663,7 +1663,8 @@ static void init_type_iterator(struct ewah_or_iterator *it,\n>  static void show_objects_for_type(\n>  \tstruct bitmap_index *bitmap_git,\n>  \tenum object_type object_type,\n> -\tshow_reachable_fn show_reach)\n> +\tshow_reachable_fn show_reach,\n\nWhat would you think about adding the `_fn` to `show_reach`? Because the\nfunction is passed on to `show_objects_for_type()`, I think it improves\nthe readability if it's called `show_reach_fn` or somethin?g\n\n-- \nToon\n"},{"id":"515430","messageId":"CAOLa=ZR4YmfHvQGvs1mzdtRu5z+8XW-xY+t_wBAoTs1m+rrO9A@mail.gmail.com","threadId":"63203","inReplyTo":"20250327-pks-cat-file-object-type-filter-v2-3-4bbc7085d7c5@pks.im","subject":"Re: [PATCH v2 03/10] builtin/cat-file: support \"blob:none\" objects filter","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-04-01T12:22:34Z","receivedAt":"2025-04-01T12:22:35Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Implement support for the \"blob:none\" filter in git-cat-file(1), which\n> causes us to omit all blobs.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  Documentation/git-cat-file.adoc |  2 ++\n>  builtin/cat-file.c              | 11 ++++++++++-\n>  t/t1006-cat-file.sh             | 33 +++++++++++++++++++++++++++++++--\n>  3 files changed, 43 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc\n> index f7f57b7f538..bb32f715944 100644\n> --- a/Documentation/git-cat-file.adoc\n> +++ b/Documentation/git-cat-file.adoc\n> @@ -86,6 +86,8 @@ OPTIONS\n>  \tOmit objects from the list of printed objects. This can only be used in\n>  \tcombination with one of the batched modes. The '<filter-spec>' may be\n>  \tone of the following:\n> ++\n> +The form '--filter=blob:none' omits all blobs.\n>\n>  --path=<path>::\n>  \tFor use with `--textconv` or `--filters`, to allow specifying an object\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index 940900d92ad..e783dbbad58 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -472,7 +472,8 @@ static void batch_object_write(const char *obj_name,\n>  \tif (!data->skip_object_info) {\n>  \t\tint ret;\n>\n> -\t\tif (use_mailmap)\n> +\t\tif (use_mailmap ||\n> +\t\t    opt->objects_filter.choice == LOFC_BLOB_NONE)\n>  \t\t\tdata->info.typep = &data->type;\n>\n\nI didn't understand why we need to do this, below we only check for\n`data->type`. The only other place we use `data->info.typep` going\nforward seems to be `print_object_or_die()`, but that flow is only\nfollowed for `opt->batch_mode == BATCH_MODE_CONTENTS`. We already have\n\n    if (opt->batch_mode == BATCH_MODE_CONTENTS)\n\t\tdata.info.typep = &data.type;\n\nin `batch_objects()` before this, shouldn't that cover this scenario\ntoo? Maybe we can add a comment with the reasoning\n\n[snip]\n"},{"id":"515431","messageId":"CAOLa=ZRjhqsSxA7JCPPU4e1HFJBeK0PFgCZwPc07emp+2pawyA@mail.gmail.com","threadId":"63203","inReplyTo":"CAOLa=ZR4YmfHvQGvs1mzdtRu5z+8XW-xY+t_wBAoTs1m+rrO9A@mail.gmail.com","subject":"Re: [PATCH v2 03/10] builtin/cat-file: support \"blob:none\" objects filter","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-04-01T12:31:24Z","receivedAt":"2025-04-01T12:31:26Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n> Patrick Steinhardt <ps@pks.im> writes:\n>\n>> Implement support for the \"blob:none\" filter in git-cat-file(1), which\n>> causes us to omit all blobs.\n>>\n>> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n>> ---\n>>  Documentation/git-cat-file.adoc |  2 ++\n>>  builtin/cat-file.c              | 11 ++++++++++-\n>>  t/t1006-cat-file.sh             | 33 +++++++++++++++++++++++++++++++--\n>>  3 files changed, 43 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc\n>> index f7f57b7f538..bb32f715944 100644\n>> --- a/Documentation/git-cat-file.adoc\n>> +++ b/Documentation/git-cat-file.adoc\n>> @@ -86,6 +86,8 @@ OPTIONS\n>>  \tOmit objects from the list of printed objects. This can only be used in\n>>  \tcombination with one of the batched modes. The '<filter-spec>' may be\n>>  \tone of the following:\n>> ++\n>> +The form '--filter=blob:none' omits all blobs.\n>>\n>>  --path=<path>::\n>>  \tFor use with `--textconv` or `--filters`, to allow specifying an object\n>> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n>> index 940900d92ad..e783dbbad58 100644\n>> --- a/builtin/cat-file.c\n>> +++ b/builtin/cat-file.c\n>> @@ -472,7 +472,8 @@ static void batch_object_write(const char *obj_name,\n>>  \tif (!data->skip_object_info) {\n>>  \t\tint ret;\n>>\n>> -\t\tif (use_mailmap)\n>> +\t\tif (use_mailmap ||\n>> +\t\t    opt->objects_filter.choice == LOFC_BLOB_NONE)\n>>  \t\t\tdata->info.typep = &data->type;\n>>\n>\n> I didn't understand why we need to do this, below we only check for\n> `data->type`. The only other place we use `data->info.typep` going\n> forward seems to be `print_object_or_die()`, but that flow is only\n> followed for `opt->batch_mode == BATCH_MODE_CONTENTS`. We already have\n>\n>     if (opt->batch_mode == BATCH_MODE_CONTENTS)\n> \t\tdata.info.typep = &data.type;\n>\n> in `batch_objects()` before this, shouldn't that cover this scenario\n> too? Maybe we can add a comment with the reasoning\n>\n> [snip]\n\nAfter playing around more, I understand now, we set the pointer\n`data->info.typep` to point to `data->type`, so when the data is parsed\nin `packed_object_info()` or `oid_object_info_extended()`, that\ninformation would be set into `data->type`. So we can skip as needed.\n\nAll good here!\n"},{"id":"515495","messageId":"Z-0bwFewNxUM7HUe@pks.im","threadId":"63203","inReplyTo":"CAOLa=ZTukdPiciDSyH1oGwYN_bdCw+vYjsHaV9AwHEpxJ5hBgQ@mail.gmail.com","subject":"Re: [PATCH v2 01/10] builtin/cat-file: rename variable that tracks usage","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-02T11:13:04Z","receivedAt":"2025-04-02T11:13:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Apr 01, 2025 at 02:51:01AM -0700, Karthik Nayak wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > The usage strings for git-cat-file(1) that we pass to `parse_options()`\n> > and `usage_msg_optf()` are stored in a variable called `usage`. This\n> > variable shadows the declaration of `usage()`, which we'll want to use\n> > in a subsequent commit.\n> >\n> > Rename the variable to `builtin_catfile_usage`, which is in line with\n> > how the variable is typically called in other builtins.\n> >\n> > Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> > ---\n> >  builtin/cat-file.c | 32 ++++++++++++++++----------------\n> >  1 file changed, 16 insertions(+), 16 deletions(-)\n> >\n> > diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> > index b13561cf73b..8e40016dd24 100644\n> > --- a/builtin/cat-file.c\n> > +++ b/builtin/cat-file.c\n> > @@ -941,7 +941,7 @@ int cmd_cat_file(int argc,\n> >  \tint input_nul_terminated = 0;\n> >  \tint nul_terminated = 0;\n> >\n> > -\tconst char * const usage[] = {\n> > +\tconst char * const builtin_catfile_usage[] = {\n> \n> Nit: Style: we use a right pointer alignment, while it is not part of\n> your code change, would be nice to fix.\n\nNot in this case though:\n\n $ git grep 'const char \\*const' | wc -l\n 85\n $ git grep 'const char \\* const' | wc -l\n 180\n\nIt's mixed, but we do have more cases of the latter.\n\nPatrick\n"},{"id":"515496","messageId":"Z-0bxM6AjLDAjDx2@pks.im","threadId":"63203","inReplyTo":"87h638f54w.fsf@iotcl.com","subject":"Re: [PATCH v2 06/10] pack-bitmap: allow passing payloads to `show_reachable_fn()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-02T11:13:08Z","receivedAt":"2025-04-02T11:13:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Apr 01, 2025 at 02:17:03PM +0200, Toon Claes wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> > diff --git a/pack-bitmap.c b/pack-bitmap.c\n> > index 6f7fd94c36f..d192fb87da9 100644\n> > --- a/pack-bitmap.c\n> > +++ b/pack-bitmap.c\n> > @@ -1663,7 +1663,8 @@ static void init_type_iterator(struct ewah_or_iterator *it,\n> >  static void show_objects_for_type(\n> >  \tstruct bitmap_index *bitmap_git,\n> >  \tenum object_type object_type,\n> > -\tshow_reachable_fn show_reach)\n> > +\tshow_reachable_fn show_reach,\n> \n> What would you think about adding the `_fn` to `show_reach`? Because the\n> function is passed on to `show_objects_for_type()`, I think it improves\n> the readability if it's called `show_reach_fn` or somethin?g\n\nWe don't have that suffix anywhere else where `show_reachable_fn` is\naccepted. So for the sake of consistency I'd rather leave it this way.\n\nPatrick\n"},{"id":"515497","messageId":"Z-0byX56V2E1pGKO@pks.im","threadId":"63203","inReplyTo":"87jz84f5a2.fsf@iotcl.com","subject":"Re: [PATCH v2 09/10] builtin/cat-file: deduplicate logic to iterate over all objects","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-02T11:13:13Z","receivedAt":"2025-04-02T11:13:17Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Apr 01, 2025 at 02:13:57PM +0200, Toon Claes wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> > diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> > index 430320adfe9..6f5dbc821a2 100644\n> > --- a/builtin/cat-file.c\n> > +++ b/builtin/cat-file.c\n> > @@ -803,6 +779,45 @@ static void batch_objects_command(struct batch_options *opt,\n> >  \n> >  #define DEFAULT_FORMAT \"%(objectname) %(objecttype) %(objectsize)\"\n> >  \n> > +typedef int (*for_each_object_fn)(const struct object_id *oid, struct packed_git *pack,\n> > +\t\t\t\t  off_t offset, void *data);\n> > +\n> > +struct for_each_object_payload {\n> > +\tfor_each_object_fn callback;\n> > +\tvoid *payload;\n> > +};\n> > +\n> > +static int batch_one_object_loose(const struct object_id *oid,\n> > +\t\t\t\t  const char *path UNUSED,\n> > +\t\t\t\t  void *_payload)\n> > +{\n> > +\tstruct for_each_object_payload *payload = _payload;\n> > +\treturn payload->callback(oid, NULL, 0, payload->payload);\n> > +}\n> > +\n> > +static int batch_one_object_packed(const struct object_id *oid,\n> > +\t\t\t\t   struct packed_git *pack,\n> > +\t\t\t\t   uint32_t pos,\n> > +\t\t\t\t   void *_payload)\n> > +{\n> > +\tstruct for_each_object_payload *payload = _payload;\n> > +\treturn payload->callback(oid, pack, nth_packed_object_offset(pack, pos),\n> > +\t\t\t\t payload->payload);\n> > +}\n> > +\n> > +static void batch_each_object(for_each_object_fn callback,\n> > +\t\t\t      unsigned flags,\n> > +\t\t\t      void *_payload)\n> \n> Why is this `_payload` typeless? I see it only getting passed in\n> `struct object_cb_data`, is there a reason to hide this type? With\n> payload being wrapped in payload I think it's beneficial to keep type\n> info where possible.\n\nBecause the payload gets forwarded to the callback, and that callback\naccepts arbitrary types. You can already see this now: we call the\nfunction once with a `struct object_cb_data` pointer, and once with a\n`struct oid_array` pointer.\n\nPatrick\n"},{"id":"515498","messageId":"Z-0bzW56m8IJ_ysa@pks.im","threadId":"63203","inReplyTo":"87plhwf6ke.fsf@iotcl.com","subject":"Re: [PATCH v2 08/10] pack-bitmap: introduce function to check whether a pack is bitmapped","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-02T11:13:17Z","receivedAt":"2025-04-02T11:13:21Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Apr 01, 2025 at 01:46:09PM +0200, Toon Claes wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > Introduce a function that allows us to verify whether a pack is\n> > bitmapped or not. This functionality will be used in a subsequent\n> > commit.\n> >\n> > Helped-by: Taylor Blau <me@ttaylorr.com>\n> > Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> > ---\n> >  pack-bitmap.c | 15 +++++++++++++++\n> >  pack-bitmap.h |  7 +++++++\n> >  2 files changed, 22 insertions(+)\n> >\n> > diff --git a/pack-bitmap.c b/pack-bitmap.c\n> > index 6adb8aaa1c2..edc8f42122d 100644\n> > --- a/pack-bitmap.c\n> > +++ b/pack-bitmap.c\n> > @@ -745,6 +745,21 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)\n> >  \treturn NULL;\n> >  }\n> >  \n> > +int bitmap_index_contains_pack(struct bitmap_index *bitmap, struct packed_git *pack)\n> > +{\n> > +\tfor (; bitmap; bitmap = bitmap->base) {\n> > +\t\tif (bitmap_is_midx(bitmap)) {\n> > +\t\t\tfor (size_t i = 0; i < bitmap->midx->num_packs; i++)\n> > +\t\t\t\tif (bitmap->midx->packs[i] == pack)\n> > +\t\t\t\t\treturn 1;\n> > +\t\t} else if (bitmap->pack == pack) {\n> \n> Here, and two lines above, we compare packs by their pointer address,\n> this doesn't seem to be common practice to me. Or is it in the Git\n> codebase? Do we expect any problems with this, for example when we stop\n> using `the_repository`?\n\nI don't expect any problems unless we have multiple `struct repository`\ninstances pointing to the same underlying repository. We never do that\nto the best of my knowledge though, and it would feel somewhat broken if\nwe ever started to do that.\n\nIf we had structs pointing to different repositories though this will do\nthe right thing as a packfile from repository A shouldn't be indexed by\nrepository B.\n\nPatrick\n"},{"id":"515499","messageId":"Z-0b0VI2f-rJbxxb@pks.im","threadId":"63203","inReplyTo":"CAOLa=ZRjhqsSxA7JCPPU4e1HFJBeK0PFgCZwPc07emp+2pawyA@mail.gmail.com","subject":"Re: [PATCH v2 03/10] builtin/cat-file: support \"blob:none\" objects filter","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-02T11:13:21Z","receivedAt":"2025-04-02T11:13:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Apr 01, 2025 at 05:31:24AM -0700, Karthik Nayak wrote:\n> Karthik Nayak <karthik.188@gmail.com> writes:\n> > Patrick Steinhardt <ps@pks.im> writes:\n> >> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> >> index 940900d92ad..e783dbbad58 100644\n> >> --- a/builtin/cat-file.c\n> >> +++ b/builtin/cat-file.c\n> >> @@ -472,7 +472,8 @@ static void batch_object_write(const char *obj_name,\n> >>  \tif (!data->skip_object_info) {\n> >>  \t\tint ret;\n> >>\n> >> -\t\tif (use_mailmap)\n> >> +\t\tif (use_mailmap ||\n> >> +\t\t    opt->objects_filter.choice == LOFC_BLOB_NONE)\n> >>  \t\t\tdata->info.typep = &data->type;\n> >>\n> >\n> > I didn't understand why we need to do this, below we only check for\n> > `data->type`. The only other place we use `data->info.typep` going\n> > forward seems to be `print_object_or_die()`, but that flow is only\n> > followed for `opt->batch_mode == BATCH_MODE_CONTENTS`. We already have\n> >\n> >     if (opt->batch_mode == BATCH_MODE_CONTENTS)\n> > \t\tdata.info.typep = &data.type;\n> >\n> > in `batch_objects()` before this, shouldn't that cover this scenario\n> > too? Maybe we can add a comment with the reasoning\n> >\n> > [snip]\n> \n> After playing around more, I understand now, we set the pointer\n> `data->info.typep` to point to `data->type`, so when the data is parsed\n> in `packed_object_info()` or `oid_object_info_extended()`, that\n> information would be set into `data->type`. So we can skip as needed.\n> \n> All good here!\n\nI've adapted the commit message to better explain this.\n\nPatrick\n"},{"id":"515500","messageId":"Z-0b1l3Q-lYH5z3E@pks.im","threadId":"63203","inReplyTo":"CAOLa=ZTgU+E3Y6DRCA0EOg9uOvNoggCRPBNYjDZTOinaDVj95Q@mail.gmail.com","subject":"Re: [PATCH v2 02/10] builtin/cat-file: wire up an option to filter objects","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-02T11:13:26Z","receivedAt":"2025-04-02T11:13:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Apr 01, 2025 at 05:05:17AM -0700, Karthik Nayak wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > In batch mode, git-cat-file(1) enumerates all objects and prints them\n> > by iterating through both loose and packed objects. This works without\n> \n> Nit: I assume you're referring to the `--batch-all-objects` mode. So\n> would be nice to specify here perhaps?\n\nIt's not though, the filter works with all batch modes.\n`--batch-all-objects` of course is the most likely usecase as it may not\nmake much sense to use a filter in other contexts. But they still work.\n\n> > diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> > index 8e40016dd24..940900d92ad 100644\n> > --- a/builtin/cat-file.c\n> > +++ b/builtin/cat-file.c\n> > @@ -15,6 +15,7 @@\n> >  #include \"gettext.h\"\n> >  #include \"hex.h\"\n> >  #include \"ident.h\"\n> > +#include \"list-objects-filter-options.h\"\n> >  #include \"parse-options.h\"\n> >  #include \"userdiff.h\"\n> >  #include \"streaming.h\"\n> > @@ -35,6 +36,7 @@ enum batch_mode {\n> >  };\n> >\n> >  struct batch_options {\n> > +\tstruct list_objects_filter_options objects_filter;\n> >  \tint enabled;\n> >  \tint follow_symlinks;\n> >  \tenum batch_mode batch_mode;\n> > @@ -487,6 +489,13 @@ static void batch_object_write(const char *obj_name,\n> >  \t\t\treturn;\n> >  \t\t}\n> >\n> > +\t\tswitch (opt->objects_filter.choice) {\n> > +\t\tcase LOFC_DISABLED:\n> > +\t\t\tbreak;\n> > +\t\tdefault:\n> > +\t\t\tBUG(\"unsupported objects filter\");\n> > +\t\t}\n> > +\n> \n> Okay here it seems like it also applies to other batch modes. So it\n> would be nice to perhaps clarify how this works when not used with\n> `--batch-all-objects`?\n\nThe filter works the same in all batch modes: if the filter says that an\nobject should be excluded, it's just not printed at all. So none of the\nmodes get treated specially, and the documentation already says that the\nfilters apply to all batched modes.\n\nBut this does raise an interesting question: when using `--batch` we\nbasically follow a request-response schema. So when the object gets\nfiltered, we'd skip the response altogether. Which raises the question\nhow the script would learn about this in the first place, because they\nwould continue to wait for the output.\n\nMaybe we should do something similar to what we do for missing objects\nand also print excluded objects.\n\nI'll implement this and update the documentation accordingly.\n\n> > diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n> > index 398865d6ebe..1246d3119f8 100755\n> > --- a/t/t1006-cat-file.sh\n> > +++ b/t/t1006-cat-file.sh\n> > @@ -1353,4 +1353,36 @@ test_expect_success PERL '--batch-command info is unbuffered by default' '\n> >  \tperl -e \"$script\" -- --batch-command $hello_oid \"$expect\" \"info \"\n> >  '\n> >\n> > +test_expect_success 'setup for objects filter' '\n> > +\tgit init repo\n> > +'\n> > +\n> > +test_expect_success 'objects filter with unknown option' '\n> > +\tcat >expect <<-EOF &&\n> > +\tfatal: invalid filter-spec ${SQ}unknown${SQ}\n> > +\tEOF\n> > +\ttest_must_fail git -C repo cat-file --filter=unknown 2>err &&\n> > +\ttest_cmp expect err\n> > +'\n> > +\n> \n> Would it be also worthwhile to test the `--no-filter` option?\n\nYeah, let's.\n\nPatrick\n"},{"id":"515501","messageId":"Z-0b2e-Y3T7W5Jka@pks.im","threadId":"63203","inReplyTo":"87r02cf6l1.fsf@iotcl.com","subject":"Re: [PATCH v2 02/10] builtin/cat-file: wire up an option to filter objects","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-02T11:13:29Z","receivedAt":"2025-04-02T11:13:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Apr 01, 2025 at 01:45:46PM +0200, Toon Claes wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> > diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> > index 8e40016dd24..940900d92ad 100644\n> > --- a/builtin/cat-file.c\n> > +++ b/builtin/cat-file.c\n> > @@ -1000,6 +1013,8 @@ int cmd_cat_file(int argc,\n> >  \t\t\t    N_(\"run filters on object's content\"), 'w'),\n> >  \t\tOPT_STRING(0, \"path\", &force_path, N_(\"blob|tree\"),\n> >  \t\t\t   N_(\"use a <path> for (--textconv | --filters); Not with 'batch'\")),\n> > +\t\tOPT_CALLBACK(0, \"filter\", &batch.objects_filter, N_(\"args\"),\n> > +\t\t\t     N_(\"object filtering\"), opt_parse_list_objects_filter),\n> \n> Because we've decided on `--filter` we can use\n> `OPT_PARSE_LIST_OBJECTS_FILTER` here now.\n\nAh, indeed, well-spotted.\n\nPatrick\n"},{"id":"515603","messageId":"87a58xf6ib.fsf@iotcl.com","threadId":"63203","inReplyTo":"Z-0byX56V2E1pGKO@pks.im","subject":"Re: [PATCH v2 09/10] builtin/cat-file: deduplicate logic to iterate over all objects","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-04-03T18:24:12Z","receivedAt":"2025-04-03T18:24:27Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Because the payload gets forwarded to the callback, and that callback\n> accepts arbitrary types. You can already see this now: we call the\n> function once with a `struct object_cb_data` pointer, and once with a\n> `struct oid_array` pointer.\n\nThanks for your replies, it all makes sense now. I've got no further\ncomments.\n\nI approve.\n\n-- \nToon\n"},{"id":"515800","messageId":"xmqqecy3n2h4.fsf@gitster.g","threadId":"63203","inReplyTo":"Z-0bwFewNxUM7HUe@pks.im","subject":"Re: [PATCH v2 01/10] builtin/cat-file: rename variable that tracks usage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-04-07T20:25:27Z","receivedAt":"2025-04-07T20:25:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n>> > -\tconst char * const usage[] = {\n>> > +\tconst char * const builtin_catfile_usage[] = {\n>> \n>> Nit: Style: we use a right pointer alignment, while it is not part of\n>> your code change, would be nice to fix.\n\nSorry, but I do not get the \"right pointer alignment\" here.  There\nis a rule to say that the asterisk sticks to variable (and member in\na struct/union) rather than to type, and since <type> comes before\nthe <variable> being declared in C, it would be <type> *<variable>,\nbut that is different from \"write asterisk stuck to the right\nidentifier\".\n\n> Not in this case though:\n>\n>  $ git grep 'const char \\*const' | wc -l\n>  85\n>  $ git grep 'const char \\* const' | wc -l\n>  180\n>\n> It's mixed, but we do have more cases of the latter.\n\nI think what you wrote is fine.  Thanks.\n"}]}