Hi,
this patch series introduces a generic `odb_for_each_object()` function to iterate through objects and adapts callers to use it. The intent is to make iteration through objects independent of the actual storage backend.
The series is structured as follows:
- Commits 1 to 2 do some cleanups for the for-each-object flags.
- Commits 3 to 7 introduce the infrastructure for
`odb_for_each_object()`. - Commits 8 to 13 convert a couple of callers to use the new
interfaces. - Commit 14 drops now-unused functions.
The patch series is built on top of 8745eae506 (The 17th batch, 2026-01-11) with the following two series merged into it:
- ps/read-object-info-improvements at a282a8f163 (packfile: move MIDX
into packfile store, 2026-01-09). - ps/packfile-store-in-odb-source at 12d3b58b55 (packfile: drop
repository parameter from `packed_object_info()`, 2026-01-12) .Changes in v4:
- Convert the `odb_for_each_object()` object info into a read-only
request parameter. Instead, we now read into a "fresh" object info
in the backends so that there can be no stale data.
- Fix typo in `struct object_info *io` parameter.
- Document what's still missing to convert `batch_each_object()` to
use the generic `odb_for_each_object()` function.
- Document ambiguity of the `mtime`.
- Re-add the `ODB_FOR_EACH_OBJECT_LOCAL_ONLY` flag in
`add_object_in_unpacked_pack()`. It shouldn't make any difference,
but it makes the conversion a bit more straight-forward.
- Link to v3: https://lore.kernel.org/r/20260121-pks-odb-for-each-object-v3-0-12c4dfd24227@pks.imChanges in v3:
- Fix error code propagation in last commit.
- Link to v2: https://lore.kernel.org/r/20260120-pks-odb-for-each-object-v2-0-d05cbfd3d6f8@pks.im
Changes in v2:
- Clarify the comment of `odb_for_each_object()` to point out that
it's the callback that can abort iteration by returning a non-zero
error code.
- Document in the commit message that we don't yet convert all sites
to use `odb_for_each_object()`.
- Link to v1: https://lore.kernel.org/r/20260115-pks-odb-for-each-object-v1-0-5418a91d5d99@pks.imThanks!
Patrick
---
Patrick Steinhardt (14):
odb: rename `FOR_EACH_OBJECT_*` flags
odb: fix flags parameter to be unsigned
object-file: extract function to read object info from path
object-file: introduce function to iterate through objects
packfile: extract function to iterate through objects of a store
packfile: introduce function to iterate through objects
odb: introduce `odb_for_each_object()`
builtin/fsck: refactor to use `odb_for_each_object()`
treewide: enumerate promisor objects via `odb_for_each_object()`
treewide: drop uses of `for_each_{loose,packed}_object()`
odb: introduce mtime fields for object info requests
builtin/pack-objects: use `packfile_store_for_each_object()`
reachable: convert to use `odb_for_each_object()`
odb: drop unused `for_each_{loose,packed}_object()` functions builtin/cat-file.c | 36 ++++++++--
builtin/fsck.c | 57 ++++-----------
builtin/pack-objects.c | 48 +++++++------
commit-graph.c | 46 +++++++++----
object-file.c | 125 ++++++++++++++++++++++-----------
object-file.h | 22 +++---
odb.c | 31 +++++++++
odb.h | 58 ++++++++++++++--
packfile.c | 184 +++++++++++++++++++++++++++++++++----------------
packfile.h | 19 ++++-
reachable.c | 129 ++++++++++------------------------
repack-promisor.c | 8 +--
revision.c | 10 ++-
13 files changed, 462 insertions(+), 311 deletions(-)
Range-diff versus v3:
1: a080e62c44 = 1: e7fa63f733 odb: rename `FOR_EACH_OBJECT_*` flags
2: 7980f241a9 = 2: b462808c07 odb: fix flags parameter to be unsigned
3: 14b9251711 = 3: 00d77e9e45 object-file: extract function to read object info from path
4: 93af71f3c7 ! 4: b9899bd1cb object-file: introduce function to iterate through objects
@@ object-file.c: int for_each_loose_object(struct object_database *odb,
+struct for_each_object_wrapper_data {
+ struct odb_source *source;
-+ struct object_info *oi;
++ const struct object_info *request;
+ odb_for_each_object_cb cb;
+ void *cb_data;
+};
@@ object-file.c: int for_each_loose_object(struct object_database *odb,
+ void *cb_data)
+{
+ struct for_each_object_wrapper_data *data = cb_data;
-+ if (data->oi &&
-+ read_object_info_from_path(data->source, path, oid, data->oi, 0) < 0)
++
++ if (data->request) {
++ struct object_info oi = *data->request;
++
++ if (read_object_info_from_path(data->source, path, oid, &oi, 0) < 0)
+ return -1;
-+ return data->cb(oid, data->oi, data->cb_data);
++
++ return data->cb(oid, &oi, data->cb_data);
++ } else {
++ return data->cb(oid, NULL, data->cb_data);
++ }
+}
+
+int odb_source_loose_for_each_object(struct odb_source *source,
-+ struct object_info *oi,
++ const struct object_info *request,
+ odb_for_each_object_cb cb,
+ void *cb_data,
+ unsigned flags)
+{
+ struct for_each_object_wrapper_data data = {
+ .source = source,
-+ .oi = oi,
++ .request = request,
+ .cb = cb,
+ .cb_data = cb_data,
+ };
@@ object-file.h: int for_each_loose_object(struct object_database *odb,
+ * `odb_source_loose_read_object_info()` on the object.
+ */
+int odb_source_loose_for_each_object(struct odb_source *source,
-+ struct object_info *oi,
++ const struct object_info *request,
+ odb_for_each_object_cb cb,
+ void *cb_data,
+ unsigned flags);
5: ad0a28e2bb = 5: 03fe7d5b3b packfile: extract function to iterate through objects of a store
6: e87126ddee ! 6: 4648a18a9b packfile: introduce function to iterate through objects
@@ Commit message
packfile: introduce function to iterate through objects
Introduce a new function `packfile_store_for_each_object()`. This
- function is the equivalent to `odb_source_loose_for_each_object()` in
+ function is equivalent to `odb_source_loose_for_each_object()`, except
that it:
- - Works on a single packfile store and thus per object source.
+ - Works on a single packfile store instead of working on the object
+ database level. Consequently, it will only yield packed objects of a
+ single object database source.
- Passes a `struct object_info` to the callback function.
@@ packfile.c: int for_each_packed_object(struct repository *repo, each_packed_obje
+struct packfile_store_for_each_object_wrapper_data {
+ struct packfile_store *store;
-+ struct object_info *oi;
++ const struct object_info *request;
+ odb_for_each_object_cb cb;
+ void *cb_data;
+};
@@ packfile.c: int for_each_packed_object(struct repository *repo, each_packed_obje
+{
+ struct packfile_store_for_each_object_wrapper_data *data = cb_data;
+
-+ if (data->oi) {
++ if (data->request) {
+ off_t offset = nth_packed_object_offset(pack, index_pos);
++ struct object_info oi = *data->request;
+
-+ if (packed_object_info(pack, offset, data->oi) < 0) {
++ if (packed_object_info(pack, offset, &oi) < 0) {
+ mark_bad_packed_object(pack, oid);
+ return -1;
+ }
-+ }
+
-+ return data->cb(oid, data->oi, data->cb_data);
++ return data->cb(oid, &oi, data->cb_data);
++ } else {
++ return data->cb(oid, NULL, data->cb_data);
++ }
+}
+
+int packfile_store_for_each_object(struct packfile_store *store,
-+ struct object_info *oi,
++ const struct object_info *request,
+ odb_for_each_object_cb cb,
+ void *cb_data,
+ unsigned flags)
+{
+ struct packfile_store_for_each_object_wrapper_data data = {
+ .store = store,
-+ .oi = oi,
++ .request = request,
+ .cb = cb,
+ .cb_data = cb_data,
+ };
@@ packfile.h: int for_each_object_in_pack(struct packed_git *p,
+/*
+ * Iterate through all packed objects in the given packfile store and invoke
-+ * the callback function for each of them. If given, the object info will be
-+ * populated with the object's data as if you had called
-+ * `packfile_store_read_object_info()` on the object.
++ * the callback function for each of them. If an object info request is given,
++ * then the object info will be read for every individual object and passed to
++ * the callback as if `packfile_store_read_object_info()` was called for the
++ * object.
+ *
+ * The flags parameter is a combination of `odb_for_each_object_flags`.
+ */
+int packfile_store_for_each_object(struct packfile_store *store,
-+ struct object_info *oi,
++ const struct object_info *request,
+ odb_for_each_object_cb cb,
+ void *cb_data,
+ unsigned flags);
7: f437198d7a ! 7: 3ec85ee10f odb: introduce `odb_for_each_object()`
@@ Commit message
Signed-off-by: Patrick Steinhardt <ps@pks.im>
+ ## object-file.h ##
+@@ object-file.h: int for_each_loose_object(struct object_database *odb,
+
+ /*
+ * Iterate through all loose objects in the given object database source and
+- * invoke the callback function for each of them. If given, the object info
+- * will be populated with the object's data as if you had called
+- * `odb_source_loose_read_object_info()` on the object.
++ * invoke the callback function for each of them. If an object info request is
++ * given, then the object info will be read for every individual object and
++ * passed to the callback as if `odb_source_loose_read_object_info()` was
++ * called for the object.
+ */
+ int odb_source_loose_for_each_object(struct odb_source *source,
+ const struct object_info *request,
+
## odb.c ##
@@ odb.c: int odb_freshen_object(struct object_database *odb,
return 0;
}
+int odb_for_each_object(struct object_database *odb,
-+ struct object_info *oi,
++ const struct object_info *request,
+ odb_for_each_object_cb cb,
+ void *cb_data,
+ unsigned flags)
@@ odb.c: int odb_freshen_object(struct object_database *odb,
+ continue;
+
+ if (!(flags & ODB_FOR_EACH_OBJECT_PROMISOR_ONLY)) {
-+ ret = odb_source_loose_for_each_object(source, oi, cb, cb_data, flags);
++ ret = odb_source_loose_for_each_object(source, request,
++ cb, cb_data, flags);
+ if (ret)
+ return ret;
+ }
+
-+ ret = packfile_store_for_each_object(source->packfiles, oi, cb, cb_data, flags);
++ ret = packfile_store_for_each_object(source->packfiles, request,
++ cb, cb_data, flags);
+ if (ret)
+ return ret;
+ }
@@ odb.h: typedef int (*odb_for_each_object_cb)(const struct object_id *oid,
+ * Iterate through all objects contained in the object database. Note that
+ * objects may be iterated over multiple times in case they are either stored
+ * in different backends or in case they are stored in multiple sources.
++ * If an object info request is given, then the object info will be read and
++ * passed to the callback as if `odb_read_object_info()` was called for the
++ * object.
+ *
+ * Returning a non-zero error code from the callback function will cause
+ * iteration to abort. The error code will be propagated.
@@ odb.h: typedef int (*odb_for_each_object_cb)(const struct object_id *oid,
+ * an arbitrary non-zero error code returned by the callback itself.
+ */
+int odb_for_each_object(struct object_database *odb,
-+ struct object_info *oi,
++ const struct object_info *request,
+ odb_for_each_object_cb cb,
+ void *cb_data,
+ unsigned flags);
8: 75c0e7fb54 ! 8: 069bcb600b builtin/fsck: refactor to use `odb_for_each_object()`
@@ Commit message
Refactor these callsites accordingly.
+ Note that `odb_for_each_object()` may iterate over the same object
+ multiple times, for example when it exists both in packed and loose
+ format. But this has already been the case beforehand, so this does not
+ result in a change in behaviour.
+
Signed-off-by: Patrick Steinhardt <ps@pks.im>
## builtin/fsck.c ##
@@ builtin/fsck.c: static int mark_used(struct object *obj, enum object_type type U
-static void mark_unreachable_referents(const struct object_id *oid)
+static int mark_unreachable_referents(const struct object_id *oid,
-+ struct object_info *io UNUSED,
++ struct object_info *oi UNUSED,
+ void *data UNUSED)
{
struct fsck_options options = FSCK_OPTIONS_DEFAULT;
9: 5a1c71af5f = 9: cb472da9d5 treewide: enumerate promisor objects via `odb_for_each_object()`
10: b6dcd01b19 ! 10: 505243613c treewide: drop uses of `for_each_{loose,packed}_object()`
@@ builtin/cat-file.c: static void batch_each_object(struct batch_options *opt,
+ struct odb_source *source;
- for_each_loose_object(the_repository->objects, batch_one_object_loose, &payload, 0);
++ /*
++ * 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) {
+ int ret = odb_source_loose_for_each_object(source, NULL, batch_one_object_oi,
11: 92a8225bca ! 11: 3dc547bb9d odb: introduce mtime fields for object info requests
@@ Commit message
to request an object's mtime. This new field will be used in a
subsequent commit.
+ Note that the concept of "mtime" is ambiguous: given an object, it may
+ be stored multiple times in the object database, and each of these
+ instances may have a different mtime. Disambiguating these mtimes is
+ nothing that can happen on the generic ODB layer: the caller may search
+ for the oldest object, the newest object, or even the relation of object
+ mtimes depending on the specific source they are located in. As such, it
+ is the responsibility of the caller to disambiguate mtimes.
+
+ A consequence of this is that it's most likely incorrect to look up the
+ mtime via `odb_read_object_info()`, as this interface does not give us
+ enough information to disambiguate the mtime. Document this accordingly
+ and tell users to use `odb_for_each_object()` instead.
+
+ Even with this gotcha though it's sensible to have this request as part
+ of the object info, as the mtime is a property of the object storage
+ format. If we for example had a "black-box" storage backend, we'd still
+ need to be able to query it for the mtime info in a generic way.
+
+ We could introduce a safety mechanism that for example calls `BUG()` in
+ case we look up the mtime outside of `odb_for_each_object()`. But that
+ feels somewhat heavy-handed.
+
Signed-off-by: Patrick Steinhardt <ps@pks.im>
## object-file.c ##
@@ odb.c: static int do_oid_object_info_extended(struct object_database *odb,
## odb.h ##
@@ odb.h: struct object_info {
- off_t *disk_sizep;
struct object_id *delta_base_oid;
void **contentp;
-+ time_t *mtimep;
++ /*
++ * The time the given looked-up object has been last modified.
++ *
++ * Note: the mtime may be ambiguous in case the object exists multiple
++ * times in the object database. It is thus _not_ recommended to use
++ * this field outside of contexts where you would read every instance
++ * of the object, like for example with `odb_for_each_object()`. As it
++ * is impossible to say at the ODB level what the intent of the caller
++ * is (e.g. whether to find the oldest or newest object), it is the
++ * responsibility of the caller to disambiguate the mtimes.
++ */
++ time_t *mtimep;
++
/* Response */
enum {
+ OI_CACHED,
## packfile.c ##
@@ packfile.c: static void add_delta_base_cache(struct packed_git *p, off_t base_offset,
@@ packfile.c: int packed_object_info(struct packed_git *p,
+ uint32_t index_pos;
+
+ if (load_pack_mtimes(p) < 0)
-+ die(_("could not load cruft pack .mtimes"));
++ die(_("could not load .mtimes for cruft pack '%s'"),
++ pack_basename(p));
+
+ if (maybe_index_pos)
+ index_pos = *maybe_index_pos;
@@ packfile.c: int packed_object_info(struct packed_git *p,
struct pack_window **w_curs,
off_t curpos,
@@ packfile.c: static int packfile_store_for_each_object_wrapper(const struct object_id *oid,
- if (data->oi) {
off_t offset = nth_packed_object_offset(pack, index_pos);
+ struct object_info oi = *data->request;
-- if (packed_object_info(pack, offset, data->oi) < 0) {
+- if (packed_object_info(pack, offset, &oi) < 0) {
+ if (packed_object_info_with_index_pos(pack, offset,
-+ &index_pos, data->oi) < 0) {
++ &index_pos, &oi) < 0) {
mark_bad_packed_object(pack, oid);
return -1;
}
12: 658cbf8f12 ! 12: 0047a40d16 builtin/pack-objects: use `packfile_store_for_each_object()`
@@ builtin/pack-objects.c: static int add_object_in_unpacked_pack(const struct obje
+ if (packfile_store_for_each_object(source->packfiles, &oi,
+ add_object_in_unpacked_pack, NULL,
+ ODB_FOR_EACH_OBJECT_PACK_ORDER |
++ ODB_FOR_EACH_OBJECT_LOCAL_ONLY |
+ ODB_FOR_EACH_OBJECT_SKIP_IN_CORE_KEPT_PACKS |
+ ODB_FOR_EACH_OBJECT_SKIP_ON_DISK_KEPT_PACKS))
+ die(_("cannot open pack index"));
13: a28907a4b6 = 13: c3bde2e822 reachable: convert to use `odb_for_each_object()`
14: 7d235b6529 ! 14: bf2f3c39a6 odb: drop unused `for_each_{loose,packed}_object()` functions
@@ object-file.c: int for_each_loose_file_in_source(struct odb_source *source,
-
struct for_each_object_wrapper_data {
struct odb_source *source;
- struct object_info *oi;
+ const struct object_info *request;
## object-file.h ##
@@ object-file.h: int for_each_loose_file_in_source(struct odb_source *source,
@@ object-file.h: int for_each_loose_file_in_source(struct odb_source *source,
-
/*
* Iterate through all loose objects in the given object database source and
- * invoke the callback function for each of them. If given, the object info
+ * invoke the callback function for each of them. If an object info request is
## packfile.c ##
@@ packfile.c: int for_each_object_in_pack(struct packed_git *p,
@@ packfile.c: int for_each_object_in_pack(struct packed_git *p,
-
struct packfile_store_for_each_object_wrapper_data {
struct packfile_store *store;
- struct object_info *oi;
+ const struct object_info *request;
@@ packfile.c: int packfile_store_for_each_object(struct packfile_store *store,
.cb = cb,
.cb_data = cb_data,--- base-commit: 1ff0e42d332523a11cc3d61b8d8463db5f9f14e8 change-id: 20260115-pks-odb-for-each-object-60b78cde09fd
Patrick Steinhardt
