Volume XXII, number 279Tuesday, October 6, 2026Latest message 41 minutes ago

The Git List

News and archive of git@vger.kernel.org, since April 2005

patch, 7 partsodb: introduce object filters to `odb_for_each_object()`

75 messages between Jul 9, 2026 and Jul 15, 2026, from Patrick Steinhardt, Junio C Hamano, Justin Tobler, Taylor Blau, Jeff King, Toon Claes.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

Patrick SteinhardtJul 9, 2026, 08:35 UTC on lore
Hi,

this patch series introduces object filters to `odb_for_each_object()`. The intent of this is to make `git cat-file --batch-all-objects` work with pluggable object databases. Right now it doesn't because it reaches into internals of the "packed" backend to efficiently handle bitmapped objects.

The series is built on top of f85a7e6620 (Start Git 2.56 cycle, 2026-07-06) with ps/odb-drop-whence at 8a7ad23e11 (odb: document object info fields, 2026-07-02) merged into it.

Thanks!
Patrick
---
Patrick Steinhardt (7):
      odb/source-packed: improve lookup when enumerating objects
      pack-bitmap: mark object filter as `const`
      pack-bitmap: allow aborting iteration of bitmapped objects
      pack-bitmap: iterate object sources when opening bitmaps
      pack-bitmap: introduce function to open bitmap for a single source
      odb: introduce object filters to `odb_for_each_object()`
      builtin/cat-file: filter objects via object database
 builtin/cat-file.c     |  76 +++-----------------------------
 builtin/pack-objects.c |   2 +-
 builtin/rev-list.c     |   2 +-
 odb.h                  |  12 ++++++
 odb/source-packed.c    |  77 ++++++++++++++++++++++++++++++---
 pack-bitmap.c          | 115 ++++++++++++++++++++++++++++---------------------
 pack-bitmap.h          |  10 ++++-
 7 files changed, 164 insertions(+), 130 deletions(-)

--- base-commit: 3c8e2790f2ce15e8b5d4b4e6ced711b12649f32a change-id: 20260708-pks-odb-for-each-object-filter-13286fa3523d

Patrick SteinhardtJul 9, 2026, 08:35 UTC in reply to Patrick Steinhardt on lore

[PATCH 1/7] odb/source-packed: improve lookup when enumerating objects

When iterating through packed objects via `odb_for_each_object()` we do so via two different mechanisms:

  - When a multi-pack index is available we use that one to efficiently
    loop through all objects.
  - We then loop through all packfiles that aren't covered by a
    multi-pack index.

Regardless of which mechanism we use, we then iterate through all the objects indexed by the respective data structure. Curiously though, while we use the indices for enumerating the objects, we completely ignore it for the actual object lookup. Instead, we call into the generic `odb_source_read_object_info()` function, which will itself consult the indices to figure out where the object in question even lives.

This has two consequences:
  - It's inefficient, as we basically have to figure out the position of
    the object a second time.
  - It's subtly wrong, as it may now happen that a specific object will
    be looked up via a different pack in case it exists multiple times.

Fix the issue by using `packed_object_info()` directly. While at it, rename the `store` variable to `source`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-packed.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
Show changes to odb/source-packed.c +8 −7
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 0edea5356d..9cfa02b7a2 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -143,7 +143,7 @@ static bool should_exclude_pack(struct packed_git *p, enum odb_for_each_object_f
 }
 
 static int for_each_prefixed_object_in_midx(
-	struct odb_source_packed *store,
+	struct odb_source_packed *source,
 	struct multi_pack_index *m,
 	const struct odb_for_each_object_options *opts,
 	struct odb_source_packed_for_each_object_wrapper_data *data)
@@ -170,6 +170,7 @@ static int for_each_prefixed_object_in_midx(
 		 */
 		for (i = first; i < num; i++) {
 			const struct object_id *current = NULL;
+			struct packed_git *pack;
 			struct object_id oid;
 
 			current = nth_midxed_object_oid(&oid, m, i);
@@ -177,9 +178,8 @@ static int for_each_prefixed_object_in_midx(
 			if (!match_hash(len, opts->prefix->hash, current->hash))
 				break;
 
-			if (opts->flags) {
+			if (opts->flags || data->request) {
 				uint32_t pack_id = nth_midxed_pack_int_id(m, i);
-				struct packed_git *pack;
 
 				if (prepare_midx_pack(m, pack_id)) {
 					pack_errors = true;
@@ -193,9 +193,9 @@ static int for_each_prefixed_object_in_midx(
 
 			if (data->request) {
 				struct object_info oi = *data->request;
+				off_t offset = nth_midxed_offset(m, i);
 
-				ret = odb_source_read_object_info(&store->base, current,
-								  &oi, 0);
+				ret = packed_object_info(source, pack, offset, &oi);
 				if (ret)
 					goto out;
 
@@ -219,7 +219,7 @@ static int for_each_prefixed_object_in_midx(
 }
 
 static int for_each_prefixed_object_in_pack(
-	struct odb_source_packed *store,
+	struct odb_source_packed *source,
 	struct packed_git *p,
 	const struct odb_for_each_object_options *opts,
 	struct odb_source_packed_for_each_object_wrapper_data *data)
@@ -246,8 +246,9 @@ static int for_each_prefixed_object_in_pack(
 
 		if (data->request) {
 			struct object_info oi = *data->request;
+			off_t offset = nth_packed_object_offset(p, i);
 
-			ret = odb_source_read_object_info(&store->base, &oid, &oi, 0);
+			ret = packed_object_info(source, p, offset, &oi);
 			if (ret)
 				goto out;
 
-- 
2.55.0.175.ge4962bd3d5.dirty
Patrick SteinhardtJul 9, 2026, 08:35 UTC in reply to Patrick Steinhardt on lore

[PATCH 2/7] pack-bitmap: mark object filter as `const`

The function `for_each_bitmapped_object()` accepts an optional object filter. This filter is never modified by the function, but is not declared as `const`. Fix this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pack-bitmap.c | 6 +++---
 pack-bitmap.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)
Show changes to 2 files +4 −4

pack-bitmap.c, pack-bitmap.h

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 35774b6f0c..a47c231632 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1976,7 +1976,7 @@ static void filter_bitmap_object_type(struct bitmap_index *bitmap_git,
 static int filter_bitmap(struct bitmap_index *bitmap_git,
 			 struct object_list *tip_objects,
 			 struct bitmap *to_filter,
-			 struct list_objects_filter_options *filter)
+			 const struct list_objects_filter_options *filter)
 {
 	if (!filter || filter->choice == LOFC_DISABLED)
 		return 0;
@@ -2027,7 +2027,7 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
 	return -1;
 }
 
-static int can_filter_bitmap(struct list_objects_filter_options *filter)
+static int can_filter_bitmap(const struct list_objects_filter_options *filter)
 {
 	return !filter_bitmap(NULL, NULL, NULL, filter);
 }
@@ -2058,7 +2058,7 @@ static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
 }
 
 int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
-			      struct list_objects_filter_options *filter,
+			      const struct list_objects_filter_options *filter,
 			      show_reachable_fn show_reach,
 			      void *payload)
 {
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 19a8655457..47935eb24e 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -96,7 +96,7 @@ struct list_objects_filter_options;
  * not supported, `0` otherwise.
  */
 int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
-			      struct list_objects_filter_options *filter,
+			      const struct list_objects_filter_options *filter,
 			      show_reachable_fn show_reach,
 			      void *payload);
 
-- 
2.55.0.175.ge4962bd3d5.dirty
Patrick SteinhardtJul 9, 2026, 08:35 UTC in reply to Patrick Steinhardt on lore

[PATCH 3/7] pack-bitmap: allow aborting iteration of bitmapped objects

In a subsequent commit we'll lift iteration of bitmapped objects into the "packed" backend and make it accessible via `odb_for_each_object()`. The calling convention for that function is that the callback may return a non-zero exit code, and if so we'll abort iteration. This is currently impossible to realize though, as `for_each_bitmapped_object()` will ignore any return value and just churn through all objects completely.

This doesn't matter to the callers of `for_each_bitmapped_object()`, as there's only one of them in git-cat-file(1), and the callbacks we pass always return zero. But once we move the logic into the generic infrastructure it becomes a latent bug waiting to happen.

Refactor the code so that the return value of the `show_reach` callback is not ignored anymore. Instead, returning a non-zero value will cause us to abort iteration in both `show_objects_for_type()` and in `for_each_bitmapped_object()`.

Note though that there's a second user of `show_objects_for_type()` with `traverse_bitmap_commit_list()`, and that function does indeed invoke callbacks that may return non-zero. This non-zero return value never had any effect at all though, and the callbacks that return non-zero values are only ever invoked via `traverse_bitmap_commit_list()`. Consequently, we adapt them to always return 0.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/pack-objects.c |  2 +-
 builtin/rev-list.c     |  2 +-
 pack-bitmap.c          | 31 +++++++++++++++++++++----------
 pack-bitmap.h          |  3 ++-
 4 files changed, 25 insertions(+), 13 deletions(-)
Show changes to 4 files +25 −13

builtin/pack-objects.c, builtin/rev-list.c, pack-bitmap.c, pack-bitmap.h

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ea5eab4cf8..8ff92c5272 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1909,7 +1909,7 @@ static int add_object_entry_from_bitmap(const struct object_id *oid,
 		return 0;
 
 	create_object_entry(oid, type, name_hash, 0, 0, pack, offset);
-	return 1;
+	return 0;
 }
 
 struct pbase_tree_cache {
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 8f63003709..02818b81c6 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -486,7 +486,7 @@ static int show_object_fast(
 	void *payload UNUSED)
 {
 	fprintf(stdout, "%s\n", oid_to_hex(oid));
-	return 1;
+	return 0;
 }
 
 static void print_disk_usage(off_t size)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index a47c231632..eda38a5433 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1695,7 +1695,7 @@ static void init_type_iterator(struct ewah_or_iterator *it,
 	}
 }
 
-static void show_objects_for_type(
+static int show_objects_for_type(
 	struct bitmap_index *bitmap_git,
 	struct bitmap *objects,
 	enum object_type object_type,
@@ -1704,6 +1704,7 @@ static void show_objects_for_type(
 {
 	size_t i = 0;
 	uint32_t offset;
+	int ret;
 
 	struct ewah_or_iterator it;
 	eword_t filter;
@@ -1749,11 +1750,17 @@ static void show_objects_for_type(
 
 			hash = bitmap_name_hash(bitmap_git, index_pos);
 
-			show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
+			ret = show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
+			if (ret)
+				goto out;
 		}
 	}
 
+	ret = 0;
+
+out:
 	ewah_or_iterator_release(&it);
+	return ret;
 }
 
 static int in_bitmapped_pack(struct bitmap_index *bitmap_git,
@@ -2062,6 +2069,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
 			      show_reachable_fn show_reach,
 			      void *payload)
 {
+	const enum object_type types[] = {
+		OBJ_COMMIT,
+		OBJ_TREE,
+		OBJ_BLOB,
+		OBJ_TAG,
+	};
 	struct bitmap *filtered_bitmap = NULL;
 	uint32_t objects_nr;
 	size_t full_word_count;
@@ -2086,14 +2099,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
 		goto out;
 	}
 
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_COMMIT, show_reach, payload);
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_TREE, show_reach, payload);
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_BLOB, show_reach, payload);
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_TAG, show_reach, payload);
+	for (size_t i = 0; i < ARRAY_SIZE(types); i++) {
+		ret = show_objects_for_type(bitmap_git, filtered_bitmap,
+					    types[i], show_reach, payload);
+		if (ret)
+			goto out;
+	}
 
 	ret = 0;
 out:
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 47935eb24e..ae8dc491ac 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -93,7 +93,8 @@ struct list_objects_filter_options;
 /*
  * Filter bitmapped objects and iterate through all resulting objects,
  * executing `show_reach` for each of them. Returns `-1` in case the filter is
- * not supported, `0` otherwise.
+ * not supported, `0` otherwise. Aborts iteration and bubbles up the return
+ * value in case `show_reach()` returns non-zero.
  */
 int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
 			      const struct list_objects_filter_options *filter,
-- 
2.55.0.175.ge4962bd3d5.dirty
Patrick SteinhardtJul 9, 2026, 08:35 UTC in reply to Patrick Steinhardt on lore

[PATCH 4/7] pack-bitmap: iterate object sources when opening bitmaps

When opening a bitmap for a repository we perform two steps:
  - We first look for a multi-pack index bitmap in any of the object
    sources connected to the repository.
  - We then look for a packfile bitmap in any of the packfiles of any of
    the object sources.

Both of these steps thus iterate through object sources themselves, one via `odb_prepare_alternates()` and one via `repo_for_each_pack()`. This layout makes it hard to introduce a way to open the bitmap of one specific object source, which is functionality that we'll require in a subsequent commit.

Reverse the loop so that we instead loop through all sources in the outer loop, and then for each source we try to load its bitmap via either the multi-pack index or via a packfile.

Note that this changes the precedence of bitmaps in one specific edge case: when an earlier object source only has a packfile bitmap, but a later source has a multi-pack index bitmap, we now pick the packfile bitmap of the earlier source. Previously, a multi-pack index bitmap from any source would have taken precedence over all packfile bitmaps. Given that object sources are ordered such that the local source comes first, this arguably is an improvement, as we now prefer local bitmaps over bitmaps in alternates. Furthermore, we already warn about repositories that have multiple bitmaps, so this setup is broken and thus arguably not worth worrying about too much.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pack-bitmap.c | 65 ++++++++++++++++++++++++++---------------------------------
 1 file changed, 29 insertions(+), 36 deletions(-)
Show changes to pack-bitmap.c +29 −36
diff --git a/pack-bitmap.c b/pack-bitmap.c
index eda38a5433..0e3e18a557 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -680,60 +680,53 @@ static int load_bitmap(struct repository *r, struct bitmap_index *bitmap_git,
 	return 0;
 }
 
-static int open_pack_bitmap(struct repository *r,
-			    struct bitmap_index *bitmap_git)
+static int open_bitmap_for_source(struct odb_source_packed *source,
+				  struct bitmap_index *bitmap_git)
 {
-	struct packed_git *p;
+	struct multi_pack_index *midx = get_multi_pack_index(source);
+	struct packfile_list_entry *e;
 	int ret = -1;
 
-	repo_for_each_pack(r, p) {
-		if (open_pack_bitmap_1(bitmap_git, p) == 0) {
-			ret = 0;
-			/*
-			 * The only reason to keep looking is to report
-			 * duplicates.
-			 */
-			if (!trace2_is_enabled())
-				break;
-		}
+	if (midx && !open_midx_bitmap_1(bitmap_git, midx))
+		ret = 0;
+
+	for (e = packfile_store_get_packs(source); e; e = e->next) {
+		/*
+		 * When tracing is enabled we want to keep looking to report
+		 * duplicates even if we have already found a bitmap.
+		 */
+		if (!ret && !trace2_is_enabled())
+			break;
+
+		if (open_pack_bitmap_1(bitmap_git, e->pack))
+			continue;
+		ret = 0;
 	}
 
 	return ret;
 }
 
-static int open_midx_bitmap(struct repository *r,
-			    struct bitmap_index *bitmap_git)
+static int open_bitmap(struct repository *r,
+		       struct bitmap_index *bitmap_git)
 {
 	struct odb_source *source;
-	int ret = -1;
+	int found = 0;
 
 	assert(!bitmap_git->map);
 
 	odb_prepare_alternates(r->objects);
 	for (source = r->objects->sources; source; source = source->next) {
 		struct odb_source_files *files = odb_source_files_downcast(source);
-		struct multi_pack_index *midx = get_multi_pack_index(files->packed);
-		if (midx && !open_midx_bitmap_1(bitmap_git, midx))
-			ret = 0;
-	}
-	return ret;
-}
-
-static int open_bitmap(struct repository *r,
-		       struct bitmap_index *bitmap_git)
-{
-	int found;
 
-	assert(!bitmap_git->map);
+		found |= !open_bitmap_for_source(files->packed, bitmap_git);
 
-	found = !open_midx_bitmap(r, bitmap_git);
-
-	/*
-	 * these will all be skipped if we opened a midx bitmap; but run it
-	 * anyway if tracing is enabled to report the duplicates
-	 */
-	if (!found || trace2_is_enabled())
-		found |= !open_pack_bitmap(r, bitmap_git);
+		/*
+		 * The only reason to keep looking after having found a bitmap
+		 * is to report duplicates.
+		 */
+		if (found && !trace2_is_enabled())
+			break;
+	}
 
 	return found ? 0 : -1;
 }
-- 
2.55.0.175.ge4962bd3d5.dirty
Patrick SteinhardtJul 9, 2026, 08:35 UTC in reply to Patrick Steinhardt on lore

[PATCH 5/7] pack-bitmap: introduce function to open bitmap for a single source

The function `prepare_bitmap_git()` opens the first bitmap it can find in any of the object sources connected to the repository. In a subsequent commit, the "packed" object database backend will learn to use bitmaps to answer object filters when enumerating objects. That backend operates on a single object source though, so using a bitmap that potentially belongs to a different source would be wrong:

  - The source would yield objects that are not part of the source
    itself.
  - The object source info would be attributed to the wrong source.
  - With multiple sources, each source would enumerate the same bitmap
    another time.

Introduce a new function `prepare_source_bitmap_git()` that only opens bitmaps belonging to the given object source.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pack-bitmap.c | 12 ++++++++++++
 pack-bitmap.h |  2 ++
 2 files changed, 14 insertions(+)
Show changes to 2 files +14 −0

pack-bitmap.c, pack-bitmap.h

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 0e3e18a557..5d2af96e2f 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -753,6 +753,18 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
 	return NULL;
 }
 
+struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source)
+{
+	struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
+
+	if (!open_bitmap_for_source(source, bitmap_git) &&
+	    !load_bitmap(source->base.odb->repo, bitmap_git, 0))
+		return bitmap_git;
+
+	free_bitmap_index(bitmap_git);
+	return NULL;
+}
+
 int bitmap_index_contains_pack(struct bitmap_index *bitmap, struct packed_git *pack)
 {
 	for (; bitmap; bitmap = bitmap->base) {
diff --git a/pack-bitmap.h b/pack-bitmap.h
index ae8dc491ac..9f20fb6e56 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -9,6 +9,7 @@
 #include "string-list.h"
 
 struct commit;
+struct odb_source_packed;
 struct repository;
 struct rev_info;
 
@@ -68,6 +69,7 @@ struct bitmapped_pack {
 
 struct bitmap_index *prepare_bitmap_git(struct repository *r);
 struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx);
+struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source);
 
 /*
  * Given a bitmap index, determine whether it contains the pack either directly
-- 
2.55.0.175.ge4962bd3d5.dirty
Patrick SteinhardtJul 9, 2026, 08:35 UTC in reply to Patrick Steinhardt on lore

[PATCH 6/7] odb: introduce object filters to `odb_for_each_object()`

The function `for_each_bitmapped_object()` can be used to iterate through all objects covered by a bitmap. The benefit of this function is that it allows the caller to efficiently handle some object filters. For example, this can be used to filter out objects of a specific type with some simple bitmap operations. But callers are currently required to manually wire up the use of bitmaps though, and to do so they have to reach into internals of a given object database source.

Introduce a new `struct odb_for_each_object_options::filter` field so that the interface becomes generic. When set, then a backend may optionally use the filter to skip some objects that it would have otherwise yielded.

Note that the respective backends are free to ignore this field if they cannot meaningfully optimize for a given filter, and consequently callers need to verify whether they actually want the returned objects. While annoying, we cannot easily lift this restriction anyway as the object filter infrastructure supports some filters that cannot be answered by the object database alone.

Implement the logic for the "packed" source. Note that we use the new function `prepare_source_bitmap_git()` to open the bitmap: as the backend operates on a single object source, we must only use bitmaps that belong to that specific source. Otherwise we might yield objects that are not part of the source at all, and with multiple sources we would enumerate the same bitmap once per source.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.h               | 12 +++++++++++
 odb/source-packed.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 pack-bitmap.c       |  3 +--
 pack-bitmap.h       |  3 +++
 4 files changed, 78 insertions(+), 2 deletions(-)
Show changes to 4 files +78 −2

odb.h, odb/source-packed.c, pack-bitmap.c, pack-bitmap.h

diff --git a/odb.h b/odb.h
index a1e222f605..67d0b34942 100644
--- a/odb.h
+++ b/odb.h
@@ -8,6 +8,7 @@
 #include "thread-utils.h"
 
 struct cached_object_entry;
+struct list_objects_filter_options;
 struct odb_source_inmemory;
 struct packed_git;
 struct repository;
@@ -490,6 +491,17 @@ struct odb_for_each_object_options {
 	 */
 	const struct object_id *prefix;
 	size_t prefix_hex_len;
+
+	/*
+	 * Optional object filter that allows backends to skip yielding
+	 * objects that are excluded by the filter as an optimization. The
+	 * filter is a best-effort hint: backends may use it to skip
+	 * excluded objects (e.g. by consulting a reachability bitmap), but
+	 * are also free to ignore it entirely and yield every object. As a
+	 * consequence, callers must re-apply the filter on yielded objects
+	 * if they require strict filtering semantics.
+	 */
+	const struct list_objects_filter_options *filter;
 };
 
 /*
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 9cfa02b7a2..4777395053 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -3,11 +3,13 @@
 #include "chdir-notify.h"
 #include "dir.h"
 #include "git-zlib.h"
+#include "list-objects-filter-options.h"
 #include "mergesort.h"
 #include "midx.h"
 #include "odb/source-packed.h"
 #include "odb/streaming.h"
 #include "packfile.h"
+#include "pack-bitmap.h"
 
 static int find_pack_entry(struct odb_source_packed *store,
 			   const struct object_id *oid,
@@ -315,6 +317,37 @@ static int odb_source_packed_for_each_prefixed_object(
 	return ret;
 }
 
+struct bitmapped_for_each_object_data {
+	struct odb_source_packed *packed;
+	const struct object_info *request;
+	const struct odb_for_each_object_options *opts;
+	odb_for_each_object_cb cb;
+	void *cb_data;
+};
+
+static int bitmapped_for_each_object(const struct object_id *oid,
+				     enum object_type type UNUSED,
+				     int flags UNUSED,
+				     uint32_t hash UNUSED,
+				     struct packed_git *pack,
+				     off_t offset,
+				     void *cb_data)
+{
+	struct bitmapped_for_each_object_data *data = cb_data;
+
+	if (should_exclude_pack(pack, data->opts->flags))
+		return 0;
+
+	if (data->request) {
+		struct object_info oi = *data->request;
+		if (packed_object_info(data->packed, pack, offset, &oi) < 0)
+			return -1;
+		return data->cb(oid, &oi, data->cb_data);
+	}
+
+	return data->cb(oid, NULL, data->cb_data);
+}
+
 static int odb_source_packed_for_each_object(struct odb_source *source,
 					     const struct object_info *request,
 					     odb_for_each_object_cb cb,
@@ -328,12 +361,33 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
 		.cb = cb,
 		.cb_data = cb_data,
 	};
+	struct bitmap_index *bitmap = NULL;
 	struct packfile_list_entry *e;
 	int pack_errors = 0, ret;
 
 	if (opts->prefix)
 		return odb_source_packed_for_each_prefixed_object(packed, opts, &data);
 
+	if (opts->filter &&
+	    opts->filter->choice != LOFC_DISABLED &&
+	    can_filter_bitmap(opts->filter))
+		bitmap = prepare_bitmap_git_for_source(packed);
+	if (bitmap) {
+		struct bitmapped_for_each_object_data bitmap_data = {
+			.packed = packed,
+			.request = request,
+			.opts = opts,
+			.cb = cb,
+			.cb_data = cb_data,
+		};
+
+		ret = for_each_bitmapped_object(bitmap, opts->filter,
+						bitmapped_for_each_object,
+						&bitmap_data);
+		if (ret)
+			goto out;
+	}
+
 	packed->skip_mru_updates = true;
 
 	for (e = packfile_store_get_packs(packed); e; e = e->next) {
@@ -342,6 +396,13 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
 		if (should_exclude_pack(p, opts->flags))
 			continue;
 
+		/*
+		 * Objects covered by the bitmap have already been yielded
+		 * above; skip them here to avoid duplicates.
+		 */
+		if (bitmap && bitmap_index_contains_pack(bitmap, p))
+			continue;
+
 		if (open_pack_index(p)) {
 			pack_errors = 1;
 			continue;
@@ -357,6 +418,7 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
 
 out:
 	packed->skip_mru_updates = false;
+	free_bitmap_index(bitmap);
 
 	if (!ret && pack_errors)
 		ret = -1;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 5d2af96e2f..ac9da9545f 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2039,12 +2039,11 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
 	return -1;
 }
 
-static int can_filter_bitmap(const struct list_objects_filter_options *filter)
+bool can_filter_bitmap(const struct list_objects_filter_options *filter)
 {
 	return !filter_bitmap(NULL, NULL, NULL, filter);
 }
 
-
 static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
 					      struct bitmap *result)
 {
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 9f20fb6e56..1385027c1f 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -92,6 +92,9 @@ int test_bitmap_pseudo_merge_objects(struct repository *r, uint32_t n);
 
 struct list_objects_filter_options;
 
+/* Check whether the filter can be computed via the bitmap. */
+bool can_filter_bitmap(const struct list_objects_filter_options *filter);
+
 /*
  * Filter bitmapped objects and iterate through all resulting objects,
  * executing `show_reach` for each of them. Returns `-1` in case the filter is
-- 
2.55.0.175.ge4962bd3d5.dirty
Patrick SteinhardtJul 9, 2026, 08:35 UTC in reply to Patrick Steinhardt on lore

[PATCH 7/7] builtin/cat-file: filter objects via object database

Refactor git-cat-file(1) to use the new object filter option when batching all objects. This significantly simplifies the logic and ensures that we don't have to reach into internals of the "files" source anymore.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/cat-file.c | 76 +++++-------------------------------------------------
 1 file changed, 7 insertions(+), 69 deletions(-)
Show changes to builtin/cat-file.c +7 −69
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index b4b99a73da..1458dd76d6 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,7 +20,6 @@
 #include "userdiff.h"
 #include "oid-array.h"
 #include "packfile.h"
-#include "pack-bitmap.h"
 #include "object-file.h"
 #include "object-name.h"
 #include "odb.h"
@@ -844,28 +843,6 @@ static int batch_one_object_oi(const struct object_id *oid,
 	return payload->callback(oid, NULL, 0, payload->payload);
 }
 
-static int batch_one_object_packed(const struct object_id *oid,
-				   struct packed_git *pack,
-				   uint32_t pos,
-				   void *_payload)
-{
-	struct for_each_object_payload *payload = _payload;
-	return payload->callback(oid, pack, nth_packed_object_offset(pack, pos),
-				 payload->payload);
-}
-
-static int batch_one_object_bitmapped(const struct object_id *oid,
-				      enum object_type type UNUSED,
-				      int flags UNUSED,
-				      uint32_t hash UNUSED,
-				      struct packed_git *pack,
-				      off_t offset,
-				      void *_payload)
-{
-	struct for_each_object_payload *payload = _payload;
-	return payload->callback(oid, pack, offset, payload->payload);
-}
-
 static void batch_each_object(struct batch_options *opt,
 			      for_each_object_fn callback,
 			      unsigned flags,
@@ -875,56 +852,17 @@ static void batch_each_object(struct batch_options *opt,
 		.callback = callback,
 		.payload = _payload,
 	};
+	struct odb_source_info source_info;
+	struct object_info oi = {
+		.source_infop = &source_info,
+	};
 	struct odb_for_each_object_options opts = {
 		.flags = flags,
+		.filter = &opt->objects_filter,
 	};
-	struct bitmap_index *bitmap = NULL;
-	struct odb_source *source;
-
-	/*
-	 * TODO: we still need to tap into implementation details of the object
-	 * database sources. Ideally, we should extend `odb_for_each_object()`
-	 * to handle object filters itself so that we can move the filtering
-	 * logic into the individual sources.
-	 */
-	odb_prepare_alternates(the_repository->objects);
-	for (source = the_repository->objects->sources; source; source = source->next) {
-		struct odb_source_files *files = odb_source_files_downcast(source);
-		int ret = odb_source_for_each_object(&files->loose->base, NULL, batch_one_object_oi,
-						     &payload, &opts);
-		if (ret)
-			break;
-	}
-
-	if (opt->objects_filter.choice != LOFC_DISABLED &&
-	    (bitmap = prepare_bitmap_git(the_repository)) &&
-	    !for_each_bitmapped_object(bitmap, &opt->objects_filter,
-				       batch_one_object_bitmapped, &payload)) {
-		struct packed_git *pack;
-
-		repo_for_each_pack(the_repository, pack) {
-			if (bitmap_index_contains_pack(bitmap, pack) ||
-			    open_pack_index(pack))
-				continue;
-			for_each_object_in_pack(pack, batch_one_object_packed,
-						&payload, flags);
-		}
-	} else {
-		struct odb_source_info source_info;
-		struct object_info oi = {
-			.source_infop = &source_info,
-		};
-
-		for (source = the_repository->objects->sources; source; source = source->next) {
-			struct odb_source_files *files = odb_source_files_downcast(source);
-			int ret = odb_source_for_each_object(&files->packed->base, &oi,
-							     batch_one_object_oi, &payload, &opts);
-			if (ret)
-				break;
-		}
-	}
 
-	free_bitmap_index(bitmap);
+	odb_for_each_object_ext(the_repository->objects, &oi,
+				batch_one_object_oi, &payload, &opts);
 }
 
 static int batch_objects(struct batch_options *opt)
-- 
2.55.0.175.ge4962bd3d5.dirty
Junio C HamanoJul 9, 2026, 18:59 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH 7/7] builtin/cat-file: filter objects via object database

Patrick Steinhardt <ps@pks.im> writes:
> Refactor git-cat-file(1) to use the new object filter option when
> batching all objects. This significantly simplifies the logic and
> ensures that we don't have to reach into internals of the "files" source
> anymore.

This would become more convincing if you spent a few lines before presenting the solution to give an observation of what the current code does, e.g.,

    When batching all objects, git-cat-file(1) reaches into the
    internals of the object database and manually manages bitmaps to
    apply object filters. This creates coupling between the command
    and ODB backend internals.

to highlight the perceived problem in it. That would flow naturally to the description of your solution.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/cat-file.c | 76 +++++-------------------------------------------------
>  1 file changed, 7 insertions(+), 69 deletions(-)
Very nice.
Show 105 quoted lines
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index b4b99a73da..1458dd76d6 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -20,7 +20,6 @@
>  #include "userdiff.h"
>  #include "oid-array.h"
>  #include "packfile.h"
> -#include "pack-bitmap.h"
>  #include "object-file.h"
>  #include "object-name.h"
>  #include "odb.h"
> @@ -844,28 +843,6 @@ static int batch_one_object_oi(const struct object_id *oid,
>  	return payload->callback(oid, NULL, 0, payload->payload);
>  }
>  
> -static int batch_one_object_packed(const struct object_id *oid,
> -				   struct packed_git *pack,
> -				   uint32_t pos,
> -				   void *_payload)
> -{
> -	struct for_each_object_payload *payload = _payload;
> -	return payload->callback(oid, pack, nth_packed_object_offset(pack, pos),
> -				 payload->payload);
> -}
> -
> -static int batch_one_object_bitmapped(const struct object_id *oid,
> -				      enum object_type type UNUSED,
> -				      int flags UNUSED,
> -				      uint32_t hash UNUSED,
> -				      struct packed_git *pack,
> -				      off_t offset,
> -				      void *_payload)
> -{
> -	struct for_each_object_payload *payload = _payload;
> -	return payload->callback(oid, pack, offset, payload->payload);
> -}
> -
>  static void batch_each_object(struct batch_options *opt,
>  			      for_each_object_fn callback,
>  			      unsigned flags,
> @@ -875,56 +852,17 @@ static void batch_each_object(struct batch_options *opt,
>  		.callback = callback,
>  		.payload = _payload,
>  	};
> +	struct odb_source_info source_info;
> +	struct object_info oi = {
> +		.source_infop = &source_info,
> +	};
>  	struct odb_for_each_object_options opts = {
>  		.flags = flags,
> +		.filter = &opt->objects_filter,
>  	};
> -	struct bitmap_index *bitmap = NULL;
> -	struct odb_source *source;
> -
> -	/*
> -	 * TODO: we still need to tap into implementation details of the object
> -	 * database sources. Ideally, we should extend `odb_for_each_object()`
> -	 * to handle object filters itself so that we can move the filtering
> -	 * logic into the individual sources.
> -	 */
> -	odb_prepare_alternates(the_repository->objects);
> -	for (source = the_repository->objects->sources; source; source = source->next) {
> -		struct odb_source_files *files = odb_source_files_downcast(source);
> -		int ret = odb_source_for_each_object(&files->loose->base, NULL, batch_one_object_oi,
> -						     &payload, &opts);
> -		if (ret)
> -			break;
> -	}
> -
> -	if (opt->objects_filter.choice != LOFC_DISABLED &&
> -	    (bitmap = prepare_bitmap_git(the_repository)) &&
> -	    !for_each_bitmapped_object(bitmap, &opt->objects_filter,
> -				       batch_one_object_bitmapped, &payload)) {
> -		struct packed_git *pack;
> -
> -		repo_for_each_pack(the_repository, pack) {
> -			if (bitmap_index_contains_pack(bitmap, pack) ||
> -			    open_pack_index(pack))
> -				continue;
> -			for_each_object_in_pack(pack, batch_one_object_packed,
> -						&payload, flags);
> -		}
> -	} else {
> -		struct odb_source_info source_info;
> -		struct object_info oi = {
> -			.source_infop = &source_info,
> -		};
> -
> -		for (source = the_repository->objects->sources; source; source = source->next) {
> -			struct odb_source_files *files = odb_source_files_downcast(source);
> -			int ret = odb_source_for_each_object(&files->packed->base, &oi,
> -							     batch_one_object_oi, &payload, &opts);
> -			if (ret)
> -				break;
> -		}
> -	}
>  
> -	free_bitmap_index(bitmap);
> +	odb_for_each_object_ext(the_repository->objects, &oi,
> +				batch_one_object_oi, &payload, &opts);
>  }
>  
>  static int batch_objects(struct batch_options *opt)
Justin ToblerJul 9, 2026, 19:54 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH 1/7] odb/source-packed: improve lookup when enumerating objects

On 26/07/09 10:35AM, Patrick Steinhardt wrote:
Show 8 quoted lines
> When iterating through packed objects via `odb_for_each_object()` we
> do so via two different mechanisms:
> 
>   - When a multi-pack index is available we use that one to efficiently
>     loop through all objects.
> 
>   - We then loop through all packfiles that aren't covered by a
>     multi-pack index.

To be specific, we are talking only about the for_each_object callback for the packed source `odb_source_packed_for_each_object()` correct? Also, this appears to only matter when we are enumerating OIDs with a specific prefix.

Show 12 quoted lines
> Regardless of which mechanism we use, we then iterate through all the
> objects indexed by the respective data structure. Curiously though,
> while we use the indices for enumerating the objects, we completely
> ignore it for the actual object lookup. Instead, we call into the
> generic `odb_source_read_object_info()` function, which will itself
> consult the indices to figure out where the object in question even
> lives.
> 
> This has two consequences:
> 
>   - It's inefficient, as we basically have to figure out the position of
>     the object a second time.

Since we already have the position from the index, there is no need to start over. Makes sense.

>   - It's subtly wrong, as it may now happen that a specific object will
>     be looked up via a different pack in case it exists multiple times.

Naive question: Is there any real harm in reading the same object, but from a different packfile here?

Regardless I do think it's a good idea to just reuse the same packfile to get the same object here.

Show 35 quoted lines
> Fix the issue by using `packed_object_info()` directly. While at it,
> rename the `store` variable to `source`.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb/source-packed.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/odb/source-packed.c b/odb/source-packed.c
> index 0edea5356d..9cfa02b7a2 100644
> --- a/odb/source-packed.c
> +++ b/odb/source-packed.c
> @@ -143,7 +143,7 @@ static bool should_exclude_pack(struct packed_git *p, enum odb_for_each_object_f
>  }
>  
>  static int for_each_prefixed_object_in_midx(
> -	struct odb_source_packed *store,
> +	struct odb_source_packed *source,
>  	struct multi_pack_index *m,
>  	const struct odb_for_each_object_options *opts,
>  	struct odb_source_packed_for_each_object_wrapper_data *data)
> @@ -170,6 +170,7 @@ static int for_each_prefixed_object_in_midx(
>  		 */
>  		for (i = first; i < num; i++) {
>  			const struct object_id *current = NULL;
> +			struct packed_git *pack;
>  			struct object_id oid;
>  
>  			current = nth_midxed_object_oid(&oid, m, i);
> @@ -177,9 +178,8 @@ static int for_each_prefixed_object_in_midx(
>  			if (!match_hash(len, opts->prefix->hash, current->hash))
>  				break;
>  
> -			if (opts->flags) {
> +			if (opts->flags || data->request) {
I'm not sure I follow why the above condition needed to change.
Show 14 quoted lines
>  				uint32_t pack_id = nth_midxed_pack_int_id(m, i);
> -				struct packed_git *pack;
>  
>  				if (prepare_midx_pack(m, pack_id)) {
>  					pack_errors = true;
> @@ -193,9 +193,9 @@ static int for_each_prefixed_object_in_midx(
>  
>  			if (data->request) {
>  				struct object_info oi = *data->request;
> +				off_t offset = nth_midxed_offset(m, i);
>  
> -				ret = odb_source_read_object_info(&store->base, current,
> -								  &oi, 0);
> +				ret = packed_object_info(source, pack, offset, &oi);

We not longer use the generic function to read object info. This ensures the exact same object is read.

Show 20 quoted lines
>  				if (ret)
>  					goto out;
>  
> @@ -219,7 +219,7 @@ static int for_each_prefixed_object_in_midx(
>  }
>  
>  static int for_each_prefixed_object_in_pack(
> -	struct odb_source_packed *store,
> +	struct odb_source_packed *source,
>  	struct packed_git *p,
>  	const struct odb_for_each_object_options *opts,
>  	struct odb_source_packed_for_each_object_wrapper_data *data)
> @@ -246,8 +246,9 @@ static int for_each_prefixed_object_in_pack(
>  
>  		if (data->request) {
>  			struct object_info oi = *data->request;
> +			off_t offset = nth_packed_object_offset(p, i);
>  
> -			ret = odb_source_read_object_info(&store->base, &oid, &oi, 0);
> +			ret = packed_object_info(source, p, offset, &oi);
And we do the same thing here when reading the object from a packfile.
-Justin
Justin ToblerJul 9, 2026, 20:19 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH 3/7] pack-bitmap: allow aborting iteration of bitmapped objects

On 26/07/09 10:35AM, Patrick Steinhardt wrote:
Show 6 quoted lines
> In a subsequent commit we'll lift iteration of bitmapped objects into
> the "packed" backend and make it accessible via `odb_for_each_object()`.
> The calling convention for that function is that the callback may return
> a non-zero exit code, and if so we'll abort iteration. This is currently
> impossible to realize though, as `for_each_bitmapped_object()` will
> ignore any return value and just churn through all objects completely.
Ok.
Show 9 quoted lines
> This doesn't matter to the callers of `for_each_bitmapped_object()`, as
> there's only one of them in git-cat-file(1), and the callbacks we pass
> always return zero. But once we move the logic into the generic
> infrastructure it becomes a latent bug waiting to happen.
> 
> Refactor the code so that the return value of the `show_reach` callback
> is not ignored anymore. Instead, returning a non-zero value will cause
> us to abort iteration in both `show_objects_for_type()` and in
> `for_each_bitmapped_object()`.

Make sense. We want to ensure that the `show_reach` callback can properly signal back to `for_each_bitmapped_object()` to abort.

Show 25 quoted lines
> Note though that there's a second user of `show_objects_for_type()` with
> `traverse_bitmap_commit_list()`, and that function does indeed invoke
> callbacks that may return non-zero. This non-zero return value never had
> any effect at all though, and the callbacks that return non-zero values
> are only ever invoked via `traverse_bitmap_commit_list()`. Consequently,
> we adapt them to always return 0.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/pack-objects.c |  2 +-
>  builtin/rev-list.c     |  2 +-
>  pack-bitmap.c          | 31 +++++++++++++++++++++----------
>  pack-bitmap.h          |  3 ++-
>  4 files changed, 25 insertions(+), 13 deletions(-)
> 
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index ea5eab4cf8..8ff92c5272 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1909,7 +1909,7 @@ static int add_object_entry_from_bitmap(const struct object_id *oid,
>  		return 0;
>  
>  	create_object_entry(oid, type, name_hash, 0, 0, pack, offset);
> -	return 1;
> +	return 0;

I wonder why this was even returning 1 to begin with? As you mentioned, the return value appears to be ignored anyways. I'm assuming it was signal that an object entry was created?

Show 13 quoted lines
>  }
>  
>  struct pbase_tree_cache {
> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 8f63003709..02818b81c6 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -486,7 +486,7 @@ static int show_object_fast(
>  	void *payload UNUSED)
>  {
>  	fprintf(stdout, "%s\n", oid_to_hex(oid));
> -	return 1;
> +	return 0;
Also curious about this one too. It probably doesn't matter though.
Show 32 quoted lines
>  }
>  
>  static void print_disk_usage(off_t size)
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index a47c231632..eda38a5433 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -1695,7 +1695,7 @@ static void init_type_iterator(struct ewah_or_iterator *it,
>  	}
>  }
>  
> -static void show_objects_for_type(
> +static int show_objects_for_type(
>  	struct bitmap_index *bitmap_git,
>  	struct bitmap *objects,
>  	enum object_type object_type,
> @@ -1704,6 +1704,7 @@ static void show_objects_for_type(
>  {
>  	size_t i = 0;
>  	uint32_t offset;
> +	int ret;
>  
>  	struct ewah_or_iterator it;
>  	eword_t filter;
> @@ -1749,11 +1750,17 @@ static void show_objects_for_type(
>  
>  			hash = bitmap_name_hash(bitmap_git, index_pos);
>  
> -			show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
> +			ret = show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
> +			if (ret)
> +				goto out;
The show_reach callback now wires back its return code.
Show 42 quoted lines
>  		}
>  	}
>  
> +	ret = 0;
> +
> +out:
>  	ewah_or_iterator_release(&it);
> +	return ret;
>  }
>  
>  static int in_bitmapped_pack(struct bitmap_index *bitmap_git,
> @@ -2062,6 +2069,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
>  			      show_reachable_fn show_reach,
>  			      void *payload)
>  {
> +	const enum object_type types[] = {
> +		OBJ_COMMIT,
> +		OBJ_TREE,
> +		OBJ_BLOB,
> +		OBJ_TAG,
> +	};
>  	struct bitmap *filtered_bitmap = NULL;
>  	uint32_t objects_nr;
>  	size_t full_word_count;
> @@ -2086,14 +2099,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
>  		goto out;
>  	}
>  
> -	show_objects_for_type(bitmap_git, filtered_bitmap,
> -			      OBJ_COMMIT, show_reach, payload);
> -	show_objects_for_type(bitmap_git, filtered_bitmap,
> -			      OBJ_TREE, show_reach, payload);
> -	show_objects_for_type(bitmap_git, filtered_bitmap,
> -			      OBJ_BLOB, show_reach, payload);
> -	show_objects_for_type(bitmap_git, filtered_bitmap,
> -			      OBJ_TAG, show_reach, payload);
> +	for (size_t i = 0; i < ARRAY_SIZE(types); i++) {
> +		ret = show_objects_for_type(bitmap_git, filtered_bitmap,
> +					    types[i], show_reach, payload);
> +		if (ret)
> +			goto out;
> +	}

`for_each_bitmapped_object()` now has access to the underlying return code and can abort. Looks good.

-Justin
Justin ToblerJul 9, 2026, 21:08 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH 4/7] pack-bitmap: iterate object sources when opening bitmaps

On 26/07/09 10:35AM, Patrick Steinhardt wrote:
Show 7 quoted lines
> When opening a bitmap for a repository we perform two steps:
> 
>   - We first look for a multi-pack index bitmap in any of the object
>     sources connected to the repository.
> 
>   - We then look for a packfile bitmap in any of the packfiles of any of
>     the object sources.
So IIUC, we generally stop searching for a bitmap once we find one.
Show 9 quoted lines
> Both of these steps thus iterate through object sources themselves, one
> via `odb_prepare_alternates()` and one via `repo_for_each_pack()`. This
> layout makes it hard to introduce a way to open the bitmap of one
> specific object source, which is functionality that we'll require in a
> subsequent commit.
> 
> Reverse the loop so that we instead loop through all sources in the
> outer loop, and then for each source we try to load its bitmap via
> either the multi-pack index or via a packfile.
Conceptually, I think this is a lot easier to follow too which is nice.
Show 10 quoted lines
> Note that this changes the precedence of bitmaps in one specific edge
> case: when an earlier object source only has a packfile bitmap, but a
> later source has a multi-pack index bitmap, we now pick the packfile
> bitmap of the earlier source. Previously, a multi-pack index bitmap from
> any source would have taken precedence over all packfile bitmaps. Given
> that object sources are ordered such that the local source comes first,
> this arguably is an improvement, as we now prefer local bitmaps over
> bitmaps in alternates. Furthermore, we already warn about repositories
> that have multiple bitmaps, so this setup is broken and thus arguably
> not worth worrying about too much.

I agree that the change in bitmap precedent is probably not a big deal. Having multiple bitmaps in a repository is already something we warn against so I think this should be fine.

Show 35 quoted lines
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  pack-bitmap.c | 65 ++++++++++++++++++++++++++---------------------------------
>  1 file changed, 29 insertions(+), 36 deletions(-)
> 
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index eda38a5433..0e3e18a557 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -680,60 +680,53 @@ static int load_bitmap(struct repository *r, struct bitmap_index *bitmap_git,
>  	return 0;
>  }
>  
> -static int open_pack_bitmap(struct repository *r,
> -			    struct bitmap_index *bitmap_git)
> +static int open_bitmap_for_source(struct odb_source_packed *source,
> +				  struct bitmap_index *bitmap_git)
>  {
> -	struct packed_git *p;
> +	struct multi_pack_index *midx = get_multi_pack_index(source);
> +	struct packfile_list_entry *e;
>  	int ret = -1;
>  
> -	repo_for_each_pack(r, p) {
> -		if (open_pack_bitmap_1(bitmap_git, p) == 0) {
> -			ret = 0;
> -			/*
> -			 * The only reason to keep looking is to report
> -			 * duplicates.
> -			 */
> -			if (!trace2_is_enabled())
> -				break;
> -		}
> +	if (midx && !open_midx_bitmap_1(bitmap_git, midx))
> +		ret = 0;

Ok, open_midx_bitmap_1() returns 0 if it find a MIDX and -1 otherwise. Probably just a matter of preference, but I think writing out like below is a little bit easier on the eyes:

  if (midx)
    ret = open_midx_bitmap_1(bitmap_git, midx);

it might just be that I find the return values a bit confusing though. Maybe we could instead use `found` like a bit later in this patch.

Show 8 quoted lines
> +
> +	for (e = packfile_store_get_packs(source); e; e = e->next) {
> +		/*
> +		 * When tracing is enabled we want to keep looking to report
> +		 * duplicates even if we have already found a bitmap.
> +		 */
> +		if (!ret && !trace2_is_enabled())
> +			break;

So if have already found a bitmap from the MIDX and tracing is not enabled, we don't continue searching for bitmaps in this source.

Show 53 quoted lines
> +
> +		if (open_pack_bitmap_1(bitmap_git, e->pack))
> +			continue;
> +		ret = 0;
>  	}
>  
>  	return ret;
>  }
>  
> -static int open_midx_bitmap(struct repository *r,
> -			    struct bitmap_index *bitmap_git)
> +static int open_bitmap(struct repository *r,
> +		       struct bitmap_index *bitmap_git)
>  {
>  	struct odb_source *source;
> -	int ret = -1;
> +	int found = 0;
>  
>  	assert(!bitmap_git->map);
>  
>  	odb_prepare_alternates(r->objects);
>  	for (source = r->objects->sources; source; source = source->next) {
>  		struct odb_source_files *files = odb_source_files_downcast(source);
> -		struct multi_pack_index *midx = get_multi_pack_index(files->packed);
> -		if (midx && !open_midx_bitmap_1(bitmap_git, midx))
> -			ret = 0;
> -	}
> -	return ret;
> -}
> -
> -static int open_bitmap(struct repository *r,
> -		       struct bitmap_index *bitmap_git)
> -{
> -	int found;
>  
> -	assert(!bitmap_git->map);
> +		found |= !open_bitmap_for_source(files->packed, bitmap_git);
>  
> -	found = !open_midx_bitmap(r, bitmap_git);
> -
> -	/*
> -	 * these will all be skipped if we opened a midx bitmap; but run it
> -	 * anyway if tracing is enabled to report the duplicates
> -	 */
> -	if (!found || trace2_is_enabled())
> -		found |= !open_pack_bitmap(r, bitmap_git);
> +		/*
> +		 * The only reason to keep looking after having found a bitmap
> +		 * is to report duplicates.
> +		 */
> +		if (found && !trace2_is_enabled())
> +			break;
> +	}

Ok, we only advance to the next source if tracing is enabled to print warnings for multiple bitmaps. Makes sense.

Overall I quite like the direction of this patch.
-Justin
Justin ToblerJul 9, 2026, 21:43 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH 6/7] odb: introduce object filters to `odb_for_each_object()`

On 26/07/09 10:35AM, Patrick Steinhardt wrote:
Show 19 quoted lines
> The function `for_each_bitmapped_object()` can be used to iterate
> through all objects covered by a bitmap. The benefit of this function is
> that it allows the caller to efficiently handle some object filters. For
> example, this can be used to filter out objects of a specific type with
> some simple bitmap operations. But callers are currently required to
> manually wire up the use of bitmaps though, and to do so they have to
> reach into internals of a given object database source.
> 
> Introduce a new `struct odb_for_each_object_options::filter` field so
> that the interface becomes generic. When set, then a backend may
> optionally use the filter to skip some objects that it would have
> otherwise yielded.
> 
> Note that the respective backends are free to ignore this field if they
> cannot meaningfully optimize for a given filter, and consequently
> callers need to verify whether they actually want the returned objects.
> While annoying, we cannot easily lift this restriction anyway as the
> object filter infrastructure supports some filters that cannot be
> answered by the object database alone.

Huh, this feels rather awkward. So callers will always still have to ensure correctness by filtering the result a second time? IIUC, the idea is that the backend may be able to more efficiently process object filtering so we would want it to attempt the first pass.

Is there a subset of object filters that we should expect any backend to be able to answer? If so, maybe we should define a separate list of object filter options specific to this interface? Any filtering not supported would have to be deligated to the caller then.

-Justin
Patrick SteinhardtJul 10, 2026, 07:08 UTC in reply to Justin Tobler on lore

Re: [PATCH 1/7] odb/source-packed: improve lookup when enumerating objects

On Thu, Jul 09, 2026 at 02:54:17PM -0500, Justin Tobler wrote:
Show 14 quoted lines
> On 26/07/09 10:35AM, Patrick Steinhardt wrote:
> > When iterating through packed objects via `odb_for_each_object()` we
> > do so via two different mechanisms:
> > 
> >   - When a multi-pack index is available we use that one to efficiently
> >     loop through all objects.
> > 
> >   - We then loop through all packfiles that aren't covered by a
> >     multi-pack index.
> 
> To be specific, we are talking only about the for_each_object callback
> for the packed source `odb_source_packed_for_each_object()` correct?
> Also, this appears to only matter when we are enumerating OIDs with a
> specific prefix.
Yeah, true. I'll clarify this a bit.
Show 21 quoted lines
> > Regardless of which mechanism we use, we then iterate through all the
> > objects indexed by the respective data structure. Curiously though,
> > while we use the indices for enumerating the objects, we completely
> > ignore it for the actual object lookup. Instead, we call into the
> > generic `odb_source_read_object_info()` function, which will itself
> > consult the indices to figure out where the object in question even
> > lives.
> > 
> > This has two consequences:
> > 
> >   - It's inefficient, as we basically have to figure out the position of
> >     the object a second time.
> 
> Since we already have the position from the index, there is no need to
> start over. Makes sense.
> 
> >   - It's subtly wrong, as it may now happen that a specific object will
> >     be looked up via a different pack in case it exists multiple times.
> 
> Naive question: Is there any real harm in reading the same object, but
> from a different packfile here?

The answer is probably "no". At least I cannot think of any case where it'd really matter, but semantically it's the wrong thing to do anyway.

Show 12 quoted lines
> > diff --git a/odb/source-packed.c b/odb/source-packed.c
> > index 0edea5356d..9cfa02b7a2 100644
> > --- a/odb/source-packed.c
> > +++ b/odb/source-packed.c
> > @@ -177,9 +178,8 @@ static int for_each_prefixed_object_in_midx(
> >  			if (!match_hash(len, opts->prefix->hash, current->hash))
> >  				break;
> >  
> > -			if (opts->flags) {
> > +			if (opts->flags || data->request) {
> 
> I'm not sure I follow why the above condition needed to change.

This needs to change because we now require access to the pack so that we can call `packed_object_info()`. Otherwise the pack would be not be populated if we're invoked without any flags.

Patrick
Patrick SteinhardtJul 10, 2026, 07:08 UTC in reply to Justin Tobler on lore

Re: [PATCH 3/7] pack-bitmap: allow aborting iteration of bitmapped objects

On Thu, Jul 09, 2026 at 03:19:52PM -0500, Justin Tobler wrote:
Show 14 quoted lines
> > diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> > index ea5eab4cf8..8ff92c5272 100644
> > --- a/builtin/pack-objects.c
> > +++ b/builtin/pack-objects.c
> > @@ -1909,7 +1909,7 @@ static int add_object_entry_from_bitmap(const struct object_id *oid,
> >  		return 0;
> >  
> >  	create_object_entry(oid, type, name_hash, 0, 0, pack, offset);
> > -	return 1;
> > +	return 0;
> 
> I wonder why this was even returning 1 to begin with? As you mentioned,
> the return value appears to be ignored anyways. I'm assuming it was
> signal that an object entry was created?

The function is only called from a single location, and the return value was completely ignored until this commit. It has always been this way since the function was originally introduced in 6b8fda2db1 (pack-objects: use bitmaps when packing objects, 2013-12-21), so it never seemed to have any purpose. The commit message doesn't mention anything either.

Show 12 quoted lines
> > diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> > index 8f63003709..02818b81c6 100644
> > --- a/builtin/rev-list.c
> > +++ b/builtin/rev-list.c
> > @@ -486,7 +486,7 @@ static int show_object_fast(
> >  	void *payload UNUSED)
> >  {
> >  	fprintf(stdout, "%s\n", oid_to_hex(oid));
> > -	return 1;
> > +	return 0;
> 
> Also curious about this one too. It probably doesn't matter though.

Likewise, this was introduced in aa32939fea (rev-list: add bitmap mode to speed up object lists, 2013-12-21), and the return value wasn't ever used for anything.

Patrick
Patrick SteinhardtJul 10, 2026, 07:08 UTC in reply to Justin Tobler on lore

Re: [PATCH 4/7] pack-bitmap: iterate object sources when opening bitmaps

On Thu, Jul 09, 2026 at 04:08:31PM -0500, Justin Tobler wrote:
Show 10 quoted lines
> On 26/07/09 10:35AM, Patrick Steinhardt wrote:
> > When opening a bitmap for a repository we perform two steps:
> > 
> >   - We first look for a multi-pack index bitmap in any of the object
> >     sources connected to the repository.
> > 
> >   - We then look for a packfile bitmap in any of the packfiles of any of
> >     the object sources.
> 
> So IIUC, we generally stop searching for a bitmap once we find one.

Except that we continue searching so that we can print a warning, but later results are simply being ignored.

Show 40 quoted lines
> > diff --git a/pack-bitmap.c b/pack-bitmap.c
> > index eda38a5433..0e3e18a557 100644
> > --- a/pack-bitmap.c
> > +++ b/pack-bitmap.c
> > @@ -680,60 +680,53 @@ static int load_bitmap(struct repository *r, struct bitmap_index *bitmap_git,
> >  	return 0;
> >  }
> >  
> > -static int open_pack_bitmap(struct repository *r,
> > -			    struct bitmap_index *bitmap_git)
> > +static int open_bitmap_for_source(struct odb_source_packed *source,
> > +				  struct bitmap_index *bitmap_git)
> >  {
> > -	struct packed_git *p;
> > +	struct multi_pack_index *midx = get_multi_pack_index(source);
> > +	struct packfile_list_entry *e;
> >  	int ret = -1;
> >  
> > -	repo_for_each_pack(r, p) {
> > -		if (open_pack_bitmap_1(bitmap_git, p) == 0) {
> > -			ret = 0;
> > -			/*
> > -			 * The only reason to keep looking is to report
> > -			 * duplicates.
> > -			 */
> > -			if (!trace2_is_enabled())
> > -				break;
> > -		}
> > +	if (midx && !open_midx_bitmap_1(bitmap_git, midx))
> > +		ret = 0;
> 
> Ok, open_midx_bitmap_1() returns 0 if it find a MIDX and -1 otherwise.
> Probably just a matter of preference, but I think writing out like below
> is a little bit easier on the eyes:
> 
>   if (midx)
>     ret = open_midx_bitmap_1(bitmap_git, midx);
> 
> it might just be that I find the return values a bit confusing though.
> Maybe we could instead use `found` like a bit later in this patch.

That's fair, and the result would be both easier to reason about and more consistent indeed. I'll adapt this accordingly.

Patrick
Patrick SteinhardtJul 10, 2026, 07:09 UTC in reply to Justin Tobler on lore

Re: [PATCH 6/7] odb: introduce object filters to `odb_for_each_object()`

On Thu, Jul 09, 2026 at 04:43:58PM -0500, Justin Tobler wrote:
Show 30 quoted lines
> On 26/07/09 10:35AM, Patrick Steinhardt wrote:
> > The function `for_each_bitmapped_object()` can be used to iterate
> > through all objects covered by a bitmap. The benefit of this function is
> > that it allows the caller to efficiently handle some object filters. For
> > example, this can be used to filter out objects of a specific type with
> > some simple bitmap operations. But callers are currently required to
> > manually wire up the use of bitmaps though, and to do so they have to
> > reach into internals of a given object database source.
> > 
> > Introduce a new `struct odb_for_each_object_options::filter` field so
> > that the interface becomes generic. When set, then a backend may
> > optionally use the filter to skip some objects that it would have
> > otherwise yielded.
> > 
> > Note that the respective backends are free to ignore this field if they
> > cannot meaningfully optimize for a given filter, and consequently
> > callers need to verify whether they actually want the returned objects.
> > While annoying, we cannot easily lift this restriction anyway as the
> > object filter infrastructure supports some filters that cannot be
> > answered by the object database alone.
> 
> Huh, this feels rather awkward. So callers will always still have to
> ensure correctness by filtering the result a second time? IIUC, the idea
> is that the backend may be able to more efficiently process object
> filtering so we would want it to attempt the first pass.
> 
> Is there a subset of object filters that we should expect any backend to
> be able to answer? If so, maybe we should define a separate list of
> object filter options specific to this interface? Any filtering not
> supported would have to be deligated to the caller then.

It's a bit awkward, but it's also similar to how we handle this for example in the reference backends with the exclude patterns. I don't really think it makes sense to enforce that backends may only handle a subset of object filters that we know the current backends support, as that would artificially limit us.

For example, the "loose" backend already cannot efficiently handle many of the filters that the "packed" backend can handle, like for example filtering by type. So ultimately, the subset of filters that can be handled efficiently by both backends is empty. And as the "files" backend always combines both of these backends we wouldn't be able to ever use the object filter at all there.

The same could be true for any future backend: we cannot assume how they store their objects, so they might be able to efficiently handle filters that the current backends cannot.

An alternative going forward could be to perform filtering of yielded objects inside `odb_for_each_object()` itself so that it will filter out any objects that the backends themselves couldn't filter efficiently. But I'm not sure I want to go there as part of this series -- we only have a single caller anyway that iterates with a filter, and that caller already knows to manually filter references.

I'll add a bit of an explanation to the commit message.
Patrick
Patrick SteinhardtJul 10, 2026, 07:09 UTC in reply to Junio C Hamano on lore

Re: [PATCH 7/7] builtin/cat-file: filter objects via object database

On Thu, Jul 09, 2026 at 11:59:29AM -0700, Junio C Hamano wrote:
Show 18 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Refactor git-cat-file(1) to use the new object filter option when
> > batching all objects. This significantly simplifies the logic and
> > ensures that we don't have to reach into internals of the "files" source
> > anymore.
> 
> This would become more convincing if you spent a few lines before
> presenting the solution to give an observation of what the current
> code does, e.g.,
> 
>     When batching all objects, git-cat-file(1) reaches into the
>     internals of the object database and manually manages bitmaps to
>     apply object filters. This creates coupling between the command
>     and ODB backend internals.
> 
> to highlight the perceived problem in it.  That would flow naturally
> to the description of your solution.
Good point, will add.
Patrick
Patrick SteinhardtJul 10, 2026, 08:48 UTC in reply to Patrick Steinhardt on lore

[PATCH v2 0/8] odb: introduce object filters to `odb_for_each_object()`

Hi,

this patch series introduces object filters to `odb_for_each_object()`. The intent of this is to make `git cat-file --batch-all-objects` work with pluggable object databases. Right now it doesn't because it reaches into internals of the "packed" backend to efficiently handle bitmapped objects.

The series is built on top of f85a7e6620 (Start Git 2.56 cycle, 2026-07-06) with ps/odb-drop-whence at 8a7ad23e11 (odb: document object info fields, 2026-07-02) merged into it.

Changes in v2:
  - Add another patch to drop the `_1()` prefixes that aren't required
    anymore.
  - Change the approach in `open_bitmap_for_source()` to also use a
    `found` boolean instead of a confusing integer.
  - Add some more explanations to commit messages.
  - Link to v1: https://patch.msgid.link/20260709-pks-odb-for-each-object-filter-v1-0-82fe014b12b3@pks.im
Thanks!
Patrick
---
Patrick Steinhardt (8):
      odb/source-packed: improve lookup when enumerating objects
      pack-bitmap: mark object filter as `const`
      pack-bitmap: allow aborting iteration of bitmapped objects
      pack-bitmap: iterate object sources when opening bitmaps
      pack-bitmap: drop `_1` suffix from functions that open bitmaps
      pack-bitmap: introduce function to open bitmap for a single source
      odb: introduce object filters to `odb_for_each_object()`
      builtin/cat-file: filter objects via object database
 builtin/cat-file.c     |  76 +++--------------------------
 builtin/pack-objects.c |   2 +-
 builtin/rev-list.c     |   2 +-
 odb.h                  |  12 +++++
 odb/source-packed.c    |  77 ++++++++++++++++++++++++++---
 pack-bitmap.c          | 129 +++++++++++++++++++++++++++----------------------
 pack-bitmap.h          |  10 +++-
 7 files changed, 171 insertions(+), 137 deletions(-)
Range-diff versus v1:
1:  7a1a92acbe ! 1:  b675967b78 odb/source-packed: improve lookup when enumerating objects
    @@ Metadata
      ## Commit message ##
         odb/source-packed: improve lookup when enumerating objects
     
    -    When iterating through packed objects via `odb_for_each_object()` we
    -    do so via two different mechanisms:
    +    When iterating through objects of a packed source that have a specific
    +    prefix we do so via two different methods:
     
           - When a multi-pack index is available we use that one to efficiently
             loop through all objects.
    @@ Commit message
     
           - It's subtly wrong, as it may now happen that a specific object will
             be looked up via a different pack in case it exists multiple times.
    +        This is unlikely to have any real-world consequences, but it's still
    +        the wrong thing to do.
     
         Fix the issue by using `packed_object_info()` directly. While at it,
         rename the `store` variable to `source`.
2:  a6c8bd7a61 = 2:  d3f9b2f781 pack-bitmap: mark object filter as `const`
3:  c38b06636b = 3:  825920205a pack-bitmap: allow aborting iteration of bitmapped objects
4:  450cdd13b7 ! 4:  a33ca8fa3b pack-bitmap: iterate object sources when opening bitmaps
    @@ pack-bitmap.c: static int load_bitmap(struct repository *r, struct bitmap_index
     +				  struct bitmap_index *bitmap_git)
      {
     -	struct packed_git *p;
    +-	int ret = -1;
     +	struct multi_pack_index *midx = get_multi_pack_index(source);
     +	struct packfile_list_entry *e;
    - 	int ret = -1;
    ++	bool found = false;
      
     -	repo_for_each_pack(r, p) {
     -		if (open_pack_bitmap_1(bitmap_git, p) == 0) {
    @@ pack-bitmap.c: static int load_bitmap(struct repository *r, struct bitmap_index
     -				break;
     -		}
     +	if (midx && !open_midx_bitmap_1(bitmap_git, midx))
    -+		ret = 0;
    ++		found = true;
     +
     +	for (e = packfile_store_get_packs(source); e; e = e->next) {
     +		/*
     +		 * When tracing is enabled we want to keep looking to report
     +		 * duplicates even if we have already found a bitmap.
     +		 */
    -+		if (!ret && !trace2_is_enabled())
    ++		if (found && !trace2_is_enabled())
     +			break;
     +
    -+		if (open_pack_bitmap_1(bitmap_git, e->pack))
    -+			continue;
    -+		ret = 0;
    ++		if (!open_pack_bitmap_1(bitmap_git, e->pack))
    ++			found = true;
      	}
      
    - 	return ret;
    +-	return ret;
    ++	return found ? 0 : -1;
      }
      
     -static int open_midx_bitmap(struct repository *r,
    @@ pack-bitmap.c: static int load_bitmap(struct repository *r, struct bitmap_index
      {
      	struct odb_source *source;
     -	int ret = -1;
    -+	int found = 0;
    ++	bool found = false;
      
      	assert(!bitmap_git->map);
      
    @@ pack-bitmap.c: static int load_bitmap(struct repository *r, struct bitmap_index
     -	int found;
      
     -	assert(!bitmap_git->map);
    -+		found |= !open_bitmap_for_source(files->packed, bitmap_git);
    ++		if (!open_bitmap_for_source(files->packed, bitmap_git))
    ++			found = true;
      
     -	found = !open_midx_bitmap(r, bitmap_git);
     -
-:  ---------- > 5:  b890ed7163 pack-bitmap: drop `_1` suffix from functions that open bitmaps
5:  26b1957f8b = 6:  f7e466217b pack-bitmap: introduce function to open bitmap for a single source
6:  722727c76d ! 7:  27ecc0802f odb: introduce object filters to `odb_for_each_object()`
    @@ Commit message
         object filter infrastructure supports some filters that cannot be
         answered by the object database alone.
     
    +    An alternative might be to limit the filters to only those that _can_ be
    +    answered by backends. But ultimately, the filters that can be answered
    +    efficiently by the "packed" backend are completely disjunct from those
    +    that can be answered by the "loose" backend, and consequently the set of
    +    filters supported by all backends would be empty. Furthermore, it would
    +    require us to make assumptions about capabilities of future backends,
    +    which may be able to efficiently handle more filters than current ones.
    +    So in the end, this alternative would only limit us artificially.
    +
         Implement the logic for the "packed" source. Note that we use the new
         function `prepare_source_bitmap_git()` to open the bitmap: as the
         backend operates on a single object source, we must only use bitmaps
7:  90be28e904 ! 8:  e4c6aeab0a builtin/cat-file: filter objects via object database
    @@ Metadata
      ## Commit message ##
         builtin/cat-file: filter objects via object database
     
    +    When batching all objects, git-cat-file(1) reaches into the internals of
    +    the object database and manually manages bitmaps to apply object
    +    filters. This creates coupling between the command and the internals of
    +    the respective backend.
    +
         Refactor git-cat-file(1) to use the new object filter option when
         batching all objects. This significantly simplifies the logic and
         ensures that we don't have to reach into internals of the "files" source

--- base-commit: 3c8e2790f2ce15e8b5d4b4e6ced711b12649f32a change-id: 20260708-pks-odb-for-each-object-filter-13286fa3523d

Patrick SteinhardtJul 10, 2026, 08:48 UTC in reply to Patrick Steinhardt on lore

[PATCH v2 1/8] odb/source-packed: improve lookup when enumerating objects

When iterating through objects of a packed source that have a specific prefix we do so via two different methods:

  - When a multi-pack index is available we use that one to efficiently
    loop through all objects.
  - We then loop through all packfiles that aren't covered by a
    multi-pack index.

Regardless of which mechanism we use, we then iterate through all the objects indexed by the respective data structure. Curiously though, while we use the indices for enumerating the objects, we completely ignore it for the actual object lookup. Instead, we call into the generic `odb_source_read_object_info()` function, which will itself consult the indices to figure out where the object in question even lives.

This has two consequences:
  - It's inefficient, as we basically have to figure out the position of
    the object a second time.
  - It's subtly wrong, as it may now happen that a specific object will
    be looked up via a different pack in case it exists multiple times.
    This is unlikely to have any real-world consequences, but it's still
    the wrong thing to do.

Fix the issue by using `packed_object_info()` directly. While at it, rename the `store` variable to `source`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-packed.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
Show changes to odb/source-packed.c +8 −7
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 0edea5356d..9cfa02b7a2 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -143,7 +143,7 @@ static bool should_exclude_pack(struct packed_git *p, enum odb_for_each_object_f
 }
 
 static int for_each_prefixed_object_in_midx(
-	struct odb_source_packed *store,
+	struct odb_source_packed *source,
 	struct multi_pack_index *m,
 	const struct odb_for_each_object_options *opts,
 	struct odb_source_packed_for_each_object_wrapper_data *data)
@@ -170,6 +170,7 @@ static int for_each_prefixed_object_in_midx(
 		 */
 		for (i = first; i < num; i++) {
 			const struct object_id *current = NULL;
+			struct packed_git *pack;
 			struct object_id oid;
 
 			current = nth_midxed_object_oid(&oid, m, i);
@@ -177,9 +178,8 @@ static int for_each_prefixed_object_in_midx(
 			if (!match_hash(len, opts->prefix->hash, current->hash))
 				break;
 
-			if (opts->flags) {
+			if (opts->flags || data->request) {
 				uint32_t pack_id = nth_midxed_pack_int_id(m, i);
-				struct packed_git *pack;
 
 				if (prepare_midx_pack(m, pack_id)) {
 					pack_errors = true;
@@ -193,9 +193,9 @@ static int for_each_prefixed_object_in_midx(
 
 			if (data->request) {
 				struct object_info oi = *data->request;
+				off_t offset = nth_midxed_offset(m, i);
 
-				ret = odb_source_read_object_info(&store->base, current,
-								  &oi, 0);
+				ret = packed_object_info(source, pack, offset, &oi);
 				if (ret)
 					goto out;
 
@@ -219,7 +219,7 @@ static int for_each_prefixed_object_in_midx(
 }
 
 static int for_each_prefixed_object_in_pack(
-	struct odb_source_packed *store,
+	struct odb_source_packed *source,
 	struct packed_git *p,
 	const struct odb_for_each_object_options *opts,
 	struct odb_source_packed_for_each_object_wrapper_data *data)
@@ -246,8 +246,9 @@ static int for_each_prefixed_object_in_pack(
 
 		if (data->request) {
 			struct object_info oi = *data->request;
+			off_t offset = nth_packed_object_offset(p, i);
 
-			ret = odb_source_read_object_info(&store->base, &oid, &oi, 0);
+			ret = packed_object_info(source, p, offset, &oi);
 			if (ret)
 				goto out;
 
-- 
2.55.0.229.g6434b31f56.dirty
Patrick SteinhardtJul 10, 2026, 08:48 UTC in reply to Patrick Steinhardt on lore

[PATCH v2 2/8] pack-bitmap: mark object filter as `const`

The function `for_each_bitmapped_object()` accepts an optional object filter. This filter is never modified by the function, but is not declared as `const`. Fix this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pack-bitmap.c | 6 +++---
 pack-bitmap.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)
Show changes to 2 files +4 −4

pack-bitmap.c, pack-bitmap.h

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 35774b6f0c..a47c231632 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1976,7 +1976,7 @@ static void filter_bitmap_object_type(struct bitmap_index *bitmap_git,
 static int filter_bitmap(struct bitmap_index *bitmap_git,
 			 struct object_list *tip_objects,
 			 struct bitmap *to_filter,
-			 struct list_objects_filter_options *filter)
+			 const struct list_objects_filter_options *filter)
 {
 	if (!filter || filter->choice == LOFC_DISABLED)
 		return 0;
@@ -2027,7 +2027,7 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
 	return -1;
 }
 
-static int can_filter_bitmap(struct list_objects_filter_options *filter)
+static int can_filter_bitmap(const struct list_objects_filter_options *filter)
 {
 	return !filter_bitmap(NULL, NULL, NULL, filter);
 }
@@ -2058,7 +2058,7 @@ static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
 }
 
 int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
-			      struct list_objects_filter_options *filter,
+			      const struct list_objects_filter_options *filter,
 			      show_reachable_fn show_reach,
 			      void *payload)
 {
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 19a8655457..47935eb24e 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -96,7 +96,7 @@ struct list_objects_filter_options;
  * not supported, `0` otherwise.
  */
 int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
-			      struct list_objects_filter_options *filter,
+			      const struct list_objects_filter_options *filter,
 			      show_reachable_fn show_reach,
 			      void *payload);
 
-- 
2.55.0.229.g6434b31f56.dirty
Patrick SteinhardtJul 10, 2026, 08:48 UTC in reply to Patrick Steinhardt on lore

[PATCH v2 3/8] pack-bitmap: allow aborting iteration of bitmapped objects

In a subsequent commit we'll lift iteration of bitmapped objects into the "packed" backend and make it accessible via `odb_for_each_object()`. The calling convention for that function is that the callback may return a non-zero exit code, and if so we'll abort iteration. This is currently impossible to realize though, as `for_each_bitmapped_object()` will ignore any return value and just churn through all objects completely.

This doesn't matter to the callers of `for_each_bitmapped_object()`, as there's only one of them in git-cat-file(1), and the callbacks we pass always return zero. But once we move the logic into the generic infrastructure it becomes a latent bug waiting to happen.

Refactor the code so that the return value of the `show_reach` callback is not ignored anymore. Instead, returning a non-zero value will cause us to abort iteration in both `show_objects_for_type()` and in `for_each_bitmapped_object()`.

Note though that there's a second user of `show_objects_for_type()` with `traverse_bitmap_commit_list()`, and that function does indeed invoke callbacks that may return non-zero. This non-zero return value never had any effect at all though, and the callbacks that return non-zero values are only ever invoked via `traverse_bitmap_commit_list()`. Consequently, we adapt them to always return 0.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/pack-objects.c |  2 +-
 builtin/rev-list.c     |  2 +-
 pack-bitmap.c          | 31 +++++++++++++++++++++----------
 pack-bitmap.h          |  3 ++-
 4 files changed, 25 insertions(+), 13 deletions(-)
Show changes to 4 files +25 −13

builtin/pack-objects.c, builtin/rev-list.c, pack-bitmap.c, pack-bitmap.h

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ea5eab4cf8..8ff92c5272 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1909,7 +1909,7 @@ static int add_object_entry_from_bitmap(const struct object_id *oid,
 		return 0;
 
 	create_object_entry(oid, type, name_hash, 0, 0, pack, offset);
-	return 1;
+	return 0;
 }
 
 struct pbase_tree_cache {
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 8f63003709..02818b81c6 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -486,7 +486,7 @@ static int show_object_fast(
 	void *payload UNUSED)
 {
 	fprintf(stdout, "%s\n", oid_to_hex(oid));
-	return 1;
+	return 0;
 }
 
 static void print_disk_usage(off_t size)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index a47c231632..eda38a5433 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1695,7 +1695,7 @@ static void init_type_iterator(struct ewah_or_iterator *it,
 	}
 }
 
-static void show_objects_for_type(
+static int show_objects_for_type(
 	struct bitmap_index *bitmap_git,
 	struct bitmap *objects,
 	enum object_type object_type,
@@ -1704,6 +1704,7 @@ static void show_objects_for_type(
 {
 	size_t i = 0;
 	uint32_t offset;
+	int ret;
 
 	struct ewah_or_iterator it;
 	eword_t filter;
@@ -1749,11 +1750,17 @@ static void show_objects_for_type(
 
 			hash = bitmap_name_hash(bitmap_git, index_pos);
 
-			show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
+			ret = show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
+			if (ret)
+				goto out;
 		}
 	}
 
+	ret = 0;
+
+out:
 	ewah_or_iterator_release(&it);
+	return ret;
 }
 
 static int in_bitmapped_pack(struct bitmap_index *bitmap_git,
@@ -2062,6 +2069,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
 			      show_reachable_fn show_reach,
 			      void *payload)
 {
+	const enum object_type types[] = {
+		OBJ_COMMIT,
+		OBJ_TREE,
+		OBJ_BLOB,
+		OBJ_TAG,
+	};
 	struct bitmap *filtered_bitmap = NULL;
 	uint32_t objects_nr;
 	size_t full_word_count;
@@ -2086,14 +2099,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
 		goto out;
 	}
 
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_COMMIT, show_reach, payload);
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_TREE, show_reach, payload);
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_BLOB, show_reach, payload);
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_TAG, show_reach, payload);
+	for (size_t i = 0; i < ARRAY_SIZE(types); i++) {
+		ret = show_objects_for_type(bitmap_git, filtered_bitmap,
+					    types[i], show_reach, payload);
+		if (ret)
+			goto out;
+	}
 
 	ret = 0;
 out:
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 47935eb24e..ae8dc491ac 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -93,7 +93,8 @@ struct list_objects_filter_options;
 /*
  * Filter bitmapped objects and iterate through all resulting objects,
  * executing `show_reach` for each of them. Returns `-1` in case the filter is
- * not supported, `0` otherwise.
+ * not supported, `0` otherwise. Aborts iteration and bubbles up the return
+ * value in case `show_reach()` returns non-zero.
  */
 int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
 			      const struct list_objects_filter_options *filter,
-- 
2.55.0.229.g6434b31f56.dirty
Patrick SteinhardtJul 10, 2026, 08:48 UTC in reply to Patrick Steinhardt on lore

[PATCH v2 4/8] pack-bitmap: iterate object sources when opening bitmaps

When opening a bitmap for a repository we perform two steps:
  - We first look for a multi-pack index bitmap in any of the object
    sources connected to the repository.
  - We then look for a packfile bitmap in any of the packfiles of any of
    the object sources.

Both of these steps thus iterate through object sources themselves, one via `odb_prepare_alternates()` and one via `repo_for_each_pack()`. This layout makes it hard to introduce a way to open the bitmap of one specific object source, which is functionality that we'll require in a subsequent commit.

Reverse the loop so that we instead loop through all sources in the outer loop, and then for each source we try to load its bitmap via either the multi-pack index or via a packfile.

Note that this changes the precedence of bitmaps in one specific edge case: when an earlier object source only has a packfile bitmap, but a later source has a multi-pack index bitmap, we now pick the packfile bitmap of the earlier source. Previously, a multi-pack index bitmap from any source would have taken precedence over all packfile bitmaps. Given that object sources are ordered such that the local source comes first, this arguably is an improvement, as we now prefer local bitmaps over bitmaps in alternates. Furthermore, we already warn about repositories that have multiple bitmaps, so this setup is broken and thus arguably not worth worrying about too much.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pack-bitmap.c | 69 +++++++++++++++++++++++++++--------------------------------
 1 file changed, 31 insertions(+), 38 deletions(-)
Show changes to pack-bitmap.c +31 −38
diff --git a/pack-bitmap.c b/pack-bitmap.c
index eda38a5433..e32795a595 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -680,60 +680,53 @@ static int load_bitmap(struct repository *r, struct bitmap_index *bitmap_git,
 	return 0;
 }
 
-static int open_pack_bitmap(struct repository *r,
-			    struct bitmap_index *bitmap_git)
+static int open_bitmap_for_source(struct odb_source_packed *source,
+				  struct bitmap_index *bitmap_git)
 {
-	struct packed_git *p;
-	int ret = -1;
+	struct multi_pack_index *midx = get_multi_pack_index(source);
+	struct packfile_list_entry *e;
+	bool found = false;
 
-	repo_for_each_pack(r, p) {
-		if (open_pack_bitmap_1(bitmap_git, p) == 0) {
-			ret = 0;
-			/*
-			 * The only reason to keep looking is to report
-			 * duplicates.
-			 */
-			if (!trace2_is_enabled())
-				break;
-		}
+	if (midx && !open_midx_bitmap_1(bitmap_git, midx))
+		found = true;
+
+	for (e = packfile_store_get_packs(source); e; e = e->next) {
+		/*
+		 * When tracing is enabled we want to keep looking to report
+		 * duplicates even if we have already found a bitmap.
+		 */
+		if (found && !trace2_is_enabled())
+			break;
+
+		if (!open_pack_bitmap_1(bitmap_git, e->pack))
+			found = true;
 	}
 
-	return ret;
+	return found ? 0 : -1;
 }
 
-static int open_midx_bitmap(struct repository *r,
-			    struct bitmap_index *bitmap_git)
+static int open_bitmap(struct repository *r,
+		       struct bitmap_index *bitmap_git)
 {
 	struct odb_source *source;
-	int ret = -1;
+	bool found = false;
 
 	assert(!bitmap_git->map);
 
 	odb_prepare_alternates(r->objects);
 	for (source = r->objects->sources; source; source = source->next) {
 		struct odb_source_files *files = odb_source_files_downcast(source);
-		struct multi_pack_index *midx = get_multi_pack_index(files->packed);
-		if (midx && !open_midx_bitmap_1(bitmap_git, midx))
-			ret = 0;
-	}
-	return ret;
-}
-
-static int open_bitmap(struct repository *r,
-		       struct bitmap_index *bitmap_git)
-{
-	int found;
 
-	assert(!bitmap_git->map);
+		if (!open_bitmap_for_source(files->packed, bitmap_git))
+			found = true;
 
-	found = !open_midx_bitmap(r, bitmap_git);
-
-	/*
-	 * these will all be skipped if we opened a midx bitmap; but run it
-	 * anyway if tracing is enabled to report the duplicates
-	 */
-	if (!found || trace2_is_enabled())
-		found |= !open_pack_bitmap(r, bitmap_git);
+		/*
+		 * The only reason to keep looking after having found a bitmap
+		 * is to report duplicates.
+		 */
+		if (found && !trace2_is_enabled())
+			break;
+	}
 
 	return found ? 0 : -1;
 }
-- 
2.55.0.229.g6434b31f56.dirty
Patrick SteinhardtJul 10, 2026, 08:48 UTC in reply to Patrick Steinhardt on lore

[PATCH v2 5/8] pack-bitmap: drop `_1` suffix from functions that open bitmaps

In the preceding commit we've refactored how we open bitmaps. As part of the refactoring we have consolidated `open_pack_bitmap()` as well as `open_midx_bitmap()` into `open_bitmap_for_source()`. Consequently, we only have their `open_pack_bitmap_1()` and `open_midx_bitmap_1()` variants left over, where the `_1` suffix doesn't really make much sense anymore.

Drop the suffix.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pack-bitmap.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
Show changes to pack-bitmap.c +7 −7
diff --git a/pack-bitmap.c b/pack-bitmap.c
index e32795a595..72c8ae3228 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -460,8 +460,8 @@ char *pack_bitmap_filename(struct packed_git *p)
 	return xstrfmt("%.*s.bitmap", (int)len, p->pack_name);
 }
 
-static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
-			      struct multi_pack_index *midx)
+static int open_midx_bitmap(struct bitmap_index *bitmap_git,
+			    struct multi_pack_index *midx)
 {
 	struct stat st;
 	char *bitmap_name = midx_bitmap_filename(midx);
@@ -539,7 +539,7 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
 	return -1;
 }
 
-static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git *packfile)
+static int open_pack_bitmap(struct bitmap_index *bitmap_git, struct packed_git *packfile)
 {
 	int fd;
 	struct stat st;
@@ -603,7 +603,7 @@ static int load_reverse_index(struct repository *r, struct bitmap_index *bitmap_
 
 		/*
 		 * The multi-pack-index's .rev file is already loaded via
-		 * open_pack_bitmap_1().
+		 * open_pack_bitmap().
 		 *
 		 * But we still need to open the individual pack .rev files,
 		 * since we will need to make use of them in pack-objects.
@@ -687,7 +687,7 @@ static int open_bitmap_for_source(struct odb_source_packed *source,
 	struct packfile_list_entry *e;
 	bool found = false;
 
-	if (midx && !open_midx_bitmap_1(bitmap_git, midx))
+	if (midx && !open_midx_bitmap(bitmap_git, midx))
 		found = true;
 
 	for (e = packfile_store_get_packs(source); e; e = e->next) {
@@ -698,7 +698,7 @@ static int open_bitmap_for_source(struct odb_source_packed *source,
 		if (found && !trace2_is_enabled())
 			break;
 
-		if (!open_pack_bitmap_1(bitmap_git, e->pack))
+		if (!open_pack_bitmap(bitmap_git, e->pack))
 			found = true;
 	}
 
@@ -746,7 +746,7 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
 {
 	struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
 
-	if (!open_midx_bitmap_1(bitmap_git, midx))
+	if (!open_midx_bitmap(bitmap_git, midx))
 		return bitmap_git;
 
 	free_bitmap_index(bitmap_git);
-- 
2.55.0.229.g6434b31f56.dirty
Patrick SteinhardtJul 10, 2026, 08:48 UTC in reply to Patrick Steinhardt on lore

[PATCH v2 6/8] pack-bitmap: introduce function to open bitmap for a single source

The function `prepare_bitmap_git()` opens the first bitmap it can find in any of the object sources connected to the repository. In a subsequent commit, the "packed" object database backend will learn to use bitmaps to answer object filters when enumerating objects. That backend operates on a single object source though, so using a bitmap that potentially belongs to a different source would be wrong:

  - The source would yield objects that are not part of the source
    itself.
  - The object source info would be attributed to the wrong source.
  - With multiple sources, each source would enumerate the same bitmap
    another time.

Introduce a new function `prepare_source_bitmap_git()` that only opens bitmaps belonging to the given object source.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pack-bitmap.c | 12 ++++++++++++
 pack-bitmap.h |  2 ++
 2 files changed, 14 insertions(+)
Show changes to 2 files +14 −0

pack-bitmap.c, pack-bitmap.h

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 72c8ae3228..09ba15d26b 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -753,6 +753,18 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
 	return NULL;
 }
 
+struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source)
+{
+	struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
+
+	if (!open_bitmap_for_source(source, bitmap_git) &&
+	    !load_bitmap(source->base.odb->repo, bitmap_git, 0))
+		return bitmap_git;
+
+	free_bitmap_index(bitmap_git);
+	return NULL;
+}
+
 int bitmap_index_contains_pack(struct bitmap_index *bitmap, struct packed_git *pack)
 {
 	for (; bitmap; bitmap = bitmap->base) {
diff --git a/pack-bitmap.h b/pack-bitmap.h
index ae8dc491ac..9f20fb6e56 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -9,6 +9,7 @@
 #include "string-list.h"
 
 struct commit;
+struct odb_source_packed;
 struct repository;
 struct rev_info;
 
@@ -68,6 +69,7 @@ struct bitmapped_pack {
 
 struct bitmap_index *prepare_bitmap_git(struct repository *r);
 struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx);
+struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source);
 
 /*
  * Given a bitmap index, determine whether it contains the pack either directly
-- 
2.55.0.229.g6434b31f56.dirty
Patrick SteinhardtJul 10, 2026, 08:48 UTC in reply to Patrick Steinhardt on lore

[PATCH v2 7/8] odb: introduce object filters to `odb_for_each_object()`

The function `for_each_bitmapped_object()` can be used to iterate through all objects covered by a bitmap. The benefit of this function is that it allows the caller to efficiently handle some object filters. For example, this can be used to filter out objects of a specific type with some simple bitmap operations. But callers are currently required to manually wire up the use of bitmaps though, and to do so they have to reach into internals of a given object database source.

Introduce a new `struct odb_for_each_object_options::filter` field so that the interface becomes generic. When set, then a backend may optionally use the filter to skip some objects that it would have otherwise yielded.

Note that the respective backends are free to ignore this field if they cannot meaningfully optimize for a given filter, and consequently callers need to verify whether they actually want the returned objects. While annoying, we cannot easily lift this restriction anyway as the object filter infrastructure supports some filters that cannot be answered by the object database alone.

An alternative might be to limit the filters to only those that _can_ be answered by backends. But ultimately, the filters that can be answered efficiently by the "packed" backend are completely disjunct from those that can be answered by the "loose" backend, and consequently the set of filters supported by all backends would be empty. Furthermore, it would require us to make assumptions about capabilities of future backends, which may be able to efficiently handle more filters than current ones. So in the end, this alternative would only limit us artificially.

Implement the logic for the "packed" source. Note that we use the new function `prepare_source_bitmap_git()` to open the bitmap: as the backend operates on a single object source, we must only use bitmaps that belong to that specific source. Otherwise we might yield objects that are not part of the source at all, and with multiple sources we would enumerate the same bitmap once per source.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.h               | 12 +++++++++++
 odb/source-packed.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 pack-bitmap.c       |  3 +--
 pack-bitmap.h       |  3 +++
 4 files changed, 78 insertions(+), 2 deletions(-)
Show changes to 4 files +78 −2

odb.h, odb/source-packed.c, pack-bitmap.c, pack-bitmap.h

diff --git a/odb.h b/odb.h
index a1e222f605..67d0b34942 100644
--- a/odb.h
+++ b/odb.h
@@ -8,6 +8,7 @@
 #include "thread-utils.h"
 
 struct cached_object_entry;
+struct list_objects_filter_options;
 struct odb_source_inmemory;
 struct packed_git;
 struct repository;
@@ -490,6 +491,17 @@ struct odb_for_each_object_options {
 	 */
 	const struct object_id *prefix;
 	size_t prefix_hex_len;
+
+	/*
+	 * Optional object filter that allows backends to skip yielding
+	 * objects that are excluded by the filter as an optimization. The
+	 * filter is a best-effort hint: backends may use it to skip
+	 * excluded objects (e.g. by consulting a reachability bitmap), but
+	 * are also free to ignore it entirely and yield every object. As a
+	 * consequence, callers must re-apply the filter on yielded objects
+	 * if they require strict filtering semantics.
+	 */
+	const struct list_objects_filter_options *filter;
 };
 
 /*
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 9cfa02b7a2..4777395053 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -3,11 +3,13 @@
 #include "chdir-notify.h"
 #include "dir.h"
 #include "git-zlib.h"
+#include "list-objects-filter-options.h"
 #include "mergesort.h"
 #include "midx.h"
 #include "odb/source-packed.h"
 #include "odb/streaming.h"
 #include "packfile.h"
+#include "pack-bitmap.h"
 
 static int find_pack_entry(struct odb_source_packed *store,
 			   const struct object_id *oid,
@@ -315,6 +317,37 @@ static int odb_source_packed_for_each_prefixed_object(
 	return ret;
 }
 
+struct bitmapped_for_each_object_data {
+	struct odb_source_packed *packed;
+	const struct object_info *request;
+	const struct odb_for_each_object_options *opts;
+	odb_for_each_object_cb cb;
+	void *cb_data;
+};
+
+static int bitmapped_for_each_object(const struct object_id *oid,
+				     enum object_type type UNUSED,
+				     int flags UNUSED,
+				     uint32_t hash UNUSED,
+				     struct packed_git *pack,
+				     off_t offset,
+				     void *cb_data)
+{
+	struct bitmapped_for_each_object_data *data = cb_data;
+
+	if (should_exclude_pack(pack, data->opts->flags))
+		return 0;
+
+	if (data->request) {
+		struct object_info oi = *data->request;
+		if (packed_object_info(data->packed, pack, offset, &oi) < 0)
+			return -1;
+		return data->cb(oid, &oi, data->cb_data);
+	}
+
+	return data->cb(oid, NULL, data->cb_data);
+}
+
 static int odb_source_packed_for_each_object(struct odb_source *source,
 					     const struct object_info *request,
 					     odb_for_each_object_cb cb,
@@ -328,12 +361,33 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
 		.cb = cb,
 		.cb_data = cb_data,
 	};
+	struct bitmap_index *bitmap = NULL;
 	struct packfile_list_entry *e;
 	int pack_errors = 0, ret;
 
 	if (opts->prefix)
 		return odb_source_packed_for_each_prefixed_object(packed, opts, &data);
 
+	if (opts->filter &&
+	    opts->filter->choice != LOFC_DISABLED &&
+	    can_filter_bitmap(opts->filter))
+		bitmap = prepare_bitmap_git_for_source(packed);
+	if (bitmap) {
+		struct bitmapped_for_each_object_data bitmap_data = {
+			.packed = packed,
+			.request = request,
+			.opts = opts,
+			.cb = cb,
+			.cb_data = cb_data,
+		};
+
+		ret = for_each_bitmapped_object(bitmap, opts->filter,
+						bitmapped_for_each_object,
+						&bitmap_data);
+		if (ret)
+			goto out;
+	}
+
 	packed->skip_mru_updates = true;
 
 	for (e = packfile_store_get_packs(packed); e; e = e->next) {
@@ -342,6 +396,13 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
 		if (should_exclude_pack(p, opts->flags))
 			continue;
 
+		/*
+		 * Objects covered by the bitmap have already been yielded
+		 * above; skip them here to avoid duplicates.
+		 */
+		if (bitmap && bitmap_index_contains_pack(bitmap, p))
+			continue;
+
 		if (open_pack_index(p)) {
 			pack_errors = 1;
 			continue;
@@ -357,6 +418,7 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
 
 out:
 	packed->skip_mru_updates = false;
+	free_bitmap_index(bitmap);
 
 	if (!ret && pack_errors)
 		ret = -1;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 09ba15d26b..f55a0859ea 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2039,12 +2039,11 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
 	return -1;
 }
 
-static int can_filter_bitmap(const struct list_objects_filter_options *filter)
+bool can_filter_bitmap(const struct list_objects_filter_options *filter)
 {
 	return !filter_bitmap(NULL, NULL, NULL, filter);
 }
 
-
 static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
 					      struct bitmap *result)
 {
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 9f20fb6e56..1385027c1f 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -92,6 +92,9 @@ int test_bitmap_pseudo_merge_objects(struct repository *r, uint32_t n);
 
 struct list_objects_filter_options;
 
+/* Check whether the filter can be computed via the bitmap. */
+bool can_filter_bitmap(const struct list_objects_filter_options *filter);
+
 /*
  * Filter bitmapped objects and iterate through all resulting objects,
  * executing `show_reach` for each of them. Returns `-1` in case the filter is
-- 
2.55.0.229.g6434b31f56.dirty
Patrick SteinhardtJul 10, 2026, 08:49 UTC in reply to Patrick Steinhardt on lore

[PATCH v2 8/8] builtin/cat-file: filter objects via object database

When batching all objects, git-cat-file(1) reaches into the internals of the object database and manually manages bitmaps to apply object filters. This creates coupling between the command and the internals of the respective backend.

Refactor git-cat-file(1) to use the new object filter option when batching all objects. This significantly simplifies the logic and ensures that we don't have to reach into internals of the "files" source anymore.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/cat-file.c | 76 +++++-------------------------------------------------
 1 file changed, 7 insertions(+), 69 deletions(-)
Show changes to builtin/cat-file.c +7 −69
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index b4b99a73da..1458dd76d6 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,7 +20,6 @@
 #include "userdiff.h"
 #include "oid-array.h"
 #include "packfile.h"
-#include "pack-bitmap.h"
 #include "object-file.h"
 #include "object-name.h"
 #include "odb.h"
@@ -844,28 +843,6 @@ static int batch_one_object_oi(const struct object_id *oid,
 	return payload->callback(oid, NULL, 0, payload->payload);
 }
 
-static int batch_one_object_packed(const struct object_id *oid,
-				   struct packed_git *pack,
-				   uint32_t pos,
-				   void *_payload)
-{
-	struct for_each_object_payload *payload = _payload;
-	return payload->callback(oid, pack, nth_packed_object_offset(pack, pos),
-				 payload->payload);
-}
-
-static int batch_one_object_bitmapped(const struct object_id *oid,
-				      enum object_type type UNUSED,
-				      int flags UNUSED,
-				      uint32_t hash UNUSED,
-				      struct packed_git *pack,
-				      off_t offset,
-				      void *_payload)
-{
-	struct for_each_object_payload *payload = _payload;
-	return payload->callback(oid, pack, offset, payload->payload);
-}
-
 static void batch_each_object(struct batch_options *opt,
 			      for_each_object_fn callback,
 			      unsigned flags,
@@ -875,56 +852,17 @@ static void batch_each_object(struct batch_options *opt,
 		.callback = callback,
 		.payload = _payload,
 	};
+	struct odb_source_info source_info;
+	struct object_info oi = {
+		.source_infop = &source_info,
+	};
 	struct odb_for_each_object_options opts = {
 		.flags = flags,
+		.filter = &opt->objects_filter,
 	};
-	struct bitmap_index *bitmap = NULL;
-	struct odb_source *source;
-
-	/*
-	 * TODO: we still need to tap into implementation details of the object
-	 * database sources. Ideally, we should extend `odb_for_each_object()`
-	 * to handle object filters itself so that we can move the filtering
-	 * logic into the individual sources.
-	 */
-	odb_prepare_alternates(the_repository->objects);
-	for (source = the_repository->objects->sources; source; source = source->next) {
-		struct odb_source_files *files = odb_source_files_downcast(source);
-		int ret = odb_source_for_each_object(&files->loose->base, NULL, batch_one_object_oi,
-						     &payload, &opts);
-		if (ret)
-			break;
-	}
-
-	if (opt->objects_filter.choice != LOFC_DISABLED &&
-	    (bitmap = prepare_bitmap_git(the_repository)) &&
-	    !for_each_bitmapped_object(bitmap, &opt->objects_filter,
-				       batch_one_object_bitmapped, &payload)) {
-		struct packed_git *pack;
-
-		repo_for_each_pack(the_repository, pack) {
-			if (bitmap_index_contains_pack(bitmap, pack) ||
-			    open_pack_index(pack))
-				continue;
-			for_each_object_in_pack(pack, batch_one_object_packed,
-						&payload, flags);
-		}
-	} else {
-		struct odb_source_info source_info;
-		struct object_info oi = {
-			.source_infop = &source_info,
-		};
-
-		for (source = the_repository->objects->sources; source; source = source->next) {
-			struct odb_source_files *files = odb_source_files_downcast(source);
-			int ret = odb_source_for_each_object(&files->packed->base, &oi,
-							     batch_one_object_oi, &payload, &opts);
-			if (ret)
-				break;
-		}
-	}
 
-	free_bitmap_index(bitmap);
+	odb_for_each_object_ext(the_repository->objects, &oi,
+				batch_one_object_oi, &payload, &opts);
 }
 
 static int batch_objects(struct batch_options *opt)
-- 
2.55.0.229.g6434b31f56.dirty
Taylor BlauJul 10, 2026, 22:25 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v2 1/8] odb/source-packed: improve lookup when enumerating objects

On Fri, Jul 10, 2026 at 10:48:53AM +0200, Patrick Steinhardt wrote:
> Fix the issue by using `packed_object_info()` directly.

What you wrote here makes sense to me insofar as I understand the pluggable ODB code.

However, I am confused by the way this function is written in general. We use `bsearch_one_midx()` to locate the first possible MIDX position in which an object matching the given prefix may exist, which is sensible. However, we go from that position up to "num", where "num" is the total number of objects in the MIDX!

Functionally this is not incorrect as we will happily discard objects that do not match the prefix. But it causes us to waste CPU cycles repeatedly calling `match_hash()` (at least for the first byte of the prefix) for objects that we know will match.

How often do we call this function with a prefix longer than a single byte? I have no idea, but I would suspect that it makes up the majority of calls. If we read the OID fanout chunk, we could narrow the range that we enumerate through, and only compare the second byte onwards of the given prefix, if one exists. In the single-byte prefix case, this means that we shouldn't have to do any memory comparisons at all.

> While at it, rename the `store` variable to `source`.

Unrelated, but please keep these to a minimum, as they make the patch more difficult to read than is necessary.

Thanks, Taylor

Taylor BlauJul 10, 2026, 22:25 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v2 2/8] pack-bitmap: mark object filter as `const`

On Fri, Jul 10, 2026 at 10:48:54AM +0200, Patrick Steinhardt wrote:
> The function `for_each_bitmapped_object()` accepts an optional object
> filter. This filter is never modified by the function, but is not
> declared as `const`. Fix this.

Makes sense. "Fix" this seems to imply that the existing behavior was broken or otherwise incorrect, but I think this is fine.

Show 5 quoted lines
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  pack-bitmap.c | 6 +++---
>  pack-bitmap.h | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)

Thanks, Taylor

Taylor BlauJul 10, 2026, 22:34 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v2 3/8] pack-bitmap: allow aborting iteration of bitmapped objects

On Fri, Jul 10, 2026 at 10:48:55AM +0200, Patrick Steinhardt wrote:
Show 11 quoted lines
> In a subsequent commit we'll lift iteration of bitmapped objects into
> the "packed" backend and make it accessible via `odb_for_each_object()`.
> The calling convention for that function is that the callback may return
> a non-zero exit code, and if so we'll abort iteration. This is currently
> impossible to realize though, as `for_each_bitmapped_object()` will
> ignore any return value and just churn through all objects completely.
>
> This doesn't matter to the callers of `for_each_bitmapped_object()`, as
> there's only one of them in git-cat-file(1), and the callbacks we pass
> always return zero. But once we move the logic into the generic
> infrastructure it becomes a latent bug waiting to happen.
Makes sense.
Show 11 quoted lines
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index ea5eab4cf8..8ff92c5272 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1909,7 +1909,7 @@ static int add_object_entry_from_bitmap(const struct object_id *oid,
>  		return 0;
>
>  	create_object_entry(oid, type, name_hash, 0, 0, pack, offset);
> -	return 1;
> +	return 0;
>  }

I was initially rather surprised to read this diff. I suspected that this was a "we used to return non-zero to indicate success but now return zero to match the project conventions", but was stumped by the unchanged "return 0" in the context above.

But I suppose that is demonstrating the thing that you're trying to fix here, which is that the caller doesn't actually care what is returned from the callback, so the change here (and analogous ones below) make sense to me.

Show 10 quoted lines
> -static void show_objects_for_type(
> +static int show_objects_for_type(
>  	struct bitmap_index *bitmap_git,
>  	struct bitmap *objects,
>  	enum object_type object_type,
> @@ -1704,6 +1704,7 @@ static void show_objects_for_type(
>  {
>  	size_t i = 0;
>  	uint32_t offset;
> +	int ret;

This has a broader scope than is strictly necessary, but I think that is OK.

Show 32 quoted lines
>  static int in_bitmapped_pack(struct bitmap_index *bitmap_git,
> @@ -2062,6 +2069,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
>  			      show_reachable_fn show_reach,
>  			      void *payload)
>  {
> +	const enum object_type types[] = {
> +		OBJ_COMMIT,
> +		OBJ_TREE,
> +		OBJ_BLOB,
> +		OBJ_TAG,
> +	};
>  	struct bitmap *filtered_bitmap = NULL;
>  	uint32_t objects_nr;
>  	size_t full_word_count;
> @@ -2086,14 +2099,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
>  		goto out;
>  	}
>
> -	show_objects_for_type(bitmap_git, filtered_bitmap,
> -			      OBJ_COMMIT, show_reach, payload);
> -	show_objects_for_type(bitmap_git, filtered_bitmap,
> -			      OBJ_TREE, show_reach, payload);
> -	show_objects_for_type(bitmap_git, filtered_bitmap,
> -			      OBJ_BLOB, show_reach, payload);
> -	show_objects_for_type(bitmap_git, filtered_bitmap,
> -			      OBJ_TAG, show_reach, payload);
> +	for (size_t i = 0; i < ARRAY_SIZE(types); i++) {
> +		ret = show_objects_for_type(bitmap_git, filtered_bitmap,
> +					    types[i], show_reach, payload);
> +		if (ret)
> +			goto out;
> +	}

OK. So now we call this function in a loop instead of the unrolled version, presumably because we want to propagate a failure from any one of these before falling through to the remaining object types.

That makes sense, and I think the clean-up is well justified here.

However, the remaining `show_objects_for_type()` callers from within `traverse_bitmap_commit_list()` do *not* bother to inspect the return value, despite taking in an arbitrary 'show_reachable_fn', which itself may return a non-zero value.

I guess this must be effectively OK in practice with respect to the existing code for the same reason you indicate in the commit message above, but we should change this function to *also* propagate non-zero return values to eliminate the foot-gun completely.

Thanks, Taylor

Taylor BlauJul 10, 2026, 22:40 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v2 4/8] pack-bitmap: iterate object sources when opening bitmaps

On Fri, Jul 10, 2026 at 10:48:56AM +0200, Patrick Steinhardt wrote:
Show 17 quoted lines
> When opening a bitmap for a repository we perform two steps:
>
>   - We first look for a multi-pack index bitmap in any of the object
>     sources connected to the repository.
>
>   - We then look for a packfile bitmap in any of the packfiles of any of
>     the object sources.
>
> Both of these steps thus iterate through object sources themselves, one
> via `odb_prepare_alternates()` and one via `repo_for_each_pack()`. This
> layout makes it hard to introduce a way to open the bitmap of one
> specific object source, which is functionality that we'll require in a
> subsequent commit.
>
> Reverse the loop so that we instead loop through all sources in the
> outer loop, and then for each source we try to load its bitmap via
> either the multi-pack index or via a packfile.

This makes sense. An individual object store should be considered to have a bitmap in the abstract sense if it provides either a multi-pack bitmap (or an incremental multi-pack bitmap ), or a single-pack bitmap.

Show 10 quoted lines
> Note that this changes the precedence of bitmaps in one specific edge
> case: when an earlier object source only has a packfile bitmap, but a
> later source has a multi-pack index bitmap, we now pick the packfile
> bitmap of the earlier source. Previously, a multi-pack index bitmap from
> any source would have taken precedence over all packfile bitmaps. Given
> that object sources are ordered such that the local source comes first,
> this arguably is an improvement, as we now prefer local bitmaps over
> bitmaps in alternates. Furthermore, we already warn about repositories
> that have multiple bitmaps, so this setup is broken and thus arguably
> not worth worrying about too much.

Yeah, I think the existing behavior should be considered broken, so I think that this behavior change is a positive one.

Thanks, Taylor

Taylor BlauJul 10, 2026, 22:41 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v2 5/8] pack-bitmap: drop `_1` suffix from functions that open bitmaps

On Fri, Jul 10, 2026 at 10:48:57AM +0200, Patrick Steinhardt wrote:
Show 8 quoted lines
> In the preceding commit we've refactored how we open bitmaps. As part of
> the refactoring we have consolidated `open_pack_bitmap()` as well as
> `open_midx_bitmap()` into `open_bitmap_for_source()`. Consequently, we
> only have their `open_pack_bitmap_1()` and `open_midx_bitmap_1()`
> variants left over, where the `_1` suffix doesn't really make much sense
> anymore.
>
> Drop the suffix.
Makes sense. Thanks for keeping this in a separate commit.

Thanks, Taylor

Taylor BlauJul 10, 2026, 22:42 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v2 7/8] odb: introduce object filters to `odb_for_each_object()`

On Fri, Jul 10, 2026 at 10:48:59AM +0200, Patrick Steinhardt wrote:
Show 6 quoted lines
> ---
>  odb.h               | 12 +++++++++++
>  odb/source-packed.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  pack-bitmap.c       |  3 +--
>  pack-bitmap.h       |  3 +++
>  4 files changed, 78 insertions(+), 2 deletions(-)

This all looks about as expected to me. As mentioned earlier in this thread, I am not as familiar with the pluggable ODB code as I'd like to be, but the patch looks plausibly correct to me.

Thanks, Taylor

Jeff KingJul 11, 2026, 07:47 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH 3/7] pack-bitmap: allow aborting iteration of bitmapped objects

On Fri, Jul 10, 2026 at 09:08:44AM +0200, Patrick Steinhardt wrote:
Show 22 quoted lines
> On Thu, Jul 09, 2026 at 03:19:52PM -0500, Justin Tobler wrote:
> > > diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> > > index ea5eab4cf8..8ff92c5272 100644
> > > --- a/builtin/pack-objects.c
> > > +++ b/builtin/pack-objects.c
> > > @@ -1909,7 +1909,7 @@ static int add_object_entry_from_bitmap(const struct object_id *oid,
> > >  		return 0;
> > >  
> > >  	create_object_entry(oid, type, name_hash, 0, 0, pack, offset);
> > > -	return 1;
> > > +	return 0;
> > 
> > I wonder why this was even returning 1 to begin with? As you mentioned,
> > the return value appears to be ignored anyways. I'm assuming it was
> > signal that an object entry was created?
> 
> The function is only called from a single location, and the return value
> was completely ignored until this commit. It has always been this way
> since the function was originally introduced in 6b8fda2db1
> (pack-objects: use bitmaps when packing objects, 2013-12-21), so it
> never seemed to have any purpose. The commit message doesn't mention
> anything either.

I think it was copying the semantics of its non-bitmap counterpart, add_object_entry(). Of course nobody looks at that return value either!

Long ago there were callers that cared about whether we actually created an entry, but I think the last one went away in 5379a5c5ee (Thin pack generation: optimization., 2006-04-05), which was quite some time ago.

So I think we could probably drop the return value from add_object_entry() entirely (but of course we can't do the same for the bitmap variant, because of its use as a callback).

I mention this mostly as answering Justin's "I wonder why...", but it might be worth cleaning up add_object_entry() here, as its return value semantics have diverged from add_object_entry_from_bitmap().

-Peff
Jeff KingJul 11, 2026, 07:58 UTC in reply to Patrick Steinhardt on lore

[PATCH v2 9/8?] pack-objects: drop unused return value from add_object_entry()

On Fri, Jul 10, 2026 at 10:48:52AM +0200, Patrick Steinhardt wrote:
> The series is built on top of f85a7e6620 (Start Git 2.56 cycle,
> 2026-07-06) with ps/odb-drop-whence at 8a7ad23e11 (odb: document object
> info fields, 2026-07-02) merged into it.
Here's a patch doing the cleanup I proposed upthread.
-- >8 --
Subject: pack-objects: drop unused return value from add_object_entry()

This function returns 0/1 to its caller to tell them whether we actually added a new entry (or if we considered it redundant). But nobody has relied on that behavior since 5379a5c5ee (Thin pack generation: optimization., 2006-04-05).

The extra return does not hurt much, but it recently became a bit more confusing. We have a sister function, add_object_entry_from_bitmap(), which had the same return value semantics. That function recently changed to always return 0 (not void, because it must conform to a callback function interface). So now we have two related functions which both return an "int" but with different semantics.

Let's drop the unused "int" return from add_object_entry() entirely, which makes it more clear that the two functions have diverged.

Signed-off-by: Jeff King <peff@peff.net>
---
I couldn't reference the commit by its id, since Junio has not yet
picked up the v2 sent a few hours ago. ;)
 builtin/pack-objects.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
Show changes to builtin/pack-objects.c +4 −5
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8ff92c5272..3673b14b89 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1867,16 +1867,16 @@ static const char no_closure_warning[] = N_(
 "disabling bitmap writing, as some objects are not being packed"
 );
 
-static int add_object_entry(const struct object_id *oid, enum object_type type,
-			    const char *name, int exclude)
+static void add_object_entry(const struct object_id *oid, enum object_type type,
+			     const char *name, int exclude)
 {
 	struct packed_git *found_pack = NULL;
 	off_t found_offset = 0;
 
 	display_progress(progress_state, ++nr_seen);
 
 	if (have_duplicate_entry(oid, exclude))
-		return 0;
+		return;
 
 	if (!want_object_in_pack(oid, exclude, &found_pack, &found_offset)) {
 		/* The pack is missing an object, so it will not have closure */
@@ -1885,13 +1885,12 @@ static int add_object_entry(const struct object_id *oid, enum object_type type,
 				warning(_(no_closure_warning));
 			write_bitmap_index = 0;
 		}
-		return 0;
+		return;
 	}
 
 	create_object_entry(oid, type, pack_name_hash_fn(name),
 			    exclude, name && no_try_delta(name),
 			    found_pack, found_offset);
-	return 1;
 }
 
 static int add_object_entry_from_bitmap(const struct object_id *oid,
-- 
2.55.0.580.gbbcb530e9e
Jeff KingJul 11, 2026, 08:01 UTC in reply to Taylor Blau on lore

Re: [PATCH v2 3/8] pack-bitmap: allow aborting iteration of bitmapped objects

On Fri, Jul 10, 2026 at 03:34:53PM -0700, Taylor Blau wrote:
Show 9 quoted lines
> However, the remaining `show_objects_for_type()` callers from within
> `traverse_bitmap_commit_list()` do *not* bother to inspect the return
> value, despite taking in an arbitrary 'show_reachable_fn', which itself
> may return a non-zero value.
> 
> I guess this must be effectively OK in practice with respect to the
> existing code for the same reason you indicate in the commit message
> above, but we should change this function to *also* propagate non-zero
> return values to eliminate the foot-gun completely.

The matching non-bitmap traverse_commit_list() does not allow aborting based on callback returns, either. In fact, its callbacks return void!

Whichever direction we go, those two should probably stay in sync (so either both should allow aborting early with a non-zero return, or both should return void).

-Peff
Junio C HamanoJul 11, 2026, 16:42 UTC in reply to Jeff King on lore

Re: [PATCH v2 9/8?] pack-objects: drop unused return value from add_object_entry()

Jeff King <peff@peff.net> writes:
Show 30 quoted lines
> On Fri, Jul 10, 2026 at 10:48:52AM +0200, Patrick Steinhardt wrote:
>
>> The series is built on top of f85a7e6620 (Start Git 2.56 cycle,
>> 2026-07-06) with ps/odb-drop-whence at 8a7ad23e11 (odb: document object
>> info fields, 2026-07-02) merged into it.
>
> Here's a patch doing the cleanup I proposed upthread.
>
> -- >8 --
> Subject: pack-objects: drop unused return value from add_object_entry()
>
> This function returns 0/1 to its caller to tell them whether we actually
> added a new entry (or if we considered it redundant). But nobody has
> relied on that behavior since 5379a5c5ee (Thin pack generation:
> optimization., 2006-04-05).
>
> The extra return does not hurt much, but it recently became a bit more
> confusing. We have a sister function, add_object_entry_from_bitmap(),
> which had the same return value semantics. That function recently
> changed to always return 0 (not void, because it must conform to a
> callback function interface). So now we have two related functions which
> both return an "int" but with different semantics.
>
> Let's drop the unused "int" return from add_object_entry() entirely,
> which makes it more clear that the two functions have diverged.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I couldn't reference the commit by its id, since Junio has not yet
> picked up the v2 sent a few hours ago. ;)

Heh, if you do intend to make this a part of the series as 9/8, you can just say "earlier in the series" with its title, and that should be sufficient to identify which patch, as I never make a fast-forward merge when merging topics into integration branches (which means that Michael's "git when-merged" works well).

And if we ever see v3 of this series, you and Patrick can work together to see if it makes sense to squash it in, or move it earlier in a series to as preliminary clean-up, etc.

Thanks. I agree with the reasoning upthread that led to this change.

Patrick SteinhardtJul 13, 2026, 09:53 UTC in reply to Jeff King on lore

Re: [PATCH v2 3/8] pack-bitmap: allow aborting iteration of bitmapped objects

On Sat, Jul 11, 2026 at 04:01:14AM -0400, Jeff King wrote:
Show 18 quoted lines
> On Fri, Jul 10, 2026 at 03:34:53PM -0700, Taylor Blau wrote:
> 
> > However, the remaining `show_objects_for_type()` callers from within
> > `traverse_bitmap_commit_list()` do *not* bother to inspect the return
> > value, despite taking in an arbitrary 'show_reachable_fn', which itself
> > may return a non-zero value.
> > 
> > I guess this must be effectively OK in practice with respect to the
> > existing code for the same reason you indicate in the commit message
> > above, but we should change this function to *also* propagate non-zero
> > return values to eliminate the foot-gun completely.
> 
> The matching non-bitmap traverse_commit_list() does not allow aborting
> based on callback returns, either. In fact, its callbacks return void!
> 
> Whichever direction we go, those two should probably stay in sync (so
> either both should allow aborting early with a non-zero return, or both
> should return void).

That's fair. But adapting `traverse_commit_list()` requires tons of changes all over the tree, so I'm inclined to rather leave both `traverse_bitmap_commit_list()` and `traverse_commit_list()` as-is. Does that work for both of you?

Patrick
Patrick SteinhardtJul 13, 2026, 09:54 UTC in reply to Taylor Blau on lore

Re: [PATCH v2 1/8] odb/source-packed: improve lookup when enumerating objects

On Fri, Jul 10, 2026 at 03:25:10PM -0700, Taylor Blau wrote:
Show 16 quoted lines
> On Fri, Jul 10, 2026 at 10:48:53AM +0200, Patrick Steinhardt wrote:
> > Fix the issue by using `packed_object_info()` directly.
> 
> What you wrote here makes sense to me insofar as I understand the
> pluggable ODB code.
> 
> However, I am confused by the way this function is written in general.
> We use `bsearch_one_midx()` to locate the first possible MIDX position
> in which an object matching the given prefix may exist, which is
> sensible. However, we go from that position up to "num", where "num" is
> the total number of objects in the MIDX!
> 
> Functionally this is not incorrect as we will happily discard objects
> that do not match the prefix. But it causes us to waste CPU cycles
> repeatedly calling `match_hash()` (at least for the first byte of the
> prefix) for objects that we know will match.

That's not quite true though, as we abort iteration as soon as `match_hash()` tells us that the prefix doesn't match anymore.

Or do you mean that `num` should only be `m->num_objects` instead of also iterating through `num_objects_in_base`? I have to admit that I'm alwas struggling with the chained MIDX. It's never quite clear to me whether a given function cares about the complete chain or whether it really only cares about a single MIDX.

In any case, this code ultimately derives from 3f5f1cff92 (midx: introduce `bsearch_one_midx()`, 2024-08-06). If one squints a bit you can see that it's still roughly in the same shape.

Show 7 quoted lines
> How often do we call this function with a prefix longer than a
> single byte? I have no idea, but I would suspect that it makes up the
> majority of calls. If we read the OID fanout chunk, we could narrow the
> range that we enumerate through, and only compare the second byte
> onwards of the given prefix, if one exists. In the single-byte prefix
> case, this means that we shouldn't have to do any memory comparisons at
> all.

The function is currently used to find unique prefixes and to disambiguate object names. So whenever we either want to abbreviate a object ID or in case we cant to figure out whether a given object ID prefix is unique we'll end up calling it.

If this logic is currently wrong (or at least wasteful) though I'd propose to fix this in a separate series, as it's been this way for quite a while.

Thanks!
Patrick
Patrick SteinhardtJul 13, 2026, 14:41 UTC in reply to Patrick Steinhardt on lore

[PATCH v3 0/9] odb: introduce object filters to `odb_for_each_object()`

Hi,

this patch series introduces object filters to `odb_for_each_object()`. The intent of this is to make `git cat-file --batch-all-objects` work with pluggable object databases. Right now it doesn't because it reaches into internals of the "packed" backend to efficiently handle bitmapped objects.

The series is built on top of f85a7e6620 (Start Git 2.56 cycle, 2026-07-06) with ps/odb-drop-whence at 8a7ad23e11 (odb: document object info fields, 2026-07-02) merged into it.

Changes in v3:
  - Weave Peff's patch into the patch series.
  - Link to v2: https://patch.msgid.link/20260710-pks-odb-for-each-object-filter-v2-0-3710a9cc165a@pks.im
Changes in v2:
  - Add another patch to drop the `_1()` prefixes that aren't required
    anymore.
  - Change the approach in `open_bitmap_for_source()` to also use a
    `found` boolean instead of a confusing integer.
  - Add some more explanations to commit messages.
  - Link to v1: https://patch.msgid.link/20260709-pks-odb-for-each-object-filter-v1-0-82fe014b12b3@pks.im
Thanks!
Patrick
---
Jeff King (1):
      pack-objects: drop unused return value from add_object_entry()
Patrick Steinhardt (8):
      odb/source-packed: improve lookup when enumerating objects
      pack-bitmap: mark object filter as `const`
      pack-bitmap: allow aborting iteration of bitmapped objects
      pack-bitmap: iterate object sources when opening bitmaps
      pack-bitmap: drop `_1` suffix from functions that open bitmaps
      pack-bitmap: introduce function to open bitmap for a single source
      odb: introduce object filters to `odb_for_each_object()`
      builtin/cat-file: filter objects via object database
 builtin/cat-file.c     |  76 +++--------------------------
 builtin/pack-objects.c |  11 ++---
 builtin/rev-list.c     |   2 +-
 odb.h                  |  12 +++++
 odb/source-packed.c    |  77 ++++++++++++++++++++++++++---
 pack-bitmap.c          | 129 +++++++++++++++++++++++++++----------------------
 pack-bitmap.h          |  10 +++-
 7 files changed, 175 insertions(+), 142 deletions(-)
Range-diff versus v2:
 1:  baf2adb012 =  1:  7c0dc1be0d odb/source-packed: improve lookup when enumerating objects
 2:  57eecf3031 =  2:  2e5908c9c3 pack-bitmap: mark object filter as `const`
 -:  ---------- >  3:  f4d66ccfc6 pack-objects: drop unused return value from add_object_entry()
 3:  92dd6a6f6e =  4:  af475654b8 pack-bitmap: allow aborting iteration of bitmapped objects
 4:  92fe41577d =  5:  6ca42587c9 pack-bitmap: iterate object sources when opening bitmaps
 5:  e5d59959e3 =  6:  f62c3bbc81 pack-bitmap: drop `_1` suffix from functions that open bitmaps
 6:  ab3547ac2b =  7:  b2d25b6e9b pack-bitmap: introduce function to open bitmap for a single source
 7:  026f21f522 =  8:  a5bf309bec odb: introduce object filters to `odb_for_each_object()`
 8:  534b25c817 =  9:  600b15a907 builtin/cat-file: filter objects via object database

--- base-commit: 3c8e2790f2ce15e8b5d4b4e6ced711b12649f32a change-id: 20260708-pks-odb-for-each-object-filter-13286fa3523d

Patrick SteinhardtJul 13, 2026, 14:41 UTC in reply to Patrick Steinhardt on lore

[PATCH v3 1/9] odb/source-packed: improve lookup when enumerating objects

When iterating through objects of a packed source that have a specific prefix we do so via two different methods:

  - When a multi-pack index is available we use that one to efficiently
    loop through all objects.
  - We then loop through all packfiles that aren't covered by a
    multi-pack index.

Regardless of which mechanism we use, we then iterate through all the objects indexed by the respective data structure. Curiously though, while we use the indices for enumerating the objects, we completely ignore it for the actual object lookup. Instead, we call into the generic `odb_source_read_object_info()` function, which will itself consult the indices to figure out where the object in question even lives.

This has two consequences:
  - It's inefficient, as we basically have to figure out the position of
    the object a second time.
  - It's subtly wrong, as it may now happen that a specific object will
    be looked up via a different pack in case it exists multiple times.
    This is unlikely to have any real-world consequences, but it's still
    the wrong thing to do.

Fix the issue by using `packed_object_info()` directly. While at it, rename the `store` variable to `source`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-packed.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
Show changes to odb/source-packed.c +8 −7
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 0edea5356d..9cfa02b7a2 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -143,7 +143,7 @@ static bool should_exclude_pack(struct packed_git *p, enum odb_for_each_object_f
 }
 
 static int for_each_prefixed_object_in_midx(
-	struct odb_source_packed *store,
+	struct odb_source_packed *source,
 	struct multi_pack_index *m,
 	const struct odb_for_each_object_options *opts,
 	struct odb_source_packed_for_each_object_wrapper_data *data)
@@ -170,6 +170,7 @@ static int for_each_prefixed_object_in_midx(
 		 */
 		for (i = first; i < num; i++) {
 			const struct object_id *current = NULL;
+			struct packed_git *pack;
 			struct object_id oid;
 
 			current = nth_midxed_object_oid(&oid, m, i);
@@ -177,9 +178,8 @@ static int for_each_prefixed_object_in_midx(
 			if (!match_hash(len, opts->prefix->hash, current->hash))
 				break;
 
-			if (opts->flags) {
+			if (opts->flags || data->request) {
 				uint32_t pack_id = nth_midxed_pack_int_id(m, i);
-				struct packed_git *pack;
 
 				if (prepare_midx_pack(m, pack_id)) {
 					pack_errors = true;
@@ -193,9 +193,9 @@ static int for_each_prefixed_object_in_midx(
 
 			if (data->request) {
 				struct object_info oi = *data->request;
+				off_t offset = nth_midxed_offset(m, i);
 
-				ret = odb_source_read_object_info(&store->base, current,
-								  &oi, 0);
+				ret = packed_object_info(source, pack, offset, &oi);
 				if (ret)
 					goto out;
 
@@ -219,7 +219,7 @@ static int for_each_prefixed_object_in_midx(
 }
 
 static int for_each_prefixed_object_in_pack(
-	struct odb_source_packed *store,
+	struct odb_source_packed *source,
 	struct packed_git *p,
 	const struct odb_for_each_object_options *opts,
 	struct odb_source_packed_for_each_object_wrapper_data *data)
@@ -246,8 +246,9 @@ static int for_each_prefixed_object_in_pack(
 
 		if (data->request) {
 			struct object_info oi = *data->request;
+			off_t offset = nth_packed_object_offset(p, i);
 
-			ret = odb_source_read_object_info(&store->base, &oid, &oi, 0);
+			ret = packed_object_info(source, p, offset, &oi);
 			if (ret)
 				goto out;
 
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 13, 2026, 14:41 UTC in reply to Patrick Steinhardt on lore

[PATCH v3 2/9] pack-bitmap: mark object filter as `const`

The function `for_each_bitmapped_object()` accepts an optional object filter. This filter is never modified by the function, but is not declared as `const`. Fix this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pack-bitmap.c | 6 +++---
 pack-bitmap.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)
Show changes to 2 files +4 −4

pack-bitmap.c, pack-bitmap.h

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 35774b6f0c..a47c231632 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1976,7 +1976,7 @@ static void filter_bitmap_object_type(struct bitmap_index *bitmap_git,
 static int filter_bitmap(struct bitmap_index *bitmap_git,
 			 struct object_list *tip_objects,
 			 struct bitmap *to_filter,
-			 struct list_objects_filter_options *filter)
+			 const struct list_objects_filter_options *filter)
 {
 	if (!filter || filter->choice == LOFC_DISABLED)
 		return 0;
@@ -2027,7 +2027,7 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
 	return -1;
 }
 
-static int can_filter_bitmap(struct list_objects_filter_options *filter)
+static int can_filter_bitmap(const struct list_objects_filter_options *filter)
 {
 	return !filter_bitmap(NULL, NULL, NULL, filter);
 }
@@ -2058,7 +2058,7 @@ static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
 }
 
 int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
-			      struct list_objects_filter_options *filter,
+			      const struct list_objects_filter_options *filter,
 			      show_reachable_fn show_reach,
 			      void *payload)
 {
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 19a8655457..47935eb24e 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -96,7 +96,7 @@ struct list_objects_filter_options;
  * not supported, `0` otherwise.
  */
 int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
-			      struct list_objects_filter_options *filter,
+			      const struct list_objects_filter_options *filter,
 			      show_reachable_fn show_reach,
 			      void *payload);
 
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 13, 2026, 14:41 UTC in reply to Patrick Steinhardt on lore

[PATCH v3 3/9] pack-objects: drop unused return value from add_object_entry()

From: Jeff King <peff@peff.net>

This function returns 0/1 to its caller to tell them whether we actually added a new entry (or if we considered it redundant). But nobody has relied on that behavior since 5379a5c5ee (Thin pack generation: optimization., 2006-04-05).

The extra return does not hurt much, but it is a bit confusing. We have a sister function, add_object_entry_from_bitmap(), which has the same return value semantics. That function is about to change to always return 0 (not void, because it must conform to a callback function interface). So with that change, we'd have two related functions which both return an "int" but with different semantics.

Let's drop the unused "int" return from add_object_entry() entirely, which makes it more clear that the two functions have diverged.

Signed-off-by: Jeff King <peff@peff.net>
[ps: slightly massaged the commit message]
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/pack-objects.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
Show changes to builtin/pack-objects.c +4 −5
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ea5eab4cf8..188c4f6d4b 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1867,8 +1867,8 @@ static const char no_closure_warning[] = N_(
 "disabling bitmap writing, as some objects are not being packed"
 );
 
-static int add_object_entry(const struct object_id *oid, enum object_type type,
-			    const char *name, int exclude)
+static void add_object_entry(const struct object_id *oid, enum object_type type,
+			     const char *name, int exclude)
 {
 	struct packed_git *found_pack = NULL;
 	off_t found_offset = 0;
@@ -1876,7 +1876,7 @@ static int add_object_entry(const struct object_id *oid, enum object_type type,
 	display_progress(progress_state, ++nr_seen);
 
 	if (have_duplicate_entry(oid, exclude))
-		return 0;
+		return;
 
 	if (!want_object_in_pack(oid, exclude, &found_pack, &found_offset)) {
 		/* The pack is missing an object, so it will not have closure */
@@ -1885,13 +1885,12 @@ static int add_object_entry(const struct object_id *oid, enum object_type type,
 				warning(_(no_closure_warning));
 			write_bitmap_index = 0;
 		}
-		return 0;
+		return;
 	}
 
 	create_object_entry(oid, type, pack_name_hash_fn(name),
 			    exclude, name && no_try_delta(name),
 			    found_pack, found_offset);
-	return 1;
 }
 
 static int add_object_entry_from_bitmap(const struct object_id *oid,
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 13, 2026, 14:41 UTC in reply to Patrick Steinhardt on lore

[PATCH v3 4/9] pack-bitmap: allow aborting iteration of bitmapped objects

In a subsequent commit we'll lift iteration of bitmapped objects into the "packed" backend and make it accessible via `odb_for_each_object()`. The calling convention for that function is that the callback may return a non-zero exit code, and if so we'll abort iteration. This is currently impossible to realize though, as `for_each_bitmapped_object()` will ignore any return value and just churn through all objects completely.

This doesn't matter to the callers of `for_each_bitmapped_object()`, as there's only one of them in git-cat-file(1), and the callbacks we pass always return zero. But once we move the logic into the generic infrastructure it becomes a latent bug waiting to happen.

Refactor the code so that the return value of the `show_reach` callback is not ignored anymore. Instead, returning a non-zero value will cause us to abort iteration in both `show_objects_for_type()` and in `for_each_bitmapped_object()`.

Note though that there's a second user of `show_objects_for_type()` with `traverse_bitmap_commit_list()`, and that function does indeed invoke callbacks that may return non-zero. This non-zero return value never had any effect at all though, and the callbacks that return non-zero values are only ever invoked via `traverse_bitmap_commit_list()`. Consequently, we adapt them to always return 0.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/pack-objects.c |  2 +-
 builtin/rev-list.c     |  2 +-
 pack-bitmap.c          | 31 +++++++++++++++++++++----------
 pack-bitmap.h          |  3 ++-
 4 files changed, 25 insertions(+), 13 deletions(-)
Show changes to 4 files +25 −13

builtin/pack-objects.c, builtin/rev-list.c, pack-bitmap.c, pack-bitmap.h

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 188c4f6d4b..3673b14b89 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1908,7 +1908,7 @@ static int add_object_entry_from_bitmap(const struct object_id *oid,
 		return 0;
 
 	create_object_entry(oid, type, name_hash, 0, 0, pack, offset);
-	return 1;
+	return 0;
 }
 
 struct pbase_tree_cache {
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 8f63003709..02818b81c6 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -486,7 +486,7 @@ static int show_object_fast(
 	void *payload UNUSED)
 {
 	fprintf(stdout, "%s\n", oid_to_hex(oid));
-	return 1;
+	return 0;
 }
 
 static void print_disk_usage(off_t size)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index a47c231632..eda38a5433 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1695,7 +1695,7 @@ static void init_type_iterator(struct ewah_or_iterator *it,
 	}
 }
 
-static void show_objects_for_type(
+static int show_objects_for_type(
 	struct bitmap_index *bitmap_git,
 	struct bitmap *objects,
 	enum object_type object_type,
@@ -1704,6 +1704,7 @@ static void show_objects_for_type(
 {
 	size_t i = 0;
 	uint32_t offset;
+	int ret;
 
 	struct ewah_or_iterator it;
 	eword_t filter;
@@ -1749,11 +1750,17 @@ static void show_objects_for_type(
 
 			hash = bitmap_name_hash(bitmap_git, index_pos);
 
-			show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
+			ret = show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
+			if (ret)
+				goto out;
 		}
 	}
 
+	ret = 0;
+
+out:
 	ewah_or_iterator_release(&it);
+	return ret;
 }
 
 static int in_bitmapped_pack(struct bitmap_index *bitmap_git,
@@ -2062,6 +2069,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
 			      show_reachable_fn show_reach,
 			      void *payload)
 {
+	const enum object_type types[] = {
+		OBJ_COMMIT,
+		OBJ_TREE,
+		OBJ_BLOB,
+		OBJ_TAG,
+	};
 	struct bitmap *filtered_bitmap = NULL;
 	uint32_t objects_nr;
 	size_t full_word_count;
@@ -2086,14 +2099,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
 		goto out;
 	}
 
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_COMMIT, show_reach, payload);
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_TREE, show_reach, payload);
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_BLOB, show_reach, payload);
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_TAG, show_reach, payload);
+	for (size_t i = 0; i < ARRAY_SIZE(types); i++) {
+		ret = show_objects_for_type(bitmap_git, filtered_bitmap,
+					    types[i], show_reach, payload);
+		if (ret)
+			goto out;
+	}
 
 	ret = 0;
 out:
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 47935eb24e..ae8dc491ac 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -93,7 +93,8 @@ struct list_objects_filter_options;
 /*
  * Filter bitmapped objects and iterate through all resulting objects,
  * executing `show_reach` for each of them. Returns `-1` in case the filter is
- * not supported, `0` otherwise.
+ * not supported, `0` otherwise. Aborts iteration and bubbles up the return
+ * value in case `show_reach()` returns non-zero.
  */
 int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
 			      const struct list_objects_filter_options *filter,
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 13, 2026, 14:41 UTC in reply to Patrick Steinhardt on lore

[PATCH v3 5/9] pack-bitmap: iterate object sources when opening bitmaps

When opening a bitmap for a repository we perform two steps:
  - We first look for a multi-pack index bitmap in any of the object
    sources connected to the repository.
  - We then look for a packfile bitmap in any of the packfiles of any of
    the object sources.

Both of these steps thus iterate through object sources themselves, one via `odb_prepare_alternates()` and one via `repo_for_each_pack()`. This layout makes it hard to introduce a way to open the bitmap of one specific object source, which is functionality that we'll require in a subsequent commit.

Reverse the loop so that we instead loop through all sources in the outer loop, and then for each source we try to load its bitmap via either the multi-pack index or via a packfile.

Note that this changes the precedence of bitmaps in one specific edge case: when an earlier object source only has a packfile bitmap, but a later source has a multi-pack index bitmap, we now pick the packfile bitmap of the earlier source. Previously, a multi-pack index bitmap from any source would have taken precedence over all packfile bitmaps. Given that object sources are ordered such that the local source comes first, this arguably is an improvement, as we now prefer local bitmaps over bitmaps in alternates. Furthermore, we already warn about repositories that have multiple bitmaps, so this setup is broken and thus arguably not worth worrying about too much.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pack-bitmap.c | 69 +++++++++++++++++++++++++++--------------------------------
 1 file changed, 31 insertions(+), 38 deletions(-)
Show changes to pack-bitmap.c +31 −38
diff --git a/pack-bitmap.c b/pack-bitmap.c
index eda38a5433..e32795a595 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -680,60 +680,53 @@ static int load_bitmap(struct repository *r, struct bitmap_index *bitmap_git,
 	return 0;
 }
 
-static int open_pack_bitmap(struct repository *r,
-			    struct bitmap_index *bitmap_git)
+static int open_bitmap_for_source(struct odb_source_packed *source,
+				  struct bitmap_index *bitmap_git)
 {
-	struct packed_git *p;
-	int ret = -1;
+	struct multi_pack_index *midx = get_multi_pack_index(source);
+	struct packfile_list_entry *e;
+	bool found = false;
 
-	repo_for_each_pack(r, p) {
-		if (open_pack_bitmap_1(bitmap_git, p) == 0) {
-			ret = 0;
-			/*
-			 * The only reason to keep looking is to report
-			 * duplicates.
-			 */
-			if (!trace2_is_enabled())
-				break;
-		}
+	if (midx && !open_midx_bitmap_1(bitmap_git, midx))
+		found = true;
+
+	for (e = packfile_store_get_packs(source); e; e = e->next) {
+		/*
+		 * When tracing is enabled we want to keep looking to report
+		 * duplicates even if we have already found a bitmap.
+		 */
+		if (found && !trace2_is_enabled())
+			break;
+
+		if (!open_pack_bitmap_1(bitmap_git, e->pack))
+			found = true;
 	}
 
-	return ret;
+	return found ? 0 : -1;
 }
 
-static int open_midx_bitmap(struct repository *r,
-			    struct bitmap_index *bitmap_git)
+static int open_bitmap(struct repository *r,
+		       struct bitmap_index *bitmap_git)
 {
 	struct odb_source *source;
-	int ret = -1;
+	bool found = false;
 
 	assert(!bitmap_git->map);
 
 	odb_prepare_alternates(r->objects);
 	for (source = r->objects->sources; source; source = source->next) {
 		struct odb_source_files *files = odb_source_files_downcast(source);
-		struct multi_pack_index *midx = get_multi_pack_index(files->packed);
-		if (midx && !open_midx_bitmap_1(bitmap_git, midx))
-			ret = 0;
-	}
-	return ret;
-}
-
-static int open_bitmap(struct repository *r,
-		       struct bitmap_index *bitmap_git)
-{
-	int found;
 
-	assert(!bitmap_git->map);
+		if (!open_bitmap_for_source(files->packed, bitmap_git))
+			found = true;
 
-	found = !open_midx_bitmap(r, bitmap_git);
-
-	/*
-	 * these will all be skipped if we opened a midx bitmap; but run it
-	 * anyway if tracing is enabled to report the duplicates
-	 */
-	if (!found || trace2_is_enabled())
-		found |= !open_pack_bitmap(r, bitmap_git);
+		/*
+		 * The only reason to keep looking after having found a bitmap
+		 * is to report duplicates.
+		 */
+		if (found && !trace2_is_enabled())
+			break;
+	}
 
 	return found ? 0 : -1;
 }
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 13, 2026, 14:41 UTC in reply to Patrick Steinhardt on lore

[PATCH v3 6/9] pack-bitmap: drop `_1` suffix from functions that open bitmaps

In the preceding commit we've refactored how we open bitmaps. As part of the refactoring we have consolidated `open_pack_bitmap()` as well as `open_midx_bitmap()` into `open_bitmap_for_source()`. Consequently, we only have their `open_pack_bitmap_1()` and `open_midx_bitmap_1()` variants left over, where the `_1` suffix doesn't really make much sense anymore.

Drop the suffix.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pack-bitmap.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
Show changes to pack-bitmap.c +7 −7
diff --git a/pack-bitmap.c b/pack-bitmap.c
index e32795a595..72c8ae3228 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -460,8 +460,8 @@ char *pack_bitmap_filename(struct packed_git *p)
 	return xstrfmt("%.*s.bitmap", (int)len, p->pack_name);
 }
 
-static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
-			      struct multi_pack_index *midx)
+static int open_midx_bitmap(struct bitmap_index *bitmap_git,
+			    struct multi_pack_index *midx)
 {
 	struct stat st;
 	char *bitmap_name = midx_bitmap_filename(midx);
@@ -539,7 +539,7 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
 	return -1;
 }
 
-static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git *packfile)
+static int open_pack_bitmap(struct bitmap_index *bitmap_git, struct packed_git *packfile)
 {
 	int fd;
 	struct stat st;
@@ -603,7 +603,7 @@ static int load_reverse_index(struct repository *r, struct bitmap_index *bitmap_
 
 		/*
 		 * The multi-pack-index's .rev file is already loaded via
-		 * open_pack_bitmap_1().
+		 * open_pack_bitmap().
 		 *
 		 * But we still need to open the individual pack .rev files,
 		 * since we will need to make use of them in pack-objects.
@@ -687,7 +687,7 @@ static int open_bitmap_for_source(struct odb_source_packed *source,
 	struct packfile_list_entry *e;
 	bool found = false;
 
-	if (midx && !open_midx_bitmap_1(bitmap_git, midx))
+	if (midx && !open_midx_bitmap(bitmap_git, midx))
 		found = true;
 
 	for (e = packfile_store_get_packs(source); e; e = e->next) {
@@ -698,7 +698,7 @@ static int open_bitmap_for_source(struct odb_source_packed *source,
 		if (found && !trace2_is_enabled())
 			break;
 
-		if (!open_pack_bitmap_1(bitmap_git, e->pack))
+		if (!open_pack_bitmap(bitmap_git, e->pack))
 			found = true;
 	}
 
@@ -746,7 +746,7 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
 {
 	struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
 
-	if (!open_midx_bitmap_1(bitmap_git, midx))
+	if (!open_midx_bitmap(bitmap_git, midx))
 		return bitmap_git;
 
 	free_bitmap_index(bitmap_git);
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 13, 2026, 14:41 UTC in reply to Patrick Steinhardt on lore

[PATCH v3 7/9] pack-bitmap: introduce function to open bitmap for a single source

The function `prepare_bitmap_git()` opens the first bitmap it can find in any of the object sources connected to the repository. In a subsequent commit, the "packed" object database backend will learn to use bitmaps to answer object filters when enumerating objects. That backend operates on a single object source though, so using a bitmap that potentially belongs to a different source would be wrong:

  - The source would yield objects that are not part of the source
    itself.
  - The object source info would be attributed to the wrong source.
  - With multiple sources, each source would enumerate the same bitmap
    another time.

Introduce a new function `prepare_source_bitmap_git()` that only opens bitmaps belonging to the given object source.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pack-bitmap.c | 12 ++++++++++++
 pack-bitmap.h |  2 ++
 2 files changed, 14 insertions(+)
Show changes to 2 files +14 −0

pack-bitmap.c, pack-bitmap.h

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 72c8ae3228..09ba15d26b 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -753,6 +753,18 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
 	return NULL;
 }
 
+struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source)
+{
+	struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
+
+	if (!open_bitmap_for_source(source, bitmap_git) &&
+	    !load_bitmap(source->base.odb->repo, bitmap_git, 0))
+		return bitmap_git;
+
+	free_bitmap_index(bitmap_git);
+	return NULL;
+}
+
 int bitmap_index_contains_pack(struct bitmap_index *bitmap, struct packed_git *pack)
 {
 	for (; bitmap; bitmap = bitmap->base) {
diff --git a/pack-bitmap.h b/pack-bitmap.h
index ae8dc491ac..9f20fb6e56 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -9,6 +9,7 @@
 #include "string-list.h"
 
 struct commit;
+struct odb_source_packed;
 struct repository;
 struct rev_info;
 
@@ -68,6 +69,7 @@ struct bitmapped_pack {
 
 struct bitmap_index *prepare_bitmap_git(struct repository *r);
 struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx);
+struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source);
 
 /*
  * Given a bitmap index, determine whether it contains the pack either directly
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 13, 2026, 14:41 UTC in reply to Patrick Steinhardt on lore

[PATCH v3 8/9] odb: introduce object filters to `odb_for_each_object()`

The function `for_each_bitmapped_object()` can be used to iterate through all objects covered by a bitmap. The benefit of this function is that it allows the caller to efficiently handle some object filters. For example, this can be used to filter out objects of a specific type with some simple bitmap operations. But callers are currently required to manually wire up the use of bitmaps though, and to do so they have to reach into internals of a given object database source.

Introduce a new `struct odb_for_each_object_options::filter` field so that the interface becomes generic. When set, then a backend may optionally use the filter to skip some objects that it would have otherwise yielded.

Note that the respective backends are free to ignore this field if they cannot meaningfully optimize for a given filter, and consequently callers need to verify whether they actually want the returned objects. While annoying, we cannot easily lift this restriction anyway as the object filter infrastructure supports some filters that cannot be answered by the object database alone.

An alternative might be to limit the filters to only those that _can_ be answered by backends. But ultimately, the filters that can be answered efficiently by the "packed" backend are completely disjunct from those that can be answered by the "loose" backend, and consequently the set of filters supported by all backends would be empty. Furthermore, it would require us to make assumptions about capabilities of future backends, which may be able to efficiently handle more filters than current ones. So in the end, this alternative would only limit us artificially.

Implement the logic for the "packed" source. Note that we use the new function `prepare_source_bitmap_git()` to open the bitmap: as the backend operates on a single object source, we must only use bitmaps that belong to that specific source. Otherwise we might yield objects that are not part of the source at all, and with multiple sources we would enumerate the same bitmap once per source.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.h               | 12 +++++++++++
 odb/source-packed.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 pack-bitmap.c       |  3 +--
 pack-bitmap.h       |  3 +++
 4 files changed, 78 insertions(+), 2 deletions(-)
Show changes to 4 files +78 −2

odb.h, odb/source-packed.c, pack-bitmap.c, pack-bitmap.h

diff --git a/odb.h b/odb.h
index a1e222f605..67d0b34942 100644
--- a/odb.h
+++ b/odb.h
@@ -8,6 +8,7 @@
 #include "thread-utils.h"
 
 struct cached_object_entry;
+struct list_objects_filter_options;
 struct odb_source_inmemory;
 struct packed_git;
 struct repository;
@@ -490,6 +491,17 @@ struct odb_for_each_object_options {
 	 */
 	const struct object_id *prefix;
 	size_t prefix_hex_len;
+
+	/*
+	 * Optional object filter that allows backends to skip yielding
+	 * objects that are excluded by the filter as an optimization. The
+	 * filter is a best-effort hint: backends may use it to skip
+	 * excluded objects (e.g. by consulting a reachability bitmap), but
+	 * are also free to ignore it entirely and yield every object. As a
+	 * consequence, callers must re-apply the filter on yielded objects
+	 * if they require strict filtering semantics.
+	 */
+	const struct list_objects_filter_options *filter;
 };
 
 /*
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 9cfa02b7a2..4777395053 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -3,11 +3,13 @@
 #include "chdir-notify.h"
 #include "dir.h"
 #include "git-zlib.h"
+#include "list-objects-filter-options.h"
 #include "mergesort.h"
 #include "midx.h"
 #include "odb/source-packed.h"
 #include "odb/streaming.h"
 #include "packfile.h"
+#include "pack-bitmap.h"
 
 static int find_pack_entry(struct odb_source_packed *store,
 			   const struct object_id *oid,
@@ -315,6 +317,37 @@ static int odb_source_packed_for_each_prefixed_object(
 	return ret;
 }
 
+struct bitmapped_for_each_object_data {
+	struct odb_source_packed *packed;
+	const struct object_info *request;
+	const struct odb_for_each_object_options *opts;
+	odb_for_each_object_cb cb;
+	void *cb_data;
+};
+
+static int bitmapped_for_each_object(const struct object_id *oid,
+				     enum object_type type UNUSED,
+				     int flags UNUSED,
+				     uint32_t hash UNUSED,
+				     struct packed_git *pack,
+				     off_t offset,
+				     void *cb_data)
+{
+	struct bitmapped_for_each_object_data *data = cb_data;
+
+	if (should_exclude_pack(pack, data->opts->flags))
+		return 0;
+
+	if (data->request) {
+		struct object_info oi = *data->request;
+		if (packed_object_info(data->packed, pack, offset, &oi) < 0)
+			return -1;
+		return data->cb(oid, &oi, data->cb_data);
+	}
+
+	return data->cb(oid, NULL, data->cb_data);
+}
+
 static int odb_source_packed_for_each_object(struct odb_source *source,
 					     const struct object_info *request,
 					     odb_for_each_object_cb cb,
@@ -328,12 +361,33 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
 		.cb = cb,
 		.cb_data = cb_data,
 	};
+	struct bitmap_index *bitmap = NULL;
 	struct packfile_list_entry *e;
 	int pack_errors = 0, ret;
 
 	if (opts->prefix)
 		return odb_source_packed_for_each_prefixed_object(packed, opts, &data);
 
+	if (opts->filter &&
+	    opts->filter->choice != LOFC_DISABLED &&
+	    can_filter_bitmap(opts->filter))
+		bitmap = prepare_bitmap_git_for_source(packed);
+	if (bitmap) {
+		struct bitmapped_for_each_object_data bitmap_data = {
+			.packed = packed,
+			.request = request,
+			.opts = opts,
+			.cb = cb,
+			.cb_data = cb_data,
+		};
+
+		ret = for_each_bitmapped_object(bitmap, opts->filter,
+						bitmapped_for_each_object,
+						&bitmap_data);
+		if (ret)
+			goto out;
+	}
+
 	packed->skip_mru_updates = true;
 
 	for (e = packfile_store_get_packs(packed); e; e = e->next) {
@@ -342,6 +396,13 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
 		if (should_exclude_pack(p, opts->flags))
 			continue;
 
+		/*
+		 * Objects covered by the bitmap have already been yielded
+		 * above; skip them here to avoid duplicates.
+		 */
+		if (bitmap && bitmap_index_contains_pack(bitmap, p))
+			continue;
+
 		if (open_pack_index(p)) {
 			pack_errors = 1;
 			continue;
@@ -357,6 +418,7 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
 
 out:
 	packed->skip_mru_updates = false;
+	free_bitmap_index(bitmap);
 
 	if (!ret && pack_errors)
 		ret = -1;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 09ba15d26b..f55a0859ea 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2039,12 +2039,11 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
 	return -1;
 }
 
-static int can_filter_bitmap(const struct list_objects_filter_options *filter)
+bool can_filter_bitmap(const struct list_objects_filter_options *filter)
 {
 	return !filter_bitmap(NULL, NULL, NULL, filter);
 }
 
-
 static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
 					      struct bitmap *result)
 {
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 9f20fb6e56..1385027c1f 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -92,6 +92,9 @@ int test_bitmap_pseudo_merge_objects(struct repository *r, uint32_t n);
 
 struct list_objects_filter_options;
 
+/* Check whether the filter can be computed via the bitmap. */
+bool can_filter_bitmap(const struct list_objects_filter_options *filter);
+
 /*
  * Filter bitmapped objects and iterate through all resulting objects,
  * executing `show_reach` for each of them. Returns `-1` in case the filter is
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 13, 2026, 14:41 UTC in reply to Patrick Steinhardt on lore

[PATCH v3 9/9] builtin/cat-file: filter objects via object database

When batching all objects, git-cat-file(1) reaches into the internals of the object database and manually manages bitmaps to apply object filters. This creates coupling between the command and the internals of the respective backend.

Refactor git-cat-file(1) to use the new object filter option when batching all objects. This significantly simplifies the logic and ensures that we don't have to reach into internals of the "files" source anymore.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/cat-file.c | 76 +++++-------------------------------------------------
 1 file changed, 7 insertions(+), 69 deletions(-)
Show changes to builtin/cat-file.c +7 −69
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index b4b99a73da..1458dd76d6 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,7 +20,6 @@
 #include "userdiff.h"
 #include "oid-array.h"
 #include "packfile.h"
-#include "pack-bitmap.h"
 #include "object-file.h"
 #include "object-name.h"
 #include "odb.h"
@@ -844,28 +843,6 @@ static int batch_one_object_oi(const struct object_id *oid,
 	return payload->callback(oid, NULL, 0, payload->payload);
 }
 
-static int batch_one_object_packed(const struct object_id *oid,
-				   struct packed_git *pack,
-				   uint32_t pos,
-				   void *_payload)
-{
-	struct for_each_object_payload *payload = _payload;
-	return payload->callback(oid, pack, nth_packed_object_offset(pack, pos),
-				 payload->payload);
-}
-
-static int batch_one_object_bitmapped(const struct object_id *oid,
-				      enum object_type type UNUSED,
-				      int flags UNUSED,
-				      uint32_t hash UNUSED,
-				      struct packed_git *pack,
-				      off_t offset,
-				      void *_payload)
-{
-	struct for_each_object_payload *payload = _payload;
-	return payload->callback(oid, pack, offset, payload->payload);
-}
-
 static void batch_each_object(struct batch_options *opt,
 			      for_each_object_fn callback,
 			      unsigned flags,
@@ -875,56 +852,17 @@ static void batch_each_object(struct batch_options *opt,
 		.callback = callback,
 		.payload = _payload,
 	};
+	struct odb_source_info source_info;
+	struct object_info oi = {
+		.source_infop = &source_info,
+	};
 	struct odb_for_each_object_options opts = {
 		.flags = flags,
+		.filter = &opt->objects_filter,
 	};
-	struct bitmap_index *bitmap = NULL;
-	struct odb_source *source;
-
-	/*
-	 * TODO: we still need to tap into implementation details of the object
-	 * database sources. Ideally, we should extend `odb_for_each_object()`
-	 * to handle object filters itself so that we can move the filtering
-	 * logic into the individual sources.
-	 */
-	odb_prepare_alternates(the_repository->objects);
-	for (source = the_repository->objects->sources; source; source = source->next) {
-		struct odb_source_files *files = odb_source_files_downcast(source);
-		int ret = odb_source_for_each_object(&files->loose->base, NULL, batch_one_object_oi,
-						     &payload, &opts);
-		if (ret)
-			break;
-	}
-
-	if (opt->objects_filter.choice != LOFC_DISABLED &&
-	    (bitmap = prepare_bitmap_git(the_repository)) &&
-	    !for_each_bitmapped_object(bitmap, &opt->objects_filter,
-				       batch_one_object_bitmapped, &payload)) {
-		struct packed_git *pack;
-
-		repo_for_each_pack(the_repository, pack) {
-			if (bitmap_index_contains_pack(bitmap, pack) ||
-			    open_pack_index(pack))
-				continue;
-			for_each_object_in_pack(pack, batch_one_object_packed,
-						&payload, flags);
-		}
-	} else {
-		struct odb_source_info source_info;
-		struct object_info oi = {
-			.source_infop = &source_info,
-		};
-
-		for (source = the_repository->objects->sources; source; source = source->next) {
-			struct odb_source_files *files = odb_source_files_downcast(source);
-			int ret = odb_source_for_each_object(&files->packed->base, &oi,
-							     batch_one_object_oi, &payload, &opts);
-			if (ret)
-				break;
-		}
-	}
 
-	free_bitmap_index(bitmap);
+	odb_for_each_object_ext(the_repository->objects, &oi,
+				batch_one_object_oi, &payload, &opts);
 }
 
 static int batch_objects(struct batch_options *opt)
-- 
2.55.0.313.g8d093f411d.dirty
Taylor BlauJul 14, 2026, 03:49 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v2 1/8] odb/source-packed: improve lookup when enumerating objects

On Mon, Jul 13, 2026 at 11:54:43AM +0200, Patrick Steinhardt wrote:
Show 20 quoted lines
> On Fri, Jul 10, 2026 at 03:25:10PM -0700, Taylor Blau wrote:
> > On Fri, Jul 10, 2026 at 10:48:53AM +0200, Patrick Steinhardt wrote:
> > > Fix the issue by using `packed_object_info()` directly.
> >
> > What you wrote here makes sense to me insofar as I understand the
> > pluggable ODB code.
> >
> > However, I am confused by the way this function is written in general.
> > We use `bsearch_one_midx()` to locate the first possible MIDX position
> > in which an object matching the given prefix may exist, which is
> > sensible. However, we go from that position up to "num", where "num" is
> > the total number of objects in the MIDX!
> >
> > Functionally this is not incorrect as we will happily discard objects
> > that do not match the prefix. But it causes us to waste CPU cycles
> > repeatedly calling `match_hash()` (at least for the first byte of the
> > prefix) for objects that we know will match.
>
> That's not quite true though, as we abort iteration as soon as
> `match_hash()` tells us that the prefix doesn't match anymore.

Right, we neither iterate through more objects than necessary once we know that `match_hash()` will stop returning true, nor do we emit objects that don't actually match the prefix.

What I was trying to say above is that in the special case where our prefix is a single byte long, we don't have to call `match_hash()` at *all*, since we can enumerate just the portion of the fanout for that specific byte, and we know that all such entries will match.

Show 5 quoted lines
> Or do you mean that `num` should only be `m->num_objects` instead of
> also iterating through `num_objects_in_base`? I have to admit that I'm
> alwas struggling with the chained MIDX. It's never quite clear to me
> whether a given function cares about the complete chain or whether it
> really only cares about a single MIDX.

If the goal is to yield all such objects that match the prefix, then we need to enumerate each layer. The analogy that I have had in my head while working on these is that they are the same conceptually as the incremental commit-graph format.

Thanks, Taylor

Taylor BlauJul 14, 2026, 03:58 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v2 3/8] pack-bitmap: allow aborting iteration of bitmapped objects

On Mon, Jul 13, 2026 at 11:53:50AM +0200, Patrick Steinhardt wrote:
Show 24 quoted lines
> On Sat, Jul 11, 2026 at 04:01:14AM -0400, Jeff King wrote:
> > On Fri, Jul 10, 2026 at 03:34:53PM -0700, Taylor Blau wrote:
> >
> > > However, the remaining `show_objects_for_type()` callers from within
> > > `traverse_bitmap_commit_list()` do *not* bother to inspect the return
> > > value, despite taking in an arbitrary 'show_reachable_fn', which itself
> > > may return a non-zero value.
> > >
> > > I guess this must be effectively OK in practice with respect to the
> > > existing code for the same reason you indicate in the commit message
> > > above, but we should change this function to *also* propagate non-zero
> > > return values to eliminate the foot-gun completely.
> >
> > The matching non-bitmap traverse_commit_list() does not allow aborting
> > based on callback returns, either. In fact, its callbacks return void!
> >
> > Whichever direction we go, those two should probably stay in sync (so
> > either both should allow aborting early with a non-zero return, or both
> > should return void).
>
> That's fair. But adapting `traverse_commit_list()` requires tons of
> changes all over the tree, so I'm inclined to rather leave both
> `traverse_bitmap_commit_list()` and `traverse_commit_list()` as-is.
> Does that work for both of you?

I think that it's fine to leave it as-is for the purpose of this series, though I would like to address it.

I don't think we need to adapt `traverse_commit_list()`, though. We can go in the other direction Peff suggested, which would be to split the callback type used by `for_each_bitmapped_object()` from `show_reachable_fn`, keep the former abortable, and make the latter return void.

That keeps `traverse_bitmap_commit_list()` in sync with `traverse_commit_list()` without changing the non-bitmap traversal machinery. I have a small two-patch follow-up on top of v3 that does this, which I'll send separately.

Thanks, Taylor

Taylor BlauJul 14, 2026, 03:59 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v3 0/9] odb: introduce object filters to `odb_for_each_object()`

On Mon, Jul 13, 2026 at 04:41:24PM +0200, Patrick Steinhardt wrote:
Show 11 quoted lines
> Range-diff versus v2:
>
>  1:  baf2adb012 =  1:  7c0dc1be0d odb/source-packed: improve lookup when enumerating objects
>  2:  57eecf3031 =  2:  2e5908c9c3 pack-bitmap: mark object filter as `const`
>  -:  ---------- >  3:  f4d66ccfc6 pack-objects: drop unused return value from add_object_entry()
>  3:  92dd6a6f6e =  4:  af475654b8 pack-bitmap: allow aborting iteration of bitmapped objects
>  4:  92fe41577d =  5:  6ca42587c9 pack-bitmap: iterate object sources when opening bitmaps
>  5:  e5d59959e3 =  6:  f62c3bbc81 pack-bitmap: drop `_1` suffix from functions that open bitmaps
>  6:  ab3547ac2b =  7:  b2d25b6e9b pack-bitmap: introduce function to open bitmap for a single source
>  7:  026f21f522 =  8:  a5bf309bec odb: introduce object filters to `odb_for_each_object()`
>  8:  534b25c817 =  9:  600b15a907 builtin/cat-file: filter objects via object database
Thanks, this version looks good to me.

Thanks, Taylor

Patrick SteinhardtJul 14, 2026, 05:35 UTC in reply to Taylor Blau on lore

Re: [PATCH v2 1/8] odb/source-packed: improve lookup when enumerating objects

On Mon, Jul 13, 2026 at 08:49:43PM -0700, Taylor Blau wrote:
Show 30 quoted lines
> On Mon, Jul 13, 2026 at 11:54:43AM +0200, Patrick Steinhardt wrote:
> > On Fri, Jul 10, 2026 at 03:25:10PM -0700, Taylor Blau wrote:
> > > On Fri, Jul 10, 2026 at 10:48:53AM +0200, Patrick Steinhardt wrote:
> > > > Fix the issue by using `packed_object_info()` directly.
> > >
> > > What you wrote here makes sense to me insofar as I understand the
> > > pluggable ODB code.
> > >
> > > However, I am confused by the way this function is written in general.
> > > We use `bsearch_one_midx()` to locate the first possible MIDX position
> > > in which an object matching the given prefix may exist, which is
> > > sensible. However, we go from that position up to "num", where "num" is
> > > the total number of objects in the MIDX!
> > >
> > > Functionally this is not incorrect as we will happily discard objects
> > > that do not match the prefix. But it causes us to waste CPU cycles
> > > repeatedly calling `match_hash()` (at least for the first byte of the
> > > prefix) for objects that we know will match.
> >
> > That's not quite true though, as we abort iteration as soon as
> > `match_hash()` tells us that the prefix doesn't match anymore.
> 
> Right, we neither iterate through more objects than necessary once we
> know that `match_hash()` will stop returning true, nor do we emit
> objects that don't actually match the prefix.
> 
> What I was trying to say above is that in the special case where our
> prefix is a single byte long, we don't have to call `match_hash()` at
> *all*, since we can enumerate just the portion of the fanout for that
> specific byte, and we know that all such entries will match.

Oh, now that's what you're getting at. I don't think that this case ever happens at all right now. I think the shortest prefix that we're ever using should be at least 2 bytes, as we don't treat anything shorter than 4 hex characters as an abbreviated object ID.

Thanks for clarifying!
Patrick
Patrick SteinhardtJul 14, 2026, 05:36 UTC in reply to Taylor Blau on lore

Re: [PATCH v3 0/9] odb: introduce object filters to `odb_for_each_object()`

On Mon, Jul 13, 2026 at 08:59:39PM -0700, Taylor Blau wrote:
Show 14 quoted lines
> On Mon, Jul 13, 2026 at 04:41:24PM +0200, Patrick Steinhardt wrote:
> > Range-diff versus v2:
> >
> >  1:  baf2adb012 =  1:  7c0dc1be0d odb/source-packed: improve lookup when enumerating objects
> >  2:  57eecf3031 =  2:  2e5908c9c3 pack-bitmap: mark object filter as `const`
> >  -:  ---------- >  3:  f4d66ccfc6 pack-objects: drop unused return value from add_object_entry()
> >  3:  92dd6a6f6e =  4:  af475654b8 pack-bitmap: allow aborting iteration of bitmapped objects
> >  4:  92fe41577d =  5:  6ca42587c9 pack-bitmap: iterate object sources when opening bitmaps
> >  5:  e5d59959e3 =  6:  f62c3bbc81 pack-bitmap: drop `_1` suffix from functions that open bitmaps
> >  6:  ab3547ac2b =  7:  b2d25b6e9b pack-bitmap: introduce function to open bitmap for a single source
> >  7:  026f21f522 =  8:  a5bf309bec odb: introduce object filters to `odb_for_each_object()`
> >  8:  534b25c817 =  9:  600b15a907 builtin/cat-file: filter objects via object database
> 
> Thanks, this version looks good to me.
Thanks for your review!
Patrick
Jeff KingJul 14, 2026, 07:17 UTC in reply to Taylor Blau on lore

Re: [PATCH v2 3/8] pack-bitmap: allow aborting iteration of bitmapped objects

On Mon, Jul 13, 2026 at 08:58:54PM -0700, Taylor Blau wrote:
Show 7 quoted lines
> > That's fair. But adapting `traverse_commit_list()` requires tons of
> > changes all over the tree, so I'm inclined to rather leave both
> > `traverse_bitmap_commit_list()` and `traverse_commit_list()` as-is.
> > Does that work for both of you?
> 
> I think that it's fine to leave it as-is for the purpose of this series,
> though I would like to address it.
Me too.
Show 10 quoted lines
> I don't think we need to adapt `traverse_commit_list()`, though. We can
> go in the other direction Peff suggested, which would be to split the
> callback type used by `for_each_bitmapped_object()` from
> `show_reachable_fn`, keep the former abortable, and make the latter
> return void.
> 
> That keeps `traverse_bitmap_commit_list()` in sync with
> `traverse_commit_list()` without changing the non-bitmap traversal
> machinery. I have a small two-patch follow-up on top of v3 that does
> this, which I'll send separately.

That would be a nice cleanup if it's possible, but I wondered if you would find that one or more of the callbacks actually rely on this abort feature. Only one way to find out. :)

-Peff
Jeff KingJul 14, 2026, 07:17 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v3 0/9] odb: introduce object filters to `odb_for_each_object()`

On Mon, Jul 13, 2026 at 04:41:24PM +0200, Patrick Steinhardt wrote:
> Changes in v3:
>   - Weave Peff's patch into the patch series.
>   - Link to v2: https://patch.msgid.link/20260710-pks-odb-for-each-object-filter-v2-0-3710a9cc165a@pks.im
Yay, thank you. :)
-Peff
Toon ClaesJul 15, 2026, 05:09 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH 6/7] odb: introduce object filters to `odb_for_each_object()`

Patrick Steinhardt <ps@pks.im> writes:
Show 6 quoted lines
> An alternative going forward could be to perform filtering of yielded
> objects inside `odb_for_each_object()` itself so that it will filter out
> any objects that the backends themselves couldn't filter efficiently.
> But I'm not sure I want to go there as part of this series -- we only
> have a single caller anyway that iterates with a filter, and that caller
> already knows to manually filter references.
Ah, that's a valid point. I didn't think of that.
If only we were using a language that has lazy iterators ;)
-- 
Cheers,
Toon
Toon ClaesJul 15, 2026, 05:25 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v3 7/9] pack-bitmap: introduce function to open bitmap for a single source

Patrick Steinhardt <ps@pks.im> writes:
Show 17 quoted lines
> The function `prepare_bitmap_git()` opens the first bitmap it can find
> in any of the object sources connected to the repository. In a
> subsequent commit, the "packed" object database backend will learn to
> use bitmaps to answer object filters when enumerating objects. That
> backend operates on a single object source though, so using a bitmap
> that potentially belongs to a different source would be wrong:
>
>   - The source would yield objects that are not part of the source
>     itself.
>
>   - The object source info would be attributed to the wrong source.
>
>   - With multiple sources, each source would enumerate the same bitmap
>     another time.
>
> Introduce a new function `prepare_source_bitmap_git()` that only opens
> bitmaps belonging to the given object source.
Tinies nit: this should be `prepare_bitmap_git_for_source()`
Show 54 quoted lines
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  pack-bitmap.c | 12 ++++++++++++
>  pack-bitmap.h |  2 ++
>  2 files changed, 14 insertions(+)
>
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index 72c8ae3228..09ba15d26b 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -753,6 +753,18 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
>  	return NULL;
>  }
>  
> +struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source)
> +{
> +	struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
> +
> +	if (!open_bitmap_for_source(source, bitmap_git) &&
> +	    !load_bitmap(source->base.odb->repo, bitmap_git, 0))
> +		return bitmap_git;
> +
> +	free_bitmap_index(bitmap_git);
> +	return NULL;
> +}
> +
>  int bitmap_index_contains_pack(struct bitmap_index *bitmap, struct packed_git *pack)
>  {
>  	for (; bitmap; bitmap = bitmap->base) {
> diff --git a/pack-bitmap.h b/pack-bitmap.h
> index ae8dc491ac..9f20fb6e56 100644
> --- a/pack-bitmap.h
> +++ b/pack-bitmap.h
> @@ -9,6 +9,7 @@
>  #include "string-list.h"
>  
>  struct commit;
> +struct odb_source_packed;
>  struct repository;
>  struct rev_info;
>  
> @@ -68,6 +69,7 @@ struct bitmapped_pack {
>  
>  struct bitmap_index *prepare_bitmap_git(struct repository *r);
>  struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx);
> +struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source);
>  
>  /*
>   * Given a bitmap index, determine whether it contains the pack either directly
>
> -- 
> 2.55.0.313.g8d093f411d.dirty
>
>
-- 
Cheers,
Toon
Toon ClaesJul 15, 2026, 05:27 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v3 8/9] odb: introduce object filters to `odb_for_each_object()`

Patrick Steinhardt <ps@pks.im> writes:
Show 31 quoted lines
> The function `for_each_bitmapped_object()` can be used to iterate
> through all objects covered by a bitmap. The benefit of this function is
> that it allows the caller to efficiently handle some object filters. For
> example, this can be used to filter out objects of a specific type with
> some simple bitmap operations. But callers are currently required to
> manually wire up the use of bitmaps though, and to do so they have to
> reach into internals of a given object database source.
>
> Introduce a new `struct odb_for_each_object_options::filter` field so
> that the interface becomes generic. When set, then a backend may
> optionally use the filter to skip some objects that it would have
> otherwise yielded.
>
> Note that the respective backends are free to ignore this field if they
> cannot meaningfully optimize for a given filter, and consequently
> callers need to verify whether they actually want the returned objects.
> While annoying, we cannot easily lift this restriction anyway as the
> object filter infrastructure supports some filters that cannot be
> answered by the object database alone.
>
> An alternative might be to limit the filters to only those that _can_ be
> answered by backends. But ultimately, the filters that can be answered
> efficiently by the "packed" backend are completely disjunct from those
> that can be answered by the "loose" backend, and consequently the set of
> filters supported by all backends would be empty. Furthermore, it would
> require us to make assumptions about capabilities of future backends,
> which may be able to efficiently handle more filters than current ones.
> So in the end, this alternative would only limit us artificially.
>
> Implement the logic for the "packed" source. Note that we use the new
> function `prepare_source_bitmap_git()` to open the bitmap: as the
Also here: `prepare_bitmap_git_for_source()`
> backend operates on a single object source, we must only use bitmaps
> that belong to that specific source. Otherwise we might yield objects
> that are not part of the source at all, and with multiple sources we
> would enumerate the same bitmap once per source.
-- 
Cheers,
Toon
Toon ClaesJul 15, 2026, 05:41 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v3 9/9] builtin/cat-file: filter objects via object database

Patrick Steinhardt <ps@pks.im> writes:
Show 67 quoted lines
> When batching all objects, git-cat-file(1) reaches into the internals of
> the object database and manually manages bitmaps to apply object
> filters. This creates coupling between the command and the internals of
> the respective backend.
>
> Refactor git-cat-file(1) to use the new object filter option when
> batching all objects. This significantly simplifies the logic and
> ensures that we don't have to reach into internals of the "files" source
> anymore.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/cat-file.c | 76 +++++-------------------------------------------------
>  1 file changed, 7 insertions(+), 69 deletions(-)
>
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index b4b99a73da..1458dd76d6 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -20,7 +20,6 @@
>  #include "userdiff.h"
>  #include "oid-array.h"
>  #include "packfile.h"
> -#include "pack-bitmap.h"
>  #include "object-file.h"
>  #include "object-name.h"
>  #include "odb.h"
> @@ -844,28 +843,6 @@ static int batch_one_object_oi(const struct object_id *oid,
>  	return payload->callback(oid, NULL, 0, payload->payload);
>  }
>  
> -static int batch_one_object_packed(const struct object_id *oid,
> -				   struct packed_git *pack,
> -				   uint32_t pos,
> -				   void *_payload)
> -{
> -	struct for_each_object_payload *payload = _payload;
> -	return payload->callback(oid, pack, nth_packed_object_offset(pack, pos),
> -				 payload->payload);
> -}
> -
> -static int batch_one_object_bitmapped(const struct object_id *oid,
> -				      enum object_type type UNUSED,
> -				      int flags UNUSED,
> -				      uint32_t hash UNUSED,
> -				      struct packed_git *pack,
> -				      off_t offset,
> -				      void *_payload)
> -{
> -	struct for_each_object_payload *payload = _payload;
> -	return payload->callback(oid, pack, offset, payload->payload);
> -}
> -
>  static void batch_each_object(struct batch_options *opt,
>  			      for_each_object_fn callback,
>  			      unsigned flags,
> @@ -875,56 +852,17 @@ static void batch_each_object(struct batch_options *opt,
>  		.callback = callback,
>  		.payload = _payload,
>  	};
> +	struct odb_source_info source_info;
> +	struct object_info oi = {
> +		.source_infop = &source_info,
> +	};
>  	struct odb_for_each_object_options opts = {
>  		.flags = flags,
> +		.filter = &opt->objects_filter,
Ahha, so here we pass the filter down.
Show 23 quoted lines
>  	};
> -	struct bitmap_index *bitmap = NULL;
> -	struct odb_source *source;
> -
> -	/*
> -	 * TODO: we still need to tap into implementation details of the object
> -	 * database sources. Ideally, we should extend `odb_for_each_object()`
> -	 * to handle object filters itself so that we can move the filtering
> -	 * logic into the individual sources.
> -	 */
> -	odb_prepare_alternates(the_repository->objects);
> -	for (source = the_repository->objects->sources; source; source = source->next) {
> -		struct odb_source_files *files = odb_source_files_downcast(source);
> -		int ret = odb_source_for_each_object(&files->loose->base, NULL, batch_one_object_oi,
> -						     &payload, &opts);
> -		if (ret)
> -			break;
> -	}
> -
> -	if (opt->objects_filter.choice != LOFC_DISABLED &&
> -	    (bitmap = prepare_bitmap_git(the_repository)) &&
> -	    !for_each_bitmapped_object(bitmap, &opt->objects_filter,
> -				       batch_one_object_bitmapped, &payload)) {

Similar to the new code, the filter is used if possible. It is batch_object_write() which ensures the filter is applied. That didn't change.

Show 27 quoted lines
> -		struct packed_git *pack;
> -
> -		repo_for_each_pack(the_repository, pack) {
> -			if (bitmap_index_contains_pack(bitmap, pack) ||
> -			    open_pack_index(pack))
> -				continue;
> -			for_each_object_in_pack(pack, batch_one_object_packed,
> -						&payload, flags);
> -		}
> -	} else {
> -		struct odb_source_info source_info;
> -		struct object_info oi = {
> -			.source_infop = &source_info,
> -		};
> -
> -		for (source = the_repository->objects->sources; source; source = source->next) {
> -			struct odb_source_files *files = odb_source_files_downcast(source);
> -			int ret = odb_source_for_each_object(&files->packed->base, &oi,
> -							     batch_one_object_oi, &payload, &opts);
> -			if (ret)
> -				break;
> -		}
> -	}
>  
> -	free_bitmap_index(bitmap);
> +	odb_for_each_object_ext(the_repository->objects, &oi,
> +				batch_one_object_oi, &payload, &opts);
Nice to see it abstracted out like this!
Show 8 quoted lines
>  }
>  
>  static int batch_objects(struct batch_options *opt)
>
> -- 
> 2.55.0.313.g8d093f411d.dirty
>
>
-- 
Cheers,
Toon
Toon ClaesJul 15, 2026, 05:43 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v3 0/9] odb: introduce object filters to `odb_for_each_object()`

Patrick Steinhardt <ps@pks.im> writes:
Show 15 quoted lines
> On Mon, Jul 13, 2026 at 08:59:39PM -0700, Taylor Blau wrote:
>> On Mon, Jul 13, 2026 at 04:41:24PM +0200, Patrick Steinhardt wrote:
>> > Range-diff versus v2:
>> >
>> >  1:  baf2adb012 =  1:  7c0dc1be0d odb/source-packed: improve lookup when enumerating objects
>> >  2:  57eecf3031 =  2:  2e5908c9c3 pack-bitmap: mark object filter as `const`
>> >  -:  ---------- >  3:  f4d66ccfc6 pack-objects: drop unused return value from add_object_entry()
>> >  3:  92dd6a6f6e =  4:  af475654b8 pack-bitmap: allow aborting iteration of bitmapped objects
>> >  4:  92fe41577d =  5:  6ca42587c9 pack-bitmap: iterate object sources when opening bitmaps
>> >  5:  e5d59959e3 =  6:  f62c3bbc81 pack-bitmap: drop `_1` suffix from functions that open bitmaps
>> >  6:  ab3547ac2b =  7:  b2d25b6e9b pack-bitmap: introduce function to open bitmap for a single source
>> >  7:  026f21f522 =  8:  a5bf309bec odb: introduce object filters to `odb_for_each_object()`
>> >  8:  534b25c817 =  9:  600b15a907 builtin/cat-file: filter objects via object database
>> 
>> Thanks, this version looks good to me.

I only posted a nit about function name in the commit messages 7 & 8 isn't correct, but other than that I'm happy with these patches too.

-- 
Cheers,
Toon
Patrick SteinhardtJul 15, 2026, 06:21 UTC in reply to Toon Claes on lore

Re: [PATCH v3 0/9] odb: introduce object filters to `odb_for_each_object()`

On Wed, Jul 15, 2026 at 07:43:56AM +0200, Toon Claes wrote:
Show 20 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > On Mon, Jul 13, 2026 at 08:59:39PM -0700, Taylor Blau wrote:
> >> On Mon, Jul 13, 2026 at 04:41:24PM +0200, Patrick Steinhardt wrote:
> >> > Range-diff versus v2:
> >> >
> >> >  1:  baf2adb012 =  1:  7c0dc1be0d odb/source-packed: improve lookup when enumerating objects
> >> >  2:  57eecf3031 =  2:  2e5908c9c3 pack-bitmap: mark object filter as `const`
> >> >  -:  ---------- >  3:  f4d66ccfc6 pack-objects: drop unused return value from add_object_entry()
> >> >  3:  92dd6a6f6e =  4:  af475654b8 pack-bitmap: allow aborting iteration of bitmapped objects
> >> >  4:  92fe41577d =  5:  6ca42587c9 pack-bitmap: iterate object sources when opening bitmaps
> >> >  5:  e5d59959e3 =  6:  f62c3bbc81 pack-bitmap: drop `_1` suffix from functions that open bitmaps
> >> >  6:  ab3547ac2b =  7:  b2d25b6e9b pack-bitmap: introduce function to open bitmap for a single source
> >> >  7:  026f21f522 =  8:  a5bf309bec odb: introduce object filters to `odb_for_each_object()`
> >> >  8:  534b25c817 =  9:  600b15a907 builtin/cat-file: filter objects via object database
> >> 
> >> Thanks, this version looks good to me.
> 
> I only posted a nit about function name in the commit messages 7 & 8
> isn't correct, but other than that I'm happy with these patches too.

Thanks for your review! I'll send one more iteration that fixes these nits.

Patrick
Patrick SteinhardtJul 15, 2026, 06:22 UTC in reply to Toon Claes on lore

Re: [PATCH v3 7/9] pack-bitmap: introduce function to open bitmap for a single source

On Wed, Jul 15, 2026 at 07:25:54AM +0200, Toon Claes wrote:
Show 21 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > The function `prepare_bitmap_git()` opens the first bitmap it can find
> > in any of the object sources connected to the repository. In a
> > subsequent commit, the "packed" object database backend will learn to
> > use bitmaps to answer object filters when enumerating objects. That
> > backend operates on a single object source though, so using a bitmap
> > that potentially belongs to a different source would be wrong:
> >
> >   - The source would yield objects that are not part of the source
> >     itself.
> >
> >   - The object source info would be attributed to the wrong source.
> >
> >   - With multiple sources, each source would enumerate the same bitmap
> >     another time.
> >
> > Introduce a new function `prepare_source_bitmap_git()` that only opens
> > bitmaps belonging to the given object source.
> 
> Tinies nit: this should be `prepare_bitmap_git_for_source()`

Indeed it should be, good catch. Shows that I've been iterating on the name a bit :)

Patrick
Patrick SteinhardtJul 15, 2026, 06:22 UTC in reply to Patrick Steinhardt on lore

[PATCH v4 0/9] odb: introduce object filters to `odb_for_each_object()`

Hi,

this patch series introduces object filters to `odb_for_each_object()`. The intent of this is to make `git cat-file --batch-all-objects` work with pluggable object databases. Right now it doesn't because it reaches into internals of the "packed" backend to efficiently handle bitmapped objects.

The series is built on top of f85a7e6620 (Start Git 2.56 cycle, 2026-07-06) with ps/odb-drop-whence at 8a7ad23e11 (odb: document object info fields, 2026-07-02) merged into it.

Changes in v4:
  - Fix references to an old function name in commit messages.
  - Link to v3: https://patch.msgid.link/20260713-pks-odb-for-each-object-filter-v3-0-b3c65c641073@pks.im
Changes in v3:
  - Weave Peff's patch into the patch series.
  - Link to v2: https://patch.msgid.link/20260710-pks-odb-for-each-object-filter-v2-0-3710a9cc165a@pks.im
Changes in v2:
  - Add another patch to drop the `_1()` prefixes that aren't required
    anymore.
  - Change the approach in `open_bitmap_for_source()` to also use a
    `found` boolean instead of a confusing integer.
  - Add some more explanations to commit messages.
  - Link to v1: https://patch.msgid.link/20260709-pks-odb-for-each-object-filter-v1-0-82fe014b12b3@pks.im
Thanks!
Patrick
---
Jeff King (1):
      pack-objects: drop unused return value from add_object_entry()
Patrick Steinhardt (8):
      odb/source-packed: improve lookup when enumerating objects
      pack-bitmap: mark object filter as `const`
      pack-bitmap: allow aborting iteration of bitmapped objects
      pack-bitmap: iterate object sources when opening bitmaps
      pack-bitmap: drop `_1` suffix from functions that open bitmaps
      pack-bitmap: introduce function to open bitmap for a single source
      odb: introduce object filters to `odb_for_each_object()`
      builtin/cat-file: filter objects via object database
 builtin/cat-file.c     |  76 +++--------------------------
 builtin/pack-objects.c |  11 ++---
 builtin/rev-list.c     |   2 +-
 odb.h                  |  12 +++++
 odb/source-packed.c    |  77 ++++++++++++++++++++++++++---
 pack-bitmap.c          | 129 +++++++++++++++++++++++++++----------------------
 pack-bitmap.h          |  10 +++-
 7 files changed, 175 insertions(+), 142 deletions(-)
Range-diff versus v3:
 1:  2d4bc229ac =  1:  30cf75ce12 odb/source-packed: improve lookup when enumerating objects
 2:  dfa942f6da =  2:  735eb8ad5c pack-bitmap: mark object filter as `const`
 3:  9bfba42b27 =  3:  a1f32c5ea6 pack-objects: drop unused return value from add_object_entry()
 4:  405c77bad3 =  4:  acb0aad580 pack-bitmap: allow aborting iteration of bitmapped objects
 5:  70e68f10aa =  5:  199db72f4c pack-bitmap: iterate object sources when opening bitmaps
 6:  08ba2c1db3 =  6:  69d5882b57 pack-bitmap: drop `_1` suffix from functions that open bitmaps
 7:  d410cfa4e2 !  7:  a1eabafebd pack-bitmap: introduce function to open bitmap for a single source
    @@ Commit message
           - With multiple sources, each source would enumerate the same bitmap
             another time.
     
    -    Introduce a new function `prepare_source_bitmap_git()` that only opens
    -    bitmaps belonging to the given object source.
    +    Introduce a new function `prepare_bitmap_git_for_source()` that only
    +    opens bitmaps belonging to the given object source.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
 8:  e0df97e318 !  8:  6ef9885006 odb: introduce object filters to `odb_for_each_object()`
    @@ Commit message
         So in the end, this alternative would only limit us artificially.
     
         Implement the logic for the "packed" source. Note that we use the new
    -    function `prepare_source_bitmap_git()` to open the bitmap: as the
    +    function `prepare_bitmap_git_for_source()` to open the bitmap: as the
         backend operates on a single object source, we must only use bitmaps
         that belong to that specific source. Otherwise we might yield objects
         that are not part of the source at all, and with multiple sources we
 9:  e866b1f4b8 =  9:  104da65906 builtin/cat-file: filter objects via object database

--- base-commit: 3c8e2790f2ce15e8b5d4b4e6ced711b12649f32a change-id: 20260708-pks-odb-for-each-object-filter-13286fa3523d

Patrick SteinhardtJul 15, 2026, 06:22 UTC in reply to Patrick Steinhardt on lore

[PATCH v4 1/9] odb/source-packed: improve lookup when enumerating objects

When iterating through objects of a packed source that have a specific prefix we do so via two different methods:

  - When a multi-pack index is available we use that one to efficiently
    loop through all objects.
  - We then loop through all packfiles that aren't covered by a
    multi-pack index.

Regardless of which mechanism we use, we then iterate through all the objects indexed by the respective data structure. Curiously though, while we use the indices for enumerating the objects, we completely ignore it for the actual object lookup. Instead, we call into the generic `odb_source_read_object_info()` function, which will itself consult the indices to figure out where the object in question even lives.

This has two consequences:
  - It's inefficient, as we basically have to figure out the position of
    the object a second time.
  - It's subtly wrong, as it may now happen that a specific object will
    be looked up via a different pack in case it exists multiple times.
    This is unlikely to have any real-world consequences, but it's still
    the wrong thing to do.

Fix the issue by using `packed_object_info()` directly. While at it, rename the `store` variable to `source`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-packed.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
Show changes to odb/source-packed.c +8 −7
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 0edea5356d..9cfa02b7a2 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -143,7 +143,7 @@ static bool should_exclude_pack(struct packed_git *p, enum odb_for_each_object_f
 }
 
 static int for_each_prefixed_object_in_midx(
-	struct odb_source_packed *store,
+	struct odb_source_packed *source,
 	struct multi_pack_index *m,
 	const struct odb_for_each_object_options *opts,
 	struct odb_source_packed_for_each_object_wrapper_data *data)
@@ -170,6 +170,7 @@ static int for_each_prefixed_object_in_midx(
 		 */
 		for (i = first; i < num; i++) {
 			const struct object_id *current = NULL;
+			struct packed_git *pack;
 			struct object_id oid;
 
 			current = nth_midxed_object_oid(&oid, m, i);
@@ -177,9 +178,8 @@ static int for_each_prefixed_object_in_midx(
 			if (!match_hash(len, opts->prefix->hash, current->hash))
 				break;
 
-			if (opts->flags) {
+			if (opts->flags || data->request) {
 				uint32_t pack_id = nth_midxed_pack_int_id(m, i);
-				struct packed_git *pack;
 
 				if (prepare_midx_pack(m, pack_id)) {
 					pack_errors = true;
@@ -193,9 +193,9 @@ static int for_each_prefixed_object_in_midx(
 
 			if (data->request) {
 				struct object_info oi = *data->request;
+				off_t offset = nth_midxed_offset(m, i);
 
-				ret = odb_source_read_object_info(&store->base, current,
-								  &oi, 0);
+				ret = packed_object_info(source, pack, offset, &oi);
 				if (ret)
 					goto out;
 
@@ -219,7 +219,7 @@ static int for_each_prefixed_object_in_midx(
 }
 
 static int for_each_prefixed_object_in_pack(
-	struct odb_source_packed *store,
+	struct odb_source_packed *source,
 	struct packed_git *p,
 	const struct odb_for_each_object_options *opts,
 	struct odb_source_packed_for_each_object_wrapper_data *data)
@@ -246,8 +246,9 @@ static int for_each_prefixed_object_in_pack(
 
 		if (data->request) {
 			struct object_info oi = *data->request;
+			off_t offset = nth_packed_object_offset(p, i);
 
-			ret = odb_source_read_object_info(&store->base, &oid, &oi, 0);
+			ret = packed_object_info(source, p, offset, &oi);
 			if (ret)
 				goto out;
 
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 15, 2026, 06:22 UTC in reply to Patrick Steinhardt on lore

[PATCH v4 2/9] pack-bitmap: mark object filter as `const`

The function `for_each_bitmapped_object()` accepts an optional object filter. This filter is never modified by the function, but is not declared as `const`. Fix this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pack-bitmap.c | 6 +++---
 pack-bitmap.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)
Show changes to 2 files +4 −4

pack-bitmap.c, pack-bitmap.h

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 35774b6f0c..a47c231632 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1976,7 +1976,7 @@ static void filter_bitmap_object_type(struct bitmap_index *bitmap_git,
 static int filter_bitmap(struct bitmap_index *bitmap_git,
 			 struct object_list *tip_objects,
 			 struct bitmap *to_filter,
-			 struct list_objects_filter_options *filter)
+			 const struct list_objects_filter_options *filter)
 {
 	if (!filter || filter->choice == LOFC_DISABLED)
 		return 0;
@@ -2027,7 +2027,7 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
 	return -1;
 }
 
-static int can_filter_bitmap(struct list_objects_filter_options *filter)
+static int can_filter_bitmap(const struct list_objects_filter_options *filter)
 {
 	return !filter_bitmap(NULL, NULL, NULL, filter);
 }
@@ -2058,7 +2058,7 @@ static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
 }
 
 int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
-			      struct list_objects_filter_options *filter,
+			      const struct list_objects_filter_options *filter,
 			      show_reachable_fn show_reach,
 			      void *payload)
 {
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 19a8655457..47935eb24e 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -96,7 +96,7 @@ struct list_objects_filter_options;
  * not supported, `0` otherwise.
  */
 int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
-			      struct list_objects_filter_options *filter,
+			      const struct list_objects_filter_options *filter,
 			      show_reachable_fn show_reach,
 			      void *payload);
 
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 15, 2026, 06:22 UTC in reply to Patrick Steinhardt on lore

[PATCH v4 3/9] pack-objects: drop unused return value from add_object_entry()

From: Jeff King <peff@peff.net>

This function returns 0/1 to its caller to tell them whether we actually added a new entry (or if we considered it redundant). But nobody has relied on that behavior since 5379a5c5ee (Thin pack generation: optimization., 2006-04-05).

The extra return does not hurt much, but it is a bit confusing. We have a sister function, add_object_entry_from_bitmap(), which has the same return value semantics. That function is about to change to always return 0 (not void, because it must conform to a callback function interface). So with that change, we'd have two related functions which both return an "int" but with different semantics.

Let's drop the unused "int" return from add_object_entry() entirely, which makes it more clear that the two functions have diverged.

Signed-off-by: Jeff King <peff@peff.net>
[ps: slightly massaged the commit message]
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/pack-objects.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
Show changes to builtin/pack-objects.c +4 −5
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ea5eab4cf8..188c4f6d4b 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1867,8 +1867,8 @@ static const char no_closure_warning[] = N_(
 "disabling bitmap writing, as some objects are not being packed"
 );
 
-static int add_object_entry(const struct object_id *oid, enum object_type type,
-			    const char *name, int exclude)
+static void add_object_entry(const struct object_id *oid, enum object_type type,
+			     const char *name, int exclude)
 {
 	struct packed_git *found_pack = NULL;
 	off_t found_offset = 0;
@@ -1876,7 +1876,7 @@ static int add_object_entry(const struct object_id *oid, enum object_type type,
 	display_progress(progress_state, ++nr_seen);
 
 	if (have_duplicate_entry(oid, exclude))
-		return 0;
+		return;
 
 	if (!want_object_in_pack(oid, exclude, &found_pack, &found_offset)) {
 		/* The pack is missing an object, so it will not have closure */
@@ -1885,13 +1885,12 @@ static int add_object_entry(const struct object_id *oid, enum object_type type,
 				warning(_(no_closure_warning));
 			write_bitmap_index = 0;
 		}
-		return 0;
+		return;
 	}
 
 	create_object_entry(oid, type, pack_name_hash_fn(name),
 			    exclude, name && no_try_delta(name),
 			    found_pack, found_offset);
-	return 1;
 }
 
 static int add_object_entry_from_bitmap(const struct object_id *oid,
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 15, 2026, 06:22 UTC in reply to Patrick Steinhardt on lore

[PATCH v4 4/9] pack-bitmap: allow aborting iteration of bitmapped objects

In a subsequent commit we'll lift iteration of bitmapped objects into the "packed" backend and make it accessible via `odb_for_each_object()`. The calling convention for that function is that the callback may return a non-zero exit code, and if so we'll abort iteration. This is currently impossible to realize though, as `for_each_bitmapped_object()` will ignore any return value and just churn through all objects completely.

This doesn't matter to the callers of `for_each_bitmapped_object()`, as there's only one of them in git-cat-file(1), and the callbacks we pass always return zero. But once we move the logic into the generic infrastructure it becomes a latent bug waiting to happen.

Refactor the code so that the return value of the `show_reach` callback is not ignored anymore. Instead, returning a non-zero value will cause us to abort iteration in both `show_objects_for_type()` and in `for_each_bitmapped_object()`.

Note though that there's a second user of `show_objects_for_type()` with `traverse_bitmap_commit_list()`, and that function does indeed invoke callbacks that may return non-zero. This non-zero return value never had any effect at all though, and the callbacks that return non-zero values are only ever invoked via `traverse_bitmap_commit_list()`. Consequently, we adapt them to always return 0.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/pack-objects.c |  2 +-
 builtin/rev-list.c     |  2 +-
 pack-bitmap.c          | 31 +++++++++++++++++++++----------
 pack-bitmap.h          |  3 ++-
 4 files changed, 25 insertions(+), 13 deletions(-)
Show changes to 4 files +25 −13

builtin/pack-objects.c, builtin/rev-list.c, pack-bitmap.c, pack-bitmap.h

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 188c4f6d4b..3673b14b89 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1908,7 +1908,7 @@ static int add_object_entry_from_bitmap(const struct object_id *oid,
 		return 0;
 
 	create_object_entry(oid, type, name_hash, 0, 0, pack, offset);
-	return 1;
+	return 0;
 }
 
 struct pbase_tree_cache {
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 8f63003709..02818b81c6 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -486,7 +486,7 @@ static int show_object_fast(
 	void *payload UNUSED)
 {
 	fprintf(stdout, "%s\n", oid_to_hex(oid));
-	return 1;
+	return 0;
 }
 
 static void print_disk_usage(off_t size)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index a47c231632..eda38a5433 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1695,7 +1695,7 @@ static void init_type_iterator(struct ewah_or_iterator *it,
 	}
 }
 
-static void show_objects_for_type(
+static int show_objects_for_type(
 	struct bitmap_index *bitmap_git,
 	struct bitmap *objects,
 	enum object_type object_type,
@@ -1704,6 +1704,7 @@ static void show_objects_for_type(
 {
 	size_t i = 0;
 	uint32_t offset;
+	int ret;
 
 	struct ewah_or_iterator it;
 	eword_t filter;
@@ -1749,11 +1750,17 @@ static void show_objects_for_type(
 
 			hash = bitmap_name_hash(bitmap_git, index_pos);
 
-			show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
+			ret = show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
+			if (ret)
+				goto out;
 		}
 	}
 
+	ret = 0;
+
+out:
 	ewah_or_iterator_release(&it);
+	return ret;
 }
 
 static int in_bitmapped_pack(struct bitmap_index *bitmap_git,
@@ -2062,6 +2069,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
 			      show_reachable_fn show_reach,
 			      void *payload)
 {
+	const enum object_type types[] = {
+		OBJ_COMMIT,
+		OBJ_TREE,
+		OBJ_BLOB,
+		OBJ_TAG,
+	};
 	struct bitmap *filtered_bitmap = NULL;
 	uint32_t objects_nr;
 	size_t full_word_count;
@@ -2086,14 +2099,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
 		goto out;
 	}
 
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_COMMIT, show_reach, payload);
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_TREE, show_reach, payload);
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_BLOB, show_reach, payload);
-	show_objects_for_type(bitmap_git, filtered_bitmap,
-			      OBJ_TAG, show_reach, payload);
+	for (size_t i = 0; i < ARRAY_SIZE(types); i++) {
+		ret = show_objects_for_type(bitmap_git, filtered_bitmap,
+					    types[i], show_reach, payload);
+		if (ret)
+			goto out;
+	}
 
 	ret = 0;
 out:
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 47935eb24e..ae8dc491ac 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -93,7 +93,8 @@ struct list_objects_filter_options;
 /*
  * Filter bitmapped objects and iterate through all resulting objects,
  * executing `show_reach` for each of them. Returns `-1` in case the filter is
- * not supported, `0` otherwise.
+ * not supported, `0` otherwise. Aborts iteration and bubbles up the return
+ * value in case `show_reach()` returns non-zero.
  */
 int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
 			      const struct list_objects_filter_options *filter,
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 15, 2026, 06:22 UTC in reply to Patrick Steinhardt on lore

[PATCH v4 5/9] pack-bitmap: iterate object sources when opening bitmaps

When opening a bitmap for a repository we perform two steps:
  - We first look for a multi-pack index bitmap in any of the object
    sources connected to the repository.
  - We then look for a packfile bitmap in any of the packfiles of any of
    the object sources.

Both of these steps thus iterate through object sources themselves, one via `odb_prepare_alternates()` and one via `repo_for_each_pack()`. This layout makes it hard to introduce a way to open the bitmap of one specific object source, which is functionality that we'll require in a subsequent commit.

Reverse the loop so that we instead loop through all sources in the outer loop, and then for each source we try to load its bitmap via either the multi-pack index or via a packfile.

Note that this changes the precedence of bitmaps in one specific edge case: when an earlier object source only has a packfile bitmap, but a later source has a multi-pack index bitmap, we now pick the packfile bitmap of the earlier source. Previously, a multi-pack index bitmap from any source would have taken precedence over all packfile bitmaps. Given that object sources are ordered such that the local source comes first, this arguably is an improvement, as we now prefer local bitmaps over bitmaps in alternates. Furthermore, we already warn about repositories that have multiple bitmaps, so this setup is broken and thus arguably not worth worrying about too much.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pack-bitmap.c | 69 +++++++++++++++++++++++++++--------------------------------
 1 file changed, 31 insertions(+), 38 deletions(-)
Show changes to pack-bitmap.c +31 −38
diff --git a/pack-bitmap.c b/pack-bitmap.c
index eda38a5433..e32795a595 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -680,60 +680,53 @@ static int load_bitmap(struct repository *r, struct bitmap_index *bitmap_git,
 	return 0;
 }
 
-static int open_pack_bitmap(struct repository *r,
-			    struct bitmap_index *bitmap_git)
+static int open_bitmap_for_source(struct odb_source_packed *source,
+				  struct bitmap_index *bitmap_git)
 {
-	struct packed_git *p;
-	int ret = -1;
+	struct multi_pack_index *midx = get_multi_pack_index(source);
+	struct packfile_list_entry *e;
+	bool found = false;
 
-	repo_for_each_pack(r, p) {
-		if (open_pack_bitmap_1(bitmap_git, p) == 0) {
-			ret = 0;
-			/*
-			 * The only reason to keep looking is to report
-			 * duplicates.
-			 */
-			if (!trace2_is_enabled())
-				break;
-		}
+	if (midx && !open_midx_bitmap_1(bitmap_git, midx))
+		found = true;
+
+	for (e = packfile_store_get_packs(source); e; e = e->next) {
+		/*
+		 * When tracing is enabled we want to keep looking to report
+		 * duplicates even if we have already found a bitmap.
+		 */
+		if (found && !trace2_is_enabled())
+			break;
+
+		if (!open_pack_bitmap_1(bitmap_git, e->pack))
+			found = true;
 	}
 
-	return ret;
+	return found ? 0 : -1;
 }
 
-static int open_midx_bitmap(struct repository *r,
-			    struct bitmap_index *bitmap_git)
+static int open_bitmap(struct repository *r,
+		       struct bitmap_index *bitmap_git)
 {
 	struct odb_source *source;
-	int ret = -1;
+	bool found = false;
 
 	assert(!bitmap_git->map);
 
 	odb_prepare_alternates(r->objects);
 	for (source = r->objects->sources; source; source = source->next) {
 		struct odb_source_files *files = odb_source_files_downcast(source);
-		struct multi_pack_index *midx = get_multi_pack_index(files->packed);
-		if (midx && !open_midx_bitmap_1(bitmap_git, midx))
-			ret = 0;
-	}
-	return ret;
-}
-
-static int open_bitmap(struct repository *r,
-		       struct bitmap_index *bitmap_git)
-{
-	int found;
 
-	assert(!bitmap_git->map);
+		if (!open_bitmap_for_source(files->packed, bitmap_git))
+			found = true;
 
-	found = !open_midx_bitmap(r, bitmap_git);
-
-	/*
-	 * these will all be skipped if we opened a midx bitmap; but run it
-	 * anyway if tracing is enabled to report the duplicates
-	 */
-	if (!found || trace2_is_enabled())
-		found |= !open_pack_bitmap(r, bitmap_git);
+		/*
+		 * The only reason to keep looking after having found a bitmap
+		 * is to report duplicates.
+		 */
+		if (found && !trace2_is_enabled())
+			break;
+	}
 
 	return found ? 0 : -1;
 }
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 15, 2026, 06:22 UTC in reply to Patrick Steinhardt on lore

[PATCH v4 6/9] pack-bitmap: drop `_1` suffix from functions that open bitmaps

In the preceding commit we've refactored how we open bitmaps. As part of the refactoring we have consolidated `open_pack_bitmap()` as well as `open_midx_bitmap()` into `open_bitmap_for_source()`. Consequently, we only have their `open_pack_bitmap_1()` and `open_midx_bitmap_1()` variants left over, where the `_1` suffix doesn't really make much sense anymore.

Drop the suffix.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pack-bitmap.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
Show changes to pack-bitmap.c +7 −7
diff --git a/pack-bitmap.c b/pack-bitmap.c
index e32795a595..72c8ae3228 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -460,8 +460,8 @@ char *pack_bitmap_filename(struct packed_git *p)
 	return xstrfmt("%.*s.bitmap", (int)len, p->pack_name);
 }
 
-static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
-			      struct multi_pack_index *midx)
+static int open_midx_bitmap(struct bitmap_index *bitmap_git,
+			    struct multi_pack_index *midx)
 {
 	struct stat st;
 	char *bitmap_name = midx_bitmap_filename(midx);
@@ -539,7 +539,7 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
 	return -1;
 }
 
-static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git *packfile)
+static int open_pack_bitmap(struct bitmap_index *bitmap_git, struct packed_git *packfile)
 {
 	int fd;
 	struct stat st;
@@ -603,7 +603,7 @@ static int load_reverse_index(struct repository *r, struct bitmap_index *bitmap_
 
 		/*
 		 * The multi-pack-index's .rev file is already loaded via
-		 * open_pack_bitmap_1().
+		 * open_pack_bitmap().
 		 *
 		 * But we still need to open the individual pack .rev files,
 		 * since we will need to make use of them in pack-objects.
@@ -687,7 +687,7 @@ static int open_bitmap_for_source(struct odb_source_packed *source,
 	struct packfile_list_entry *e;
 	bool found = false;
 
-	if (midx && !open_midx_bitmap_1(bitmap_git, midx))
+	if (midx && !open_midx_bitmap(bitmap_git, midx))
 		found = true;
 
 	for (e = packfile_store_get_packs(source); e; e = e->next) {
@@ -698,7 +698,7 @@ static int open_bitmap_for_source(struct odb_source_packed *source,
 		if (found && !trace2_is_enabled())
 			break;
 
-		if (!open_pack_bitmap_1(bitmap_git, e->pack))
+		if (!open_pack_bitmap(bitmap_git, e->pack))
 			found = true;
 	}
 
@@ -746,7 +746,7 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
 {
 	struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
 
-	if (!open_midx_bitmap_1(bitmap_git, midx))
+	if (!open_midx_bitmap(bitmap_git, midx))
 		return bitmap_git;
 
 	free_bitmap_index(bitmap_git);
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 15, 2026, 06:22 UTC in reply to Patrick Steinhardt on lore

[PATCH v4 7/9] pack-bitmap: introduce function to open bitmap for a single source

The function `prepare_bitmap_git()` opens the first bitmap it can find in any of the object sources connected to the repository. In a subsequent commit, the "packed" object database backend will learn to use bitmaps to answer object filters when enumerating objects. That backend operates on a single object source though, so using a bitmap that potentially belongs to a different source would be wrong:

  - The source would yield objects that are not part of the source
    itself.
  - The object source info would be attributed to the wrong source.
  - With multiple sources, each source would enumerate the same bitmap
    another time.

Introduce a new function `prepare_bitmap_git_for_source()` that only opens bitmaps belonging to the given object source.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 pack-bitmap.c | 12 ++++++++++++
 pack-bitmap.h |  2 ++
 2 files changed, 14 insertions(+)
Show changes to 2 files +14 −0

pack-bitmap.c, pack-bitmap.h

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 72c8ae3228..09ba15d26b 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -753,6 +753,18 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
 	return NULL;
 }
 
+struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source)
+{
+	struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
+
+	if (!open_bitmap_for_source(source, bitmap_git) &&
+	    !load_bitmap(source->base.odb->repo, bitmap_git, 0))
+		return bitmap_git;
+
+	free_bitmap_index(bitmap_git);
+	return NULL;
+}
+
 int bitmap_index_contains_pack(struct bitmap_index *bitmap, struct packed_git *pack)
 {
 	for (; bitmap; bitmap = bitmap->base) {
diff --git a/pack-bitmap.h b/pack-bitmap.h
index ae8dc491ac..9f20fb6e56 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -9,6 +9,7 @@
 #include "string-list.h"
 
 struct commit;
+struct odb_source_packed;
 struct repository;
 struct rev_info;
 
@@ -68,6 +69,7 @@ struct bitmapped_pack {
 
 struct bitmap_index *prepare_bitmap_git(struct repository *r);
 struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx);
+struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source);
 
 /*
  * Given a bitmap index, determine whether it contains the pack either directly
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 15, 2026, 06:22 UTC in reply to Patrick Steinhardt on lore

[PATCH v4 8/9] odb: introduce object filters to `odb_for_each_object()`

The function `for_each_bitmapped_object()` can be used to iterate through all objects covered by a bitmap. The benefit of this function is that it allows the caller to efficiently handle some object filters. For example, this can be used to filter out objects of a specific type with some simple bitmap operations. But callers are currently required to manually wire up the use of bitmaps though, and to do so they have to reach into internals of a given object database source.

Introduce a new `struct odb_for_each_object_options::filter` field so that the interface becomes generic. When set, then a backend may optionally use the filter to skip some objects that it would have otherwise yielded.

Note that the respective backends are free to ignore this field if they cannot meaningfully optimize for a given filter, and consequently callers need to verify whether they actually want the returned objects. While annoying, we cannot easily lift this restriction anyway as the object filter infrastructure supports some filters that cannot be answered by the object database alone.

An alternative might be to limit the filters to only those that _can_ be answered by backends. But ultimately, the filters that can be answered efficiently by the "packed" backend are completely disjunct from those that can be answered by the "loose" backend, and consequently the set of filters supported by all backends would be empty. Furthermore, it would require us to make assumptions about capabilities of future backends, which may be able to efficiently handle more filters than current ones. So in the end, this alternative would only limit us artificially.

Implement the logic for the "packed" source. Note that we use the new function `prepare_bitmap_git_for_source()` to open the bitmap: as the backend operates on a single object source, we must only use bitmaps that belong to that specific source. Otherwise we might yield objects that are not part of the source at all, and with multiple sources we would enumerate the same bitmap once per source.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.h               | 12 +++++++++++
 odb/source-packed.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 pack-bitmap.c       |  3 +--
 pack-bitmap.h       |  3 +++
 4 files changed, 78 insertions(+), 2 deletions(-)
Show changes to 4 files +78 −2

odb.h, odb/source-packed.c, pack-bitmap.c, pack-bitmap.h

diff --git a/odb.h b/odb.h
index a1e222f605..67d0b34942 100644
--- a/odb.h
+++ b/odb.h
@@ -8,6 +8,7 @@
 #include "thread-utils.h"
 
 struct cached_object_entry;
+struct list_objects_filter_options;
 struct odb_source_inmemory;
 struct packed_git;
 struct repository;
@@ -490,6 +491,17 @@ struct odb_for_each_object_options {
 	 */
 	const struct object_id *prefix;
 	size_t prefix_hex_len;
+
+	/*
+	 * Optional object filter that allows backends to skip yielding
+	 * objects that are excluded by the filter as an optimization. The
+	 * filter is a best-effort hint: backends may use it to skip
+	 * excluded objects (e.g. by consulting a reachability bitmap), but
+	 * are also free to ignore it entirely and yield every object. As a
+	 * consequence, callers must re-apply the filter on yielded objects
+	 * if they require strict filtering semantics.
+	 */
+	const struct list_objects_filter_options *filter;
 };
 
 /*
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 9cfa02b7a2..4777395053 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -3,11 +3,13 @@
 #include "chdir-notify.h"
 #include "dir.h"
 #include "git-zlib.h"
+#include "list-objects-filter-options.h"
 #include "mergesort.h"
 #include "midx.h"
 #include "odb/source-packed.h"
 #include "odb/streaming.h"
 #include "packfile.h"
+#include "pack-bitmap.h"
 
 static int find_pack_entry(struct odb_source_packed *store,
 			   const struct object_id *oid,
@@ -315,6 +317,37 @@ static int odb_source_packed_for_each_prefixed_object(
 	return ret;
 }
 
+struct bitmapped_for_each_object_data {
+	struct odb_source_packed *packed;
+	const struct object_info *request;
+	const struct odb_for_each_object_options *opts;
+	odb_for_each_object_cb cb;
+	void *cb_data;
+};
+
+static int bitmapped_for_each_object(const struct object_id *oid,
+				     enum object_type type UNUSED,
+				     int flags UNUSED,
+				     uint32_t hash UNUSED,
+				     struct packed_git *pack,
+				     off_t offset,
+				     void *cb_data)
+{
+	struct bitmapped_for_each_object_data *data = cb_data;
+
+	if (should_exclude_pack(pack, data->opts->flags))
+		return 0;
+
+	if (data->request) {
+		struct object_info oi = *data->request;
+		if (packed_object_info(data->packed, pack, offset, &oi) < 0)
+			return -1;
+		return data->cb(oid, &oi, data->cb_data);
+	}
+
+	return data->cb(oid, NULL, data->cb_data);
+}
+
 static int odb_source_packed_for_each_object(struct odb_source *source,
 					     const struct object_info *request,
 					     odb_for_each_object_cb cb,
@@ -328,12 +361,33 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
 		.cb = cb,
 		.cb_data = cb_data,
 	};
+	struct bitmap_index *bitmap = NULL;
 	struct packfile_list_entry *e;
 	int pack_errors = 0, ret;
 
 	if (opts->prefix)
 		return odb_source_packed_for_each_prefixed_object(packed, opts, &data);
 
+	if (opts->filter &&
+	    opts->filter->choice != LOFC_DISABLED &&
+	    can_filter_bitmap(opts->filter))
+		bitmap = prepare_bitmap_git_for_source(packed);
+	if (bitmap) {
+		struct bitmapped_for_each_object_data bitmap_data = {
+			.packed = packed,
+			.request = request,
+			.opts = opts,
+			.cb = cb,
+			.cb_data = cb_data,
+		};
+
+		ret = for_each_bitmapped_object(bitmap, opts->filter,
+						bitmapped_for_each_object,
+						&bitmap_data);
+		if (ret)
+			goto out;
+	}
+
 	packed->skip_mru_updates = true;
 
 	for (e = packfile_store_get_packs(packed); e; e = e->next) {
@@ -342,6 +396,13 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
 		if (should_exclude_pack(p, opts->flags))
 			continue;
 
+		/*
+		 * Objects covered by the bitmap have already been yielded
+		 * above; skip them here to avoid duplicates.
+		 */
+		if (bitmap && bitmap_index_contains_pack(bitmap, p))
+			continue;
+
 		if (open_pack_index(p)) {
 			pack_errors = 1;
 			continue;
@@ -357,6 +418,7 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
 
 out:
 	packed->skip_mru_updates = false;
+	free_bitmap_index(bitmap);
 
 	if (!ret && pack_errors)
 		ret = -1;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 09ba15d26b..f55a0859ea 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2039,12 +2039,11 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
 	return -1;
 }
 
-static int can_filter_bitmap(const struct list_objects_filter_options *filter)
+bool can_filter_bitmap(const struct list_objects_filter_options *filter)
 {
 	return !filter_bitmap(NULL, NULL, NULL, filter);
 }
 
-
 static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
 					      struct bitmap *result)
 {
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 9f20fb6e56..1385027c1f 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -92,6 +92,9 @@ int test_bitmap_pseudo_merge_objects(struct repository *r, uint32_t n);
 
 struct list_objects_filter_options;
 
+/* Check whether the filter can be computed via the bitmap. */
+bool can_filter_bitmap(const struct list_objects_filter_options *filter);
+
 /*
  * Filter bitmapped objects and iterate through all resulting objects,
  * executing `show_reach` for each of them. Returns `-1` in case the filter is
-- 
2.55.0.313.g8d093f411d.dirty
Patrick SteinhardtJul 15, 2026, 06:22 UTC in reply to Patrick Steinhardt on lore

[PATCH v4 9/9] builtin/cat-file: filter objects via object database

When batching all objects, git-cat-file(1) reaches into the internals of the object database and manually manages bitmaps to apply object filters. This creates coupling between the command and the internals of the respective backend.

Refactor git-cat-file(1) to use the new object filter option when batching all objects. This significantly simplifies the logic and ensures that we don't have to reach into internals of the "files" source anymore.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/cat-file.c | 76 +++++-------------------------------------------------
 1 file changed, 7 insertions(+), 69 deletions(-)
Show changes to builtin/cat-file.c +7 −69
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index b4b99a73da..1458dd76d6 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,7 +20,6 @@
 #include "userdiff.h"
 #include "oid-array.h"
 #include "packfile.h"
-#include "pack-bitmap.h"
 #include "object-file.h"
 #include "object-name.h"
 #include "odb.h"
@@ -844,28 +843,6 @@ static int batch_one_object_oi(const struct object_id *oid,
 	return payload->callback(oid, NULL, 0, payload->payload);
 }
 
-static int batch_one_object_packed(const struct object_id *oid,
-				   struct packed_git *pack,
-				   uint32_t pos,
-				   void *_payload)
-{
-	struct for_each_object_payload *payload = _payload;
-	return payload->callback(oid, pack, nth_packed_object_offset(pack, pos),
-				 payload->payload);
-}
-
-static int batch_one_object_bitmapped(const struct object_id *oid,
-				      enum object_type type UNUSED,
-				      int flags UNUSED,
-				      uint32_t hash UNUSED,
-				      struct packed_git *pack,
-				      off_t offset,
-				      void *_payload)
-{
-	struct for_each_object_payload *payload = _payload;
-	return payload->callback(oid, pack, offset, payload->payload);
-}
-
 static void batch_each_object(struct batch_options *opt,
 			      for_each_object_fn callback,
 			      unsigned flags,
@@ -875,56 +852,17 @@ static void batch_each_object(struct batch_options *opt,
 		.callback = callback,
 		.payload = _payload,
 	};
+	struct odb_source_info source_info;
+	struct object_info oi = {
+		.source_infop = &source_info,
+	};
 	struct odb_for_each_object_options opts = {
 		.flags = flags,
+		.filter = &opt->objects_filter,
 	};
-	struct bitmap_index *bitmap = NULL;
-	struct odb_source *source;
-
-	/*
-	 * TODO: we still need to tap into implementation details of the object
-	 * database sources. Ideally, we should extend `odb_for_each_object()`
-	 * to handle object filters itself so that we can move the filtering
-	 * logic into the individual sources.
-	 */
-	odb_prepare_alternates(the_repository->objects);
-	for (source = the_repository->objects->sources; source; source = source->next) {
-		struct odb_source_files *files = odb_source_files_downcast(source);
-		int ret = odb_source_for_each_object(&files->loose->base, NULL, batch_one_object_oi,
-						     &payload, &opts);
-		if (ret)
-			break;
-	}
-
-	if (opt->objects_filter.choice != LOFC_DISABLED &&
-	    (bitmap = prepare_bitmap_git(the_repository)) &&
-	    !for_each_bitmapped_object(bitmap, &opt->objects_filter,
-				       batch_one_object_bitmapped, &payload)) {
-		struct packed_git *pack;
-
-		repo_for_each_pack(the_repository, pack) {
-			if (bitmap_index_contains_pack(bitmap, pack) ||
-			    open_pack_index(pack))
-				continue;
-			for_each_object_in_pack(pack, batch_one_object_packed,
-						&payload, flags);
-		}
-	} else {
-		struct odb_source_info source_info;
-		struct object_info oi = {
-			.source_infop = &source_info,
-		};
-
-		for (source = the_repository->objects->sources; source; source = source->next) {
-			struct odb_source_files *files = odb_source_files_downcast(source);
-			int ret = odb_source_for_each_object(&files->packed->base, &oi,
-							     batch_one_object_oi, &payload, &opts);
-			if (ret)
-				break;
-		}
-	}
 
-	free_bitmap_index(bitmap);
+	odb_for_each_object_ext(the_repository->objects, &oi,
+				batch_one_object_oi, &payload, &opts);
 }
 
 static int batch_objects(struct batch_options *opt)
-- 
2.55.0.313.g8d093f411d.dirty
Toon ClaesJul 15, 2026, 12:08 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v4 0/9] odb: introduce object filters to `odb_for_each_object()`

Patrick Steinhardt <ps@pks.im> writes:
Show 14 quoted lines
> Hi,
>
> this patch series introduces object filters to `odb_for_each_object()`.
> The intent of this is to make `git cat-file --batch-all-objects` work
> with pluggable object databases. Right now it doesn't because it reaches
> into internals of the "packed" backend to efficiently handle bitmapped
> objects.
>
> The series is built on top of f85a7e6620 (Start Git 2.56 cycle,
> 2026-07-06) with ps/odb-drop-whence at 8a7ad23e11 (odb: document object
> info fields, 2026-07-02) merged into it.
>
> Changes in v4:
>   - Fix references to an old function name in commit messages.
Thanks for fixing that! This version looks fine by me.
-- 
Cheers,
Toon
Junio C HamanoJul 15, 2026, 15:15 UTC in reply to Toon Claes on lore

Re: [PATCH v4 0/9] odb: introduce object filters to `odb_for_each_object()`

Toon Claes <toon@iotcl.com> writes:
Show 18 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
>
>> Hi,
>>
>> this patch series introduces object filters to `odb_for_each_object()`.
>> The intent of this is to make `git cat-file --batch-all-objects` work
>> with pluggable object databases. Right now it doesn't because it reaches
>> into internals of the "packed" backend to efficiently handle bitmapped
>> objects.
>>
>> The series is built on top of f85a7e6620 (Start Git 2.56 cycle,
>> 2026-07-06) with ps/odb-drop-whence at 8a7ad23e11 (odb: document object
>> info fields, 2026-07-02) merged into it.
>>
>> Changes in v4:
>>   - Fix references to an old function name in commit messages.
>
> Thanks for fixing that! This version looks fine by me.
Great.  Thanks all for working well together.

Back to recent threads