* [PATCH v2 11/17] odb/source-packed: wire up `for_each_object()` callback
From: Patrick Steinhardt @ 2026-06-09 8:51 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260609-pks-odb-source-packed-v2-0-839089132c8b@pks.im>
Move `packfile_store_for_each_object()` and its associated helpers from
"packfile.c" into "odb/source-packed.c" and wire it up as the
`for_each_object()` callback of the "packed" source.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/cat-file.c | 4 +-
builtin/pack-objects.c | 4 +-
commit-graph.c | 4 +-
odb/source-files.c | 2 +-
odb/source-packed.c | 258 ++++++++++++++++++++++++++++++++++++++++++++++++
packfile.c | 260 +------------------------------------------------
packfile.h | 17 +---
7 files changed, 269 insertions(+), 280 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 04b64006a5..d997011531 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -916,8 +916,8 @@ static void batch_each_object(struct batch_options *opt,
for (source = the_repository->objects->sources; source; source = source->next) {
struct odb_source_files *files = odb_source_files_downcast(source);
- int ret = packfile_store_for_each_object(files->packed, &oi,
- batch_one_object_oi, &payload, &opts);
+ int ret = odb_source_for_each_object(&files->packed->base, &oi,
+ batch_one_object_oi, &payload, &opts);
if (ret)
break;
}
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 50675481e1..5e94805478 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4503,8 +4503,8 @@ static void add_objects_in_unpacked_packs(void)
if (!source->local)
continue;
- if (packfile_store_for_each_object(files->packed, &oi,
- add_object_in_unpacked_pack, NULL, &opts))
+ if (odb_source_for_each_object(&files->packed->base, &oi,
+ add_object_in_unpacked_pack, NULL, &opts))
die(_("cannot open pack index"));
}
}
diff --git a/commit-graph.c b/commit-graph.c
index 9abe62bd5a..1e4038baf3 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -2016,8 +2016,8 @@ static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx)
odb_prepare_alternates(ctx->r->objects);
for (source = ctx->r->objects->sources; source; source = source->next) {
struct odb_source_files *files = odb_source_files_downcast(source);
- packfile_store_for_each_object(files->packed, &oi, add_packed_commits_oi,
- ctx, &opts);
+ odb_source_for_each_object(&files->packed->base, &oi, add_packed_commits_oi,
+ ctx, &opts);
}
if (ctx->progress_done < ctx->approx_nr_objects)
diff --git a/odb/source-files.c b/odb/source-files.c
index dff69d0e4e..c73a7e5f90 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -88,7 +88,7 @@ static int odb_source_files_for_each_object(struct odb_source *source,
return ret;
}
- ret = packfile_store_for_each_object(files->packed, request, cb, cb_data, opts);
+ ret = odb_source_for_each_object(&files->packed->base, request, cb, cb_data, opts);
if (ret)
return ret;
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 23d7149fe3..a61c809c8c 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -81,6 +81,263 @@ static int odb_source_packed_read_object_stream(struct odb_read_stream **out,
return packfile_read_object_stream(out, oid, e.p, e.offset);
}
+struct odb_source_packed_for_each_object_wrapper_data {
+ struct odb_source_packed *store;
+ const struct object_info *request;
+ odb_for_each_object_cb cb;
+ void *cb_data;
+};
+
+static int odb_source_packed_for_each_object_wrapper(const struct object_id *oid,
+ struct packed_git *pack,
+ uint32_t index_pos,
+ void *cb_data)
+{
+ struct odb_source_packed_for_each_object_wrapper_data *data = cb_data;
+
+ if (data->request) {
+ off_t offset = nth_packed_object_offset(pack, index_pos);
+ struct object_info oi = *data->request;
+
+ if (packed_object_info_with_index_pos(pack, offset,
+ &index_pos, &oi) < 0) {
+ mark_bad_packed_object(pack, oid);
+ return -1;
+ }
+
+ return data->cb(oid, &oi, data->cb_data);
+ } else {
+ return data->cb(oid, NULL, data->cb_data);
+ }
+}
+
+static int match_hash(unsigned len, const unsigned char *a, const unsigned char *b)
+{
+ do {
+ if (*a != *b)
+ return 0;
+ a++;
+ b++;
+ len -= 2;
+ } while (len > 1);
+ if (len)
+ if ((*a ^ *b) & 0xf0)
+ return 0;
+ return 1;
+}
+
+static int for_each_prefixed_object_in_midx(
+ struct odb_source_packed *store,
+ struct multi_pack_index *m,
+ const struct odb_for_each_object_options *opts,
+ struct odb_source_packed_for_each_object_wrapper_data *data)
+{
+ int ret;
+
+ for (; m; m = m->base_midx) {
+ uint32_t num, i, first = 0;
+ int len = opts->prefix_hex_len > m->source->odb->repo->hash_algo->hexsz ?
+ m->source->odb->repo->hash_algo->hexsz : opts->prefix_hex_len;
+
+ if (!m->num_objects)
+ continue;
+
+ num = m->num_objects + m->num_objects_in_base;
+
+ bsearch_one_midx(opts->prefix, m, &first);
+
+ /*
+ * At this point, "first" is the location of the lowest
+ * object with an object name that could match "opts->prefix".
+ * See if we have 0, 1 or more objects that actually match(es).
+ */
+ for (i = first; i < num; i++) {
+ const struct object_id *current = NULL;
+ struct object_id oid;
+
+ current = nth_midxed_object_oid(&oid, m, i);
+
+ if (!match_hash(len, opts->prefix->hash, current->hash))
+ break;
+
+ if (data->request) {
+ struct object_info oi = *data->request;
+
+ ret = odb_source_read_object_info(&store->base, current,
+ &oi, 0);
+ if (ret)
+ goto out;
+
+ ret = data->cb(&oid, &oi, data->cb_data);
+ if (ret)
+ goto out;
+ } else {
+ ret = data->cb(&oid, NULL, data->cb_data);
+ if (ret)
+ goto out;
+ }
+ }
+ }
+
+ ret = 0;
+
+out:
+ return ret;
+}
+
+static int for_each_prefixed_object_in_pack(
+ struct odb_source_packed *store,
+ struct packed_git *p,
+ const struct odb_for_each_object_options *opts,
+ struct odb_source_packed_for_each_object_wrapper_data *data)
+{
+ uint32_t num, i, first = 0;
+ int len = opts->prefix_hex_len > p->repo->hash_algo->hexsz ?
+ p->repo->hash_algo->hexsz : opts->prefix_hex_len;
+ int ret;
+
+ num = p->num_objects;
+ bsearch_pack(opts->prefix, p, &first);
+
+ /*
+ * At this point, "first" is the location of the lowest object
+ * with an object name that could match "bin_pfx". See if we have
+ * 0, 1 or more objects that actually match(es).
+ */
+ for (i = first; i < num; i++) {
+ struct object_id oid;
+
+ nth_packed_object_id(&oid, p, i);
+ if (!match_hash(len, opts->prefix->hash, oid.hash))
+ break;
+
+ if (data->request) {
+ struct object_info oi = *data->request;
+
+ ret = odb_source_read_object_info(&store->base, &oid, &oi, 0);
+ if (ret)
+ goto out;
+
+ ret = data->cb(&oid, &oi, data->cb_data);
+ if (ret)
+ goto out;
+ } else {
+ ret = data->cb(&oid, NULL, data->cb_data);
+ if (ret)
+ goto out;
+ }
+ }
+
+ ret = 0;
+
+out:
+ return ret;
+}
+
+static int odb_source_packed_for_each_prefixed_object(
+ struct odb_source_packed *store,
+ const struct odb_for_each_object_options *opts,
+ struct odb_source_packed_for_each_object_wrapper_data *data)
+{
+ struct packfile_list_entry *e;
+ struct multi_pack_index *m;
+ bool pack_errors = false;
+ int ret;
+
+ if (opts->flags)
+ BUG("flags unsupported");
+
+ store->skip_mru_updates = true;
+
+ m = get_multi_pack_index(&store->files->base);
+ if (m) {
+ ret = for_each_prefixed_object_in_midx(store, m, opts, data);
+ if (ret)
+ goto out;
+ }
+
+ for (e = packfile_store_get_packs(store); e; e = e->next) {
+ if (e->pack->multi_pack_index)
+ continue;
+
+ if (open_pack_index(e->pack)) {
+ pack_errors = true;
+ continue;
+ }
+
+ if (!e->pack->num_objects)
+ continue;
+
+ ret = for_each_prefixed_object_in_pack(store, e->pack, opts, data);
+ if (ret)
+ goto out;
+ }
+
+ ret = 0;
+
+out:
+ store->skip_mru_updates = false;
+ if (!ret && pack_errors)
+ ret = -1;
+ return ret;
+}
+
+static int odb_source_packed_for_each_object(struct odb_source *source,
+ const struct object_info *request,
+ odb_for_each_object_cb cb,
+ void *cb_data,
+ const struct odb_for_each_object_options *opts)
+{
+ struct odb_source_packed *packed = odb_source_packed_downcast(source);
+ struct odb_source_packed_for_each_object_wrapper_data data = {
+ .store = packed,
+ .request = request,
+ .cb = cb,
+ .cb_data = cb_data,
+ };
+ struct packfile_list_entry *e;
+ int pack_errors = 0, ret;
+
+ if (opts->prefix)
+ return odb_source_packed_for_each_prefixed_object(packed, opts, &data);
+
+ packed->skip_mru_updates = true;
+
+ for (e = packfile_store_get_packs(packed); e; e = e->next) {
+ struct packed_git *p = e->pack;
+
+ if ((opts->flags & ODB_FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local)
+ continue;
+ if ((opts->flags & ODB_FOR_EACH_OBJECT_PROMISOR_ONLY) &&
+ !p->pack_promisor)
+ continue;
+ if ((opts->flags & ODB_FOR_EACH_OBJECT_SKIP_IN_CORE_KEPT_PACKS) &&
+ p->pack_keep_in_core)
+ continue;
+ if ((opts->flags & ODB_FOR_EACH_OBJECT_SKIP_ON_DISK_KEPT_PACKS) &&
+ p->pack_keep)
+ continue;
+ if (open_pack_index(p)) {
+ pack_errors = 1;
+ continue;
+ }
+
+ ret = for_each_object_in_pack(p, odb_source_packed_for_each_object_wrapper,
+ &data, opts->flags);
+ if (ret)
+ goto out;
+ }
+
+ ret = 0;
+
+out:
+ packed->skip_mru_updates = false;
+
+ if (!ret && pack_errors)
+ ret = -1;
+ return ret;
+}
+
void (*report_garbage)(unsigned seen_bits, const char *path);
static void report_helper(const struct string_list *list,
@@ -291,6 +548,7 @@ struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
packed->base.reprepare = odb_source_packed_reprepare;
packed->base.read_object_info = odb_source_packed_read_object_info;
packed->base.read_object_stream = odb_source_packed_read_object_stream;
+ packed->base.for_each_object = odb_source_packed_for_each_object;
if (!is_absolute_path(parent->base.path))
chdir_notify_register(NULL, odb_source_packed_reparent, packed);
diff --git a/packfile.c b/packfile.c
index 42c84397eb..b8d6054c16 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1362,8 +1362,8 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset,
hashmap_add(&delta_base_cache, &ent->ent);
}
-static int packed_object_info_with_index_pos(struct packed_git *p, off_t obj_offset,
- uint32_t *maybe_index_pos, struct object_info *oi)
+int packed_object_info_with_index_pos(struct packed_git *p, off_t obj_offset,
+ uint32_t *maybe_index_pos, struct object_info *oi)
{
struct pack_window *w_curs = NULL;
size_t size;
@@ -2068,262 +2068,6 @@ int for_each_object_in_pack(struct packed_git *p,
return r;
}
-struct odb_source_packed_for_each_object_wrapper_data {
- struct odb_source_packed *store;
- const struct object_info *request;
- odb_for_each_object_cb cb;
- void *cb_data;
-};
-
-static int packfile_store_for_each_object_wrapper(const struct object_id *oid,
- struct packed_git *pack,
- uint32_t index_pos,
- void *cb_data)
-{
- struct odb_source_packed_for_each_object_wrapper_data *data = cb_data;
-
- if (data->request) {
- off_t offset = nth_packed_object_offset(pack, index_pos);
- struct object_info oi = *data->request;
-
- if (packed_object_info_with_index_pos(pack, offset,
- &index_pos, &oi) < 0) {
- mark_bad_packed_object(pack, oid);
- return -1;
- }
-
- return data->cb(oid, &oi, data->cb_data);
- } else {
- return data->cb(oid, NULL, data->cb_data);
- }
-}
-
-static int match_hash(unsigned len, const unsigned char *a, const unsigned char *b)
-{
- do {
- if (*a != *b)
- return 0;
- a++;
- b++;
- len -= 2;
- } while (len > 1);
- if (len)
- if ((*a ^ *b) & 0xf0)
- return 0;
- return 1;
-}
-
-static int for_each_prefixed_object_in_midx(
- struct odb_source_packed *store,
- struct multi_pack_index *m,
- const struct odb_for_each_object_options *opts,
- struct odb_source_packed_for_each_object_wrapper_data *data)
-{
- int ret;
-
- for (; m; m = m->base_midx) {
- uint32_t num, i, first = 0;
- int len = opts->prefix_hex_len > m->source->odb->repo->hash_algo->hexsz ?
- m->source->odb->repo->hash_algo->hexsz : opts->prefix_hex_len;
-
- if (!m->num_objects)
- continue;
-
- num = m->num_objects + m->num_objects_in_base;
-
- bsearch_one_midx(opts->prefix, m, &first);
-
- /*
- * At this point, "first" is the location of the lowest
- * object with an object name that could match "opts->prefix".
- * See if we have 0, 1 or more objects that actually match(es).
- */
- for (i = first; i < num; i++) {
- const struct object_id *current = NULL;
- struct object_id oid;
-
- current = nth_midxed_object_oid(&oid, m, i);
-
- if (!match_hash(len, opts->prefix->hash, current->hash))
- break;
-
- if (data->request) {
- struct object_info oi = *data->request;
-
- ret = odb_source_read_object_info(&store->base, current,
- &oi, 0);
- if (ret)
- goto out;
-
- ret = data->cb(&oid, &oi, data->cb_data);
- if (ret)
- goto out;
- } else {
- ret = data->cb(&oid, NULL, data->cb_data);
- if (ret)
- goto out;
- }
- }
- }
-
- ret = 0;
-
-out:
- return ret;
-}
-
-static int for_each_prefixed_object_in_pack(
- struct odb_source_packed *store,
- struct packed_git *p,
- const struct odb_for_each_object_options *opts,
- struct odb_source_packed_for_each_object_wrapper_data *data)
-{
- uint32_t num, i, first = 0;
- int len = opts->prefix_hex_len > p->repo->hash_algo->hexsz ?
- p->repo->hash_algo->hexsz : opts->prefix_hex_len;
- int ret;
-
- num = p->num_objects;
- bsearch_pack(opts->prefix, p, &first);
-
- /*
- * At this point, "first" is the location of the lowest object
- * with an object name that could match "bin_pfx". See if we have
- * 0, 1 or more objects that actually match(es).
- */
- for (i = first; i < num; i++) {
- struct object_id oid;
-
- nth_packed_object_id(&oid, p, i);
- if (!match_hash(len, opts->prefix->hash, oid.hash))
- break;
-
- if (data->request) {
- struct object_info oi = *data->request;
-
- ret = odb_source_read_object_info(&store->base, &oid, &oi, 0);
- if (ret)
- goto out;
-
- ret = data->cb(&oid, &oi, data->cb_data);
- if (ret)
- goto out;
- } else {
- ret = data->cb(&oid, NULL, data->cb_data);
- if (ret)
- goto out;
- }
- }
-
- ret = 0;
-
-out:
- return ret;
-}
-
-static int packfile_store_for_each_prefixed_object(
- struct odb_source_packed *store,
- const struct odb_for_each_object_options *opts,
- struct odb_source_packed_for_each_object_wrapper_data *data)
-{
- struct packfile_list_entry *e;
- struct multi_pack_index *m;
- bool pack_errors = false;
- int ret;
-
- if (opts->flags)
- BUG("flags unsupported");
-
- store->skip_mru_updates = true;
-
- m = get_multi_pack_index(&store->files->base);
- if (m) {
- ret = for_each_prefixed_object_in_midx(store, m, opts, data);
- if (ret)
- goto out;
- }
-
- for (e = packfile_store_get_packs(store); e; e = e->next) {
- if (e->pack->multi_pack_index)
- continue;
-
- if (open_pack_index(e->pack)) {
- pack_errors = true;
- continue;
- }
-
- if (!e->pack->num_objects)
- continue;
-
- ret = for_each_prefixed_object_in_pack(store, e->pack, opts, data);
- if (ret)
- goto out;
- }
-
- ret = 0;
-
-out:
- store->skip_mru_updates = false;
- if (!ret && pack_errors)
- ret = -1;
- return ret;
-}
-
-int packfile_store_for_each_object(struct odb_source_packed *store,
- const struct object_info *request,
- odb_for_each_object_cb cb,
- void *cb_data,
- const struct odb_for_each_object_options *opts)
-{
- struct odb_source_packed_for_each_object_wrapper_data data = {
- .store = store,
- .request = request,
- .cb = cb,
- .cb_data = cb_data,
- };
- struct packfile_list_entry *e;
- int pack_errors = 0, ret;
-
- if (opts->prefix)
- return packfile_store_for_each_prefixed_object(store, opts, &data);
-
- store->skip_mru_updates = true;
-
- for (e = packfile_store_get_packs(store); e; e = e->next) {
- struct packed_git *p = e->pack;
-
- if ((opts->flags & ODB_FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local)
- continue;
- if ((opts->flags & ODB_FOR_EACH_OBJECT_PROMISOR_ONLY) &&
- !p->pack_promisor)
- continue;
- if ((opts->flags & ODB_FOR_EACH_OBJECT_SKIP_IN_CORE_KEPT_PACKS) &&
- p->pack_keep_in_core)
- continue;
- if ((opts->flags & ODB_FOR_EACH_OBJECT_SKIP_ON_DISK_KEPT_PACKS) &&
- p->pack_keep)
- continue;
- if (open_pack_index(p)) {
- pack_errors = 1;
- continue;
- }
-
- ret = for_each_object_in_pack(p, packfile_store_for_each_object_wrapper,
- &data, opts->flags);
- if (ret)
- goto out;
- }
-
- ret = 0;
-
-out:
- store->skip_mru_updates = false;
-
- if (!ret && pack_errors)
- ret = -1;
- return ret;
-}
-
static int extend_abbrev_len(const struct object_id *a,
const struct object_id *b,
unsigned *out)
diff --git a/packfile.h b/packfile.h
index dd97684e70..0097de0b27 100644
--- a/packfile.h
+++ b/packfile.h
@@ -227,21 +227,6 @@ int for_each_object_in_pack(struct packed_git *p,
each_packed_object_fn, void *data,
enum odb_for_each_object_flags flags);
-/*
- * Iterate through all packed objects in the given packfile store and 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 `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 odb_source_packed *store,
- const struct object_info *request,
- odb_for_each_object_cb cb,
- void *cb_data,
- const struct odb_for_each_object_options *opts);
-
int packfile_store_find_abbrev_len(struct odb_source_packed *store,
const struct object_id *oid,
unsigned min_len,
@@ -354,6 +339,8 @@ extern int do_check_packed_object_crc;
*/
int packed_object_info(struct packed_git *pack,
off_t offset, struct object_info *);
+int packed_object_info_with_index_pos(struct packed_git *p, off_t obj_offset,
+ uint32_t *maybe_index_pos, struct object_info *oi);
void mark_bad_packed_object(struct packed_git *, const struct object_id *);
const struct packed_git *has_packed_and_bad(struct repository *, const struct object_id *);
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v2 10/17] odb/source-packed: wire up `read_object_stream()` callback
From: Patrick Steinhardt @ 2026-06-09 8:51 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260609-pks-odb-source-packed-v2-0-839089132c8b@pks.im>
Wire up the `read_object_stream()` callback for the packed source and
call it in the "files" source via the `odb_source_read_object_stream()`
interface.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-files.c | 2 +-
odb/source-packed.c | 16 ++++++++++++++++
packfile.c | 12 ------------
packfile.h | 4 ----
4 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/odb/source-files.c b/odb/source-files.c
index 8cae35d25e..dff69d0e4e 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -67,7 +67,7 @@ static int odb_source_files_read_object_stream(struct odb_read_stream **out,
const struct object_id *oid)
{
struct odb_source_files *files = odb_source_files_downcast(source);
- if (!packfile_store_read_object_stream(out, files->packed, oid) ||
+ if (!odb_source_read_object_stream(out, &files->packed->base, oid) ||
!odb_source_read_object_stream(out, &files->loose->base, oid))
return 0;
return -1;
diff --git a/odb/source-packed.c b/odb/source-packed.c
index f71a194739..23d7149fe3 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -2,9 +2,11 @@
#include "abspath.h"
#include "chdir-notify.h"
#include "dir.h"
+#include "git-zlib.h"
#include "mergesort.h"
#include "midx.h"
#include "odb/source-packed.h"
+#include "odb/streaming.h"
#include "packfile.h"
int find_pack_entry(struct odb_source_packed *store,
@@ -66,6 +68,19 @@ static int odb_source_packed_read_object_info(struct odb_source *source,
return 0;
}
+static int odb_source_packed_read_object_stream(struct odb_read_stream **out,
+ struct odb_source *source,
+ const struct object_id *oid)
+{
+ struct odb_source_packed *packed = odb_source_packed_downcast(source);
+ struct pack_entry e;
+
+ if (!find_pack_entry(packed, oid, &e))
+ return -1;
+
+ return packfile_read_object_stream(out, oid, e.p, e.offset);
+}
+
void (*report_garbage)(unsigned seen_bits, const char *path);
static void report_helper(const struct string_list *list,
@@ -275,6 +290,7 @@ struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
packed->base.close = odb_source_packed_close;
packed->base.reprepare = odb_source_packed_reprepare;
packed->base.read_object_info = odb_source_packed_read_object_info;
+ packed->base.read_object_stream = odb_source_packed_read_object_stream;
if (!is_absolute_path(parent->base.path))
chdir_notify_register(NULL, odb_source_packed_reparent, packed);
diff --git a/packfile.c b/packfile.c
index 29530532ba..42c84397eb 100644
--- a/packfile.c
+++ b/packfile.c
@@ -2658,15 +2658,3 @@ int packfile_read_object_stream(struct odb_read_stream **out,
return 0;
}
-
-int packfile_store_read_object_stream(struct odb_read_stream **out,
- struct odb_source_packed *store,
- const struct object_id *oid)
-{
- struct pack_entry e;
-
- if (!find_pack_entry(store, oid, &e))
- return -1;
-
- return packfile_read_object_stream(out, oid, e.p, e.offset);
-}
diff --git a/packfile.h b/packfile.h
index 25d458beb0..dd97684e70 100644
--- a/packfile.h
+++ b/packfile.h
@@ -124,10 +124,6 @@ static inline void repo_for_each_pack_data_next(struct repo_for_each_pack_data *
((p) = (eack_pack_data.entry ? eack_pack_data.entry->pack : NULL)); \
repo_for_each_pack_data_next(&eack_pack_data))
-int packfile_store_read_object_stream(struct odb_read_stream **out,
- struct odb_source_packed *store,
- const struct object_id *oid);
-
/*
* Open the packfile and add it to the store if it isn't yet known. Returns
* either the newly opened packfile or the preexisting packfile. Returns a
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v2 09/17] odb/source-packed: wire up `read_object_info()` callback
From: Patrick Steinhardt @ 2026-06-09 8:51 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260609-pks-odb-source-packed-v2-0-839089132c8b@pks.im>
Move the logic to read object info from a "packed" source into
"odb/source-packed.c" and wire it up as the `read_object_info()`
callback.
Note that we also move around the supporting `find_pack_entry()`, but we
still have to expose it to other callers that exist in "packfile.c".
This will be fixed in subsequent commits though, where all callers in
"packfile.c" will have been moved into "odb/source-packed.c", and at
that point we'll be able to make `find_pack_entry()` file-local again.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-files.c | 2 +-
odb/source-packed.c | 60 +++++++++++++++++++++++++++++++++++++++++++
odb/source-packed.h | 6 +++++
packfile.c | 74 ++++++-----------------------------------------------
packfile.h | 15 +++--------
5 files changed, 79 insertions(+), 78 deletions(-)
diff --git a/odb/source-files.c b/odb/source-files.c
index 7b1e0ac565..8cae35d25e 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -55,7 +55,7 @@ static int odb_source_files_read_object_info(struct odb_source *source,
{
struct odb_source_files *files = odb_source_files_downcast(source);
- if (!packfile_store_read_object_info(files->packed, oid, oi, flags) ||
+ if (!odb_source_read_object_info(&files->packed->base, oid, oi, flags) ||
!odb_source_read_object_info(&files->loose->base, oid, oi, flags))
return 0;
diff --git a/odb/source-packed.c b/odb/source-packed.c
index e8e2e5bb48..f71a194739 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -7,6 +7,65 @@
#include "odb/source-packed.h"
#include "packfile.h"
+int find_pack_entry(struct odb_source_packed *store,
+ const struct object_id *oid,
+ struct pack_entry *e)
+{
+ struct packfile_list_entry *l;
+
+ odb_source_packed_prepare(store);
+ if (store->midx && fill_midx_entry(store->midx, oid, e))
+ return 1;
+
+ for (l = store->packs.head; l; l = l->next) {
+ struct packed_git *p = l->pack;
+
+ if (!p->multi_pack_index && packfile_fill_entry(p, oid, e)) {
+ if (!store->skip_mru_updates)
+ packfile_list_prepend(&store->packs, p);
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+static int odb_source_packed_read_object_info(struct odb_source *source,
+ const struct object_id *oid,
+ struct object_info *oi,
+ enum object_info_flags flags)
+{
+ struct odb_source_packed *packed = odb_source_packed_downcast(source);
+ struct pack_entry e;
+ int ret;
+
+ /*
+ * In case the first read didn't surface the object, we have to reload
+ * packfiles. This may cause us to discover new packfiles that have
+ * been added since the last time we have prepared the packfile store.
+ */
+ if (flags & OBJECT_INFO_SECOND_READ)
+ odb_source_reprepare(source);
+
+ if (!find_pack_entry(packed, oid, &e))
+ return 1;
+
+ /*
+ * We know that the caller doesn't actually need the
+ * information below, so return early.
+ */
+ if (!oi)
+ return 0;
+
+ ret = packed_object_info(e.p, e.offset, oi);
+ if (ret < 0) {
+ mark_bad_packed_object(e.p, oid);
+ return -1;
+ }
+
+ return 0;
+}
+
void (*report_garbage)(unsigned seen_bits, const char *path);
static void report_helper(const struct string_list *list,
@@ -215,6 +274,7 @@ struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
packed->base.free = odb_source_packed_free;
packed->base.close = odb_source_packed_close;
packed->base.reprepare = odb_source_packed_reprepare;
+ packed->base.read_object_info = odb_source_packed_read_object_info;
if (!is_absolute_path(parent->base.path))
chdir_notify_register(NULL, odb_source_packed_reparent, packed);
diff --git a/odb/source-packed.h b/odb/source-packed.h
index 9d4796261a..f430ee0b94 100644
--- a/odb/source-packed.h
+++ b/odb/source-packed.h
@@ -90,4 +90,10 @@ static inline struct odb_source_packed *odb_source_packed_downcast(struct odb_so
*/
void odb_source_packed_prepare(struct odb_source_packed *source);
+struct pack_entry;
+
+int find_pack_entry(struct odb_source_packed *store,
+ const struct object_id *oid,
+ struct pack_entry *e);
+
#endif
diff --git a/packfile.c b/packfile.c
index b35afd7797..29530532ba 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1895,9 +1895,9 @@ int is_pack_valid(struct packed_git *p)
return !open_packed_git(p);
}
-static int fill_pack_entry(const struct object_id *oid,
- struct pack_entry *e,
- struct packed_git *p)
+int packfile_fill_entry(struct packed_git *p,
+ const struct object_id *oid,
+ struct pack_entry *e)
{
off_t offset;
@@ -1923,29 +1923,6 @@ static int fill_pack_entry(const struct object_id *oid,
return 1;
}
-static int find_pack_entry(struct odb_source_packed *store,
- const struct object_id *oid,
- struct pack_entry *e)
-{
- struct packfile_list_entry *l;
-
- odb_source_packed_prepare(store);
- if (store->midx && fill_midx_entry(store->midx, oid, e))
- return 1;
-
- for (l = store->packs.head; l; l = l->next) {
- struct packed_git *p = l->pack;
-
- if (!p->multi_pack_index && fill_pack_entry(oid, e, p)) {
- if (!store->skip_mru_updates)
- packfile_list_prepend(&store->packs, p);
- return 1;
- }
- }
-
- return 0;
-}
-
int packfile_store_freshen_object(struct odb_source_packed *store,
const struct object_id *oid)
{
@@ -1962,41 +1939,6 @@ int packfile_store_freshen_object(struct odb_source_packed *store,
return 1;
}
-int packfile_store_read_object_info(struct odb_source_packed *store,
- const struct object_id *oid,
- struct object_info *oi,
- enum object_info_flags flags)
-{
- struct pack_entry e;
- int ret;
-
- /*
- * In case the first read didn't surface the object, we have to reload
- * packfiles. This may cause us to discover new packfiles that have
- * been added since the last time we have prepared the packfile store.
- */
- if (flags & OBJECT_INFO_SECOND_READ)
- odb_source_reprepare(&store->base);
-
- if (!find_pack_entry(store, oid, &e))
- return 1;
-
- /*
- * We know that the caller doesn't actually need the
- * information below, so return early.
- */
- if (!oi)
- return 0;
-
- ret = packed_object_info(e.p, e.offset, oi);
- if (ret < 0) {
- mark_bad_packed_object(e.p, oid);
- return -1;
- }
-
- return 0;
-}
-
static void maybe_invalidate_kept_pack_cache(struct odb_source_packed *store,
unsigned flags)
{
@@ -2053,7 +1995,7 @@ int has_object_pack(struct repository *r, const struct object_id *oid)
odb_prepare_alternates(r->objects);
for (source = r->objects->sources; source; source = source->next) {
struct odb_source_files *files = odb_source_files_downcast(source);
- if (!packfile_store_read_object_info(files->packed, oid, NULL, 0))
+ if (!odb_source_read_object_info(&files->packed->base, oid, NULL, 0))
return 1;
}
@@ -2074,7 +2016,7 @@ int has_object_kept_pack(struct repository *r, const struct object_id *oid,
for (; *cache; cache++) {
struct packed_git *p = *cache;
- if (fill_pack_entry(oid, &e, p))
+ if (packfile_fill_entry(p, oid, &e))
return 1;
}
}
@@ -2208,8 +2150,8 @@ static int for_each_prefixed_object_in_midx(
if (data->request) {
struct object_info oi = *data->request;
- ret = packfile_store_read_object_info(store, current,
- &oi, 0);
+ ret = odb_source_read_object_info(&store->base, current,
+ &oi, 0);
if (ret)
goto out;
@@ -2259,7 +2201,7 @@ static int for_each_prefixed_object_in_pack(
if (data->request) {
struct object_info oi = *data->request;
- ret = packfile_store_read_object_info(store, &oid, &oi, 0);
+ ret = odb_source_read_object_info(&store->base, &oid, &oi, 0);
if (ret)
goto out;
diff --git a/packfile.h b/packfile.h
index 9674e573ae..25d458beb0 100644
--- a/packfile.h
+++ b/packfile.h
@@ -128,17 +128,6 @@ int packfile_store_read_object_stream(struct odb_read_stream **out,
struct odb_source_packed *store,
const struct object_id *oid);
-/*
- * Try to read the object identified by its ID from the object store and
- * populate the object info with its data. Returns 1 in case the object was
- * not found, 0 if it was and read successfully, and a negative error code in
- * case the object was corrupted.
- */
-int packfile_store_read_object_info(struct odb_source_packed *store,
- const struct object_id *oid,
- struct object_info *oi,
- enum object_info_flags flags);
-
/*
* Open the packfile and add it to the store if it isn't yet known. Returns
* either the newly opened packfile or the preexisting packfile. Returns a
@@ -340,6 +329,10 @@ off_t nth_packed_object_offset(const struct packed_git *, uint32_t n);
*/
off_t find_pack_entry_one(const struct object_id *oid, struct packed_git *);
+int packfile_fill_entry(struct packed_git *p,
+ const struct object_id *oid,
+ struct pack_entry *e);
+
int is_pack_valid(struct packed_git *);
void *unpack_entry(struct repository *r, struct packed_git *, off_t, enum object_type *, unsigned long *);
unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, size_t *sizep);
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v2 08/17] packfile: use higher-level interface to implement `has_object_pack()`
From: Patrick Steinhardt @ 2026-06-09 8:51 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260609-pks-odb-source-packed-v2-0-839089132c8b@pks.im>
In `has_object_pack()` we're checking whether a specific object exists
as part of a packfile. This is done by calling the low-level function
`find_pack_entry()`, but this function will eventually be moved into
"odb/source-packed.c" and made file-local.
Refactor the code to use `packfile_store_read_object_info()` instead.
This refactoring is functionally equivalent as that function will call
`find_pack_entry()` itself and then return immediately when it ain't got
no object info pointer as parameter.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
packfile.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/packfile.c b/packfile.c
index 65631f674f..b35afd7797 100644
--- a/packfile.c
+++ b/packfile.c
@@ -2049,14 +2049,12 @@ struct packed_git **packfile_store_get_kept_pack_cache(struct odb_source_packed
int has_object_pack(struct repository *r, const struct object_id *oid)
{
struct odb_source *source;
- struct pack_entry e;
odb_prepare_alternates(r->objects);
for (source = r->objects->sources; source; source = source->next) {
struct odb_source_files *files = odb_source_files_downcast(source);
- int ret = find_pack_entry(files->packed, oid, &e);
- if (ret)
- return ret;
+ if (!packfile_store_read_object_info(files->packed, oid, NULL, 0))
+ return 1;
}
return 0;
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v2 07/17] odb/source-packed: wire up `reprepare()` callback
From: Patrick Steinhardt @ 2026-06-09 8:51 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260609-pks-odb-source-packed-v2-0-839089132c8b@pks.im>
Move the logic to prepare and reprepare the "packed" source into
"odb/source-packed.c" and wire it up as the `reprepare()` callback.
Note that "preparing" a source is not yet generic. Eventually, it would
probably make sense to turn the existing `reprepare()` callback into a
`prepare()` callback with an optional flag to force re-preparing. But
this step will be handled in a separate patch series.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/grep.c | 2 +-
midx.c | 2 +-
odb/source-files.c | 2 +-
odb/source-packed.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++++++
odb/source-packed.h | 9 +++
packfile.c | 160 +---------------------------------------------------
packfile.h | 17 ------
7 files changed, 172 insertions(+), 177 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 6a09571903..8080d1bf5e 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1363,7 +1363,7 @@ int cmd_grep(int argc,
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);
- packfile_store_prepare(files->packed);
+ odb_source_packed_prepare(files->packed);
}
}
diff --git a/midx.c b/midx.c
index efbfbb13f4..00bbd137b2 100644
--- a/midx.c
+++ b/midx.c
@@ -102,7 +102,7 @@ static int midx_read_object_offsets(const unsigned char *chunk_start,
struct multi_pack_index *get_multi_pack_index(struct odb_source *source)
{
struct odb_source_files *files = odb_source_files_downcast(source);
- packfile_store_prepare(files->packed);
+ odb_source_packed_prepare(files->packed);
return files->packed->midx;
}
diff --git a/odb/source-files.c b/odb/source-files.c
index 9b0fa9ccdc..7b1e0ac565 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -45,7 +45,7 @@ static void odb_source_files_reprepare(struct odb_source *source)
{
struct odb_source_files *files = odb_source_files_downcast(source);
odb_source_reprepare(&files->loose->base);
- packfile_store_reprepare(files->packed);
+ odb_source_reprepare(&files->packed->base);
}
static int odb_source_files_read_object_info(struct odb_source *source,
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 74805be1dd..e8e2e5bb48 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -1,10 +1,166 @@
#include "git-compat-util.h"
#include "abspath.h"
#include "chdir-notify.h"
+#include "dir.h"
+#include "mergesort.h"
#include "midx.h"
#include "odb/source-packed.h"
#include "packfile.h"
+void (*report_garbage)(unsigned seen_bits, const char *path);
+
+static void report_helper(const struct string_list *list,
+ int seen_bits, int first, int last)
+{
+ if (seen_bits == (PACKDIR_FILE_PACK|PACKDIR_FILE_IDX))
+ return;
+
+ for (; first < last; first++)
+ report_garbage(seen_bits, list->items[first].string);
+}
+
+static void report_pack_garbage(struct string_list *list)
+{
+ int baselen = -1, first = 0, seen_bits = 0;
+
+ if (!report_garbage)
+ return;
+
+ string_list_sort(list);
+
+ for (size_t i = 0; i < list->nr; i++) {
+ const char *path = list->items[i].string;
+ if (baselen != -1 &&
+ strncmp(path, list->items[first].string, baselen)) {
+ report_helper(list, seen_bits, first, i);
+ baselen = -1;
+ seen_bits = 0;
+ }
+ if (baselen == -1) {
+ const char *dot = strrchr(path, '.');
+ if (!dot) {
+ report_garbage(PACKDIR_FILE_GARBAGE, path);
+ continue;
+ }
+ baselen = dot - path + 1;
+ first = i;
+ }
+ if (!strcmp(path + baselen, "pack"))
+ seen_bits |= 1;
+ else if (!strcmp(path + baselen, "idx"))
+ seen_bits |= 2;
+ }
+ report_helper(list, seen_bits, first, list->nr);
+}
+
+struct prepare_pack_data {
+ struct odb_source *source;
+ struct string_list *garbage;
+};
+
+static void prepare_pack(const char *full_name, size_t full_name_len,
+ const char *file_name, void *_data)
+{
+ struct prepare_pack_data *data = (struct prepare_pack_data *)_data;
+ struct odb_source_files *files = odb_source_files_downcast(data->source);
+ size_t base_len = full_name_len;
+
+ if (strip_suffix_mem(full_name, &base_len, ".idx") &&
+ !(files->packed->midx &&
+ midx_contains_pack(files->packed->midx, file_name))) {
+ char *trimmed_path = xstrndup(full_name, full_name_len);
+ packfile_store_load_pack(files->packed,
+ trimmed_path, data->source->local);
+ free(trimmed_path);
+ }
+
+ if (!report_garbage)
+ return;
+
+ if (!strcmp(file_name, "multi-pack-index") ||
+ !strcmp(file_name, "multi-pack-index.d"))
+ return;
+ if (starts_with(file_name, "multi-pack-index") &&
+ (ends_with(file_name, ".bitmap") || ends_with(file_name, ".rev")))
+ return;
+ if (ends_with(file_name, ".idx") ||
+ ends_with(file_name, ".rev") ||
+ ends_with(file_name, ".pack") ||
+ ends_with(file_name, ".bitmap") ||
+ ends_with(file_name, ".keep") ||
+ ends_with(file_name, ".promisor") ||
+ ends_with(file_name, ".mtimes"))
+ string_list_append(data->garbage, full_name);
+ else
+ report_garbage(PACKDIR_FILE_GARBAGE, full_name);
+}
+
+static void prepare_packed_git_one(struct odb_source *source)
+{
+ struct string_list garbage = STRING_LIST_INIT_DUP;
+ struct prepare_pack_data data = {
+ .source = source,
+ .garbage = &garbage,
+ };
+
+ for_each_file_in_pack_dir(source->path, prepare_pack, &data);
+
+ report_pack_garbage(data.garbage);
+ string_list_clear(data.garbage, 0);
+}
+
+DEFINE_LIST_SORT(static, sort_packs, struct packfile_list_entry, next);
+
+static int sort_pack(const struct packfile_list_entry *a,
+ const struct packfile_list_entry *b)
+{
+ int st;
+
+ /*
+ * Local packs tend to contain objects specific to our
+ * variant of the project than remote ones. In addition,
+ * remote ones could be on a network mounted filesystem.
+ * Favor local ones for these reasons.
+ */
+ st = a->pack->pack_local - b->pack->pack_local;
+ if (st)
+ return -st;
+
+ /*
+ * Younger packs tend to contain more recent objects,
+ * and more recent objects tend to get accessed more
+ * often.
+ */
+ if (a->pack->mtime < b->pack->mtime)
+ return 1;
+ else if (a->pack->mtime == b->pack->mtime)
+ return 0;
+ return -1;
+}
+
+void odb_source_packed_prepare(struct odb_source_packed *source)
+{
+ if (source->initialized)
+ return;
+
+ prepare_multi_pack_index_one(&source->files->base);
+ prepare_packed_git_one(&source->files->base);
+
+ sort_packs(&source->packs.head, sort_pack);
+ for (struct packfile_list_entry *e = source->packs.head; e; e = e->next)
+ if (!e->next)
+ source->packs.tail = e;
+
+ source->initialized = true;
+}
+
+static void odb_source_packed_reprepare(struct odb_source *source)
+{
+ struct odb_source_packed *packed = odb_source_packed_downcast(source);
+ packed->initialized = false;
+ odb_source_packed_prepare(packed);
+}
+
static void odb_source_packed_reparent(const char *name UNUSED,
const char *old_cwd,
const char *new_cwd,
@@ -58,6 +214,7 @@ struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
packed->base.free = odb_source_packed_free;
packed->base.close = odb_source_packed_close;
+ packed->base.reprepare = odb_source_packed_reprepare;
if (!is_absolute_path(parent->base.path))
chdir_notify_register(NULL, odb_source_packed_reparent, packed);
diff --git a/odb/source-packed.h b/odb/source-packed.h
index 68e64cabab..9d4796261a 100644
--- a/odb/source-packed.h
+++ b/odb/source-packed.h
@@ -81,4 +81,13 @@ static inline struct odb_source_packed *odb_source_packed_downcast(struct odb_so
return container_of(source, struct odb_source_packed, base);
}
+/*
+ * Prepare the source by loading packfiles and multi-pack indices for
+ * all alternates. This becomes a no-op if the source is already prepared.
+ *
+ * It shouldn't typically be necessary to call this function directly, as
+ * functions that access the source know to prepare it.
+ */
+void odb_source_packed_prepare(struct odb_source_packed *source);
+
#endif
diff --git a/packfile.c b/packfile.c
index e5386145a7..65631f674f 100644
--- a/packfile.c
+++ b/packfile.c
@@ -8,7 +8,6 @@
#include "pack.h"
#include "repository.h"
#include "dir.h"
-#include "mergesort.h"
#include "packfile.h"
#include "delta.h"
#include "hash-lookup.h"
@@ -812,52 +811,6 @@ struct packed_git *packfile_store_load_pack(struct odb_source_packed *store,
return p;
}
-void (*report_garbage)(unsigned seen_bits, const char *path);
-
-static void report_helper(const struct string_list *list,
- int seen_bits, int first, int last)
-{
- if (seen_bits == (PACKDIR_FILE_PACK|PACKDIR_FILE_IDX))
- return;
-
- for (; first < last; first++)
- report_garbage(seen_bits, list->items[first].string);
-}
-
-static void report_pack_garbage(struct string_list *list)
-{
- int i, baselen = -1, first = 0, seen_bits = 0;
-
- if (!report_garbage)
- return;
-
- string_list_sort(list);
-
- for (i = 0; i < list->nr; i++) {
- const char *path = list->items[i].string;
- if (baselen != -1 &&
- strncmp(path, list->items[first].string, baselen)) {
- report_helper(list, seen_bits, first, i);
- baselen = -1;
- seen_bits = 0;
- }
- if (baselen == -1) {
- const char *dot = strrchr(path, '.');
- if (!dot) {
- report_garbage(PACKDIR_FILE_GARBAGE, path);
- continue;
- }
- baselen = dot - path + 1;
- first = i;
- }
- if (!strcmp(path + baselen, "pack"))
- seen_bits |= 1;
- else if (!strcmp(path + baselen, "idx"))
- seen_bits |= 2;
- }
- report_helper(list, seen_bits, first, list->nr);
-}
-
void for_each_file_in_pack_subdir(const char *objdir,
const char *subdir,
each_file_in_pack_dir_fn fn,
@@ -900,116 +853,9 @@ void for_each_file_in_pack_dir(const char *objdir,
for_each_file_in_pack_subdir(objdir, NULL, fn, data);
}
-struct prepare_pack_data {
- struct odb_source *source;
- struct string_list *garbage;
-};
-
-static void prepare_pack(const char *full_name, size_t full_name_len,
- const char *file_name, void *_data)
-{
- struct prepare_pack_data *data = (struct prepare_pack_data *)_data;
- struct odb_source_files *files = odb_source_files_downcast(data->source);
- size_t base_len = full_name_len;
-
- if (strip_suffix_mem(full_name, &base_len, ".idx") &&
- !(files->packed->midx &&
- midx_contains_pack(files->packed->midx, file_name))) {
- char *trimmed_path = xstrndup(full_name, full_name_len);
- packfile_store_load_pack(files->packed,
- trimmed_path, data->source->local);
- free(trimmed_path);
- }
-
- if (!report_garbage)
- return;
-
- if (!strcmp(file_name, "multi-pack-index") ||
- !strcmp(file_name, "multi-pack-index.d"))
- return;
- if (starts_with(file_name, "multi-pack-index") &&
- (ends_with(file_name, ".bitmap") || ends_with(file_name, ".rev")))
- return;
- if (ends_with(file_name, ".idx") ||
- ends_with(file_name, ".rev") ||
- ends_with(file_name, ".pack") ||
- ends_with(file_name, ".bitmap") ||
- ends_with(file_name, ".keep") ||
- ends_with(file_name, ".promisor") ||
- ends_with(file_name, ".mtimes"))
- string_list_append(data->garbage, full_name);
- else
- report_garbage(PACKDIR_FILE_GARBAGE, full_name);
-}
-
-static void prepare_packed_git_one(struct odb_source *source)
-{
- struct string_list garbage = STRING_LIST_INIT_DUP;
- struct prepare_pack_data data = {
- .source = source,
- .garbage = &garbage,
- };
-
- for_each_file_in_pack_dir(source->path, prepare_pack, &data);
-
- report_pack_garbage(data.garbage);
- string_list_clear(data.garbage, 0);
-}
-
-DEFINE_LIST_SORT(static, sort_packs, struct packfile_list_entry, next);
-
-static int sort_pack(const struct packfile_list_entry *a,
- const struct packfile_list_entry *b)
-{
- int st;
-
- /*
- * Local packs tend to contain objects specific to our
- * variant of the project than remote ones. In addition,
- * remote ones could be on a network mounted filesystem.
- * Favor local ones for these reasons.
- */
- st = a->pack->pack_local - b->pack->pack_local;
- if (st)
- return -st;
-
- /*
- * Younger packs tend to contain more recent objects,
- * and more recent objects tend to get accessed more
- * often.
- */
- if (a->pack->mtime < b->pack->mtime)
- return 1;
- else if (a->pack->mtime == b->pack->mtime)
- return 0;
- return -1;
-}
-
-void packfile_store_prepare(struct odb_source_packed *store)
-{
- if (store->initialized)
- return;
-
- prepare_multi_pack_index_one(&store->files->base);
- prepare_packed_git_one(&store->files->base);
-
- sort_packs(&store->packs.head, sort_pack);
- for (struct packfile_list_entry *e = store->packs.head; e; e = e->next)
- if (!e->next)
- store->packs.tail = e;
-
- store->initialized = true;
-}
-
-void packfile_store_reprepare(struct odb_source_packed *store)
-{
- store->initialized = false;
- packfile_store_prepare(store);
-}
-
struct packfile_list_entry *packfile_store_get_packs(struct odb_source_packed *store)
{
- packfile_store_prepare(store);
+ odb_source_packed_prepare(store);
if (store->midx) {
struct multi_pack_index *m = store->midx;
@@ -2083,7 +1929,7 @@ static int find_pack_entry(struct odb_source_packed *store,
{
struct packfile_list_entry *l;
- packfile_store_prepare(store);
+ odb_source_packed_prepare(store);
if (store->midx && fill_midx_entry(store->midx, oid, e))
return 1;
@@ -2130,7 +1976,7 @@ int packfile_store_read_object_info(struct odb_source_packed *store,
* been added since the last time we have prepared the packfile store.
*/
if (flags & OBJECT_INFO_SECOND_READ)
- packfile_store_reprepare(store);
+ odb_source_reprepare(&store->base);
if (!find_pack_entry(store, oid, &e))
return 1;
diff --git a/packfile.h b/packfile.h
index 9dc3a13112..9674e573ae 100644
--- a/packfile.h
+++ b/packfile.h
@@ -55,23 +55,6 @@ struct packed_git {
char pack_name[FLEX_ARRAY]; /* more */
};
-/*
- * Prepare the packfile store by loading packfiles and multi-pack indices for
- * all alternates. This becomes a no-op if the store is already prepared.
- *
- * It shouldn't typically be necessary to call this function directly, as
- * functions that access the store know to prepare it.
- */
-void packfile_store_prepare(struct odb_source_packed *store);
-
-/*
- * Clear the packfile caches and try to look up any new packfiles that have
- * appeared since last preparing the packfiles store.
- *
- * This function must be called under the `odb_read_lock()`.
- */
-void packfile_store_reprepare(struct odb_source_packed *store);
-
/*
* Add the pack to the store so that contained objects become accessible via
* the store. This moves ownership into the store.
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v2 06/17] odb/source-packed: wire up `close()` callback
From: Patrick Steinhardt @ 2026-06-09 8:50 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260609-pks-odb-source-packed-v2-0-839089132c8b@pks.im>
Wire up a new `close()` callback for the packed source and call it from
the "files" source via the generic `odb_source_close()` interface.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-files.c | 2 +-
odb/source-packed.c | 16 ++++++++++++++++
packfile.c | 12 ------------
packfile.h | 6 ------
4 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/odb/source-files.c b/odb/source-files.c
index 3608808e7c..9b0fa9ccdc 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -38,7 +38,7 @@ static void odb_source_files_close(struct odb_source *source)
{
struct odb_source_files *files = odb_source_files_downcast(source);
odb_source_close(&files->loose->base);
- packfile_store_close(files->packed);
+ odb_source_close(&files->packed->base);
}
static void odb_source_files_reprepare(struct odb_source *source)
diff --git a/odb/source-packed.c b/odb/source-packed.c
index f81a990cbd..74805be1dd 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -1,6 +1,7 @@
#include "git-compat-util.h"
#include "abspath.h"
#include "chdir-notify.h"
+#include "midx.h"
#include "odb/source-packed.h"
#include "packfile.h"
@@ -16,6 +17,20 @@ static void odb_source_packed_reparent(const char *name UNUSED,
packed->base.path = path;
}
+static void odb_source_packed_close(struct odb_source *source)
+{
+ struct odb_source_packed *packed = odb_source_packed_downcast(source);
+
+ for (struct packfile_list_entry *e = packed->packs.head; e; e = e->next) {
+ if (e->pack->do_not_close)
+ BUG("want to close pack marked 'do-not-close'");
+ close_pack(e->pack);
+ }
+ if (packed->midx)
+ close_midx(packed->midx);
+ packed->midx = NULL;
+}
+
static void odb_source_packed_free(struct odb_source *source)
{
struct odb_source_packed *packed = odb_source_packed_downcast(source);
@@ -42,6 +57,7 @@ struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
strmap_init(&packed->packs_by_path);
packed->base.free = odb_source_packed_free;
+ packed->base.close = odb_source_packed_close;
if (!is_absolute_path(parent->base.path))
chdir_notify_register(NULL, odb_source_packed_reparent, packed);
diff --git a/packfile.c b/packfile.c
index 6d492216de..e5386145a7 100644
--- a/packfile.c
+++ b/packfile.c
@@ -2749,18 +2749,6 @@ int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *l
return 0;
}
-void packfile_store_close(struct odb_source_packed *store)
-{
- for (struct packfile_list_entry *e = store->packs.head; e; e = e->next) {
- if (e->pack->do_not_close)
- BUG("want to close pack marked 'do-not-close'");
- close_pack(e->pack);
- }
- if (store->midx)
- close_midx(store->midx);
- store->midx = NULL;
-}
-
struct odb_packed_read_stream {
struct odb_read_stream base;
struct packed_git *pack;
diff --git a/packfile.h b/packfile.h
index e8bc9349f8..9dc3a13112 100644
--- a/packfile.h
+++ b/packfile.h
@@ -55,12 +55,6 @@ struct packed_git {
char pack_name[FLEX_ARRAY]; /* more */
};
-/*
- * Close all packfiles associated with this store. The packfiles won't be
- * free'd, so they can be re-opened at a later point in time.
- */
-void packfile_store_close(struct odb_source_packed *store);
-
/*
* Prepare the packfile store by loading packfiles and multi-pack indices for
* all alternates. This becomes a no-op if the store is already prepared.
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v2 05/17] odb/source-packed: start converting to a proper `struct odb_source`
From: Patrick Steinhardt @ 2026-06-09 8:50 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260609-pks-odb-source-packed-v2-0-839089132c8b@pks.im>
Start converting `struct odb_source_packed` into a proper pluggable
`struct odb_source` by embedding the base struct and assigning it the
new `ODB_SOURCE_PACKED` type. Furthermore, wire up lifecycle management
of this source by implementing the `free` callback and taking ownership
of the chdir notifications.
Note that the packed source is not yet functional as a standalone `struct
odb_source`, as it's missing all of the callback implementations. These
will be wired up in subsequent commits.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-files.c | 2 +-
odb/source-packed.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
odb/source-packed.h | 12 ++++++++++++
odb/source.h | 3 +++
packfile.c | 10 ----------
packfile.h | 6 ------
6 files changed, 60 insertions(+), 22 deletions(-)
diff --git a/odb/source-files.c b/odb/source-files.c
index e04525fb08..3608808e7c 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -29,7 +29,7 @@ static void odb_source_files_free(struct odb_source *source)
struct odb_source_files *files = odb_source_files_downcast(source);
chdir_notify_unregister(NULL, odb_source_files_reparent, files);
odb_source_free(&files->loose->base);
- packfile_store_free(files->packed);
+ odb_source_free(&files->packed->base);
odb_source_release(&files->base);
free(files);
}
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 12e785be48..f81a990cbd 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -1,11 +1,50 @@
#include "git-compat-util.h"
+#include "abspath.h"
+#include "chdir-notify.h"
#include "odb/source-packed.h"
+#include "packfile.h"
+
+static void odb_source_packed_reparent(const char *name UNUSED,
+ const char *old_cwd,
+ const char *new_cwd,
+ void *cb_data)
+{
+ struct odb_source_packed *packed = cb_data;
+ char *path = reparent_relative_path(old_cwd, new_cwd,
+ packed->base.path);
+ free(packed->base.path);
+ packed->base.path = path;
+}
+
+static void odb_source_packed_free(struct odb_source *source)
+{
+ struct odb_source_packed *packed = odb_source_packed_downcast(source);
+
+ chdir_notify_unregister(NULL, odb_source_packed_reparent, packed);
+
+ for (struct packfile_list_entry *e = packed->packs.head; e; e = e->next)
+ free(e->pack);
+ packfile_list_clear(&packed->packs);
+
+ strmap_clear(&packed->packs_by_path, 0);
+ odb_source_release(&packed->base);
+ free(packed);
+}
struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
{
- struct odb_source_packed *store;
- CALLOC_ARRAY(store, 1);
- store->files = parent;
- strmap_init(&store->packs_by_path);
- return store;
+ struct odb_source_packed *packed;
+
+ CALLOC_ARRAY(packed, 1);
+ odb_source_init(&packed->base, parent->base.odb, ODB_SOURCE_PACKED,
+ parent->base.path, parent->base.local);
+ packed->files = parent;
+ strmap_init(&packed->packs_by_path);
+
+ packed->base.free = odb_source_packed_free;
+
+ if (!is_absolute_path(parent->base.path))
+ chdir_notify_register(NULL, odb_source_packed_reparent, packed);
+
+ return packed;
}
diff --git a/odb/source-packed.h b/odb/source-packed.h
index 3c2d229a17..68e64cabab 100644
--- a/odb/source-packed.h
+++ b/odb/source-packed.h
@@ -9,6 +9,7 @@
* A store that manages packfiles for a given object database.
*/
struct odb_source_packed {
+ struct odb_source base;
struct odb_source_files *files;
/*
@@ -69,4 +70,15 @@ struct odb_source_packed {
*/
struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent);
+/*
+ * Cast the given object database source to the packed backend. This will cause
+ * a BUG in case the source doesn't use this backend.
+ */
+static inline struct odb_source_packed *odb_source_packed_downcast(struct odb_source *source)
+{
+ if (source->type != ODB_SOURCE_PACKED)
+ BUG("trying to downcast source of type '%d' to packed", source->type);
+ return container_of(source, struct odb_source_packed, base);
+}
+
#endif
diff --git a/odb/source.h b/odb/source.h
index 8bcb67787e..6865e1f71a 100644
--- a/odb/source.h
+++ b/odb/source.h
@@ -17,6 +17,9 @@ enum odb_source_type {
/* The "loose" backend that uses loose objects, only. */
ODB_SOURCE_LOOSE,
+ /* The "packed" backend that uses packfiles. */
+ ODB_SOURCE_PACKED,
+
/* The "in-memory" backend that stores objects in memory. */
ODB_SOURCE_INMEMORY,
};
diff --git a/packfile.c b/packfile.c
index 862a24ad49..6d492216de 100644
--- a/packfile.c
+++ b/packfile.c
@@ -2749,16 +2749,6 @@ int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *l
return 0;
}
-void packfile_store_free(struct odb_source_packed *store)
-{
- for (struct packfile_list_entry *e = store->packs.head; e; e = e->next)
- free(e->pack);
- packfile_list_clear(&store->packs);
-
- strmap_clear(&store->packs_by_path, 0);
- free(store);
-}
-
void packfile_store_close(struct odb_source_packed *store)
{
for (struct packfile_list_entry *e = store->packs.head; e; e = e->next) {
diff --git a/packfile.h b/packfile.h
index 2d0bb7adbe..e8bc9349f8 100644
--- a/packfile.h
+++ b/packfile.h
@@ -55,12 +55,6 @@ struct packed_git {
char pack_name[FLEX_ARRAY]; /* more */
};
-/*
- * Free the packfile store and all its associated state. All packfiles
- * tracked by the store will be closed.
- */
-void packfile_store_free(struct odb_source_packed *store);
-
/*
* Close all packfiles associated with this store. The packfiles won't be
* free'd, so they can be re-opened at a later point in time.
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v2 04/17] odb/source-packed: store pointer to "files" instead of generic source
From: Patrick Steinhardt @ 2026-06-09 8:50 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260609-pks-odb-source-packed-v2-0-839089132c8b@pks.im>
The `struct odb_source_packed` holds a pointer to its owning parent
source. The way that Git is currently structured, this parent is always
the "files" source. In subsequent commits we're going to detangle that
so that the "packed" source doesn't have any owning parent source at
all, which makes it usable as a completely standalone source.
Detangling this mess is somewhat intricate though, and is made even more
intricate because it's not always clear which kind of source one is
holding at a specific point in time -- either the parent "files" source,
or the child "packed" source.
Make this relationship more explicit by storing a pointer to the "files"
source instead of storing a pointer to a generic `struct odb_source`.
This will help make subsequent steps a bit clearer.
Note that this is a temporary step, only. At the end of this series
we will have dropped the parent pointer completely.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-files.c | 2 +-
odb/source-packed.c | 4 ++--
odb/source-packed.h | 4 ++--
packfile.c | 12 ++++++------
4 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/odb/source-files.c b/odb/source-files.c
index 191562f316..e04525fb08 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -269,7 +269,7 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
CALLOC_ARRAY(files, 1);
odb_source_init(&files->base, odb, ODB_SOURCE_FILES, path, local);
files->loose = odb_source_loose_new(odb, path, local);
- files->packed = odb_source_packed_new(&files->base);
+ files->packed = odb_source_packed_new(files);
files->base.free = odb_source_files_free;
files->base.close = odb_source_files_close;
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 1e94b47ea0..12e785be48 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -1,11 +1,11 @@
#include "git-compat-util.h"
#include "odb/source-packed.h"
-struct odb_source_packed *odb_source_packed_new(struct odb_source *source)
+struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
{
struct odb_source_packed *store;
CALLOC_ARRAY(store, 1);
- store->source = source;
+ store->files = parent;
strmap_init(&store->packs_by_path);
return store;
}
diff --git a/odb/source-packed.h b/odb/source-packed.h
index 327be4ad65..3c2d229a17 100644
--- a/odb/source-packed.h
+++ b/odb/source-packed.h
@@ -9,7 +9,7 @@
* A store that manages packfiles for a given object database.
*/
struct odb_source_packed {
- struct odb_source *source;
+ struct odb_source_files *files;
/*
* The list of packfiles in the order in which they have been most
@@ -67,6 +67,6 @@ struct odb_source_packed {
* Allocate and initialize a new empty packfile store for the given object
* database source.
*/
-struct odb_source_packed *odb_source_packed_new(struct odb_source *source);
+struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent);
#endif
diff --git a/packfile.c b/packfile.c
index 99be5789ef..862a24ad49 100644
--- a/packfile.c
+++ b/packfile.c
@@ -802,7 +802,7 @@ struct packed_git *packfile_store_load_pack(struct odb_source_packed *store,
p = strmap_get(&store->packs_by_path, key.buf);
if (!p) {
- p = add_packed_git(store->source->odb->repo, idx_path,
+ p = add_packed_git(store->files->base.odb->repo, idx_path,
strlen(idx_path), local);
if (p)
packfile_store_add_pack(store, p);
@@ -990,8 +990,8 @@ void packfile_store_prepare(struct odb_source_packed *store)
if (store->initialized)
return;
- prepare_multi_pack_index_one(store->source);
- prepare_packed_git_one(store->source);
+ prepare_multi_pack_index_one(&store->files->base);
+ prepare_packed_git_one(&store->files->base);
sort_packs(&store->packs.head, sort_pack);
for (struct packfile_list_entry *e = store->packs.head; e; e = e->next)
@@ -1029,7 +1029,7 @@ int packfile_store_count_objects(struct odb_source_packed *store,
unsigned long count = 0;
int ret;
- m = get_multi_pack_index(store->source);
+ m = get_multi_pack_index(&store->files->base);
if (m)
count += m->num_objects + m->num_objects_in_base;
@@ -2450,7 +2450,7 @@ static int packfile_store_for_each_prefixed_object(
store->skip_mru_updates = true;
- m = get_multi_pack_index(store->source);
+ m = get_multi_pack_index(&store->files->base);
if (m) {
ret = for_each_prefixed_object_in_midx(store, m, opts, data);
if (ret)
@@ -2632,7 +2632,7 @@ int packfile_store_find_abbrev_len(struct odb_source_packed *store,
struct packfile_list_entry *e;
struct multi_pack_index *m;
- m = get_multi_pack_index(store->source);
+ m = get_multi_pack_index(&store->files->base);
if (m)
find_abbrev_len_for_midx(m, oid, min_len, &min_len);
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v2 03/17] packfile: move packed source into "odb/" subsystem
From: Patrick Steinhardt @ 2026-06-09 8:50 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260609-pks-odb-source-packed-v2-0-839089132c8b@pks.im>
In subsequent patches we'll be turning `struct odb_source_packed` into a
proper `struct odb_source`. As a first step towards this goal, move its
struct out of "packfile.{c,h}" and into "odb/source-packed.{c,h}".
This detaches the implementation of the packfile object source from the
generic packfile code, following the same convention already used by the
"files" and "in-memory" sources.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Makefile | 1 +
meson.build | 1 +
odb/source-files.c | 2 +-
odb/source-packed.c | 11 ++++++++
odb/source-packed.h | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++
packfile.c | 9 -------
packfile.h | 66 +-----------------------------------------------
7 files changed, 87 insertions(+), 75 deletions(-)
diff --git a/Makefile b/Makefile
index ed1731548e..113fa45993 100644
--- a/Makefile
+++ b/Makefile
@@ -1218,6 +1218,7 @@ LIB_OBJS += odb/source.o
LIB_OBJS += odb/source-files.o
LIB_OBJS += odb/source-inmemory.o
LIB_OBJS += odb/source-loose.o
+LIB_OBJS += odb/source-packed.o
LIB_OBJS += odb/streaming.o
LIB_OBJS += odb/transaction.o
LIB_OBJS += oid-array.o
diff --git a/meson.build b/meson.build
index 12913fc948..ca235801cf 100644
--- a/meson.build
+++ b/meson.build
@@ -406,6 +406,7 @@ libgit_sources = [
'odb/source-files.c',
'odb/source-inmemory.c',
'odb/source-loose.c',
+ 'odb/source-packed.c',
'odb/streaming.c',
'odb/transaction.c',
'oid-array.c',
diff --git a/odb/source-files.c b/odb/source-files.c
index 5bdd042922..191562f316 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -269,7 +269,7 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
CALLOC_ARRAY(files, 1);
odb_source_init(&files->base, odb, ODB_SOURCE_FILES, path, local);
files->loose = odb_source_loose_new(odb, path, local);
- files->packed = packfile_store_new(&files->base);
+ files->packed = odb_source_packed_new(&files->base);
files->base.free = odb_source_files_free;
files->base.close = odb_source_files_close;
diff --git a/odb/source-packed.c b/odb/source-packed.c
new file mode 100644
index 0000000000..1e94b47ea0
--- /dev/null
+++ b/odb/source-packed.c
@@ -0,0 +1,11 @@
+#include "git-compat-util.h"
+#include "odb/source-packed.h"
+
+struct odb_source_packed *odb_source_packed_new(struct odb_source *source)
+{
+ struct odb_source_packed *store;
+ CALLOC_ARRAY(store, 1);
+ store->source = source;
+ strmap_init(&store->packs_by_path);
+ return store;
+}
diff --git a/odb/source-packed.h b/odb/source-packed.h
new file mode 100644
index 0000000000..327be4ad65
--- /dev/null
+++ b/odb/source-packed.h
@@ -0,0 +1,72 @@
+#ifndef ODB_SOURCE_PACKED_H
+#define ODB_SOURCE_PACKED_H
+
+#include "odb/source.h"
+#include "packfile-list.h"
+#include "strmap.h"
+
+/*
+ * A store that manages packfiles for a given object database.
+ */
+struct odb_source_packed {
+ struct odb_source *source;
+
+ /*
+ * The list of packfiles in the order in which they have been most
+ * recently used.
+ */
+ struct packfile_list packs;
+
+ /*
+ * Cache of packfiles which are marked as "kept", either because there
+ * is an on-disk ".keep" file or because they are marked as "kept" in
+ * memory.
+ *
+ * Should not be accessed directly, but via
+ * `packfile_store_get_kept_pack_cache()`. The list of packs gets
+ * invalidated when the stored flags and the flags passed to
+ * `packfile_store_get_kept_pack_cache()` mismatch.
+ */
+ struct {
+ struct packed_git **packs;
+ unsigned flags;
+ } kept_cache;
+
+ /* The multi-pack index that belongs to this specific packfile store. */
+ struct multi_pack_index *midx;
+
+ /*
+ * A map of packfile names to packed_git structs for tracking which
+ * packs have been loaded already.
+ */
+ struct strmap packs_by_path;
+
+ /*
+ * Whether packfiles have already been populated with this store's
+ * packs.
+ */
+ bool initialized;
+
+ /*
+ * Usually, packfiles will be reordered to the front of the `packs`
+ * list whenever an object is looked up via them. This has the effect
+ * that packs that contain a lot of accessed objects will be located
+ * towards the front.
+ *
+ * This is usually desireable, but there are exceptions. One exception
+ * is when the looking up multiple objects in a loop for each packfile.
+ * In that case, we may easily end up with an infinite loop as the
+ * packfiles get reordered to the front repeatedly.
+ *
+ * Setting this field to `true` thus disables these reorderings.
+ */
+ bool skip_mru_updates;
+};
+
+/*
+ * Allocate and initialize a new empty packfile store for the given object
+ * database source.
+ */
+struct odb_source_packed *odb_source_packed_new(struct odb_source *source);
+
+#endif
diff --git a/packfile.c b/packfile.c
index 27ea4a8436..99be5789ef 100644
--- a/packfile.c
+++ b/packfile.c
@@ -2749,15 +2749,6 @@ int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *l
return 0;
}
-struct odb_source_packed *packfile_store_new(struct odb_source *source)
-{
- struct odb_source_packed *store;
- CALLOC_ARRAY(store, 1);
- store->source = source;
- strmap_init(&store->packs_by_path);
- return store;
-}
-
void packfile_store_free(struct odb_source_packed *store)
{
for (struct packfile_list_entry *e = store->packs.head; e; e = e->next)
diff --git a/packfile.h b/packfile.h
index 4e3d701a3a..2d0bb7adbe 100644
--- a/packfile.h
+++ b/packfile.h
@@ -5,10 +5,10 @@
#include "object.h"
#include "odb.h"
#include "odb/source-files.h"
+#include "odb/source-packed.h"
#include "oidset.h"
#include "packfile-list.h"
#include "repository.h"
-#include "strmap.h"
/* in odb.h */
struct object_info;
@@ -55,70 +55,6 @@ struct packed_git {
char pack_name[FLEX_ARRAY]; /* more */
};
-/*
- * A store that manages packfiles for a given object database.
- */
-struct odb_source_packed {
- struct odb_source *source;
-
- /*
- * The list of packfiles in the order in which they have been most
- * recently used.
- */
- struct packfile_list packs;
-
- /*
- * Cache of packfiles which are marked as "kept", either because there
- * is an on-disk ".keep" file or because they are marked as "kept" in
- * memory.
- *
- * Should not be accessed directly, but via
- * `packfile_store_get_kept_pack_cache()`. The list of packs gets
- * invalidated when the stored flags and the flags passed to
- * `packfile_store_get_kept_pack_cache()` mismatch.
- */
- struct {
- struct packed_git **packs;
- unsigned flags;
- } kept_cache;
-
- /* The multi-pack index that belongs to this specific packfile store. */
- struct multi_pack_index *midx;
-
- /*
- * A map of packfile names to packed_git structs for tracking which
- * packs have been loaded already.
- */
- struct strmap packs_by_path;
-
- /*
- * Whether packfiles have already been populated with this store's
- * packs.
- */
- bool initialized;
-
- /*
- * Usually, packfiles will be reordered to the front of the `packs`
- * list whenever an object is looked up via them. This has the effect
- * that packs that contain a lot of accessed objects will be located
- * towards the front.
- *
- * This is usually desireable, but there are exceptions. One exception
- * is when the looking up multiple objects in a loop for each packfile.
- * In that case, we may easily end up with an infinite loop as the
- * packfiles get reordered to the front repeatedly.
- *
- * Setting this field to `true` thus disables these reorderings.
- */
- bool skip_mru_updates;
-};
-
-/*
- * Allocate and initialize a new empty packfile store for the given object
- * database source.
- */
-struct odb_source_packed *packfile_store_new(struct odb_source *source);
-
/*
* Free the packfile store and all its associated state. All packfiles
* tracked by the store will be closed.
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v2 02/17] packfile: split out packfile list logic
From: Patrick Steinhardt @ 2026-06-09 8:50 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260609-pks-odb-source-packed-v2-0-839089132c8b@pks.im>
In the next commit we're about to introduce the "packed" object database
source. This source will embed a packfile list, and consequently we'll
have to include "packfile.h" to make the struct definition available.
This will unfortunately lead to a cyclic dependency that we cannot
resolve with a forward declaration.
Split out the code that relates to the packfile list into a separate
compilation unit so that both "packfile.h" and "odb/source-packed.h" can
include it.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Makefile | 1 +
meson.build | 1 +
packfile-list.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
packfile-list.h | 28 +++++++++++++++++++
packfile.c | 83 -------------------------------------------------------
packfile.h | 23 +--------------
6 files changed, 117 insertions(+), 105 deletions(-)
diff --git a/Makefile b/Makefile
index 0976a69b4c..ed1731548e 100644
--- a/Makefile
+++ b/Makefile
@@ -1233,6 +1233,7 @@ LIB_OBJS += pack-refs.o
LIB_OBJS += pack-revindex.o
LIB_OBJS += pack-write.o
LIB_OBJS += packfile.o
+LIB_OBJS += packfile-list.o
LIB_OBJS += pager.o
LIB_OBJS += parallel-checkout.o
LIB_OBJS += parse.o
diff --git a/meson.build b/meson.build
index 3247697f74..12913fc948 100644
--- a/meson.build
+++ b/meson.build
@@ -421,6 +421,7 @@ libgit_sources = [
'pack-revindex.c',
'pack-write.c',
'packfile.c',
+ 'packfile-list.c',
'pager.c',
'parallel-checkout.c',
'parse.c',
diff --git a/packfile-list.c b/packfile-list.c
new file mode 100644
index 0000000000..01fb913abf
--- /dev/null
+++ b/packfile-list.c
@@ -0,0 +1,86 @@
+#include "git-compat-util.h"
+#include "packfile.h"
+#include "packfile-list.h"
+
+void packfile_list_clear(struct packfile_list *list)
+{
+ struct packfile_list_entry *e, *next;
+
+ for (e = list->head; e; e = next) {
+ next = e->next;
+ free(e);
+ }
+
+ list->head = list->tail = NULL;
+}
+
+static struct packfile_list_entry *packfile_list_remove_internal(struct packfile_list *list,
+ struct packed_git *pack)
+{
+ struct packfile_list_entry *e, *prev;
+
+ for (e = list->head, prev = NULL; e; prev = e, e = e->next) {
+ if (e->pack != pack)
+ continue;
+
+ if (prev)
+ prev->next = e->next;
+ if (list->head == e)
+ list->head = e->next;
+ if (list->tail == e)
+ list->tail = prev;
+
+ return e;
+ }
+
+ return NULL;
+}
+
+void packfile_list_remove(struct packfile_list *list, struct packed_git *pack)
+{
+ free(packfile_list_remove_internal(list, pack));
+}
+
+void packfile_list_prepend(struct packfile_list *list, struct packed_git *pack)
+{
+ struct packfile_list_entry *entry;
+
+ entry = packfile_list_remove_internal(list, pack);
+ if (!entry) {
+ entry = xmalloc(sizeof(*entry));
+ entry->pack = pack;
+ }
+ entry->next = list->head;
+
+ list->head = entry;
+ if (!list->tail)
+ list->tail = entry;
+}
+
+void packfile_list_append(struct packfile_list *list, struct packed_git *pack)
+{
+ struct packfile_list_entry *entry;
+
+ entry = packfile_list_remove_internal(list, pack);
+ if (!entry) {
+ entry = xmalloc(sizeof(*entry));
+ entry->pack = pack;
+ }
+ entry->next = NULL;
+
+ if (list->tail) {
+ list->tail->next = entry;
+ list->tail = entry;
+ } else {
+ list->head = list->tail = entry;
+ }
+}
+
+struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs,
+ const struct object_id *oid)
+{
+ for (; packs; packs = packs->next)
+ if (find_pack_entry_one(oid, packs->pack))
+ return packs->pack;
+ return NULL;
+}
diff --git a/packfile-list.h b/packfile-list.h
new file mode 100644
index 0000000000..1b05e2aa36
--- /dev/null
+++ b/packfile-list.h
@@ -0,0 +1,28 @@
+#ifndef PACKFILE_LIST_H
+#define PACKFILE_LIST_H
+
+struct object_id;
+
+struct packfile_list {
+ struct packfile_list_entry *head, *tail;
+};
+
+struct packfile_list_entry {
+ struct packfile_list_entry *next;
+ struct packed_git *pack;
+};
+
+void packfile_list_clear(struct packfile_list *list);
+void packfile_list_remove(struct packfile_list *list, struct packed_git *pack);
+void packfile_list_prepend(struct packfile_list *list, struct packed_git *pack);
+void packfile_list_append(struct packfile_list *list, struct packed_git *pack);
+
+/*
+ * Find the pack within the "packs" list whose index contains the object
+ * "oid". For general object lookups, you probably don't want this; use
+ * find_pack_entry() instead.
+ */
+struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs,
+ const struct object_id *oid);
+
+#endif
diff --git a/packfile.c b/packfile.c
index a2d768d0ae..27ea4a8436 100644
--- a/packfile.c
+++ b/packfile.c
@@ -48,89 +48,6 @@ static size_t pack_mapped;
#define SZ_FMT PRIuMAX
static inline uintmax_t sz_fmt(size_t s) { return s; }
-void packfile_list_clear(struct packfile_list *list)
-{
- struct packfile_list_entry *e, *next;
-
- for (e = list->head; e; e = next) {
- next = e->next;
- free(e);
- }
-
- list->head = list->tail = NULL;
-}
-
-static struct packfile_list_entry *packfile_list_remove_internal(struct packfile_list *list,
- struct packed_git *pack)
-{
- struct packfile_list_entry *e, *prev;
-
- for (e = list->head, prev = NULL; e; prev = e, e = e->next) {
- if (e->pack != pack)
- continue;
-
- if (prev)
- prev->next = e->next;
- if (list->head == e)
- list->head = e->next;
- if (list->tail == e)
- list->tail = prev;
-
- return e;
- }
-
- return NULL;
-}
-
-void packfile_list_remove(struct packfile_list *list, struct packed_git *pack)
-{
- free(packfile_list_remove_internal(list, pack));
-}
-
-void packfile_list_prepend(struct packfile_list *list, struct packed_git *pack)
-{
- struct packfile_list_entry *entry;
-
- entry = packfile_list_remove_internal(list, pack);
- if (!entry) {
- entry = xmalloc(sizeof(*entry));
- entry->pack = pack;
- }
- entry->next = list->head;
-
- list->head = entry;
- if (!list->tail)
- list->tail = entry;
-}
-
-void packfile_list_append(struct packfile_list *list, struct packed_git *pack)
-{
- struct packfile_list_entry *entry;
-
- entry = packfile_list_remove_internal(list, pack);
- if (!entry) {
- entry = xmalloc(sizeof(*entry));
- entry->pack = pack;
- }
- entry->next = NULL;
-
- if (list->tail) {
- list->tail->next = entry;
- list->tail = entry;
- } else {
- list->head = list->tail = entry;
- }
-}
-
-struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs,
- const struct object_id *oid)
-{
- for (; packs; packs = packs->next)
- if (find_pack_entry_one(oid, packs->pack))
- return packs->pack;
- return NULL;
-}
-
void pack_report(struct repository *repo)
{
fprintf(stderr,
diff --git a/packfile.h b/packfile.h
index 9cec15bc50..4e3d701a3a 100644
--- a/packfile.h
+++ b/packfile.h
@@ -6,6 +6,7 @@
#include "odb.h"
#include "odb/source-files.h"
#include "oidset.h"
+#include "packfile-list.h"
#include "repository.h"
#include "strmap.h"
@@ -54,28 +55,6 @@ struct packed_git {
char pack_name[FLEX_ARRAY]; /* more */
};
-struct packfile_list {
- struct packfile_list_entry *head, *tail;
-};
-
-struct packfile_list_entry {
- struct packfile_list_entry *next;
- struct packed_git *pack;
-};
-
-void packfile_list_clear(struct packfile_list *list);
-void packfile_list_remove(struct packfile_list *list, struct packed_git *pack);
-void packfile_list_prepend(struct packfile_list *list, struct packed_git *pack);
-void packfile_list_append(struct packfile_list *list, struct packed_git *pack);
-
-/*
- * Find the pack within the "packs" list whose index contains the object
- * "oid". For general object lookups, you probably don't want this; use
- * find_pack_entry() instead.
- */
-struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs,
- const struct object_id *oid);
-
/*
* A store that manages packfiles for a given object database.
*/
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v2 01/17] packfile: rename `struct packfile_store` to `odb_source_packed`
From: Patrick Steinhardt @ 2026-06-09 8:50 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260609-pks-odb-source-packed-v2-0-839089132c8b@pks.im>
Not too long ago, we have introduced the packfile store in b7983adb51
(packfile: introduce a new `struct packfile_store`, 2025-09-23). This
struct is responsible for managing all of our access to packfiles and is
used as one of the two sources of objects for the "files" source.
Back when I introduced this structure I didn't have the clear vision yet
that it will eventually also turn into a proper object database source,
and how exactly that infrastructure will look like. Now though it's
becoming increasingly clear that it does make sense to treat it just the
same as any of our other ODB sources.
The consequence is that the naming is now a bit out-of-date: it's just
another source and will be turned into a proper `struct odb_source` over
the next couple of commits, but it's not named accordingly.
Rename the structure to `odb_source_packed` to align it with this goal
and to bring it in line with the other sources we already have.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-files.h | 4 ++--
packfile.c | 56 +++++++++++++++++++++++++++---------------------------
packfile.h | 32 +++++++++++++++----------------
3 files changed, 46 insertions(+), 46 deletions(-)
diff --git a/odb/source-files.h b/odb/source-files.h
index 23a3b4e04b..d7ac3c1c81 100644
--- a/odb/source-files.h
+++ b/odb/source-files.h
@@ -4,7 +4,7 @@
#include "odb/source.h"
struct odb_source_loose;
-struct packfile_store;
+struct odb_source_packed;
/*
* The files object database source uses a combination of loose objects and
@@ -13,7 +13,7 @@ struct packfile_store;
struct odb_source_files {
struct odb_source base;
struct odb_source_loose *loose;
- struct packfile_store *packed;
+ struct odb_source_packed *packed;
};
/* Allocate and initialize a new object source. */
diff --git a/packfile.c b/packfile.c
index 89366abfe3..a2d768d0ae 100644
--- a/packfile.c
+++ b/packfile.c
@@ -859,7 +859,7 @@ struct packed_git *add_packed_git(struct repository *r, const char *path,
return p;
}
-void packfile_store_add_pack(struct packfile_store *store,
+void packfile_store_add_pack(struct odb_source_packed *store,
struct packed_git *pack)
{
if (pack->pack_fd != -1)
@@ -869,7 +869,7 @@ void packfile_store_add_pack(struct packfile_store *store,
strmap_put(&store->packs_by_path, pack->pack_name, pack);
}
-struct packed_git *packfile_store_load_pack(struct packfile_store *store,
+struct packed_git *packfile_store_load_pack(struct odb_source_packed *store,
const char *idx_path, int local)
{
struct strbuf key = STRBUF_INIT;
@@ -1068,7 +1068,7 @@ static int sort_pack(const struct packfile_list_entry *a,
return -1;
}
-void packfile_store_prepare(struct packfile_store *store)
+void packfile_store_prepare(struct odb_source_packed *store)
{
if (store->initialized)
return;
@@ -1084,13 +1084,13 @@ void packfile_store_prepare(struct packfile_store *store)
store->initialized = true;
}
-void packfile_store_reprepare(struct packfile_store *store)
+void packfile_store_reprepare(struct odb_source_packed *store)
{
store->initialized = false;
packfile_store_prepare(store);
}
-struct packfile_list_entry *packfile_store_get_packs(struct packfile_store *store)
+struct packfile_list_entry *packfile_store_get_packs(struct odb_source_packed *store)
{
packfile_store_prepare(store);
@@ -1103,7 +1103,7 @@ struct packfile_list_entry *packfile_store_get_packs(struct packfile_store *stor
return store->packs.head;
}
-int packfile_store_count_objects(struct packfile_store *store,
+int packfile_store_count_objects(struct odb_source_packed *store,
enum odb_count_objects_flags flags UNUSED,
unsigned long *out)
{
@@ -2160,7 +2160,7 @@ static int fill_pack_entry(const struct object_id *oid,
return 1;
}
-static int find_pack_entry(struct packfile_store *store,
+static int find_pack_entry(struct odb_source_packed *store,
const struct object_id *oid,
struct pack_entry *e)
{
@@ -2183,7 +2183,7 @@ static int find_pack_entry(struct packfile_store *store,
return 0;
}
-int packfile_store_freshen_object(struct packfile_store *store,
+int packfile_store_freshen_object(struct odb_source_packed *store,
const struct object_id *oid)
{
struct pack_entry e;
@@ -2199,7 +2199,7 @@ int packfile_store_freshen_object(struct packfile_store *store,
return 1;
}
-int packfile_store_read_object_info(struct packfile_store *store,
+int packfile_store_read_object_info(struct odb_source_packed *store,
const struct object_id *oid,
struct object_info *oi,
enum object_info_flags flags)
@@ -2234,7 +2234,7 @@ int packfile_store_read_object_info(struct packfile_store *store,
return 0;
}
-static void maybe_invalidate_kept_pack_cache(struct packfile_store *store,
+static void maybe_invalidate_kept_pack_cache(struct odb_source_packed *store,
unsigned flags)
{
if (!store->kept_cache.packs)
@@ -2245,7 +2245,7 @@ static void maybe_invalidate_kept_pack_cache(struct packfile_store *store,
store->kept_cache.flags = 0;
}
-struct packed_git **packfile_store_get_kept_pack_cache(struct packfile_store *store,
+struct packed_git **packfile_store_get_kept_pack_cache(struct odb_source_packed *store,
unsigned flags)
{
maybe_invalidate_kept_pack_cache(store, flags);
@@ -2365,8 +2365,8 @@ int for_each_object_in_pack(struct packed_git *p,
return r;
}
-struct packfile_store_for_each_object_wrapper_data {
- struct packfile_store *store;
+struct odb_source_packed_for_each_object_wrapper_data {
+ struct odb_source_packed *store;
const struct object_info *request;
odb_for_each_object_cb cb;
void *cb_data;
@@ -2377,7 +2377,7 @@ static int packfile_store_for_each_object_wrapper(const struct object_id *oid,
uint32_t index_pos,
void *cb_data)
{
- struct packfile_store_for_each_object_wrapper_data *data = cb_data;
+ struct odb_source_packed_for_each_object_wrapper_data *data = cb_data;
if (data->request) {
off_t offset = nth_packed_object_offset(pack, index_pos);
@@ -2411,10 +2411,10 @@ static int match_hash(unsigned len, const unsigned char *a, const unsigned char
}
static int for_each_prefixed_object_in_midx(
- struct packfile_store *store,
+ struct odb_source_packed *store,
struct multi_pack_index *m,
const struct odb_for_each_object_options *opts,
- struct packfile_store_for_each_object_wrapper_data *data)
+ struct odb_source_packed_for_each_object_wrapper_data *data)
{
int ret;
@@ -2470,10 +2470,10 @@ static int for_each_prefixed_object_in_midx(
}
static int for_each_prefixed_object_in_pack(
- struct packfile_store *store,
+ struct odb_source_packed *store,
struct packed_git *p,
const struct odb_for_each_object_options *opts,
- struct packfile_store_for_each_object_wrapper_data *data)
+ struct odb_source_packed_for_each_object_wrapper_data *data)
{
uint32_t num, i, first = 0;
int len = opts->prefix_hex_len > p->repo->hash_algo->hexsz ?
@@ -2519,9 +2519,9 @@ static int for_each_prefixed_object_in_pack(
}
static int packfile_store_for_each_prefixed_object(
- struct packfile_store *store,
+ struct odb_source_packed *store,
const struct odb_for_each_object_options *opts,
- struct packfile_store_for_each_object_wrapper_data *data)
+ struct odb_source_packed_for_each_object_wrapper_data *data)
{
struct packfile_list_entry *e;
struct multi_pack_index *m;
@@ -2566,13 +2566,13 @@ static int packfile_store_for_each_prefixed_object(
return ret;
}
-int packfile_store_for_each_object(struct packfile_store *store,
+int packfile_store_for_each_object(struct odb_source_packed *store,
const struct object_info *request,
odb_for_each_object_cb cb,
void *cb_data,
const struct odb_for_each_object_options *opts)
{
- struct packfile_store_for_each_object_wrapper_data data = {
+ struct odb_source_packed_for_each_object_wrapper_data data = {
.store = store,
.request = request,
.cb = cb,
@@ -2707,7 +2707,7 @@ static void find_abbrev_len_for_pack(struct packed_git *p,
*out = len;
}
-int packfile_store_find_abbrev_len(struct packfile_store *store,
+int packfile_store_find_abbrev_len(struct odb_source_packed *store,
const struct object_id *oid,
unsigned min_len,
unsigned *out)
@@ -2832,16 +2832,16 @@ int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *l
return 0;
}
-struct packfile_store *packfile_store_new(struct odb_source *source)
+struct odb_source_packed *packfile_store_new(struct odb_source *source)
{
- struct packfile_store *store;
+ struct odb_source_packed *store;
CALLOC_ARRAY(store, 1);
store->source = source;
strmap_init(&store->packs_by_path);
return store;
}
-void packfile_store_free(struct packfile_store *store)
+void packfile_store_free(struct odb_source_packed *store)
{
for (struct packfile_list_entry *e = store->packs.head; e; e = e->next)
free(e->pack);
@@ -2851,7 +2851,7 @@ void packfile_store_free(struct packfile_store *store)
free(store);
}
-void packfile_store_close(struct packfile_store *store)
+void packfile_store_close(struct odb_source_packed *store)
{
for (struct packfile_list_entry *e = store->packs.head; e; e = e->next) {
if (e->pack->do_not_close)
@@ -2988,7 +2988,7 @@ int packfile_read_object_stream(struct odb_read_stream **out,
}
int packfile_store_read_object_stream(struct odb_read_stream **out,
- struct packfile_store *store,
+ struct odb_source_packed *store,
const struct object_id *oid)
{
struct pack_entry e;
diff --git a/packfile.h b/packfile.h
index 49d6bdecf6..9cec15bc50 100644
--- a/packfile.h
+++ b/packfile.h
@@ -79,7 +79,7 @@ struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs,
/*
* A store that manages packfiles for a given object database.
*/
-struct packfile_store {
+struct odb_source_packed {
struct odb_source *source;
/*
@@ -138,19 +138,19 @@ struct packfile_store {
* Allocate and initialize a new empty packfile store for the given object
* database source.
*/
-struct packfile_store *packfile_store_new(struct odb_source *source);
+struct odb_source_packed *packfile_store_new(struct odb_source *source);
/*
* Free the packfile store and all its associated state. All packfiles
* tracked by the store will be closed.
*/
-void packfile_store_free(struct packfile_store *store);
+void packfile_store_free(struct odb_source_packed *store);
/*
* Close all packfiles associated with this store. The packfiles won't be
* free'd, so they can be re-opened at a later point in time.
*/
-void packfile_store_close(struct packfile_store *store);
+void packfile_store_close(struct odb_source_packed *store);
/*
* Prepare the packfile store by loading packfiles and multi-pack indices for
@@ -159,7 +159,7 @@ void packfile_store_close(struct packfile_store *store);
* It shouldn't typically be necessary to call this function directly, as
* functions that access the store know to prepare it.
*/
-void packfile_store_prepare(struct packfile_store *store);
+void packfile_store_prepare(struct odb_source_packed *store);
/*
* Clear the packfile caches and try to look up any new packfiles that have
@@ -167,20 +167,20 @@ void packfile_store_prepare(struct packfile_store *store);
*
* This function must be called under the `odb_read_lock()`.
*/
-void packfile_store_reprepare(struct packfile_store *store);
+void packfile_store_reprepare(struct odb_source_packed *store);
/*
* Add the pack to the store so that contained objects become accessible via
* the store. This moves ownership into the store.
*/
-void packfile_store_add_pack(struct packfile_store *store,
+void packfile_store_add_pack(struct odb_source_packed *store,
struct packed_git *pack);
/*
* Get all packs managed by the given store, including packfiles that are
* referenced by multi-pack indices.
*/
-struct packfile_list_entry *packfile_store_get_packs(struct packfile_store *store);
+struct packfile_list_entry *packfile_store_get_packs(struct odb_source_packed *store);
struct repo_for_each_pack_data {
struct odb_source *source;
@@ -239,7 +239,7 @@ static inline void repo_for_each_pack_data_next(struct repo_for_each_pack_data *
repo_for_each_pack_data_next(&eack_pack_data))
int packfile_store_read_object_stream(struct odb_read_stream **out,
- struct packfile_store *store,
+ struct odb_source_packed *store,
const struct object_id *oid);
/*
@@ -248,7 +248,7 @@ int packfile_store_read_object_stream(struct odb_read_stream **out,
* not found, 0 if it was and read successfully, and a negative error code in
* case the object was corrupted.
*/
-int packfile_store_read_object_info(struct packfile_store *store,
+int packfile_store_read_object_info(struct odb_source_packed *store,
const struct object_id *oid,
struct object_info *oi,
enum object_info_flags flags);
@@ -258,10 +258,10 @@ int packfile_store_read_object_info(struct packfile_store *store,
* either the newly opened packfile or the preexisting packfile. Returns a
* `NULL` pointer in case the packfile could not be opened.
*/
-struct packed_git *packfile_store_load_pack(struct packfile_store *store,
+struct packed_git *packfile_store_load_pack(struct odb_source_packed *store,
const char *idx_path, int local);
-int packfile_store_freshen_object(struct packfile_store *store,
+int packfile_store_freshen_object(struct odb_source_packed *store,
const struct object_id *oid);
enum kept_pack_type {
@@ -276,7 +276,7 @@ enum kept_pack_type {
*
* Return 0 on success, a negative error code otherwise.
*/
-int packfile_store_count_objects(struct packfile_store *store,
+int packfile_store_count_objects(struct odb_source_packed *store,
enum odb_count_objects_flags flags,
unsigned long *out);
@@ -285,7 +285,7 @@ int packfile_store_count_objects(struct packfile_store *store,
* combination of `kept_pack_type` flags. The cache is computed on demand and
* will be recomputed whenever the flags change.
*/
-struct packed_git **packfile_store_get_kept_pack_cache(struct packfile_store *store,
+struct packed_git **packfile_store_get_kept_pack_cache(struct odb_source_packed *store,
unsigned flags);
struct pack_window {
@@ -365,13 +365,13 @@ int for_each_object_in_pack(struct packed_git *p,
*
* The flags parameter is a combination of `odb_for_each_object_flags`.
*/
-int packfile_store_for_each_object(struct packfile_store *store,
+int packfile_store_for_each_object(struct odb_source_packed *store,
const struct object_info *request,
odb_for_each_object_cb cb,
void *cb_data,
const struct odb_for_each_object_options *opts);
-int packfile_store_find_abbrev_len(struct packfile_store *store,
+int packfile_store_find_abbrev_len(struct odb_source_packed *store,
const struct object_id *oid,
unsigned min_len,
unsigned *out);
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v2 00/17] odb: make packed object source a proper `struct odb_source`
From: Patrick Steinhardt @ 2026-06-09 8:50 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260604-pks-odb-source-packed-v1-0-2e7ab31b4b5c@pks.im>
Hi,
this patch series converts the "packed" source into a proper `struct
odb_source`. It's thus the equivalent to [1], which did the same thing
for the "loose" source.
This series here is unfortunately a bit bigger, mostly because I'm also
renaming `struct packfile_store` to `struct odb_source_packed`. Back
when I introduced the packfile store I didn't yet have the full vision
of how the final layout will look like, so I didn't have the foresight
yet to call it `struct odb_source_packed`. But now that the layout has
materialized I think it's sensible to adjust its naming to match all the
other sources that we have.
Also: I don't have anything else in the pipeline anymore that moves
around large pieces of our code in the vicinity of the object database.
So after this series got merged, subsequent changes should be of a more
incremental nature.
This series is built on top of 9ac3f193c0 (The 11th batch, 2026-06-02)
with ps/odb-source-loose at ef4778bcba (odb/source-loose: drop pointer
to the "files" source, 2026-06-01) merged into it.
Changes in v2:
- Split out `struct packfile_list` into a separate code unit to fix a
cyclic dependency between "packfile.h" and "odb/souurce-packed.h".
- Fix an extraneous newline.
- Link to v1: https://patch.msgid.link/20260604-pks-odb-source-packed-v1-0-2e7ab31b4b5c@pks.im
Thanks!
Patrick
[1]: <20260521-b4-pks-odb-source-loose-v1-0-6553b399be2d@pks.im>
---
Patrick Steinhardt (17):
packfile: rename `struct packfile_store` to `odb_source_packed`
packfile: split out packfile list logic
packfile: move packed source into "odb/" subsystem
odb/source-packed: store pointer to "files" instead of generic source
odb/source-packed: start converting to a proper `struct odb_source`
odb/source-packed: wire up `close()` callback
odb/source-packed: wire up `reprepare()` callback
packfile: use higher-level interface to implement `has_object_pack()`
odb/source-packed: wire up `read_object_info()` callback
odb/source-packed: wire up `read_object_stream()` callback
odb/source-packed: wire up `for_each_object()` callback
odb/source-packed: wire up `count_objects()` callback
odb/source-packed: wire up `find_abbrev_len()` callback
odb/source-packed: wire up `freshen_object()` callback
odb/source-packed: stub out remaining functions
midx: refactor interfaces to work on "packed" source
odb/source-packed: drop pointer to "files" parent source
Makefile | 2 +
builtin/cat-file.c | 4 +-
builtin/grep.c | 2 +-
builtin/multi-pack-index.c | 29 +-
builtin/pack-objects.c | 7 +-
builtin/repack.c | 8 +-
commit-graph.c | 4 +-
meson.build | 2 +
midx-write.c | 34 +-
midx.c | 118 +++----
midx.h | 30 +-
odb/source-files.c | 20 +-
odb/source-files.h | 4 +-
odb/source-packed.c | 764 +++++++++++++++++++++++++++++++++++++++++++
odb/source-packed.h | 94 ++++++
odb/source.h | 3 +
pack-bitmap.c | 8 +-
pack-revindex.c | 6 +-
packfile-list.c | 86 +++++
packfile-list.h | 28 ++
packfile.c | 784 +--------------------------------------------
packfile.h | 180 +----------
repack-geometry.c | 3 +-
repack-midx.c | 9 +-
repack.c | 6 +-
t/helper/test-read-midx.c | 7 +-
26 files changed, 1163 insertions(+), 1079 deletions(-)
Range-diff versus v1:
1: 5fb0dcfef9 = 1: 8a5e5f5473 packfile: rename `struct packfile_store` to `odb_source_packed`
-: ---------- > 2: 179416a017 packfile: split out packfile list logic
2: a29ca59090 ! 3: 6e54f9f918 packfile: move packed source into "odb/" subsystem
@@ odb/source-packed.h (new)
+#define ODB_SOURCE_PACKED_H
+
+#include "odb/source.h"
++#include "packfile-list.h"
+#include "strmap.h"
+
-+struct packfile_list {
-+ struct packfile_list_entry *head, *tail;
-+};
-+
-+struct packfile_list_entry {
-+ struct packfile_list_entry *next;
-+ struct packed_git *pack;
-+};
-+
+/*
+ * A store that manages packfiles for a given object database.
+ */
@@ packfile.h
#include "odb/source-files.h"
+#include "odb/source-packed.h"
#include "oidset.h"
+ #include "packfile-list.h"
#include "repository.h"
-#include "strmap.h"
@@ packfile.h: struct packed_git {
char pack_name[FLEX_ARRAY]; /* more */
};
--struct packfile_list {
-- struct packfile_list_entry *head, *tail;
--};
--
--struct packfile_list_entry {
-- struct packfile_list_entry *next;
-- struct packed_git *pack;
--};
--
- void packfile_list_clear(struct packfile_list *list);
- void packfile_list_remove(struct packfile_list *list, struct packed_git *pack);
- void packfile_list_prepend(struct packfile_list *list, struct packed_git *pack);
-@@ packfile.h: void packfile_list_append(struct packfile_list *list, struct packed_git *pack);
- struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs,
- const struct object_id *oid);
-
-/*
- * A store that manages packfiles for a given object database.
- */
3: d0402d115b ! 4: 85ebdcb253 odb/source-packed: store pointer to "files" instead of generic source
@@ odb/source-packed.c
}
## odb/source-packed.h ##
-@@ odb/source-packed.h: struct packfile_list_entry {
+@@
* A store that manages packfiles for a given object database.
*/
struct odb_source_packed {
4: d92302b497 ! 5: 7d5605b9c7 odb/source-packed: start converting to a proper `struct odb_source`
@@ odb/source-packed.c
}
## odb/source-packed.h ##
-@@ odb/source-packed.h: struct packfile_list_entry {
+@@
* A store that manages packfiles for a given object database.
*/
struct odb_source_packed {
@@ packfile.c: int parse_pack_header_option(const char *in, unsigned char *out, uns
for (struct packfile_list_entry *e = store->packs.head; e; e = e->next) {
## packfile.h ##
-@@ packfile.h: void packfile_list_append(struct packfile_list *list, struct packed_git *pack);
- struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs,
- const struct object_id *oid);
+@@ packfile.h: struct packed_git {
+ char pack_name[FLEX_ARRAY]; /* more */
+ };
-/*
- * Free the packfile store and all its associated state. All packfiles
5: 9a51dac274 ! 6: 736dc22977 odb/source-packed: wire up `close()` callback
@@ packfile.c: int parse_pack_header_option(const char *in, unsigned char *out, uns
struct packed_git *pack;
## packfile.h ##
-@@ packfile.h: void packfile_list_append(struct packfile_list *list, struct packed_git *pack);
- struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs,
- const struct object_id *oid);
+@@ packfile.h: struct packed_git {
+ char pack_name[FLEX_ARRAY]; /* more */
+ };
-/*
- * Close all packfiles associated with this store. The packfiles won't be
6: b9571c28f1 ! 7: f2ec21206d odb/source-packed: wire up `reprepare()` callback
@@ packfile.c: int packfile_store_read_object_info(struct odb_source_packed *store,
return 1;
## packfile.h ##
-@@ packfile.h: void packfile_list_append(struct packfile_list *list, struct packed_git *pack);
- struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs,
- const struct object_id *oid);
+@@ packfile.h: struct packed_git {
+ char pack_name[FLEX_ARRAY]; /* more */
+ };
-/*
- * Prepare the packfile store by loading packfiles and multi-pack indices for
7: 703181be7e = 8: a41dbcf7d9 packfile: use higher-level interface to implement `has_object_pack()`
8: f1f1cb2044 = 9: 3fcdfc2686 odb/source-packed: wire up `read_object_info()` callback
9: 91fad0e9ad = 10: 7b301ce67d odb/source-packed: wire up `read_object_stream()` callback
10: 0dc3c1b836 = 11: 5298ddece5 odb/source-packed: wire up `for_each_object()` callback
11: 461be09a17 ! 12: 929d7ad5af odb/source-packed: wire up `count_objects()` callback
@@ odb/source-packed.c: static int odb_source_packed_for_each_object(struct odb_sou
+out:
+ return ret;
+}
-+
+
void (*report_garbage)(unsigned seen_bits, const char *path);
12: c206556286 ! 13: 5de59e451d odb/source-packed: wire up `find_abbrev_len()` callback
@@ odb/source-packed.c: static int odb_source_packed_count_objects(struct odb_sourc
+ *out = min_len;
+ return 0;
+}
-
++
void (*report_garbage)(unsigned seen_bits, const char *path);
+ static void report_helper(const struct string_list *list,
@@ odb/source-packed.c: struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
packed->base.read_object_stream = odb_source_packed_read_object_stream;
packed->base.for_each_object = odb_source_packed_for_each_object;
13: 3ee5394ce8 = 14: 8e7b0f746c odb/source-packed: wire up `freshen_object()` callback
14: 875ca3572a = 15: 52967e0d24 odb/source-packed: stub out remaining functions
15: 5bd35384d6 = 16: 68cf451c52 midx: refactor interfaces to work on "packed" source
16: d1a7ce9f17 ! 17: fbed40d82a odb/source-packed: drop pointer to "files" parent source
@@ odb/source-packed.c: struct odb_source_packed *odb_source_packed_new(struct odb_
return packed;
## odb/source-packed.h ##
-@@ odb/source-packed.h: struct packfile_list_entry {
+@@
*/
struct odb_source_packed {
struct odb_source base;
---
base-commit: 06d49cec508464ced5d42541890ce5d749542a61
change-id: 20260602-pks-odb-source-packed-3826c352f059
^ permalink raw reply
* Re: [PATCH v4 0/8] Auto-configure advertised remotes via URL allowlist
From: Christian Couder @ 2026-06-09 8:30 UTC (permalink / raw)
To: Toon Claes
Cc: git, Junio C Hamano, Patrick Steinhardt, Taylor Blau,
Karthik Nayak, Elijah Newren, Kristoffer Haugsbakk
In-Reply-To: <87ik7s16sg.fsf@emacs.iotcl.com>
Hi Toon,
On Tue, Jun 9, 2026 at 10:01 AM Toon Claes <toon@iotcl.com> wrote:
>
> _(resend because it seems I accidentally didn't reply-all)_
>
> Christian Couder <christian.couder@gmail.com> writes:
>
> > Changes compared to v3
> > ======================
> >
> > Thanks to Toon, Kristoffer, Patrick and Junio for reviewing the
> > previous versions of this series and of the preparatory series.
> >
> > This has been rebased onto master @ 56a4f3c3a2 (The 8th batch,
> > 2026-05-25) to avoid a trivial conflict in "urlmatch.c".
> >
> > Only minor changes have been made since v3, in the following patches:
> >
> > - Patch 4/8 ("promisor-remote: add 'local_name' to 'struct
> > promisor_info'"):
> >
> > - The promisor_info_internal_name() function has been renamed
> > promisor_info_local_name() for clarity.
> >
> > - A `const char *local` local variable has been renamed
> > `remote_name` for consistency with another similar variable.
>
> I can really appreciate these two changes. Both make things more
> consistent and cleaner.
>
> > - Patch 6/8 ("promisor-remote: trust known remotes matching
> > acceptFromServerUrl"):
>
> I previously reviewed v2 and compared to that I like the changes you've
> made toward being clear about precedence. And this consistency carries
> through in PATCH 7/8.
>
> And thanks for mentioning username and password components are ignored
> intentionally.
Thanks.
> But I previously mentioned I felt the naming of 'acceptFromServer' and
> 'acceptFromServerUrl' are a bit confusing. So I'm wondering whether we
> can consider another proposal:
>
> What if 'acceptFromServer' would configure if 'acceptFromServerUrl'
> should be used? I mean, imagine we put this in the config:
>
> [promisor]
> acceptFromServer = Match
> acceptFromServerUrl = https://my-org.com/*
>
> (we can still argue over naming, but to get the idea)
>
> So the value "Match" for 'acceptFromServer' would inform Git to use
> 'acceptFromServerUrl'. This way precedence isn't a concern no more,
> because every value for 'acceptFromServer' is mutually exclusive.
In this case I would prefer to remove 'acceptFromServerUrl' entirely
and to make acceptFromServer accept values like:
match:https://my-org.com/*
By the way "match" might not be the best term. Maybe something like
"auto-configure" would be better.
> This has one downside though, you can no longer combine
> acceptFromServer=KnownUrl with a 'acceptFromServerUrl'. So URLs
> advertised by the server can no longer fall-through to
> 'acceptFromServer' if they don't match 'acceptFromServerUrl'. You can
> argue whether that's a good thing or not.
I think it's a good thing to have this fall-through. It allows setting
up things like this:
In the global config:
[promisor]
acceptFromServerUrl = https://my-org.com/*
In the config of only a few repo that need it:
[promisor]
acceptFromServer = knownUrl
This way remotes from my-org.com are accepted in all the repos, while
other remotes are accepted only if their name and URLs have already
been configured in the repos that need them.
This allows relatively lenient security for internal repos and more
strict security for external ones, and I suspect that many users will
want something like that.
What you suggest doesn't allow that. It could force users to choose
for each repo between either URL based allowlist or local
configuration of every remote.
Also I think it's easier to explain that 'acceptFromServerUrl' is a
different mechanism (that allows auto-configuration, contrary to
'acceptFromServer') if these two variables are independent.
> What do you think? If you disagree, I'm fine with the current approach
> and I think this version looks good.
Thanks for your review and for being fine with the current approach if
I disagree.
Best,
Christian.
^ permalink raw reply
* Re: [PATCH v4 0/8] Auto-configure advertised remotes via URL allowlist
From: Toon Claes @ 2026-06-09 8:01 UTC (permalink / raw)
To: Christian Couder, git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Kristoffer Haugsbakk, Christian Couder
In-Reply-To: <20260527140820.1438165-1-christian.couder@gmail.com>
_(resend because it seems I accidentally didn't reply-all)_
Christian Couder <christian.couder@gmail.com> writes:
> Changes compared to v3
> ======================
>
> Thanks to Toon, Kristoffer, Patrick and Junio for reviewing the
> previous versions of this series and of the preparatory series.
>
> This has been rebased onto master @ 56a4f3c3a2 (The 8th batch,
> 2026-05-25) to avoid a trivial conflict in "urlmatch.c".
>
> Only minor changes have been made since v3, in the following patches:
>
> - Patch 4/8 ("promisor-remote: add 'local_name' to 'struct
> promisor_info'"):
>
> - The promisor_info_internal_name() function has been renamed
> promisor_info_local_name() for clarity.
>
> - A `const char *local` local variable has been renamed
> `remote_name` for consistency with another similar variable.
I can really appreciate these two changes. Both make things more
consistent and cleaner.
> - Patch 6/8 ("promisor-remote: trust known remotes matching
> acceptFromServerUrl"):
I previously reviewed v2 and compared to that I like the changes you've
made toward being clear about precedence. And this consistency carries
through in PATCH 7/8.
And thanks for mentioning username and password components are ignored
intentionally.
But I previously mentioned I felt the naming of 'acceptFromServer' and
'acceptFromServerUrl' are a bit confusing. So I'm wondering whether we
can consider another proposal:
What if 'acceptFromServer' would configure if 'acceptFromServerUrl'
should be used? I mean, imagine we put this in the config:
[promisor]
acceptFromServer = Match
acceptFromServerUrl = https://my-org.com/*
(we can still argue over naming, but to get the idea)
So the value "Match" for 'acceptFromServer' would inform Git to use
'acceptFromServerUrl'. This way precedence isn't a concern no more,
because every value for 'acceptFromServer' is mutually exclusive.
This has one downside though, you can no longer combine
acceptFromServer=KnownUrl with a 'acceptFromServerUrl'. So URLs
advertised by the server can no longer fall-through to
'acceptFromServer' if they don't match 'acceptFromServerUrl'. You can
argue whether that's a good thing or not.
What do you think? If you disagree, I'm fine with the current approach
and I think this version looks good.
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH v13 2/6] branch: let delete_branches warn instead of error on bulk refusal
From: Harald Nordgren @ 2026-06-09 7:52 UTC (permalink / raw)
To: Junio C Hamano
Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk,
Johannes Sixt, Phillip Wood
In-Reply-To: <xmqq4ijcvb64.fsf@gitster.g>
> This breaks t5404, t5514, and t5505, which contradicts with
> "Existing callers are unaffected".
>
> What's going on? It is troubling that the breakage happens without
> even getting merged with other topics in-flight, which means that
> the environment you are developing in and testing on and the
> environment that I apply patches on, integrate and test (something
> based on Debian testing) are somehow behaving differently.
>
> "cd t && sh t5404-*.sh -i -v" ends like so:
>
> expecting success of 5404.7 'already deleted tracking branches ignored':
> git branch -d -r origin/b3 &&
> git push origin :b3 >output 2>&1 &&
> ! grep "^error: " output
>
> error: the branch 'origin/b3' is not fully merged
> hint: If you are sure you want to delete it, run 'git branch -D origin/b3'
> hint: Disable this message with "git config set advice.forceDeleteBranch false"
> not ok 7 - already deleted tracking branches ignored
> #
> # git branch -d -r origin/b3 &&
> # git push origin :b3 >output 2>&1 &&
> # ! grep "^error: " output
> #
> 1..7
>
> but it may be possible that earlier steps are behaving differently
> with the patches applied. I didn't dig further but I think the CI
> in the recent past have been affected by the same breakage.
Thanks for directing my attention to this.
The GitHub CI has been broken for some time, maybe I should have told
you about this earlier, but it coincided with a period where other
open source projects I worked on also had mass CI failures, so I
chalked it up to upstream issues (GitHub, Linux, etc). But it seems to
have not gone away.
All of my GitHub pull requests have broken tests (see e.g. which a
quite minimal change: https://github.com/git/git/pull/2313). This
makes it harder to detect actual issues. But of course it's not an
excuse.
Harald
^ permalink raw reply
* Re: [PATCH 00/16] odb: make packed object source a proper `struct odb_source`
From: Patrick Steinhardt @ 2026-06-09 7:27 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git
In-Reply-To: <CAOLa=ZT9PLFeVpyKph=jQOz_BHXhYgKO=-3SV_VP6p4oXLxZpg@mail.gmail.com>
On Mon, Jun 08, 2026 at 09:15:09AM -0700, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > Hi,
> >
> > this patch series converts the "packed" source into a proper `struct
> > odb_source`. It's thus the equivalent to [1], which did the same thing
> > for the "loose" source.
> >
> > This series here is unfortunately a bit bigger, mostly because I'm also
> > renaming `struct packfile_store` to `struct odb_source_packed`. Back
> > when I introduced the packfile store I didn't yet have the full vision
> > of how the final layout will look like, so I didn't have the foresight
> > yet to call it `struct odb_source_packed`. But now that the layout has
> > materialized I think it's sensible to adjust its naming to match all the
> > other sources that we have.
> >
> > Also: I don't have anything else in the pipeline anymore that moves
> > around large pieces of our code in the vicinity of the object database.
> > So after this series got merged, subsequent changes should be of a more
> > incremental nature.
> >
> > This series is built on top of 9ac3f193c0 (The 11th batch, 2026-06-02)
> > with ps/odb-source-loose at ef4778bcba (odb/source-loose: drop pointer
> > to the "files" source, 2026-06-01) merged into it.
>
> This was a good read. The commits towards the end are mostly simple code
> movements. Overall the series looks to be in good shape.
Thanks for your review! Will send a new version later today.
Patrick
^ permalink raw reply
* Re: [PATCH 02/16] packfile: move packed source into "odb/" subsystem
From: Patrick Steinhardt @ 2026-06-09 7:27 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git
In-Reply-To: <CAOLa=ZQ8K53yyopSOp4_Gc-Gpq6ULA0xW6gH5OCWdWNHEyRysw@mail.gmail.com>
On Mon, Jun 08, 2026 at 08:09:06AM -0700, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/odb/source-packed.h b/odb/source-packed.h
> > new file mode 100644
> > index 0000000000..c17068a4f1
> > --- /dev/null
> > +++ b/odb/source-packed.h
> > @@ -0,0 +1,80 @@
> > +#ifndef ODB_SOURCE_PACKED_H
> > +#define ODB_SOURCE_PACKED_H
> > +
> > +#include "odb/source.h"
> > +#include "strmap.h"
> > +
> > +struct packfile_list {
> > + struct packfile_list_entry *head, *tail;
> > +};
> > +
> > +struct packfile_list_entry {
> > + struct packfile_list_entry *next;
> > + struct packed_git *pack;
> > +};
> > +
>
> So this is exposed, because outside of the odb, we also use packfiles in
> the transport layer. That makes me wonder if these two structures are
> better kept alonsigde `struct packed_git` in 'packfile.h'.
Yeah, this is quite awkward indeed as the struct and function
declarations are now split up across "packfile.h" and
"odb/source-packed.h". The reason though is that there's a cyclic
dependency between the two headers, so we have to move the code around.
Arguably though, the better fix would be to move it into a standalone
file "packfile-list.{c,h}". Will adapt the code accordingly.
Patrick
^ permalink raw reply
* Re: [PATCH 11/16] odb/source-packed: wire up `count_objects()` callback
From: Patrick Steinhardt @ 2026-06-09 7:27 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git
In-Reply-To: <CAOLa=ZQdGMo83KggkmeeKYMR475TFqLn=o-nJz4QEUX2njgaOA@mail.gmail.com>
On Mon, Jun 08, 2026 at 09:12:06AM -0700, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> [snip]
>
> > diff --git a/odb/source-packed.c b/odb/source-packed.c
> > index a61c809c8c..013d8a50f8 100644
> > --- a/odb/source-packed.c
> > +++ b/odb/source-packed.c
> > @@ -338,6 +338,39 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
> > return ret;
> > }
> >
> > +static int odb_source_packed_count_objects(struct odb_source *source,
> > + enum odb_count_objects_flags flags UNUSED,
> > + unsigned long *out)
> > +{
> > + struct odb_source_packed *packed = odb_source_packed_downcast(source);
> > + struct packfile_list_entry *e;
> > + struct multi_pack_index *m;
> > + unsigned long count = 0;
> > + int ret;
> > +
> > + m = get_multi_pack_index(&packed->files->base);
> > + if (m)
> > + count += m->num_objects + m->num_objects_in_base;
> > +
> > + for (e = packfile_store_get_packs(packed); e; e = e->next) {
> > + if (e->pack->multi_pack_index)
> > + continue;
> > + if (open_pack_index(e->pack)) {
> > + ret = -1;
> > + goto out;
> > + }
> > +
> > + count += e->pack->num_objects;
> > + }
> > +
> > + *out = count;
> > + ret = 0;
> > +
> > +out:
> > + return ret;
> > +}
> > +
> > +
>
> Nit: extra newline.
Good catch, fixed locally.
Patrick
^ permalink raw reply
* Re: [PATCH 04/16] odb/source-packed: start converting to a proper `struct odb_source`
From: Patrick Steinhardt @ 2026-06-09 7:27 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git
In-Reply-To: <CAOLa=ZQst6ucwvtVOfXC6g1ZcP9_UZAwRyAXfQdjL7WcJ6ZzxQ@mail.gmail.com>
On Mon, Jun 08, 2026 at 08:29:04AM -0700, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/odb/source-packed.c b/odb/source-packed.c
> > index 12e785be48..f81a990cbd 100644
> > --- a/odb/source-packed.c
> > +++ b/odb/source-packed.c
> > + CALLOC_ARRAY(packed, 1);
> > + odb_source_init(&packed->base, parent->base.odb, ODB_SOURCE_PACKED,
> > + parent->base.path, parent->base.local);
> > + packed->files = parent;
> > + strmap_init(&packed->packs_by_path);
> > +
> > + packed->base.free = odb_source_packed_free;
> > +
> > + if (!is_absolute_path(parent->base.path))
> > + chdir_notify_register(NULL, odb_source_packed_reparent, packed);
> > +
>
> Tangent: seems like no one sets the 'name' field within
> `chdir_notify_register()`. It is meant for tracing purposes, but if no
> one is using it, we might as well remove it...? Perhaps #leftoverbits
There are some callers: `chdir_notify_reparent()` calls
`chdir_notify_register()` with a name, and the reference backends call
that function with names.
Ultimately though I think that this infrastructure is somewhat misguided
overall: we use this to update relative paths after chdir(3p), but if we
stored absolute paths in the first place then we wouldn't have to care
about the paths changing at all.
I plan to revisit this infra for the object database going forward: we
expose and use `struct odb_source::path` in various other subsystems,
including user-facing ones. This is inherently wrong though, as there
may be sources that don't even have an on-disk path. So there's a need
to drop that field and make it an internal implementation detail of the
source's backend. And once we've done that, we can just as well start to
store absolute paths.
For the reference backends we can already do that refactoring now-ish.
I'll send a patch series later today.
Patrick
^ permalink raw reply
* Re: [PATCH] docs: fix typos
From: Kristoffer Haugsbakk @ 2026-06-09 6:34 UTC (permalink / raw)
To: Tuomas Ahola, git
In-Reply-To: <20260604131457.19215-1-taahol@utu.fi>
On Thu, Jun 4, 2026, at 15:14, Tuomas Ahola wrote:
> Fix some typos and grammar errors in comments and documentation files.
>
> Signed-off-by: Tuomas Ahola <taahol@utu.fi>
> ---
>
> Notes:
> Written mostly as an exercise on how to submit patches that depend
> on other topics.
I’ve been thinking of how to handle typos for a few days now. ;) The
following does not apply to this submission since the maintainer said
that he will apply it.
Anyway, it struck me that you might sometimes want to apply the typofix
on top of the original branch *if* the branch is scheduled to be merged
to more than just `master`.
So e.g. this does *not* apply to topic kh/name-rev-custom-format since
that topic is not scheduled for a maintenance branch (`maint`). But:
>
> $ git log --oneline --first-parent v2.54.0..
> d19e9182ab (HEAD -> ta/typofixes) docs: fix typos
> 5a7e9cc03d Merge branch 'ta/approxidate-noon-fix'
Your topic is. See `RelNotes`:
(merge b809304101 ta/approxidate-noon-fix later to maint).
So it might make sense in such cases to post a patch to
be applied on top of the topic.
Just a thought for later.
> f03649d802 Merge branch 'kh/name-rev-custom-format'
> 023a226b4b Merge branch 'jc/neuter-sideband-fixup'
>
> As can be seen, these topics have already graduated to master:
>
> $ git cherry master
> + d19e9182ab097a722e32d459a9a58c8985831e3b
>[snip]
^ permalink raw reply
* Re: [PATCH GSoC RFC v12 04/12] t1006: split test utility functions into new "lib-cat-file.sh"
From: Chandra Pratap @ 2026-06-09 6:28 UTC (permalink / raw)
To: Pablo Sabater
Cc: eric.peijian, calvinwan, chriscool, git, jltobler, jonathantanmy,
karthik.188, toon
In-Reply-To: <20260608-ps-eric-work-rebase-v12-4-5338b766e658@gmail.com>
On Mon, 8 Jun 2026 at 15:44, Pablo Sabater <pabloosabaterr@gmail.com> wrote:
>
> From: Eric Ju <eric.peijian@gmail.com>
>
> This refactor extracts utility functions from the cat-file's test
> script "t1006-cat-file.sh" into a new "lib-cat-file.sh" dedicated
> library file. The goal is to improve code reuse and readability,
> enabling future tests to leverage these utilities without duplicating
> code.
Hmm, seems like a premature change to me. Do any of the subsequent
commits require this refactor? Maybe the follow-up series that enables
%objecttype support needs it? Did someone request this change in v11's
feedback?
If any of those are true, I think it's worthwhile mentioning it here. That will
make it easier to determine whether this change is truly necessary.
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
> t/lib-cat-file.sh | 16 ++++++++++++++++
> t/t1006-cat-file.sh | 13 +------------
> 2 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/t/lib-cat-file.sh b/t/lib-cat-file.sh
> new file mode 100644
> index 0000000000..44af232d74
> --- /dev/null
> +++ b/t/lib-cat-file.sh
> @@ -0,0 +1,16 @@
> +# Library of git-cat-file related test functions.
> +
> +# Print a string without a trailing newline.
> +echo_without_newline () {
> + printf '%s' "$*"
> +}
> +
> +# Print a string without newlines and replace them with a NULL character (\0).
> +echo_without_newline_nul () {
> + echo_without_newline "$@" | tr '\n' '\0'
> +}
> +
> +# Calculate the length of a string.
> +strlen () {
> + echo_without_newline "$1" | wc -c | sed -e 's/^ *//'
> +}
> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
> index 8e2c52652c..8360f3bbd9 100755
> --- a/t/t1006-cat-file.sh
> +++ b/t/t1006-cat-file.sh
> @@ -4,6 +4,7 @@ test_description='git cat-file'
>
> . ./test-lib.sh
> . "$TEST_DIRECTORY/lib-loose.sh"
> +. "$TEST_DIRECTORY"/lib-cat-file.sh
>
> test_cmdmode_usage () {
> test_expect_code 129 "$@" 2>err &&
> @@ -99,18 +100,6 @@ do
> '
> done
>
> -echo_without_newline () {
> - printf '%s' "$*"
> -}
> -
> -echo_without_newline_nul () {
> - echo_without_newline "$@" | tr '\n' '\0'
> -}
> -
> -strlen () {
> - echo_without_newline "$1" | wc -c | sed -e 's/^ *//'
> -}
> -
> run_tests () {
> type=$1
> object_name="$2"
>
> --
> 2.54.0
^ permalink raw reply
* [GSoC Blog] Week 1&2 : Improve Disk Space Recovery for Partial Clones
From: Siddharth Shrimali @ 2026-06-09 6:24 UTC (permalink / raw)
To: git; +Cc: Christian Couder, Siddharth Asthana, Siddharth Shrimali
Hello everyone,
I have published my GSoC blog about my recent progress
on the project.
I apologize for the delay in sharing the Week 1 update on time.
I am sharing it alongside Week 2's progress.
- Week 1 Update: https://siddharth.shrimali.info/#post/4
- Week 2 Update: https://siddharth.shrimali.info/#post/5
Please feel free to review my work and share your feedback.
Always open to discussions! ;)
Regards,
Siddharth Shrimali
^ permalink raw reply
* Re: [PATCH GSoC RFC v12 02/12] git-compat-util: add strtoul_ul() with error handling
From: Chandra Pratap @ 2026-06-09 6:20 UTC (permalink / raw)
To: Pablo Sabater
Cc: eric.peijian, calvinwan, chriscool, git, jltobler, jonathantanmy,
karthik.188, toon
In-Reply-To: <20260608-ps-eric-work-rebase-v12-2-5338b766e658@gmail.com>
On Mon, 8 Jun 2026 at 15:44, Pablo Sabater <pabloosabaterr@gmail.com> wrote:
>
> From: Eric Ju <eric.peijian@gmail.com>
>
> We already have strtoul_ui() and similar functions that provide proper
> error handling using strtoul from the standard library. However,
> there isn't currently a variant that returns an unsigned long.
>
> This variant is needed in a subsequent commit.
>
> Add strtoul_ul() to address this gap, enabling the
Nit: extra space here. Also, this could be conciser. Maybe something like:
"This variant is needed in a subsequent commit to enable returning an
unsigned long with proper error handling."
> return of an unsigned long with proper error handling.
>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
> git-compat-util.h | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 8809776407..4bf569f35c 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -975,6 +975,26 @@ static inline int strtoul_ui(char const *s, int base, unsigned int *result)
> return 0;
> }
>
> +/*
> + * Convert a string to an unsigned long using the standard library's strtoul,
> + * with additional error handling to ensure robustness.
> + */
> +static inline int strtoul_ul(char const *s, int base, unsigned long *result)
> +{
> + unsigned long ul;
> + char *p;
> +
> + errno = 0;
> + /* negative values would be accepted by strtoul */
> + if (strchr(s, '-'))
> + return -1;
> + ul = strtoul(s, &p, base);
> + if (errno || *p || p == s)
> + return -1;
> + *result = ul;
> + return 0;
> +}
> +
> static inline int strtol_i(char const *s, int base, int *result)
> {
> long ul;
>
> --
> 2.54.0
^ permalink raw reply
* Re: [PATCH 2/3] config: add GIT_CONFIG_INCLUDES
From: Patrick Steinhardt @ 2026-06-09 6:06 UTC (permalink / raw)
To: Derrick Stolee; +Cc: Derrick Stolee via GitGitGadget, git, gitster
In-Reply-To: <dd971b9e-2c13-4521-b991-b9bee1c5bf5b@gmail.com>
On Mon, Jun 08, 2026 at 03:38:55PM -0400, Derrick Stolee wrote:
> On 6/8/2026 10:34 AM, Patrick Steinhardt wrote:
> > On Mon, Jun 08, 2026 at 01:57:05PM +0000, Derrick Stolee via GitGitGadget wrote:
> > That raises the question whether we can introduce the configuration in a
> > way that it allows a bit more flexibility than just "yes"/"no", like for
> > example an allow-list of locations that should be evaluated. But maybe
> > I'm overthinking this.
> I see. So we can say "avoid including into the repository worktree" but
> that will probably be incomplete.
>
> There is room for nuance in future expansions, if we can find a creative
> way to handle that nuance. For now, I think I would still want an ability
> to turn the entire feature off, at least for certain tools that care.
Yup, that's fine with me. Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH] ref-filter: restore prefix-scoped iteration
From: Patrick Steinhardt @ 2026-06-09 6:05 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Junio C Hamano, git, Karthik Nayak, Victoria Dye, ZheNing Hu
In-Reply-To: <CAJ-ks9m9gq-=JB-gqeKaL4YOLSfrP2Cm0DytZjuC3OetG-UVbA@mail.gmail.com>
On Mon, Jun 08, 2026 at 06:39:48PM -0400, Tamir Duberstein wrote:
> On Mon, Jun 8, 2026 at 2:36 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Tamir Duberstein <tamird@gmail.com> writes:
> >
> > > diff --git a/ref-filter.c b/ref-filter.c
> > > index 1da4c0e60d..2388a57b39 100644
> > > --- a/ref-filter.c
> > > +++ b/ref-filter.c
> > > @@ -3315,19 +3315,31 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, refs_for
> > > prefix = "refs/tags/";
> > >
> > > if (prefix) {
> >
> > Below, adding an extra call to get_main_ref_store(the_repository)
> > makes one line unnecessarily split and harder to read. How about
> > doing
> >
> > struct ref_store *store = get_main_ref_store(the_repository);
> >
> > upfront here, and then use that to replace these two calls of
> > get_main_ref_store(the_repository)?
>
> Yep, done in v2.
>
> Thanks for the review!
>
> By the way, how long should I wait before sending new versions of my
> patches? I have 4 outstanding at the moment.
I typically aim to send at most one version per day per patch series.
This avoids that you're "flooding" the mailing list with too many
versions of the same series, allows you to address feedback from
multiple folks in batches, and it gives you enough time to think about
the feedback without having to rush anything.
Whether I actually do end up sending a series depends on a couple of
factors:
- How big is the series? The bigger it is the more time I give folks
to perform reviews.
- How substantial were the reviews you received? Is it just a couple
of small typos? Then it probably makes sense to wait one or two more
days to get some more involved reviews. Is it something that
requires signifciant rework? Then I'd send out soon so that others
don't review a patch series that will change significantly anyway.
- How close to being merged is the series? The closer it is the less
substantial the reviews will (hopefully) get, so it makes sense to
reroll a bit faster even if you only received minor feedback.
So there isn't really a golden rule to follow here, but a lot of this
depends on gut feeling. You probably won't have that feeling yet when
starting out in a new project, but that's fine. In case we see that
behaviour doesn't quite match the norm we'll typically give a hint that
the contributor should slow down or maybe send a new iteration.
Patrick
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox