* [PATCH 01/12] cache-tree: remove dependency on `the_repository`
2026-09-01 11:08 [PATCH 00/12] odb: stop registering in-memory sources Patrick Steinhardt
@ 2026-09-01 11:09 ` Patrick Steinhardt
2026-09-01 22:03 ` Junio C Hamano
2026-09-01 22:47 ` Junio C Hamano
2026-09-01 11:09 ` [PATCH 02/12] submodule-config: remove uses of `the_repository` Patrick Steinhardt
` (12 subsequent siblings)
13 siblings, 2 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-01 11:09 UTC (permalink / raw)
To: git
The "cache-tree" subsystem still depends on `the_repository`. Adapt it
to instead use repositories provided via the context, either as a new
parameter or the one passed in via `struct index_state`.
Besides getting rid of `the_repository`, this also removes the last
dependency on registering submodule sources with the main object
database. When reading gitmodules from a submodule's index we implicitly
read that object via `the_repository`'s object database, which is of
course wrong. This works though because we would then register the
submodule's object database with the main object database, but a later
patch is going to get rid of that mechanism.
You can verify that we indeed no longer depend on this mechanism by
running tests with `GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=true`. Without
this patch we fail in t1092, with this patch we never register submodule
object databases anymore.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/checkout.c | 2 +-
builtin/commit.c | 2 +-
cache-tree.c | 88 +++++++++++++++++++++++++++++-------------------------
cache-tree.h | 7 +++--
read-cache-ll.h | 5 ++--
read-cache.c | 9 +++---
sequencer.c | 2 +-
sparse-index.c | 2 +-
unpack-trees.c | 9 +++---
9 files changed, 68 insertions(+), 58 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 55e3a89a85..60d106f4d5 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -921,7 +921,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
}
}
- if (!cache_tree_fully_valid(the_repository->index->cache_tree))
+ if (!cache_tree_fully_valid(the_repository, the_repository->index->cache_tree))
cache_tree_update(the_repository->index, WRITE_TREE_SILENT | WRITE_TREE_REPAIR);
if (write_locked_index(the_repository->index, &lock_file, COMMIT_LOCK))
diff --git a/builtin/commit.c b/builtin/commit.c
index 28f6174503..02df1c2feb 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -484,7 +484,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
LOCK_DIE_ON_ERROR);
refresh_cache_or_die(refresh_flags);
if (the_repository->index->cache_changed
- || !cache_tree_fully_valid(the_repository->index->cache_tree))
+ || !cache_tree_fully_valid(the_repository, the_repository->index->cache_tree))
cache_tree_update(the_repository->index, WRITE_TREE_SILENT);
if (write_locked_index(the_repository->index, &index_lock,
COMMIT_LOCK | SKIP_IF_UNCHANGED))
diff --git a/cache-tree.c b/cache-tree.c
index a220372a42..6e4d82f3e8 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -275,28 +274,30 @@ static void discard_unused_subtrees(struct cache_tree *it)
}
}
-int cache_tree_fully_valid(struct cache_tree *it)
+int cache_tree_fully_valid(struct repository *repo, struct cache_tree *it)
{
int i;
if (!it)
return 0;
if (it->entry_count < 0 ||
- !odb_has_object(the_repository->objects, &it->oid,
+ !odb_has_object(repo->objects, &it->oid,
ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR))
return 0;
for (i = 0; i < it->subtree_nr; i++) {
- if (!cache_tree_fully_valid(it->down[i]->cache_tree))
+ if (!cache_tree_fully_valid(repo, it->down[i]->cache_tree))
return 0;
}
return 1;
}
-static int must_check_existence(const struct cache_entry *ce)
+static int must_check_existence(const struct cache_entry *ce, void *cb_data)
{
- return !(repo_has_promisor_remote(the_repository) && ce_skip_worktree(ce));
+ struct repository *repo = cb_data;
+ return !(repo_has_promisor_remote(repo) && ce_skip_worktree(ce));
}
-static int update_one(struct cache_tree *it,
+static int update_one(struct repository *repo,
+ struct cache_tree *it,
struct cache_entry **cache,
int entries,
const char *base,
@@ -334,7 +335,7 @@ static int update_one(struct cache_tree *it,
}
if (0 <= it->entry_count &&
- odb_has_object(the_repository->objects, &it->oid,
+ odb_has_object(repo->objects, &it->oid,
ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR))
return it->entry_count;
@@ -375,7 +376,8 @@ static int update_one(struct cache_tree *it,
sub = find_subtree(it, path + baselen, sublen, 1);
if (!sub->cache_tree)
sub->cache_tree = cache_tree();
- subcnt = update_one(sub->cache_tree,
+ subcnt = update_one(repo,
+ sub->cache_tree,
cache + i, entries - i,
path,
baselen + sublen + 1,
@@ -439,10 +441,10 @@ static int update_one(struct cache_tree *it,
}
ce_missing_ok = mode == S_IFGITLINK || missing_ok ||
- !must_check_existence(ce);
+ !must_check_existence(ce, repo);
if (is_null_oid(oid) ||
(!ce_missing_ok &&
- !odb_has_object(the_repository->objects, oid,
+ !odb_has_object(repo->objects, oid,
ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR))) {
strbuf_release(&buffer);
if (expected_missing)
@@ -474,12 +476,12 @@ static int update_one(struct cache_tree *it,
/*
* "sub" can be an empty tree if all subentries are i-t-a.
*/
- if (contains_ita && is_empty_tree_oid(oid, the_repository->hash_algo))
+ if (contains_ita && is_empty_tree_oid(oid, repo->hash_algo))
continue;
strbuf_grow(&buffer, entlen + 100);
strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
- strbuf_add(&buffer, oid->hash, the_hash_algo->rawsz);
+ strbuf_add(&buffer, oid->hash, repo->hash_algo->rawsz);
#if DEBUG_CACHE_TREE
fprintf(stderr, "cache-tree update-one %o %.*s\n",
@@ -489,16 +491,16 @@ static int update_one(struct cache_tree *it,
if (repair) {
struct object_id oid;
- hash_object_file(the_hash_algo, buffer.buf, buffer.len,
+ hash_object_file(repo->hash_algo, buffer.buf, buffer.len,
OBJ_TREE, &oid);
- if (odb_has_object(the_repository->objects, &oid, ODB_HAS_OBJECT_RECHECK_PACKED))
+ if (odb_has_object(repo->objects, &oid, ODB_HAS_OBJECT_RECHECK_PACKED))
oidcpy(&it->oid, &oid);
else
to_invalidate = 1;
} else if (dryrun) {
- hash_object_file(the_hash_algo, buffer.buf, buffer.len,
+ hash_object_file(repo->hash_algo, buffer.buf, buffer.len,
OBJ_TREE, &it->oid);
- } else if (odb_write_object_ext(the_repository->objects, buffer.buf, buffer.len, OBJ_TREE,
+ } else if (odb_write_object_ext(repo->objects, buffer.buf, buffer.len, OBJ_TREE,
&it->oid, NULL, flags & WRITE_TREE_SILENT ? ODB_WRITE_OBJECT_SILENT : 0)) {
strbuf_release(&buffer);
return -1;
@@ -516,7 +518,7 @@ static int update_one(struct cache_tree *it,
int cache_tree_update(struct index_state *istate, int flags)
{
- int inflight = !!the_repository->objects->transaction;
+ int inflight = !!istate->repo->objects->transaction;
struct odb_transaction *transaction;
int skip, i;
@@ -528,14 +530,14 @@ int cache_tree_update(struct index_state *istate, int flags)
if (!istate->cache_tree)
istate->cache_tree = cache_tree();
- if (!(flags & WRITE_TREE_MISSING_OK) && repo_has_promisor_remote(the_repository))
- prefetch_cache_entries(istate, must_check_existence);
+ if (!(flags & WRITE_TREE_MISSING_OK) && repo_has_promisor_remote(istate->repo))
+ prefetch_cache_entries(istate, must_check_existence, istate->repo);
trace_performance_enter();
trace2_region_enter("cache_tree", "update", istate->repo);
if (!inflight)
- odb_transaction_begin_or_die(the_repository->objects, &transaction, 0);
- i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
+ odb_transaction_begin_or_die(istate->repo->objects, &transaction, 0);
+ i = update_one(istate->repo, istate->cache_tree, istate->cache, istate->cache_nr,
"", 0, &skip, flags);
if (!inflight)
odb_transaction_commit_and_finalize_or_die(transaction);
@@ -547,7 +549,8 @@ int cache_tree_update(struct index_state *istate, int flags)
return 0;
}
-static void write_one(struct strbuf *buffer, struct cache_tree *it,
+static void write_one(struct repository *repo,
+ struct strbuf *buffer, struct cache_tree *it,
const char *path, int pathlen)
{
int i;
@@ -573,7 +576,7 @@ static void write_one(struct strbuf *buffer, struct cache_tree *it,
#endif
if (0 <= it->entry_count) {
- strbuf_add(buffer, it->oid.hash, the_hash_algo->rawsz);
+ strbuf_add(buffer, it->oid.hash, repo->hash_algo->rawsz);
}
for (i = 0; i < it->subtree_nr; i++) {
struct cache_tree_sub *down = it->down[i];
@@ -583,15 +586,16 @@ static void write_one(struct strbuf *buffer, struct cache_tree *it,
prev->name, prev->namelen) <= 0)
die("fatal - unsorted cache subtree");
}
- write_one(buffer, down->cache_tree, down->name, down->namelen);
+ write_one(repo, buffer, down->cache_tree, down->name, down->namelen);
}
}
-void cache_tree_write(struct strbuf *sb, struct cache_tree *root)
+void cache_tree_write(struct repository *repo,
+ struct strbuf *sb, struct cache_tree *root)
{
- trace2_region_enter("cache_tree", "write", the_repository);
- write_one(sb, root, "", 0);
- trace2_region_leave("cache_tree", "write", the_repository);
+ trace2_region_enter("cache_tree", "write", repo);
+ write_one(repo, sb, root, "", 0);
+ trace2_region_leave("cache_tree", "write", repo);
}
static int parse_int(const char **ptr, unsigned long *len_p, int *out)
@@ -625,13 +629,14 @@ static int parse_int(const char **ptr, unsigned long *len_p, int *out)
return 0;
}
-static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
+static struct cache_tree *read_one(struct repository *repo,
+ const char **buffer, unsigned long *size_p)
{
const char *buf = *buffer;
unsigned long size = *size_p;
struct cache_tree *it;
int i, subtree_nr;
- const unsigned rawsz = the_hash_algo->rawsz;
+ const unsigned rawsz = repo->hash_algo->rawsz;
it = NULL;
/* skip name, but make sure name exists */
@@ -658,7 +663,7 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
if (size < rawsz)
goto free_return;
oidread(&it->oid, (const unsigned char *)buf,
- the_repository->hash_algo);
+ repo->hash_algo);
buf += rawsz;
size -= rawsz;
}
@@ -686,7 +691,7 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
struct cache_tree_sub *subtree;
const char *name = buf;
- sub = read_one(&buf, &size);
+ sub = read_one(repo, &buf, &size);
if (!sub)
goto free_return;
subtree = cache_tree_sub(it, name);
@@ -703,16 +708,17 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
return NULL;
}
-struct cache_tree *cache_tree_read(const char *buffer, unsigned long size)
+struct cache_tree *cache_tree_read(struct repository *repo,
+ const char *buffer, unsigned long size)
{
struct cache_tree *result;
if (buffer[0])
return NULL; /* not the whole tree */
- trace2_region_enter("cache_tree", "read", the_repository);
- result = read_one(&buffer, &size);
- trace2_region_leave("cache_tree", "read", the_repository);
+ trace2_region_enter("cache_tree", "read", repo);
+ result = read_one(repo, &buffer, &size);
+ trace2_region_leave("cache_tree", "read", repo);
return result;
}
@@ -775,7 +781,7 @@ struct tree *write_in_core_index_as_tree(struct repository *repo,
int was_valid, ret;
was_valid = index_state->cache_tree &&
- cache_tree_fully_valid(index_state->cache_tree);
+ cache_tree_fully_valid(repo, index_state->cache_tree);
ret = write_index_as_tree_internal(&o, index_state, was_valid, 0, NULL);
if (ret == WRITE_TREE_UNMERGED_INDEX) {
@@ -803,7 +809,7 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR);
entries = read_index_from(index_state, index_path,
- repo_get_git_dir(the_repository));
+ repo_get_git_dir(index_state->repo));
if (entries < 0) {
ret = WRITE_TREE_UNREADABLE_INDEX;
goto out;
@@ -811,7 +817,7 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
was_valid = !(flags & WRITE_TREE_IGNORE_CACHE_TREE) &&
index_state->cache_tree &&
- cache_tree_fully_valid(index_state->cache_tree);
+ cache_tree_fully_valid(index_state->repo, index_state->cache_tree);
ret = write_index_as_tree_internal(oid, index_state, was_valid, flags,
prefix);
@@ -859,7 +865,7 @@ static void prime_cache_tree_rec(struct repository *r,
struct cache_tree_sub *sub;
struct tree *subtree = lookup_tree(r, &entry.oid);
- if (repo_parse_tree(the_repository, subtree) < 0)
+ if (repo_parse_tree(r, subtree) < 0)
exit(128);
sub = cache_tree_sub(it, entry.path);
sub->cache_tree = cache_tree();
diff --git a/cache-tree.h b/cache-tree.h
index f8bddae523..5d49308de9 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -28,10 +28,11 @@ struct cache_tree_sub *cache_tree_sub(struct cache_tree *, const char *);
int cache_tree_subtree_pos(struct cache_tree *it, const char *path, int pathlen);
-void cache_tree_write(struct strbuf *, struct cache_tree *root);
-struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);
+void cache_tree_write(struct repository *repo, struct strbuf *, struct cache_tree *root);
+struct cache_tree *cache_tree_read(struct repository *repo,
+ const char *buffer, unsigned long size);
-int cache_tree_fully_valid(struct cache_tree *);
+int cache_tree_fully_valid(struct repository *, struct cache_tree *);
int cache_tree_update(struct index_state *, int);
int cache_tree_verify(struct repository *, struct index_state *);
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 8eb266cfd1..066dd8bc3b 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -269,9 +269,10 @@ void validate_cache_entries(const struct index_state *istate);
* the given predicate. This function should only be called if
* repo_has_promisor_remote() returns true.
*/
-typedef int (*must_prefetch_predicate)(const struct cache_entry *);
+typedef int (*must_prefetch_predicate)(const struct cache_entry *, void *cb_data);
void prefetch_cache_entries(const struct index_state *istate,
- must_prefetch_predicate must_prefetch);
+ must_prefetch_predicate must_prefetch,
+ void *cb_data);
/* Initialize and use the cache information */
struct lock_file;
diff --git a/read-cache.c b/read-cache.c
index 8044ff820b..e40f290bb3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1748,7 +1748,7 @@ static int read_index_extension(struct index_state *istate,
{
switch (CACHE_EXT(ext)) {
case CACHE_EXT_TREE:
- istate->cache_tree = cache_tree_read(data, sz);
+ istate->cache_tree = cache_tree_read(istate->repo, data, sz);
break;
case CACHE_EXT_RESOLVE_UNDO:
istate->resolve_undo = resolve_undo_read(data, sz, the_hash_algo);
@@ -3012,7 +3012,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
!drop_cache_tree && istate->cache_tree) {
strbuf_reset(&sb);
- cache_tree_write(&sb, istate->cache_tree);
+ cache_tree_write(istate->repo, &sb, istate->cache_tree);
err = write_index_ext_header(f, eoie_c, CACHE_EXT_TREE, sb.len) < 0;
hashwrite(f, sb.buf, sb.len);
if (err) {
@@ -3733,7 +3733,8 @@ static void write_ieot_extension(struct strbuf *sb, struct index_entry_offset_ta
}
void prefetch_cache_entries(const struct index_state *istate,
- must_prefetch_predicate must_prefetch)
+ must_prefetch_predicate must_prefetch,
+ void *cb_data)
{
int i;
struct oid_array to_fetch = OID_ARRAY_INIT;
@@ -3741,7 +3742,7 @@ void prefetch_cache_entries(const struct index_state *istate,
for (i = 0; i < istate->cache_nr; i++) {
struct cache_entry *ce = istate->cache[i];
- if (S_ISGITLINK(ce->ce_mode) || !must_prefetch(ce))
+ if (S_ISGITLINK(ce->ce_mode) || !must_prefetch(ce, cb_data))
continue;
if (!odb_read_object_info_extended(the_repository->objects,
&ce->oid, NULL,
diff --git a/sequencer.c b/sequencer.c
index 65afd100d9..97f4119bed 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -814,7 +814,7 @@ static int do_recursive_merge(struct repository *r,
static struct object_id *get_cache_tree_oid(struct index_state *istate)
{
- if (!cache_tree_fully_valid(istate->cache_tree))
+ if (!cache_tree_fully_valid(istate->repo, istate->cache_tree))
if (cache_tree_update(istate, 0)) {
error(_("unable to update cache tree"));
return NULL;
diff --git a/sparse-index.c b/sparse-index.c
index c1fa231a89..102a9dadeb 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -228,7 +228,7 @@ int convert_to_sparse(struct index_state *istate, int flags)
if (index_has_unmerged_entries(istate))
return 0;
- if (!cache_tree_fully_valid(istate->cache_tree)) {
+ if (!cache_tree_fully_valid(istate->repo, istate->cache_tree)) {
/* Clear and recompute the cache-tree */
cache_tree_free(&istate->cache_tree);
diff --git a/unpack-trees.c b/unpack-trees.c
index 154d6d40a1..20e6d6d824 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -416,7 +416,8 @@ static void report_collided_checkout(struct index_state *index)
string_list_clear(&list, 0);
}
-static int must_checkout(const struct cache_entry *ce)
+static int must_checkout(const struct cache_entry *ce,
+ void *cb_data UNUSED)
{
return ce->ce_flags & CE_UPDATE;
}
@@ -477,7 +478,7 @@ static int check_updates(struct unpack_trees_options *o,
* Prefetch the objects that are to be checked out in the loop
* below.
*/
- prefetch_cache_entries(index, must_checkout);
+ prefetch_cache_entries(index, must_checkout, NULL);
get_parallel_checkout_configs(&pc_workers, &pc_threshold);
@@ -487,7 +488,7 @@ static int check_updates(struct unpack_trees_options *o,
for (i = 0; i < index->cache_nr; i++) {
struct cache_entry *ce = index->cache[i];
- if (must_checkout(ce)) {
+ if (must_checkout(ce, NULL)) {
size_t last_pc_queue_size = pc_queue_size();
if (ce->ce_flags & CE_WT_REMOVE)
@@ -2086,7 +2087,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
}
if (!o->skip_cache_tree_update &&
- !cache_tree_fully_valid(o->internal.result.cache_tree))
+ !cache_tree_fully_valid(the_repository, o->internal.result.cache_tree))
cache_tree_update(&o->internal.result,
WRITE_TREE_SILENT |
WRITE_TREE_REPAIR);
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH 01/12] cache-tree: remove dependency on `the_repository`
2026-09-01 11:09 ` [PATCH 01/12] cache-tree: remove dependency on `the_repository` Patrick Steinhardt
@ 2026-09-01 22:03 ` Junio C Hamano
2026-09-02 10:23 ` Patrick Steinhardt
2026-09-01 22:47 ` Junio C Hamano
1 sibling, 1 reply; 54+ messages in thread
From: Junio C Hamano @ 2026-09-01 22:03 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
Patrick Steinhardt <ps@pks.im> writes:
> The "cache-tree" subsystem still depends on `the_repository`. Adapt it
> to instead use repositories provided via the context, either as a new
> parameter or the one passed in via `struct index_state`.
>
> Besides getting rid of `the_repository`, this also removes the last
> dependency on registering submodule sources with the main object
> database. When reading gitmodules from a submodule's index we implicitly
> read that object via `the_repository`'s object database, which is of
> course wrong. This works though because we would then register the
> submodule's object database with the main object database, but a later
> patch is going to get rid of that mechanism.
As "the_repository, " is quite a long string, many lines bust our
usual ~70 column limit, but other than that the changes look quite
straight-forward.
We probably should make the cache_tree_fully_valid() into file scope
static. Your new cache_tree_fully_valid() that takes the repository
and a cache_tree, which will be file-scope static, will gain a new
thin wrapper around it, i.e.,
int has_valid_cache_tree(struct index_state *istate)
{
return cache_tree_fully_valid(istate->repo, istate->cache_tree);
}
Migrate the callers outside cache_tree.c file to use it; outside the
implementation, nobody has any business talking about a subdirectory
of any cache-tree, or any cache-tree outside the context of the istate
it belongs to.
Other than that, looking good.
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PATCH 01/12] cache-tree: remove dependency on `the_repository`
2026-09-01 22:03 ` Junio C Hamano
@ 2026-09-02 10:23 ` Patrick Steinhardt
0 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-02 10:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Tue, Sep 01, 2026 at 03:03:44PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > The "cache-tree" subsystem still depends on `the_repository`. Adapt it
> > to instead use repositories provided via the context, either as a new
> > parameter or the one passed in via `struct index_state`.
> >
> > Besides getting rid of `the_repository`, this also removes the last
> > dependency on registering submodule sources with the main object
> > database. When reading gitmodules from a submodule's index we implicitly
> > read that object via `the_repository`'s object database, which is of
> > course wrong. This works though because we would then register the
> > submodule's object database with the main object database, but a later
> > patch is going to get rid of that mechanism.
>
> As "the_repository, " is quite a long string, many lines bust our
> usual ~70 column limit, but other than that the changes look quite
> straight-forward.
>
> We probably should make the cache_tree_fully_valid() into file scope
> static. Your new cache_tree_fully_valid() that takes the repository
> and a cache_tree, which will be file-scope static, will gain a new
> thin wrapper around it, i.e.,
>
> int has_valid_cache_tree(struct index_state *istate)
> {
> return cache_tree_fully_valid(istate->repo, istate->cache_tree);
> }
>
> Migrate the callers outside cache_tree.c file to use it; outside the
> implementation, nobody has any business talking about a subdirectory
> of any cache-tree, or any cache-tree outside the context of the istate
> it belongs to.
Yeah, I think that's a good idea indeed. But instead of introducing a
new `has_valid_cache_tree()`, I think it makes more sense to just adapt
the current function and introduce `cache_tree_fully_valid_recursive()`
as internal function instead. That also matches the other existing
interfaces like `cache_tree_fully_valid()` better.
Thanks!
Patrick
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 01/12] cache-tree: remove dependency on `the_repository`
2026-09-01 11:09 ` [PATCH 01/12] cache-tree: remove dependency on `the_repository` Patrick Steinhardt
2026-09-01 22:03 ` Junio C Hamano
@ 2026-09-01 22:47 ` Junio C Hamano
1 sibling, 0 replies; 54+ messages in thread
From: Junio C Hamano @ 2026-09-01 22:47 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
Patrick Steinhardt <ps@pks.im> writes:
> diff --git a/cache-tree.c b/cache-tree.c
> index a220372a42..6e4d82f3e8 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> ...
> @@ -775,7 +781,7 @@ struct tree *write_in_core_index_as_tree(struct repository *repo,
> int was_valid, ret;
>
> was_valid = index_state->cache_tree &&
> - cache_tree_fully_valid(index_state->cache_tree);
> + cache_tree_fully_valid(repo, index_state->cache_tree);
Here you use "repo". I do not know offhand if the two callers of
this function write_in_core_index_as_tree() are correct (they are in
builtin/history.c and not my code), but if they are, then "repo"
should match "index_state->repo". And if they aren't, this code is
broken.
Either way, passing index_state->repo would be the right thing to do
here. Perhaps write_in_core_index_as_tree() should be corrected to
drop its "repo" parameter.
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 154d6d40a1..20e6d6d824 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> ...
> @@ -2086,7 +2087,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
> }
>
> if (!o->skip_cache_tree_update &&
> - !cache_tree_fully_valid(o->internal.result.cache_tree))
> + !cache_tree_fully_valid(the_repository, o->internal.result.cache_tree))
The o->internal.result is a full fledged istate instance and comes
with its own repository. It may or may not be the same as
the_repository.
If this were a misconversion, it would argue strongly for a thin
wrapper I suggested in the other message, that only takes an istate
instance and infers which repository and which cache-tree to work
with.
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH 02/12] submodule-config: remove uses of `the_repository`
2026-09-01 11:08 [PATCH 00/12] odb: stop registering in-memory sources Patrick Steinhardt
2026-09-01 11:09 ` [PATCH 01/12] cache-tree: remove dependency on `the_repository` Patrick Steinhardt
@ 2026-09-01 11:09 ` Patrick Steinhardt
2026-09-01 11:09 ` [PATCH 03/12] submodule-config: stop using `the_hash_algo` Patrick Steinhardt
` (11 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-01 11:09 UTC (permalink / raw)
To: git
Several functions in the submodule-config subsystem implicitly depend
on `the_repository`. Refactor these to take a `struct repository` as
parameter and adapt callers accordingly.
Note that as usual with these refactorings, callers simply pass
`the_repository` even if they already have a different repository
available in the calling context. This simplifies the migration and
ensures that we don't have a change in behaviour.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/fetch.c | 2 +-
builtin/grep.c | 2 +-
builtin/submodule--helper.c | 8 ++++----
submodule-config.c | 49 ++++++++++++++++++++++++++-------------------
submodule-config.h | 12 +++++++----
submodule.c | 2 +-
t/helper/test-submodule.c | 4 ++--
7 files changed, 45 insertions(+), 34 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index ab7db2be06..533fdfe7d8 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -2681,7 +2681,7 @@ int cmd_fetch(int argc,
int *rs = config.recurse_submodules == RECURSE_SUBMODULES_DEFAULT
? &config.recurse_submodules : NULL;
- fetch_config_from_gitmodules(sfjc, rs);
+ fetch_config_from_gitmodules(the_repository, sfjc, rs);
}
diff --git a/builtin/grep.c b/builtin/grep.c
index d3d86abe01..073dfaaf45 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -897,7 +897,7 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,
if (recurse_submodules) {
submodule_free(opt->repo);
obj_read_lock();
- gitmodules_config_oid(&real_obj->oid);
+ gitmodules_config_oid(the_repository, &real_obj->oid);
obj_read_unlock();
}
if (grep_object(opt, pathspec, real_obj, list->objects[i].name,
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index e7cd3225fa..aaaa963fd8 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -3041,7 +3041,7 @@ static int module_update(int argc, const char **argv, const char *prefix,
NULL
};
- update_clone_config_from_gitmodules(&opt.max_jobs);
+ update_clone_config_from_gitmodules(the_repository, &opt.max_jobs);
repo_config(the_repository, git_update_clone_config, &opt.max_jobs);
argc = parse_options(argc, argv, prefix, module_update_options,
@@ -3255,7 +3255,7 @@ static int module_set_url(int argc, const char **argv, const char *prefix,
path);
config_name = xstrfmt("submodule.%s.url", sub->name);
- ret = config_set_in_gitmodules_file_gently(config_name, newurl);
+ ret = config_set_in_gitmodules_file_gently(the_repository, config_name, newurl);
if (!ret) {
repo_read_gitmodules(the_repository, 0);
@@ -3311,7 +3311,7 @@ static int module_set_branch(int argc, const char **argv, const char *prefix,
path);
config_name = xstrfmt("submodule.%s.branch", sub->name);
- ret = config_set_in_gitmodules_file_gently(config_name, opt_branch);
+ ret = config_set_in_gitmodules_file_gently(the_repository, config_name, opt_branch);
free(config_name);
return !!ret;
@@ -3510,7 +3510,7 @@ static int config_submodule_in_gitmodules(const char *name, const char *var, con
die(_("please make sure that the .gitmodules file is in the working tree"));
key = xstrfmt("submodule.%s.%s", name, var);
- ret = config_set_in_gitmodules_file_gently(key, value);
+ ret = config_set_in_gitmodules_file_gently(the_repository, key, value);
free(key);
return ret;
diff --git a/submodule-config.c b/submodule-config.c
index f75997402a..f8c2cf7a93 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -667,19 +667,20 @@ static int parse_config(const char *var, const char *value,
return ret;
}
-static int gitmodule_oid_from_commit(const struct object_id *treeish_name,
+static int gitmodule_oid_from_commit(struct repository *repo,
+ const struct object_id *treeish_name,
struct object_id *gitmodules_oid,
struct strbuf *rev)
{
int ret = 0;
if (is_null_oid(treeish_name)) {
- oidclr(gitmodules_oid, the_repository->hash_algo);
+ oidclr(gitmodules_oid, repo->hash_algo);
return 1;
}
strbuf_addf(rev, "%s:.gitmodules", oid_to_hex(treeish_name));
- if (repo_get_oid(the_repository, rev->buf, gitmodules_oid) >= 0)
+ if (repo_get_oid(repo, rev->buf, gitmodules_oid) >= 0)
ret = 1;
return ret;
@@ -689,9 +690,11 @@ static int gitmodule_oid_from_commit(const struct object_id *treeish_name,
* (key) with on-demand reading of the appropriate .gitmodules from
* revisions.
*/
-static const struct submodule *config_from(struct submodule_cache *cache,
- const struct object_id *treeish_name, const char *key,
- enum lookup_type lookup_type)
+static const struct submodule *config_from(struct repository *repo,
+ struct submodule_cache *cache,
+ const struct object_id *treeish_name,
+ const char *key,
+ enum lookup_type lookup_type)
{
struct strbuf rev = STRBUF_INIT;
size_t config_size;
@@ -718,7 +721,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
return entry->config;
}
- if (!gitmodule_oid_from_commit(treeish_name, &oid, &rev))
+ if (!gitmodule_oid_from_commit(repo, treeish_name, &oid, &rev))
goto out;
switch (lookup_type) {
@@ -732,7 +735,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
if (submodule)
goto out;
- config = odb_read_object(the_repository->objects, &oid,
+ config = odb_read_object(repo->objects, &oid,
&type, &config_size);
if (!config || type != OBJ_BLOB)
goto out;
@@ -843,21 +846,22 @@ void repo_read_gitmodules(struct repository *repo, int skip_if_read)
repo->submodule_cache->gitmodules_read = 1;
}
-void gitmodules_config_oid(const struct object_id *commit_oid)
+void gitmodules_config_oid(struct repository *repo,
+ const struct object_id *commit_oid)
{
struct strbuf rev = STRBUF_INIT;
struct object_id oid;
- submodule_cache_check_init(the_repository);
+ submodule_cache_check_init(repo);
- if (gitmodule_oid_from_commit(commit_oid, &oid, &rev)) {
+ if (gitmodule_oid_from_commit(repo, commit_oid, &oid, &rev)) {
git_config_from_blob_oid(gitmodules_cb, rev.buf,
- the_repository, &oid, the_repository,
+ repo, &oid, repo,
CONFIG_SCOPE_UNKNOWN);
}
strbuf_release(&rev);
- the_repository->submodule_cache->gitmodules_read = 1;
+ repo->submodule_cache->gitmodules_read = 1;
}
const struct submodule *submodule_from_name(struct repository *r,
@@ -865,7 +869,7 @@ const struct submodule *submodule_from_name(struct repository *r,
const char *name)
{
repo_read_gitmodules(r, 1);
- return config_from(r->submodule_cache, treeish_name, name, lookup_name);
+ return config_from(r, r->submodule_cache, treeish_name, name, lookup_name);
}
const struct submodule *submodule_from_path(struct repository *r,
@@ -873,7 +877,7 @@ const struct submodule *submodule_from_path(struct repository *r,
const char *path)
{
repo_read_gitmodules(r, 1);
- return config_from(r->submodule_cache, treeish_name, path, lookup_path);
+ return config_from(r, r->submodule_cache, treeish_name, path, lookup_path);
}
/**
@@ -980,11 +984,12 @@ int print_config_from_gitmodules(struct repository *repo, const char *key)
return 0;
}
-int config_set_in_gitmodules_file_gently(const char *key, const char *value)
+int config_set_in_gitmodules_file_gently(struct repository *repo,
+ const char *key, const char *value)
{
int ret;
- ret = repo_config_set_in_file_gently(the_repository, GITMODULES_FILE, key, NULL, value);
+ ret = repo_config_set_in_file_gently(repo, GITMODULES_FILE, key, NULL, value);
if (ret < 0)
/* Maybe the user already did that, don't error out here */
warning(_("Could not update .gitmodules entry %s"), key);
@@ -1017,13 +1022,15 @@ static int gitmodules_fetch_config(const char *var, const char *value,
return 0;
}
-void fetch_config_from_gitmodules(int *max_children, int *recurse_submodules)
+void fetch_config_from_gitmodules(struct repository *repo,
+ int *max_children,
+ int *recurse_submodules)
{
struct fetch_config config = {
.max_children = max_children,
.recurse_submodules = recurse_submodules
};
- config_from_gitmodules(gitmodules_fetch_config, the_repository, &config);
+ config_from_gitmodules(gitmodules_fetch_config, repo, &config);
}
static int gitmodules_update_clone_config(const char *var, const char *value,
@@ -1036,7 +1043,7 @@ static int gitmodules_update_clone_config(const char *var, const char *value,
return 0;
}
-void update_clone_config_from_gitmodules(int *max_jobs)
+void update_clone_config_from_gitmodules(struct repository *repo, int *max_jobs)
{
- config_from_gitmodules(gitmodules_update_clone_config, the_repository, max_jobs);
+ config_from_gitmodules(gitmodules_update_clone_config, repo, max_jobs);
}
diff --git a/submodule-config.h b/submodule-config.h
index f55d4e3b61..755570d5d1 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -57,7 +57,8 @@ int option_fetch_parse_recurse_submodules(const struct option *opt,
int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
int parse_push_recurse_submodules_arg(const char *opt, const char *arg);
void repo_read_gitmodules(struct repository *repo, int skip_if_read);
-void gitmodules_config_oid(const struct object_id *commit_oid);
+void gitmodules_config_oid(struct repository *repo,
+ const struct object_id *commit_oid);
/**
* Same as submodule_from_path but lookup by name.
@@ -80,7 +81,8 @@ const struct submodule *submodule_from_path(struct repository *r,
void submodule_free(struct repository *r);
int print_config_from_gitmodules(struct repository *repo, const char *key);
-int config_set_in_gitmodules_file_gently(const char *key, const char *value);
+int config_set_in_gitmodules_file_gently(struct repository *repo,
+ const char *key, const char *value);
/*
* Returns 0 if the name is syntactically acceptable as a submodule "name"
@@ -100,8 +102,10 @@ int check_submodule_url(const char *url);
* New helpers to retrieve arbitrary configuration from the '.gitmodules' file
* should NOT be added.
*/
-void fetch_config_from_gitmodules(int *max_children, int *recurse_submodules);
-void update_clone_config_from_gitmodules(int *max_jobs);
+void fetch_config_from_gitmodules(struct repository *repo,
+ int *max_children,
+ int *recurse_submodules);
+void update_clone_config_from_gitmodules(struct repository *repo, int *max_jobs);
/*
* Submodule entry that contains relevant information about a
diff --git a/submodule.c b/submodule.c
index 5c92575888..6fcb606f7e 100644
--- a/submodule.c
+++ b/submodule.c
@@ -133,7 +133,7 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)
strbuf_addstr(&entry, "submodule.");
strbuf_addstr(&entry, submodule->name);
strbuf_addstr(&entry, ".path");
- ret = config_set_in_gitmodules_file_gently(entry.buf, newpath);
+ ret = config_set_in_gitmodules_file_gently(the_repository, entry.buf, newpath);
strbuf_release(&entry);
return ret;
}
diff --git a/t/helper/test-submodule.c b/t/helper/test-submodule.c
index 3c5c4c4a09..ea9bef0904 100644
--- a/t/helper/test-submodule.c
+++ b/t/helper/test-submodule.c
@@ -168,7 +168,7 @@ static int cmd__submodule_config_set(int argc, const char **argv)
if (!is_writing_gitmodules_ok())
die("please make sure that the .gitmodules file is in the working tree");
- return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
+ return config_set_in_gitmodules_file_gently(the_repository, argv[1], argv[2]);
}
usage_with_options(usage, options);
}
@@ -188,7 +188,7 @@ static int cmd__submodule_config_unset(int argc, const char **argv)
if (argc == 2) {
if (!is_writing_gitmodules_ok())
die("please make sure that the .gitmodules file is in the working tree");
- return config_set_in_gitmodules_file_gently(argv[1], NULL);
+ return config_set_in_gitmodules_file_gently(the_repository, argv[1], NULL);
}
usage_with_options(usage, options);
}
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH 03/12] submodule-config: stop using `the_hash_algo`
2026-09-01 11:08 [PATCH 00/12] odb: stop registering in-memory sources Patrick Steinhardt
2026-09-01 11:09 ` [PATCH 01/12] cache-tree: remove dependency on `the_repository` Patrick Steinhardt
2026-09-01 11:09 ` [PATCH 02/12] submodule-config: remove uses of `the_repository` Patrick Steinhardt
@ 2026-09-01 11:09 ` Patrick Steinhardt
2026-09-01 11:09 ` [PATCH 04/12] submodule-config: stop registering submodule sources Patrick Steinhardt
` (10 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-01 11:09 UTC (permalink / raw)
To: git
We have two uses of `the_hash_algo` in "submodule-config.c":
- One trivial use in `gitmodules_cb`, which we can convert to use the
hash algorithm of the repository that's already available in the
caller's context.
- One use where we compute the hashmap key of an object ID. We should
only ever get valid, populated object IDs here, and consequently we
can easily adapt that function to use the hash algorithm of the
passed-in object ID.
Adapt both sites accordingly. Safeguard us against the case where the
passed-in object ID is _not_ properly initialized. While this case
shouldn't ever happen, it doesn't hurt to be defensive.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
submodule-config.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index f8c2cf7a93..7c73fa108b 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -133,7 +133,9 @@ void submodule_cache_free(struct submodule_cache *cache)
static unsigned int hash_oid_string(const struct object_id *oid,
const char *string)
{
- return memhash(oid->hash, the_hash_algo->rawsz) + strhash(string);
+ if (oid->algo == GIT_HASH_UNKNOWN)
+ BUG("hashing an object ID with unknown algorithm");
+ return memhash(oid->hash, hash_algos[oid->algo].rawsz) + strhash(string);
}
static void cache_put_path(struct submodule_cache *cache,
@@ -824,7 +826,7 @@ static int gitmodules_cb(const char *var, const char *value,
parameter.cache = repo->submodule_cache;
parameter.treeish_name = NULL;
- parameter.gitmodules_oid = null_oid(the_hash_algo);
+ parameter.gitmodules_oid = null_oid(repo->hash_algo);
parameter.overwrite = 1;
return parse_config(var, value, ctx, ¶meter);
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH 04/12] submodule-config: stop registering submodule sources
2026-09-01 11:08 [PATCH 00/12] odb: stop registering in-memory sources Patrick Steinhardt
` (2 preceding siblings ...)
2026-09-01 11:09 ` [PATCH 03/12] submodule-config: stop using `the_hash_algo` Patrick Steinhardt
@ 2026-09-01 11:09 ` Patrick Steinhardt
2026-09-01 11:09 ` [PATCH 05/12] builtin/grep: stop registering submodule ODB as source Patrick Steinhardt
` (9 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-01 11:09 UTC (permalink / raw)
To: git
When reading the ".gitmodules" file from a blob in a repository other
than `the_repository`, we register the repository's object database as
an in-memory source of `the_repository`'s object database. This call has
its origins in d9b8b8f896 (submodule-config.c: use repo_get_oid for
reading .gitmodules, 2019-04-16): back then, `config_with_options()` was
not able to read a blob from an arbitrary repository, but would always
read it via `the_repository`. So even though the blob could be resolved
in the submodule repository via `repo_get_oid()`, the submodule's object
database had to be registered as an in-memory source of `the_repository`
so that the subsequent object read was able to find the blob at all.
That need went away with e3e8bf046e (submodule-config: pass repo
upon blob config read, 2021-08-16), which taught the config machinery
to read the blob from the repository we pass to it. The same series
converted the eager submodule source registration into a lazy mechanism
that only registers submodule sources with the object database when an
object lookup failed. The intent though was that we don't ever have to
fall back to this mechanism in the first place, and to verify that this
is the case we introduced GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB. If set,
then any such lazy registration would cause us to BUG.
At the beginning of this series, we still triggered this bug in t1092.
But now that we have converted the "cache-tree" subsystem to not depend
on `the_repository` anymore it also knows to properly access objects via
the submodule. With that change, GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
does not cause any failures anymore.
Remove the call to `odb_add_submodule_source_by_path()`. This removes
the last user of `the_repository`, so at the same time we can also get
rid of `USE_THE_REPOSITORY_VARIABLE`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
submodule-config.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index 7c73fa108b..37c3be377b 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -803,9 +802,6 @@ static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void
} else if (repo_get_oid(repo, GITMODULES_INDEX, &oid) >= 0 ||
repo_get_oid(repo, GITMODULES_HEAD, &oid) >= 0) {
config_source.blob = oidstr = xstrdup(oid_to_hex(&oid));
- if (repo != the_repository)
- odb_add_submodule_source_by_path(the_repository->objects,
- repo->objects->sources->path);
} else {
goto out;
}
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH 05/12] builtin/grep: stop registering submodule ODB as source
2026-09-01 11:08 [PATCH 00/12] odb: stop registering in-memory sources Patrick Steinhardt
` (3 preceding siblings ...)
2026-09-01 11:09 ` [PATCH 04/12] submodule-config: stop registering submodule sources Patrick Steinhardt
@ 2026-09-01 11:09 ` Patrick Steinhardt
2026-09-01 11:09 ` [PATCH 06/12] odb: remove infrastructure to register submodule sources Patrick Steinhardt
` (8 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-01 11:09 UTC (permalink / raw)
To: git
Same as with the preceding commit, git-grep(1) registers each
submodule's object database as an in-memory source of the main object
database before grepping it. This was introduced as an eager alternate
registration and converted into the lazy mechanism via 8d33c3af0b (grep:
use submodule-ODB-as-alternate lazy-addition, 2021-08-16).
Starting with 0693806bf8 (grep: add repository to OID grep sources,
2021-08-16), the command instead knows to pass submodule repositories to
our workers, which means that those now use that repository to look up
objects, too. As a consequence, registering submodule sources as
alternates is not required anymore.
Remove the logic to register submodule sources. Unfortunately, this does
not allow us to get rid of the object read lock as initializing the
subrepository is still racy.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/grep.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 073dfaaf45..b045f8a488 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -463,16 +463,6 @@ static int grep_submodule(struct grep_opt *opt,
ALLOC_GROW(repos_to_free, repos_to_free_nr + 1, repos_to_free_alloc);
repos_to_free[repos_to_free_nr++] = subrepo;
- /*
- * NEEDSWORK: repo_read_gitmodules() might call
- * odb_add_to_alternates_memory() via config_from_gitmodules(). This
- * operation causes a race condition with concurrent object readings
- * performed by the worker threads. That's why we need obj_read_lock()
- * here. It should be removed once it's no longer necessary to add the
- * subrepo's odbs to the in-memory alternates list.
- */
- obj_read_lock();
-
/*
* NEEDSWORK: when reading a submodule, the sparsity settings in the
* superproject are incorrectly forgotten or misused. For example:
@@ -498,18 +488,14 @@ static int grep_submodule(struct grep_opt *opt,
* ditto.
*
* Note that this list is not exhaustive.
+ *
+ * NEEDSWORK: initializing the subrepository is not thread-safe,
+ * either, as it may cause us to race around `get_main_ref_store()`. We
+ * thus need to hold the object-read lock to serialize all readers with
+ * one another.
*/
+ obj_read_lock();
repo_read_gitmodules(subrepo, 0);
-
- /*
- * All code paths tested by test code no longer need submodule ODBs to
- * be added as alternates, but add it to the list just in case.
- * Submodule ODBs added through add_submodule_odb_by_path() will be
- * lazily registered as alternates when needed (and except in an
- * unexpected code interaction, it won't be needed).
- */
- odb_add_submodule_source_by_path(the_repository->objects,
- subrepo->objects->sources->path);
obj_read_unlock();
memcpy(&subopt, opt, sizeof(subopt));
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH 06/12] odb: remove infrastructure to register submodule sources
2026-09-01 11:08 [PATCH 00/12] odb: stop registering in-memory sources Patrick Steinhardt
` (4 preceding siblings ...)
2026-09-01 11:09 ` [PATCH 05/12] builtin/grep: stop registering submodule ODB as source Patrick Steinhardt
@ 2026-09-01 11:09 ` Patrick Steinhardt
2026-09-01 22:26 ` Junio C Hamano
2026-09-01 11:09 ` [PATCH 07/12] tmp-objdir: drop unused function to register alternate Patrick Steinhardt
` (7 subsequent siblings)
13 siblings, 1 reply; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-01 11:09 UTC (permalink / raw)
To: git
The preceding commits have removed the last two users of
`odb_add_submodule_source_by_path()`. The mechanism was only ever
meant as a transitional crutch while migrating submodule object
access away from "add the submodule ODB as an alternate of
the_repository" towards explicitly passing the submodule repository,
see a35e03dee0 (submodule: lazily add submodule ODBs as alternates,
2021-08-16). Remove it.
As GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB is now a no-op, remove its
documentation and the exports from the test suite, as well.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb.c | 36 ----------------------------------
odb.h | 14 -------------
t/README | 7 -------
t/t5526-fetch-submodules.sh | 3 ---
t/t5531-deep-submodule-push.sh | 3 ---
t/t5545-push-options.sh | 3 ---
t/t5572-pull-submodule.sh | 3 ---
t/t6437-submodule-merge.sh | 3 ---
t/t7418-submodule-sparse-gitmodules.sh | 3 ---
t/t7814-grep-recurse-submodules.sh | 3 ---
10 files changed, 78 deletions(-)
diff --git a/odb.c b/odb.c
index 6d5943e5ea..2f8a70a90c 100644
--- a/odb.c
+++ b/odb.c
@@ -388,12 +388,6 @@ struct odb_source *odb_find_source_or_die(struct object_database *odb, const cha
return source;
}
-void odb_add_submodule_source_by_path(struct object_database *odb,
- const char *path)
-{
- string_list_insert(&odb->submodule_source_paths, path);
-}
-
static void fill_alternate_refs_command(struct repository *repo,
struct child_process *cmd,
const char *repo_path)
@@ -549,23 +543,6 @@ void disable_obj_read_lock(void)
pthread_mutex_destroy(&obj_read_mutex);
}
-static int register_all_submodule_sources(struct object_database *odb)
-{
- int ret = odb->submodule_source_paths.nr;
-
- for (size_t i = 0; i < odb->submodule_source_paths.nr; i++)
- odb_add_to_alternates_memory(odb,
- odb->submodule_source_paths.items[i].string);
- if (ret) {
- string_list_clear(&odb->submodule_source_paths, 0);
- trace2_data_intmax("submodule", odb->repo,
- "register_all_submodule_sources/registered", ret);
- if (git_env_bool("GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB", 0))
- BUG("register_all_submodule_sources() called");
- }
- return ret;
-}
-
static enum odb_read_status do_oid_object_info_extended(struct object_database *odb,
const struct object_id *oid,
struct object_info *oi, unsigned flags)
@@ -614,16 +591,6 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database *
}
}
- /*
- * This might be an attempt at accessing a submodule object as
- * if it were in main object store (having called
- * `odb_add_submodule_source_by_path()` on that submodule's
- * ODB). If any such ODBs exist, register them and try again.
- */
- if (register_all_submodule_sources(odb))
- /* We added some alternates; retry */
- continue;
-
/* Check if it is a missing object */
if (odb->repo->fetch_if_missing && repo_has_promisor_remote(odb->repo) &&
!already_retried &&
@@ -1109,7 +1076,6 @@ struct object_database *odb_new(struct repository *repo,
CALLOC_ARRAY(o, 1);
o->repo = repo;
pthread_mutex_init(&o->replace_mutex, NULL);
- string_list_init_dup(&o->submodule_source_paths);
hashmap_init(&o->source_by_path, odb_source_by_path_cmp, o, 0);
o->source_paths_icase = -1;
@@ -1166,8 +1132,6 @@ void odb_free(struct object_database *o)
odb_close(o);
odb_free_sources(o);
- string_list_clear(&o->submodule_source_paths, 0);
-
free(o);
}
diff --git a/odb.h b/odb.h
index 248ee9cdfa..54548efc55 100644
--- a/odb.h
+++ b/odb.h
@@ -89,12 +89,6 @@ struct object_database {
unsigned long object_count;
unsigned object_count_flags;
unsigned object_count_valid : 1;
-
- /*
- * Submodule source paths that will be added as additional sources to
- * allow lookup of submodule objects via the main object database.
- */
- struct string_list submodule_source_paths;
};
enum odb_new_flags {
@@ -224,14 +218,6 @@ void odb_restore_primary_source(struct object_database *odb,
struct odb_source *restore_source,
const char *old_path);
-/*
- * Call odb_add_submodule_source_by_path() to add the submodule at the given
- * path to a list. The object stores of all submodules in that list will be
- * added as additional sources in the object store when looking up objects.
- */
-void odb_add_submodule_source_by_path(struct object_database *odb,
- const char *path);
-
/*
* Iterate through all alternates of the database and execute the provided
* callback function for each of them. Stop iterating once the callback
diff --git a/t/README b/t/README
index 9a9daaf2af..f831c5355b 100644
--- a/t/README
+++ b/t/README
@@ -462,13 +462,6 @@ GIT_TEST_CHECKOUT_WORKERS=<n> overrides the 'checkout.workers' setting
to <n> and 'checkout.thresholdForParallelism' to 0, forcing the
execution of the parallel-checkout code.
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=<boolean>, when true, makes
-registering submodule ODBs as alternates a fatal action. Support for
-this environment variable can be removed once the migration to
-explicitly providing repositories when accessing submodule objects is
-complete or needs to be abandoned for whatever reason (in which case the
-migrated codepaths still retain their performance benefits).
-
GIT_TEST_REQUIRE_PREREQ=<list> allows specifying a space separated list of
prereqs that are required to succeed. If a prereq in this list is triggered by
a test and then fails then the whole test run will abort. This can help to make
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 7b3b7359da..37d7373b36 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -3,9 +3,6 @@
test_description='Recursive "git fetch" for submodules'
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
pwd=$(pwd)
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 7d239dd31f..73429ec6e3 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -5,9 +5,6 @@ test_description='test push with submodules'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
test_expect_success setup '
diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh
index fb13549da7..239edd7d62 100755
--- a/t/t5545-push-options.sh
+++ b/t/t5545-push-options.sh
@@ -5,9 +5,6 @@ test_description='pushing to a repository using push options'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
mk_repo_pair () {
diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh
index 42d14328b6..9969a3294e 100755
--- a/t/t5572-pull-submodule.sh
+++ b/t/t5572-pull-submodule.sh
@@ -2,9 +2,6 @@
test_description='pull can handle submodules'
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-submodule-update.sh
diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh
index 107e13afbc..1546d5f773 100755
--- a/t/t6437-submodule-merge.sh
+++ b/t/t6437-submodule-merge.sh
@@ -5,9 +5,6 @@ test_description='merging with submodules'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
#
diff --git a/t/t7418-submodule-sparse-gitmodules.sh b/t/t7418-submodule-sparse-gitmodules.sh
index dde11ecce8..cf94e30e78 100755
--- a/t/t7418-submodule-sparse-gitmodules.sh
+++ b/t/t7418-submodule-sparse-gitmodules.sh
@@ -12,9 +12,6 @@ The test setup uses a sparse checkout, however the same scenario can be set up
also by committing .gitmodules and then just removing it from the filesystem.
'
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
test_expect_success 'setup' '
diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh
index e1cf53dc9e..3d149d34c1 100755
--- a/t/t7814-grep-recurse-submodules.sh
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -9,9 +9,6 @@ submodules.
TEST_CREATE_REPO_NO_TEMPLATE=1
. ./test-lib.sh
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
test_expect_success 'setup directory structure and submodule' '
echo "(1|2)d(3|4)" >a &&
mkdir b &&
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH 07/12] tmp-objdir: drop unused function to register alternate
2026-09-01 11:08 [PATCH 00/12] odb: stop registering in-memory sources Patrick Steinhardt
` (5 preceding siblings ...)
2026-09-01 11:09 ` [PATCH 06/12] odb: remove infrastructure to register submodule sources Patrick Steinhardt
@ 2026-09-01 11:09 ` Patrick Steinhardt
2026-09-01 11:09 ` [PATCH 08/12] odb/packed: fix memory leaks when freeing source Patrick Steinhardt
` (6 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-01 11:09 UTC (permalink / raw)
To: git
The last caller of `tmp_objdir_add_as_alternate()` went away in
bdee7b3013 (builtin/receive-pack: stage incoming objects via ODB
transactions, 2026-07-10) and is unused now. Remove the function.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
tmp-objdir.c | 5 -----
tmp-objdir.h | 6 ------
2 files changed, 11 deletions(-)
diff --git a/tmp-objdir.c b/tmp-objdir.c
index 0eaa79ffd7..deaaf6ba2e 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -321,11 +321,6 @@ const char **tmp_objdir_env(const struct tmp_objdir *t)
return t->env.v;
}
-void tmp_objdir_add_as_alternate(const struct tmp_objdir *t)
-{
- odb_add_to_alternates_memory(t->repo->objects, t->path.buf);
-}
-
struct odb_source *tmp_objdir_replace_primary_odb(struct tmp_objdir *t,
int will_destroy)
{
diff --git a/tmp-objdir.h b/tmp-objdir.h
index 81eb927413..05f0d08d10 100644
--- a/tmp-objdir.h
+++ b/tmp-objdir.h
@@ -55,12 +55,6 @@ int tmp_objdir_destroy(struct tmp_objdir *);
*/
void tmp_objdir_discard_objects(struct tmp_objdir *);
-/*
- * Add the temporary object directory as an alternate object store in the
- * current process.
- */
-void tmp_objdir_add_as_alternate(const struct tmp_objdir *);
-
/*
* Replaces the writable object store in the current process with the temporary
* object directory and makes the former main object store an alternate.
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH 08/12] odb/packed: fix memory leaks when freeing source
2026-09-01 11:08 [PATCH 00/12] odb: stop registering in-memory sources Patrick Steinhardt
` (6 preceding siblings ...)
2026-09-01 11:09 ` [PATCH 07/12] tmp-objdir: drop unused function to register alternate Patrick Steinhardt
@ 2026-09-01 11:09 ` Patrick Steinhardt
2026-09-01 11:09 ` [PATCH 09/12] builtin/multi-pack-index: refuse unknown sources with "--object-dir=" Patrick Steinhardt
` (5 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-01 11:09 UTC (permalink / raw)
To: git
When freeing a "packed" source we don't close either its packs nor its
multi-pack indices. This can cause memory leaks in case we create an
ad-hoc packed source. As we used to always link packed sources to the
main object database we never noticed this issue until now, but it's
going to surface in subsequent commits where we stop linking them.
Plug the memory leaks by closing the source first.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-packed.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 1d90e714e6..166e76e2d6 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -844,6 +844,7 @@ static void odb_source_packed_free(struct odb_source *source)
chdir_notify_unregister(odb_source_packed_reparent, packed);
+ odb_source_close(source);
for (struct packfile_list_entry *e = packed->packs.head; e; e = e->next)
free(e->pack);
packfile_list_clear(&packed->packs);
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH 09/12] builtin/multi-pack-index: refuse unknown sources with "--object-dir="
2026-09-01 11:08 [PATCH 00/12] odb: stop registering in-memory sources Patrick Steinhardt
` (7 preceding siblings ...)
2026-09-01 11:09 ` [PATCH 08/12] odb/packed: fix memory leaks when freeing source Patrick Steinhardt
@ 2026-09-01 11:09 ` Patrick Steinhardt
2026-09-01 11:09 ` [PATCH 10/12] t/helper: adapt read-midx to not link ad-hoc source anymore Patrick Steinhardt
` (4 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-01 11:09 UTC (permalink / raw)
To: git
Users can tell git-multi-pack-index(1) to access multi-pack indices that
are stored in a different object directory via the "--object-dir="
option. This allows them to for example write or verify a multi-pack
index other than the one located in the main object directory in case a
repository has alternates with multiple multi-pack indices.
But while the documentation explicitly points out that the specified
object directory must be an alternate of the current repository, we
never verify that property. Instead, starting with 017db7bb14 (midx:
load multi-pack indices via their source, 2025-08-11), we now construct
an ad-hoc source and link it to the main object directory.
Besides contradicting the documentation, it's dubious that this really
ought to work in the first place: creating a multi-pack index (and
potentially a bitmap) for a completely foreign object directory is of
questionable value, as bitmap commit selection operates on the invoking
repository's refs. Furthermore, this is the only remaining caller
outside of our test helpers that constructs an ad-hoc source and links
it to the database, and we want to get rid of this mechanism as part of
this series.
Stop constructing the ad-hoc source and instead refuse the operation.
While this results in a change in behaviour, this restriction has been
documented as such ever since f57a739691 (midx: avoid opening multiple
MIDXs when writing, 2021-09-01).
Note that this change requires us to adapt one test chain in t5319, as
it creates an object directory that is not connected to any repository
and then uses it via "--object-dir=". The setup itself already documents
this and does the necessary gymnastics to link the object directory to a
temporary repository, but subsequent tests don't. Adapt those tests to
retain and reuse the temporary repository.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/multi-pack-index.c | 3 ++-
t/t5319-multi-pack-index.sh | 9 ++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index 6e73c85cde..753bd53a70 100644
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -90,7 +90,8 @@ static struct odb_source_files *handle_object_dir_option(struct repository *repo
{
struct odb_source *source = odb_find_source(repo->objects, opts.object_dir);
if (!source)
- source = odb_add_to_alternates_memory(repo->objects, opts.object_dir);
+ die(_("object directory is not an alternate of the current repository: '%s'"),
+ opts.object_dir);
return odb_source_files_downcast(source);
}
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 68143cb5b7..00e90f163f 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -698,10 +698,9 @@ test_expect_success 'force some 64-bit offsets with pack-objects' '
corrupt_data $idx64 $(test_oid idxoff) "\02" &&
# objects64 is not a real repository, but can serve as an alternate
# anyway so we can write a MIDX into it
- git init repo &&
- test_when_finished "rm -fr repo" &&
+ git init repo64 &&
(
- cd repo &&
+ cd repo64 &&
( cd ../objects64 && pwd ) >.git/objects/info/alternates &&
midx64=$(git multi-pack-index --object-dir=../objects64 write)
) &&
@@ -709,7 +708,7 @@ test_expect_success 'force some 64-bit offsets with pack-objects' '
'
test_expect_success 'verify multi-pack-index with 64-bit offsets' '
- git multi-pack-index verify --object-dir=objects64
+ git -C repo64 multi-pack-index verify --object-dir=../objects64
'
NUM_OBJECTS=63
@@ -721,7 +720,7 @@ MIDX_BYTE_LARGE_OFFSET=$(($MIDX_OFFSET_LARGE_OFFSETS + 3))
test_expect_success 'verify incorrect 64-bit offset' '
corrupt_midx_and_verify $MIDX_BYTE_LARGE_OFFSET "\07" objects64 \
- "incorrect object offset"
+ "incorrect object offset" "git -C repo64 multi-pack-index verify --object-dir=../objects64"
'
test_expect_success 'setup expire tests' '
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH 10/12] t/helper: adapt read-midx to not link ad-hoc source anymore
2026-09-01 11:08 [PATCH 00/12] odb: stop registering in-memory sources Patrick Steinhardt
` (8 preceding siblings ...)
2026-09-01 11:09 ` [PATCH 09/12] builtin/multi-pack-index: refuse unknown sources with "--object-dir=" Patrick Steinhardt
@ 2026-09-01 11:09 ` Patrick Steinhardt
2026-09-01 11:09 ` [PATCH 11/12] t/helper: stop registering alternates in "ref-store" command Patrick Steinhardt
` (3 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-01 11:09 UTC (permalink / raw)
To: git
Same as in the preceding commit, refactor the setup of ad-hoc object
database sources when accessing a multi-pack index in an arbitrary
location to not link the newly created source into the main object
database anymore.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/helper/test-read-midx.c | 43 +++++++++++++++++++++++++++++++------------
1 file changed, 31 insertions(+), 12 deletions(-)
diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c
index 27a05da957..1f7a1927e4 100644
--- a/t/helper/test-read-midx.c
+++ b/t/helper/test-read-midx.c
@@ -5,34 +5,42 @@
#include "midx.h"
#include "repository.h"
#include "odb.h"
+#include "odb/source-packed.h"
#include "pack-bitmap.h"
#include "packfile.h"
#include "setup.h"
#include "gettext.h"
#include "pack-revindex.h"
-static struct multi_pack_index *setup_midx(const char *object_dir)
+static struct multi_pack_index *setup_midx(const char *object_dir,
+ struct odb_source_packed **out)
{
- struct odb_source_files *files;
+ struct odb_source_packed *packed;
struct odb_source *source;
+
setup_git_directory(the_repository);
+
source = odb_find_source(the_repository->objects, object_dir);
- if (!source)
- source = odb_add_to_alternates_memory(the_repository->objects,
- object_dir);
- files = odb_source_files_downcast(source);
+ if (source) {
+ packed = odb_source_files_downcast(source)->packed;
+ } else {
+ packed = odb_source_packed_new(the_repository->objects,
+ object_dir, false);
+ *out = packed;
+ }
- return load_multi_pack_index(files->packed);
+ return load_multi_pack_index(packed);
}
static int read_midx_file(const char *object_dir, const char *checksum,
int show_objects)
{
+ struct odb_source_packed *packed = NULL;
uint32_t i;
struct multi_pack_index *m, *tip;
int ret = 0;
- m = tip = setup_midx(object_dir);
+ m = tip = setup_midx(object_dir, &packed);
if (!m)
return 1;
@@ -91,29 +99,35 @@ static int read_midx_file(const char *object_dir, const char *checksum,
out:
close_midx(tip);
+ if (packed)
+ odb_source_free(&packed->base);
return ret;
}
static int read_midx_checksum(const char *object_dir)
{
+ struct odb_source_packed *packed = NULL;
struct multi_pack_index *m;
- m = setup_midx(object_dir);
+ m = setup_midx(object_dir, &packed);
if (!m)
return 1;
printf("%s\n", midx_get_checksum_hex(m));
close_midx(m);
+ if (packed)
+ odb_source_free(&packed->base);
return 0;
}
static int read_midx_preferred_pack(const char *object_dir)
{
+ struct odb_source_packed *packed = NULL;
struct multi_pack_index *midx = NULL;
uint32_t preferred_pack;
- midx = setup_midx(object_dir);
+ midx = setup_midx(object_dir, &packed);
if (!midx)
return 1;
@@ -124,17 +138,21 @@ static int read_midx_preferred_pack(const char *object_dir)
}
printf("%s\n", midx->pack_names[preferred_pack]);
+
close_midx(midx);
+ if (packed)
+ odb_source_free(&packed->base);
return 0;
}
static int read_midx_bitmapped_packs(const char *object_dir)
{
+ struct odb_source_packed *packed = NULL;
struct multi_pack_index *midx = NULL;
struct bitmapped_pack pack;
uint32_t i;
- midx = setup_midx(object_dir);
+ midx = setup_midx(object_dir, &packed);
if (!midx)
return 1;
@@ -150,7 +168,8 @@ static int read_midx_bitmapped_packs(const char *object_dir)
}
close_midx(midx);
-
+ if (packed)
+ odb_source_free(&packed->base);
return 0;
}
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH 11/12] t/helper: stop registering alternates in "ref-store" command
2026-09-01 11:08 [PATCH 00/12] odb: stop registering in-memory sources Patrick Steinhardt
` (9 preceding siblings ...)
2026-09-01 11:09 ` [PATCH 10/12] t/helper: adapt read-midx to not link ad-hoc source anymore Patrick Steinhardt
@ 2026-09-01 11:09 ` Patrick Steinhardt
2026-09-01 11:09 ` [PATCH 12/12] odb: remove the ability to link sources ad-hoc Patrick Steinhardt
` (2 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-01 11:09 UTC (permalink / raw)
To: git
When using the "ref-store" command we support access to multiple
different reference stores. As part of that we allow the caller to
explicitly exercise stores of a submodule. This allows us to verify
low-level behaviour of submodule stores, which is exercised in t1406.
When doing so we also link the submodule's object database into the main
object database. The intent of this is that it allows us to access
objects of the submodule, too. But that functionality is not even
needed anymore: when creating a submodule reference store, we will first
initialize the submodule repository and then initialize the store with
that repository. And as the reference subsystem doesn't depend on
`the_repository` anymore all subsequent object lookups performed by the
reference store will be routed to the submodule repository.
It is thus not needed anymore to register the submodule object store
with the main object database. Remove the call.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/helper/test-ref-store.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 5a9a3053d9..db58f00589 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -74,14 +74,6 @@ static const char **get_store(const char **argv, struct ref_store **refs)
} else if (!strcmp(argv[0], "main")) {
*refs = get_main_ref_store(the_repository);
} else if (skip_prefix(argv[0], "submodule:", &gitdir)) {
- struct strbuf sb = STRBUF_INIT;
-
- if (!repo_submodule_path_append(the_repository,
- &sb, gitdir, "objects/"))
- die("computing submodule path failed");
- odb_add_to_alternates_memory(the_repository->objects, sb.buf);
- strbuf_release(&sb);
-
*refs = repo_get_submodule_ref_store(the_repository, gitdir);
} else if (skip_prefix(argv[0], "worktree:", &gitdir)) {
struct worktree **p, **worktrees = get_worktrees(the_repository);
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH 12/12] odb: remove the ability to link sources ad-hoc
2026-09-01 11:08 [PATCH 00/12] odb: stop registering in-memory sources Patrick Steinhardt
` (10 preceding siblings ...)
2026-09-01 11:09 ` [PATCH 11/12] t/helper: stop registering alternates in "ref-store" command Patrick Steinhardt
@ 2026-09-01 11:09 ` Patrick Steinhardt
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 " Patrick Steinhardt
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-01 11:09 UTC (permalink / raw)
To: git
Over the course of this patch series we have adapted all callers of
`odb_add_to_alternates_memory()` to not do so anymore. Remove the
function.
This series of refactorings doesn't only simplify our code base. More
importantly, with those changes in place we can now unconditionally
assume that the list of sources linked to the object database only
consists of the primary source and its alternates. This serves as the
foundation to eventually move handling of alternates into the "files"
backend itself.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb.c | 6 ------
odb.h | 8 --------
2 files changed, 14 deletions(-)
diff --git a/odb.c b/odb.c
index 2f8a70a90c..5fe081496f 100644
--- a/odb.c
+++ b/odb.c
@@ -247,12 +247,6 @@ void odb_add_to_alternates_file(struct object_database *odb,
odb_add_alternate_recursively(odb, dir, 0);
}
-struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
- const char *dir)
-{
- return odb_add_alternate_recursively(odb, dir, 0);
-}
-
struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
const char *dir, int will_destroy,
struct odb_source **prev_source)
diff --git a/odb.h b/odb.h
index 54548efc55..9025239df5 100644
--- a/odb.h
+++ b/odb.h
@@ -258,14 +258,6 @@ int odb_has_alternates(struct object_database *odb);
void odb_add_to_alternates_file(struct object_database *odb,
const char *dir);
-/*
- * Add the directory to the in-memory list of alternate sources (along with any
- * recursive alternates it points to), but do not modify the on-disk alternates
- * file.
- */
-struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
- const char *dir);
-
/*
* Read an object from the database. Returns the object data and assigns object
* type and size to the `type` and `size` pointers, if these pointers are
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v2 00/13] odb: stop registering in-memory sources
2026-09-01 11:08 [PATCH 00/12] odb: stop registering in-memory sources Patrick Steinhardt
` (11 preceding siblings ...)
2026-09-01 11:09 ` [PATCH 12/12] odb: remove the ability to link sources ad-hoc Patrick Steinhardt
@ 2026-09-02 13:34 ` Patrick Steinhardt
2026-09-02 13:34 ` [PATCH v2 01/13] cache-tree: drop `the_repository` in `cache_tree_fully_valid()` Patrick Steinhardt
` (13 more replies)
2026-09-11 5:51 ` [PATCH v3 " Patrick Steinhardt
13 siblings, 14 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-02 13:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Hi,
the object database has a list of sources that is used for two
different purposes:
- We use it to track the list of alternates.
- We use it to track temporary in-memory sources that we create for
various purposes. Most importantly, this is used to link object
database sources from submodules into the main store.
This dual-use is quite awkward, as it mixes two different levels of
concerns and thus as a consequence makes both harder to reason about.
It's also a source of bugs: we make assumptions about the ordering of
sources all over the place, and we furthermore assume in other places
that the sources only contain alternates in the first place. I don't
think this surfaces in the form of real bugs, but I've long disliked
this dual-use.
Furthermore, we want to migrate handling of alternates into the "files"
backend itself in a subsequent patch series. This is most importantly to
fix a performance regression by making the backend own all of its
alternates, but it also fixes a couple of longer-standing design issues
that I've been struggling with [1].
Most importantly though: this whole machinery is not even needed at all.
A couple years ago we have already refactored our codebase so that
submodule sources don't even have to be linked into the main object
database anymore. And all the other use cases where we link sources into
the main object database can be trivially converted, too.
So this patch series does exactly that: it removes the mechanism to link
ad-hoc sources into the object database entirely. This ensures that the
list of sources is exactly the list of alternates, and that makes it
easier to move them into the "files" backend in a subsequent patch
series.
There is one exception though: creating transactions still creates a
temporary quarantine directory. This mechanism is left as-is for now,
but as it's an implementation detail of the "files" backend anyway
that's not conflicting with our above stated goals.
This series is built on top of 1630431f32 (The 21st batch, 2026-08-31)
with ty/repository-fetch-if-missing at 508ec9837c (repository: move
fetch_if_missing into struct repository, 2026-08-15) merged into it.
There's still two merge conflicts, but these are trivial to resolve: in
"odb.c" and "odb.h" you simply remove both ours and theirs, and in
"builtin/multi-pack-index.c" you only need to munge the parameters a
bit.
Changes in v2:
- Adapt `cache_tree_fully_valid()` to take a `struct index_state` as
input instead of taking both a repository and a cache tree, as
suggested by Junio.
- Link to v1: https://patch.msgid.link/20260901-pks-odb-registering-in-memory-sources-v1-0-97a312d5fa25@pks.im
Thanks!
Patrick
[1]: <amLgMqkqxR8mKIbT@pks.im>
---
Patrick Steinhardt (13):
cache-tree: drop `the_repository` in `cache_tree_fully_valid()`
cache-tree: remove dependency on `the_repository`
submodule-config: remove uses of `the_repository`
submodule-config: stop using `the_hash_algo`
submodule-config: stop registering submodule sources
builtin/grep: stop registering submodule ODB as source
odb: remove infrastructure to register submodule sources
tmp-objdir: drop unused function to register alternate
odb/packed: fix memory leaks when freeing source
builtin/multi-pack-index: refuse unknown sources with "--object-dir="
t/helper: adapt read-midx to not link ad-hoc source anymore
t/helper: stop registering alternates in "ref-store" command
odb: remove the ability to link sources ad-hoc
builtin/checkout.c | 2 +-
builtin/commit.c | 2 +-
builtin/fetch.c | 2 +-
builtin/grep.c | 28 +++-------
builtin/multi-pack-index.c | 3 +-
builtin/submodule--helper.c | 8 +--
cache-tree.c | 95 +++++++++++++++++++---------------
cache-tree.h | 7 +--
odb.c | 42 ---------------
odb.h | 22 --------
odb/source-packed.c | 1 +
read-cache-ll.h | 5 +-
read-cache.c | 9 ++--
sequencer.c | 2 +-
sparse-index.c | 2 +-
submodule-config.c | 59 +++++++++++----------
submodule-config.h | 12 +++--
submodule.c | 2 +-
t/README | 7 ---
t/helper/test-read-midx.c | 43 ++++++++++-----
t/helper/test-ref-store.c | 8 ---
t/helper/test-submodule.c | 4 +-
t/t5319-multi-pack-index.sh | 9 ++--
t/t5526-fetch-submodules.sh | 3 --
t/t5531-deep-submodule-push.sh | 3 --
t/t5545-push-options.sh | 3 --
t/t5572-pull-submodule.sh | 3 --
t/t6437-submodule-merge.sh | 3 --
t/t7418-submodule-sparse-gitmodules.sh | 3 --
t/t7814-grep-recurse-submodules.sh | 3 --
tmp-objdir.c | 5 --
tmp-objdir.h | 6 ---
unpack-trees.c | 9 ++--
33 files changed, 168 insertions(+), 247 deletions(-)
Range-diff versus v1:
-: ---------- > 1: 4870d0661d cache-tree: drop `the_repository` in `cache_tree_fully_valid()`
1: a4e2648526 ! 2: 755652bfe1 cache-tree: remove dependency on `the_repository`
@@ Commit message
Signed-off-by: Patrick Steinhardt <ps@pks.im>
- ## builtin/checkout.c ##
-@@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *opts,
- }
- }
-
-- if (!cache_tree_fully_valid(the_repository->index->cache_tree))
-+ if (!cache_tree_fully_valid(the_repository, the_repository->index->cache_tree))
- cache_tree_update(the_repository->index, WRITE_TREE_SILENT | WRITE_TREE_REPAIR);
-
- if (write_locked_index(the_repository->index, &lock_file, COMMIT_LOCK))
-
- ## builtin/commit.c ##
-@@ builtin/commit.c: static const char *prepare_index(const char **argv, const char *prefix,
- LOCK_DIE_ON_ERROR);
- refresh_cache_or_die(refresh_flags);
- if (the_repository->index->cache_changed
-- || !cache_tree_fully_valid(the_repository->index->cache_tree))
-+ || !cache_tree_fully_valid(the_repository, the_repository->index->cache_tree))
- cache_tree_update(the_repository->index, WRITE_TREE_SILENT);
- if (write_locked_index(the_repository->index, &index_lock,
- COMMIT_LOCK | SKIP_IF_UNCHANGED))
-
## cache-tree.c ##
@@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
-@@ cache-tree.c: static void discard_unused_subtrees(struct cache_tree *it)
- }
- }
-
--int cache_tree_fully_valid(struct cache_tree *it)
-+int cache_tree_fully_valid(struct repository *repo, struct cache_tree *it)
- {
- int i;
- if (!it)
- return 0;
- if (it->entry_count < 0 ||
-- !odb_has_object(the_repository->objects, &it->oid,
-+ !odb_has_object(repo->objects, &it->oid,
- ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR))
- return 0;
- for (i = 0; i < it->subtree_nr; i++) {
-- if (!cache_tree_fully_valid(it->down[i]->cache_tree))
-+ if (!cache_tree_fully_valid(repo, it->down[i]->cache_tree))
- return 0;
- }
- return 1;
+@@ cache-tree.c: int cache_tree_fully_valid(struct index_state *istate)
+ istate->cache_tree);
}
-static int must_check_existence(const struct cache_entry *ce)
@@ cache-tree.c: static struct cache_tree *read_one(const char **buffer, unsigned l
return result;
}
-@@ cache-tree.c: struct tree *write_in_core_index_as_tree(struct repository *repo,
- int was_valid, ret;
-
- was_valid = index_state->cache_tree &&
-- cache_tree_fully_valid(index_state->cache_tree);
-+ cache_tree_fully_valid(repo, index_state->cache_tree);
-
- ret = write_index_as_tree_internal(&o, index_state, was_valid, 0, NULL);
- if (ret == WRITE_TREE_UNMERGED_INDEX) {
@@ cache-tree.c: int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR);
@@ cache-tree.c: int write_index_as_tree(struct object_id *oid, struct index_state
if (entries < 0) {
ret = WRITE_TREE_UNREADABLE_INDEX;
goto out;
-@@ cache-tree.c: int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
-
- was_valid = !(flags & WRITE_TREE_IGNORE_CACHE_TREE) &&
- index_state->cache_tree &&
-- cache_tree_fully_valid(index_state->cache_tree);
-+ cache_tree_fully_valid(index_state->repo, index_state->cache_tree);
-
- ret = write_index_as_tree_internal(oid, index_state, was_valid, flags,
- prefix);
@@ cache-tree.c: static void prime_cache_tree_rec(struct repository *r,
struct cache_tree_sub *sub;
struct tree *subtree = lookup_tree(r, &entry.oid);
@@ cache-tree.h: struct cache_tree_sub *cache_tree_sub(struct cache_tree *, const c
+struct cache_tree *cache_tree_read(struct repository *repo,
+ const char *buffer, unsigned long size);
--int cache_tree_fully_valid(struct cache_tree *);
-+int cache_tree_fully_valid(struct repository *, struct cache_tree *);
+ int cache_tree_fully_valid(struct index_state *);
int cache_tree_update(struct index_state *, int);
- int cache_tree_verify(struct repository *, struct index_state *);
-
## read-cache-ll.h ##
@@ read-cache-ll.h: void validate_cache_entries(const struct index_state *istate);
@@ read-cache.c: void prefetch_cache_entries(const struct index_state *istate,
if (!odb_read_object_info_extended(the_repository->objects,
&ce->oid, NULL,
- ## sequencer.c ##
-@@ sequencer.c: static int do_recursive_merge(struct repository *r,
-
- static struct object_id *get_cache_tree_oid(struct index_state *istate)
- {
-- if (!cache_tree_fully_valid(istate->cache_tree))
-+ if (!cache_tree_fully_valid(istate->repo, istate->cache_tree))
- if (cache_tree_update(istate, 0)) {
- error(_("unable to update cache tree"));
- return NULL;
-
- ## sparse-index.c ##
-@@ sparse-index.c: int convert_to_sparse(struct index_state *istate, int flags)
- if (index_has_unmerged_entries(istate))
- return 0;
-
-- if (!cache_tree_fully_valid(istate->cache_tree)) {
-+ if (!cache_tree_fully_valid(istate->repo, istate->cache_tree)) {
- /* Clear and recompute the cache-tree */
- cache_tree_free(&istate->cache_tree);
-
-
## unpack-trees.c ##
@@ unpack-trees.c: static void report_collided_checkout(struct index_state *index)
string_list_clear(&list, 0);
@@ unpack-trees.c: static int check_updates(struct unpack_trees_options *o,
size_t last_pc_queue_size = pc_queue_size();
if (ce->ce_flags & CE_WT_REMOVE)
-@@ unpack-trees.c: int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
- }
-
- if (!o->skip_cache_tree_update &&
-- !cache_tree_fully_valid(o->internal.result.cache_tree))
-+ !cache_tree_fully_valid(the_repository, o->internal.result.cache_tree))
- cache_tree_update(&o->internal.result,
- WRITE_TREE_SILENT |
- WRITE_TREE_REPAIR);
2: bedb614b5c = 3: 46bbe548a6 submodule-config: remove uses of `the_repository`
3: 8fa02cdd5c = 4: 65f8634bda submodule-config: stop using `the_hash_algo`
4: ebf1dfb548 = 5: 4faf4fdd57 submodule-config: stop registering submodule sources
5: dfe1a212f7 = 6: dee5aca311 builtin/grep: stop registering submodule ODB as source
6: 1644e49419 = 7: 2ce128799a odb: remove infrastructure to register submodule sources
7: 3f3ccfba74 = 8: 0a174f6304 tmp-objdir: drop unused function to register alternate
8: f1341b797b = 9: d22c376cd2 odb/packed: fix memory leaks when freeing source
9: c1f6d9ebdd = 10: 09405ade4c builtin/multi-pack-index: refuse unknown sources with "--object-dir="
10: c5b9c1c96e = 11: 557b3ab923 t/helper: adapt read-midx to not link ad-hoc source anymore
11: 3f43f73cd8 = 12: 78b26c172a t/helper: stop registering alternates in "ref-store" command
12: 4c670d726f = 13: 1ca49caf1c odb: remove the ability to link sources ad-hoc
---
base-commit: e5d60560f61f520e9ea350645a6cc9770b0f1607
change-id: 20260811-pks-odb-registering-in-memory-sources-88648cd95735
^ permalink raw reply [flat|nested] 54+ messages in thread* [PATCH v2 01/13] cache-tree: drop `the_repository` in `cache_tree_fully_valid()`
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
@ 2026-09-02 13:34 ` Patrick Steinhardt
2026-09-02 13:34 ` [PATCH v2 02/13] cache-tree: remove dependency on `the_repository` Patrick Steinhardt
` (12 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-02 13:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
The function `cache_tree_fully_valid()` verifies whether the cache tree
owned by the index is valid or not. As part of that, the function checks
whether the objects referenced by the cache all exist. But because the
function has no repository available, it is using the object database of
`the_repository` instead.
We could of course adapt callers to pass in a repository as parameter
explicitly to get rid of this implicit dependency on global state. But
all of them pass the cache tree owned by a `struct index_state`, and
that structure already has a reference to its owning repository.
So instead, adapt the function to accept a `struct index_state`, which
ensures that callers will implicitly always pass the correct repository.
Adapt callers accordingly.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/checkout.c | 2 +-
builtin/commit.c | 2 +-
cache-tree.c | 17 ++++++++++++-----
cache-tree.h | 2 +-
sequencer.c | 2 +-
sparse-index.c | 2 +-
unpack-trees.c | 2 +-
7 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 55e3a89a85..505d3f7bf3 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -921,7 +921,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
}
}
- if (!cache_tree_fully_valid(the_repository->index->cache_tree))
+ if (!cache_tree_fully_valid(the_repository->index))
cache_tree_update(the_repository->index, WRITE_TREE_SILENT | WRITE_TREE_REPAIR);
if (write_locked_index(the_repository->index, &lock_file, COMMIT_LOCK))
diff --git a/builtin/commit.c b/builtin/commit.c
index 28f6174503..840b6b4083 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -484,7 +484,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
LOCK_DIE_ON_ERROR);
refresh_cache_or_die(refresh_flags);
if (the_repository->index->cache_changed
- || !cache_tree_fully_valid(the_repository->index->cache_tree))
+ || !cache_tree_fully_valid(the_repository->index))
cache_tree_update(the_repository->index, WRITE_TREE_SILENT);
if (write_locked_index(the_repository->index, &index_lock,
COMMIT_LOCK | SKIP_IF_UNCHANGED))
diff --git a/cache-tree.c b/cache-tree.c
index a220372a42..6103b3fcb3 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -275,22 +275,29 @@ static void discard_unused_subtrees(struct cache_tree *it)
}
}
-int cache_tree_fully_valid(struct cache_tree *it)
+static int cache_tree_fully_valid_recursive(struct object_database *odb,
+ struct cache_tree *it)
{
int i;
if (!it)
return 0;
if (it->entry_count < 0 ||
- !odb_has_object(the_repository->objects, &it->oid,
+ !odb_has_object(odb, &it->oid,
ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR))
return 0;
for (i = 0; i < it->subtree_nr; i++) {
- if (!cache_tree_fully_valid(it->down[i]->cache_tree))
+ if (!cache_tree_fully_valid_recursive(odb, it->down[i]->cache_tree))
return 0;
}
return 1;
}
+int cache_tree_fully_valid(struct index_state *istate)
+{
+ return cache_tree_fully_valid_recursive(istate->repo->objects,
+ istate->cache_tree);
+}
+
static int must_check_existence(const struct cache_entry *ce)
{
return !(repo_has_promisor_remote(the_repository) && ce_skip_worktree(ce));
@@ -775,7 +782,7 @@ struct tree *write_in_core_index_as_tree(struct repository *repo,
int was_valid, ret;
was_valid = index_state->cache_tree &&
- cache_tree_fully_valid(index_state->cache_tree);
+ cache_tree_fully_valid(index_state);
ret = write_index_as_tree_internal(&o, index_state, was_valid, 0, NULL);
if (ret == WRITE_TREE_UNMERGED_INDEX) {
@@ -811,7 +818,7 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
was_valid = !(flags & WRITE_TREE_IGNORE_CACHE_TREE) &&
index_state->cache_tree &&
- cache_tree_fully_valid(index_state->cache_tree);
+ cache_tree_fully_valid(index_state);
ret = write_index_as_tree_internal(oid, index_state, was_valid, flags,
prefix);
diff --git a/cache-tree.h b/cache-tree.h
index f8bddae523..4b3f60d6db 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -31,7 +31,7 @@ int cache_tree_subtree_pos(struct cache_tree *it, const char *path, int pathlen)
void cache_tree_write(struct strbuf *, struct cache_tree *root);
struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);
-int cache_tree_fully_valid(struct cache_tree *);
+int cache_tree_fully_valid(struct index_state *);
int cache_tree_update(struct index_state *, int);
int cache_tree_verify(struct repository *, struct index_state *);
diff --git a/sequencer.c b/sequencer.c
index 65afd100d9..11a95c031b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -814,7 +814,7 @@ static int do_recursive_merge(struct repository *r,
static struct object_id *get_cache_tree_oid(struct index_state *istate)
{
- if (!cache_tree_fully_valid(istate->cache_tree))
+ if (!cache_tree_fully_valid(istate))
if (cache_tree_update(istate, 0)) {
error(_("unable to update cache tree"));
return NULL;
diff --git a/sparse-index.c b/sparse-index.c
index c1fa231a89..3d77dadae5 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -228,7 +228,7 @@ int convert_to_sparse(struct index_state *istate, int flags)
if (index_has_unmerged_entries(istate))
return 0;
- if (!cache_tree_fully_valid(istate->cache_tree)) {
+ if (!cache_tree_fully_valid(istate)) {
/* Clear and recompute the cache-tree */
cache_tree_free(&istate->cache_tree);
diff --git a/unpack-trees.c b/unpack-trees.c
index 154d6d40a1..f6bb1e6d2b 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -2086,7 +2086,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
}
if (!o->skip_cache_tree_update &&
- !cache_tree_fully_valid(o->internal.result.cache_tree))
+ !cache_tree_fully_valid(&o->internal.result))
cache_tree_update(&o->internal.result,
WRITE_TREE_SILENT |
WRITE_TREE_REPAIR);
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v2 02/13] cache-tree: remove dependency on `the_repository`
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
2026-09-02 13:34 ` [PATCH v2 01/13] cache-tree: drop `the_repository` in `cache_tree_fully_valid()` Patrick Steinhardt
@ 2026-09-02 13:34 ` Patrick Steinhardt
2026-09-04 22:28 ` Karthik Nayak
2026-09-02 13:34 ` [PATCH v2 03/13] submodule-config: remove uses of `the_repository` Patrick Steinhardt
` (11 subsequent siblings)
13 siblings, 1 reply; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-02 13:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
The "cache-tree" subsystem still depends on `the_repository`. Adapt it
to instead use repositories provided via the context, either as a new
parameter or the one passed in via `struct index_state`.
Besides getting rid of `the_repository`, this also removes the last
dependency on registering submodule sources with the main object
database. When reading gitmodules from a submodule's index we implicitly
read that object via `the_repository`'s object database, which is of
course wrong. This works though because we would then register the
submodule's object database with the main object database, but a later
patch is going to get rid of that mechanism.
You can verify that we indeed no longer depend on this mechanism by
running tests with `GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=true`. Without
this patch we fail in t1092, with this patch we never register submodule
object databases anymore.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
cache-tree.c | 78 +++++++++++++++++++++++++++++++--------------------------
cache-tree.h | 5 ++--
read-cache-ll.h | 5 ++--
read-cache.c | 9 ++++---
unpack-trees.c | 7 +++---
5 files changed, 57 insertions(+), 47 deletions(-)
diff --git a/cache-tree.c b/cache-tree.c
index 6103b3fcb3..b8cbb5da22 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -298,12 +297,14 @@ int cache_tree_fully_valid(struct index_state *istate)
istate->cache_tree);
}
-static int must_check_existence(const struct cache_entry *ce)
+static int must_check_existence(const struct cache_entry *ce, void *cb_data)
{
- return !(repo_has_promisor_remote(the_repository) && ce_skip_worktree(ce));
+ struct repository *repo = cb_data;
+ return !(repo_has_promisor_remote(repo) && ce_skip_worktree(ce));
}
-static int update_one(struct cache_tree *it,
+static int update_one(struct repository *repo,
+ struct cache_tree *it,
struct cache_entry **cache,
int entries,
const char *base,
@@ -341,7 +342,7 @@ static int update_one(struct cache_tree *it,
}
if (0 <= it->entry_count &&
- odb_has_object(the_repository->objects, &it->oid,
+ odb_has_object(repo->objects, &it->oid,
ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR))
return it->entry_count;
@@ -382,7 +383,8 @@ static int update_one(struct cache_tree *it,
sub = find_subtree(it, path + baselen, sublen, 1);
if (!sub->cache_tree)
sub->cache_tree = cache_tree();
- subcnt = update_one(sub->cache_tree,
+ subcnt = update_one(repo,
+ sub->cache_tree,
cache + i, entries - i,
path,
baselen + sublen + 1,
@@ -446,10 +448,10 @@ static int update_one(struct cache_tree *it,
}
ce_missing_ok = mode == S_IFGITLINK || missing_ok ||
- !must_check_existence(ce);
+ !must_check_existence(ce, repo);
if (is_null_oid(oid) ||
(!ce_missing_ok &&
- !odb_has_object(the_repository->objects, oid,
+ !odb_has_object(repo->objects, oid,
ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR))) {
strbuf_release(&buffer);
if (expected_missing)
@@ -481,12 +483,12 @@ static int update_one(struct cache_tree *it,
/*
* "sub" can be an empty tree if all subentries are i-t-a.
*/
- if (contains_ita && is_empty_tree_oid(oid, the_repository->hash_algo))
+ if (contains_ita && is_empty_tree_oid(oid, repo->hash_algo))
continue;
strbuf_grow(&buffer, entlen + 100);
strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
- strbuf_add(&buffer, oid->hash, the_hash_algo->rawsz);
+ strbuf_add(&buffer, oid->hash, repo->hash_algo->rawsz);
#if DEBUG_CACHE_TREE
fprintf(stderr, "cache-tree update-one %o %.*s\n",
@@ -496,16 +498,16 @@ static int update_one(struct cache_tree *it,
if (repair) {
struct object_id oid;
- hash_object_file(the_hash_algo, buffer.buf, buffer.len,
+ hash_object_file(repo->hash_algo, buffer.buf, buffer.len,
OBJ_TREE, &oid);
- if (odb_has_object(the_repository->objects, &oid, ODB_HAS_OBJECT_RECHECK_PACKED))
+ if (odb_has_object(repo->objects, &oid, ODB_HAS_OBJECT_RECHECK_PACKED))
oidcpy(&it->oid, &oid);
else
to_invalidate = 1;
} else if (dryrun) {
- hash_object_file(the_hash_algo, buffer.buf, buffer.len,
+ hash_object_file(repo->hash_algo, buffer.buf, buffer.len,
OBJ_TREE, &it->oid);
- } else if (odb_write_object_ext(the_repository->objects, buffer.buf, buffer.len, OBJ_TREE,
+ } else if (odb_write_object_ext(repo->objects, buffer.buf, buffer.len, OBJ_TREE,
&it->oid, NULL, flags & WRITE_TREE_SILENT ? ODB_WRITE_OBJECT_SILENT : 0)) {
strbuf_release(&buffer);
return -1;
@@ -523,7 +525,7 @@ static int update_one(struct cache_tree *it,
int cache_tree_update(struct index_state *istate, int flags)
{
- int inflight = !!the_repository->objects->transaction;
+ int inflight = !!istate->repo->objects->transaction;
struct odb_transaction *transaction;
int skip, i;
@@ -535,14 +537,14 @@ int cache_tree_update(struct index_state *istate, int flags)
if (!istate->cache_tree)
istate->cache_tree = cache_tree();
- if (!(flags & WRITE_TREE_MISSING_OK) && repo_has_promisor_remote(the_repository))
- prefetch_cache_entries(istate, must_check_existence);
+ if (!(flags & WRITE_TREE_MISSING_OK) && repo_has_promisor_remote(istate->repo))
+ prefetch_cache_entries(istate, must_check_existence, istate->repo);
trace_performance_enter();
trace2_region_enter("cache_tree", "update", istate->repo);
if (!inflight)
- odb_transaction_begin_or_die(the_repository->objects, &transaction, 0);
- i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
+ odb_transaction_begin_or_die(istate->repo->objects, &transaction, 0);
+ i = update_one(istate->repo, istate->cache_tree, istate->cache, istate->cache_nr,
"", 0, &skip, flags);
if (!inflight)
odb_transaction_commit_and_finalize_or_die(transaction);
@@ -554,7 +556,8 @@ int cache_tree_update(struct index_state *istate, int flags)
return 0;
}
-static void write_one(struct strbuf *buffer, struct cache_tree *it,
+static void write_one(struct repository *repo,
+ struct strbuf *buffer, struct cache_tree *it,
const char *path, int pathlen)
{
int i;
@@ -580,7 +583,7 @@ static void write_one(struct strbuf *buffer, struct cache_tree *it,
#endif
if (0 <= it->entry_count) {
- strbuf_add(buffer, it->oid.hash, the_hash_algo->rawsz);
+ strbuf_add(buffer, it->oid.hash, repo->hash_algo->rawsz);
}
for (i = 0; i < it->subtree_nr; i++) {
struct cache_tree_sub *down = it->down[i];
@@ -590,15 +593,16 @@ static void write_one(struct strbuf *buffer, struct cache_tree *it,
prev->name, prev->namelen) <= 0)
die("fatal - unsorted cache subtree");
}
- write_one(buffer, down->cache_tree, down->name, down->namelen);
+ write_one(repo, buffer, down->cache_tree, down->name, down->namelen);
}
}
-void cache_tree_write(struct strbuf *sb, struct cache_tree *root)
+void cache_tree_write(struct repository *repo,
+ struct strbuf *sb, struct cache_tree *root)
{
- trace2_region_enter("cache_tree", "write", the_repository);
- write_one(sb, root, "", 0);
- trace2_region_leave("cache_tree", "write", the_repository);
+ trace2_region_enter("cache_tree", "write", repo);
+ write_one(repo, sb, root, "", 0);
+ trace2_region_leave("cache_tree", "write", repo);
}
static int parse_int(const char **ptr, unsigned long *len_p, int *out)
@@ -632,13 +636,14 @@ static int parse_int(const char **ptr, unsigned long *len_p, int *out)
return 0;
}
-static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
+static struct cache_tree *read_one(struct repository *repo,
+ const char **buffer, unsigned long *size_p)
{
const char *buf = *buffer;
unsigned long size = *size_p;
struct cache_tree *it;
int i, subtree_nr;
- const unsigned rawsz = the_hash_algo->rawsz;
+ const unsigned rawsz = repo->hash_algo->rawsz;
it = NULL;
/* skip name, but make sure name exists */
@@ -665,7 +670,7 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
if (size < rawsz)
goto free_return;
oidread(&it->oid, (const unsigned char *)buf,
- the_repository->hash_algo);
+ repo->hash_algo);
buf += rawsz;
size -= rawsz;
}
@@ -693,7 +698,7 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
struct cache_tree_sub *subtree;
const char *name = buf;
- sub = read_one(&buf, &size);
+ sub = read_one(repo, &buf, &size);
if (!sub)
goto free_return;
subtree = cache_tree_sub(it, name);
@@ -710,16 +715,17 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
return NULL;
}
-struct cache_tree *cache_tree_read(const char *buffer, unsigned long size)
+struct cache_tree *cache_tree_read(struct repository *repo,
+ const char *buffer, unsigned long size)
{
struct cache_tree *result;
if (buffer[0])
return NULL; /* not the whole tree */
- trace2_region_enter("cache_tree", "read", the_repository);
- result = read_one(&buffer, &size);
- trace2_region_leave("cache_tree", "read", the_repository);
+ trace2_region_enter("cache_tree", "read", repo);
+ result = read_one(repo, &buffer, &size);
+ trace2_region_leave("cache_tree", "read", repo);
return result;
}
@@ -810,7 +816,7 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR);
entries = read_index_from(index_state, index_path,
- repo_get_git_dir(the_repository));
+ repo_get_git_dir(index_state->repo));
if (entries < 0) {
ret = WRITE_TREE_UNREADABLE_INDEX;
goto out;
@@ -866,7 +872,7 @@ static void prime_cache_tree_rec(struct repository *r,
struct cache_tree_sub *sub;
struct tree *subtree = lookup_tree(r, &entry.oid);
- if (repo_parse_tree(the_repository, subtree) < 0)
+ if (repo_parse_tree(r, subtree) < 0)
exit(128);
sub = cache_tree_sub(it, entry.path);
sub->cache_tree = cache_tree();
diff --git a/cache-tree.h b/cache-tree.h
index 4b3f60d6db..7a2177de83 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -28,8 +28,9 @@ struct cache_tree_sub *cache_tree_sub(struct cache_tree *, const char *);
int cache_tree_subtree_pos(struct cache_tree *it, const char *path, int pathlen);
-void cache_tree_write(struct strbuf *, struct cache_tree *root);
-struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);
+void cache_tree_write(struct repository *repo, struct strbuf *, struct cache_tree *root);
+struct cache_tree *cache_tree_read(struct repository *repo,
+ const char *buffer, unsigned long size);
int cache_tree_fully_valid(struct index_state *);
int cache_tree_update(struct index_state *, int);
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 8eb266cfd1..066dd8bc3b 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -269,9 +269,10 @@ void validate_cache_entries(const struct index_state *istate);
* the given predicate. This function should only be called if
* repo_has_promisor_remote() returns true.
*/
-typedef int (*must_prefetch_predicate)(const struct cache_entry *);
+typedef int (*must_prefetch_predicate)(const struct cache_entry *, void *cb_data);
void prefetch_cache_entries(const struct index_state *istate,
- must_prefetch_predicate must_prefetch);
+ must_prefetch_predicate must_prefetch,
+ void *cb_data);
/* Initialize and use the cache information */
struct lock_file;
diff --git a/read-cache.c b/read-cache.c
index 8044ff820b..e40f290bb3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1748,7 +1748,7 @@ static int read_index_extension(struct index_state *istate,
{
switch (CACHE_EXT(ext)) {
case CACHE_EXT_TREE:
- istate->cache_tree = cache_tree_read(data, sz);
+ istate->cache_tree = cache_tree_read(istate->repo, data, sz);
break;
case CACHE_EXT_RESOLVE_UNDO:
istate->resolve_undo = resolve_undo_read(data, sz, the_hash_algo);
@@ -3012,7 +3012,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
!drop_cache_tree && istate->cache_tree) {
strbuf_reset(&sb);
- cache_tree_write(&sb, istate->cache_tree);
+ cache_tree_write(istate->repo, &sb, istate->cache_tree);
err = write_index_ext_header(f, eoie_c, CACHE_EXT_TREE, sb.len) < 0;
hashwrite(f, sb.buf, sb.len);
if (err) {
@@ -3733,7 +3733,8 @@ static void write_ieot_extension(struct strbuf *sb, struct index_entry_offset_ta
}
void prefetch_cache_entries(const struct index_state *istate,
- must_prefetch_predicate must_prefetch)
+ must_prefetch_predicate must_prefetch,
+ void *cb_data)
{
int i;
struct oid_array to_fetch = OID_ARRAY_INIT;
@@ -3741,7 +3742,7 @@ void prefetch_cache_entries(const struct index_state *istate,
for (i = 0; i < istate->cache_nr; i++) {
struct cache_entry *ce = istate->cache[i];
- if (S_ISGITLINK(ce->ce_mode) || !must_prefetch(ce))
+ if (S_ISGITLINK(ce->ce_mode) || !must_prefetch(ce, cb_data))
continue;
if (!odb_read_object_info_extended(the_repository->objects,
&ce->oid, NULL,
diff --git a/unpack-trees.c b/unpack-trees.c
index f6bb1e6d2b..1802809ad3 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -416,7 +416,8 @@ static void report_collided_checkout(struct index_state *index)
string_list_clear(&list, 0);
}
-static int must_checkout(const struct cache_entry *ce)
+static int must_checkout(const struct cache_entry *ce,
+ void *cb_data UNUSED)
{
return ce->ce_flags & CE_UPDATE;
}
@@ -477,7 +478,7 @@ static int check_updates(struct unpack_trees_options *o,
* Prefetch the objects that are to be checked out in the loop
* below.
*/
- prefetch_cache_entries(index, must_checkout);
+ prefetch_cache_entries(index, must_checkout, NULL);
get_parallel_checkout_configs(&pc_workers, &pc_threshold);
@@ -487,7 +488,7 @@ static int check_updates(struct unpack_trees_options *o,
for (i = 0; i < index->cache_nr; i++) {
struct cache_entry *ce = index->cache[i];
- if (must_checkout(ce)) {
+ if (must_checkout(ce, NULL)) {
size_t last_pc_queue_size = pc_queue_size();
if (ce->ce_flags & CE_WT_REMOVE)
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v2 02/13] cache-tree: remove dependency on `the_repository`
2026-09-02 13:34 ` [PATCH v2 02/13] cache-tree: remove dependency on `the_repository` Patrick Steinhardt
@ 2026-09-04 22:28 ` Karthik Nayak
2026-09-07 7:49 ` Patrick Steinhardt
0 siblings, 1 reply; 54+ messages in thread
From: Karthik Nayak @ 2026-09-04 22:28 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Junio C Hamano
[-- Attachment #1: Type: text/plain, Size: 1558 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> The "cache-tree" subsystem still depends on `the_repository`. Adapt it
> to instead use repositories provided via the context, either as a new
> parameter or the one passed in via `struct index_state`.
>
> Besides getting rid of `the_repository`, this also removes the last
> dependency on registering submodule sources with the main object
> database. When reading gitmodules from a submodule's index we implicitly
> read that object via `the_repository`'s object database, which is of
> course wrong. This works though because we would then register the
> submodule's object database with the main object database, but a later
> patch is going to get rid of that mechanism.
>
> You can verify that we indeed no longer depend on this mechanism by
> running tests with `GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=true`. Without
> this patch we fail in t1092, with this patch we never register submodule
> object databases anymore.
>
Interesting, to sum up if I understand correctly, somewhere in the call
chain of looking up the gitmodules from the submodules index, we end up
using 'the_repository' instead of the submodule's repo structure.
This is of course wrong, because we use the wrong repo, the consequence
is that the lookup fails, but we have a last ditch effort of adding all
submodules as alternates and retrying the object read, this succeeds.
So this patch fixes the repository being correctly passed down. Meaning
we no longer need to add the submodules as an alternate.
[snip]
The changes look good.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2 02/13] cache-tree: remove dependency on `the_repository`
2026-09-04 22:28 ` Karthik Nayak
@ 2026-09-07 7:49 ` Patrick Steinhardt
0 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-07 7:49 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, Junio C Hamano
On Fri, Sep 04, 2026 at 03:28:07PM -0700, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > The "cache-tree" subsystem still depends on `the_repository`. Adapt it
> > to instead use repositories provided via the context, either as a new
> > parameter or the one passed in via `struct index_state`.
> >
> > Besides getting rid of `the_repository`, this also removes the last
> > dependency on registering submodule sources with the main object
> > database. When reading gitmodules from a submodule's index we implicitly
> > read that object via `the_repository`'s object database, which is of
> > course wrong. This works though because we would then register the
> > submodule's object database with the main object database, but a later
> > patch is going to get rid of that mechanism.
> >
> > You can verify that we indeed no longer depend on this mechanism by
> > running tests with `GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=true`. Without
> > this patch we fail in t1092, with this patch we never register submodule
> > object databases anymore.
> >
>
> Interesting, to sum up if I understand correctly, somewhere in the call
> chain of looking up the gitmodules from the submodules index, we end up
> using 'the_repository' instead of the submodule's repo structure.
>
> This is of course wrong, because we use the wrong repo, the consequence
> is that the lookup fails, but we have a last ditch effort of adding all
> submodules as alternates and retrying the object read, this succeeds.
>
> So this patch fixes the repository being correctly passed down. Meaning
> we no longer need to add the submodules as an alternate.
Yup, exactly this.
Patrick
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v2 03/13] submodule-config: remove uses of `the_repository`
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
2026-09-02 13:34 ` [PATCH v2 01/13] cache-tree: drop `the_repository` in `cache_tree_fully_valid()` Patrick Steinhardt
2026-09-02 13:34 ` [PATCH v2 02/13] cache-tree: remove dependency on `the_repository` Patrick Steinhardt
@ 2026-09-02 13:34 ` Patrick Steinhardt
2026-09-02 13:34 ` [PATCH v2 04/13] submodule-config: stop using `the_hash_algo` Patrick Steinhardt
` (10 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-02 13:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Several functions in the submodule-config subsystem implicitly depend
on `the_repository`. Refactor these to take a `struct repository` as
parameter and adapt callers accordingly.
Note that as usual with these refactorings, callers simply pass
`the_repository` even if they already have a different repository
available in the calling context. This simplifies the migration and
ensures that we don't have a change in behaviour.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/fetch.c | 2 +-
builtin/grep.c | 2 +-
builtin/submodule--helper.c | 8 ++++----
submodule-config.c | 49 ++++++++++++++++++++++++++-------------------
submodule-config.h | 12 +++++++----
submodule.c | 2 +-
t/helper/test-submodule.c | 4 ++--
7 files changed, 45 insertions(+), 34 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index ab7db2be06..533fdfe7d8 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -2681,7 +2681,7 @@ int cmd_fetch(int argc,
int *rs = config.recurse_submodules == RECURSE_SUBMODULES_DEFAULT
? &config.recurse_submodules : NULL;
- fetch_config_from_gitmodules(sfjc, rs);
+ fetch_config_from_gitmodules(the_repository, sfjc, rs);
}
diff --git a/builtin/grep.c b/builtin/grep.c
index d3d86abe01..073dfaaf45 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -897,7 +897,7 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,
if (recurse_submodules) {
submodule_free(opt->repo);
obj_read_lock();
- gitmodules_config_oid(&real_obj->oid);
+ gitmodules_config_oid(the_repository, &real_obj->oid);
obj_read_unlock();
}
if (grep_object(opt, pathspec, real_obj, list->objects[i].name,
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index e7cd3225fa..aaaa963fd8 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -3041,7 +3041,7 @@ static int module_update(int argc, const char **argv, const char *prefix,
NULL
};
- update_clone_config_from_gitmodules(&opt.max_jobs);
+ update_clone_config_from_gitmodules(the_repository, &opt.max_jobs);
repo_config(the_repository, git_update_clone_config, &opt.max_jobs);
argc = parse_options(argc, argv, prefix, module_update_options,
@@ -3255,7 +3255,7 @@ static int module_set_url(int argc, const char **argv, const char *prefix,
path);
config_name = xstrfmt("submodule.%s.url", sub->name);
- ret = config_set_in_gitmodules_file_gently(config_name, newurl);
+ ret = config_set_in_gitmodules_file_gently(the_repository, config_name, newurl);
if (!ret) {
repo_read_gitmodules(the_repository, 0);
@@ -3311,7 +3311,7 @@ static int module_set_branch(int argc, const char **argv, const char *prefix,
path);
config_name = xstrfmt("submodule.%s.branch", sub->name);
- ret = config_set_in_gitmodules_file_gently(config_name, opt_branch);
+ ret = config_set_in_gitmodules_file_gently(the_repository, config_name, opt_branch);
free(config_name);
return !!ret;
@@ -3510,7 +3510,7 @@ static int config_submodule_in_gitmodules(const char *name, const char *var, con
die(_("please make sure that the .gitmodules file is in the working tree"));
key = xstrfmt("submodule.%s.%s", name, var);
- ret = config_set_in_gitmodules_file_gently(key, value);
+ ret = config_set_in_gitmodules_file_gently(the_repository, key, value);
free(key);
return ret;
diff --git a/submodule-config.c b/submodule-config.c
index f75997402a..f8c2cf7a93 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -667,19 +667,20 @@ static int parse_config(const char *var, const char *value,
return ret;
}
-static int gitmodule_oid_from_commit(const struct object_id *treeish_name,
+static int gitmodule_oid_from_commit(struct repository *repo,
+ const struct object_id *treeish_name,
struct object_id *gitmodules_oid,
struct strbuf *rev)
{
int ret = 0;
if (is_null_oid(treeish_name)) {
- oidclr(gitmodules_oid, the_repository->hash_algo);
+ oidclr(gitmodules_oid, repo->hash_algo);
return 1;
}
strbuf_addf(rev, "%s:.gitmodules", oid_to_hex(treeish_name));
- if (repo_get_oid(the_repository, rev->buf, gitmodules_oid) >= 0)
+ if (repo_get_oid(repo, rev->buf, gitmodules_oid) >= 0)
ret = 1;
return ret;
@@ -689,9 +690,11 @@ static int gitmodule_oid_from_commit(const struct object_id *treeish_name,
* (key) with on-demand reading of the appropriate .gitmodules from
* revisions.
*/
-static const struct submodule *config_from(struct submodule_cache *cache,
- const struct object_id *treeish_name, const char *key,
- enum lookup_type lookup_type)
+static const struct submodule *config_from(struct repository *repo,
+ struct submodule_cache *cache,
+ const struct object_id *treeish_name,
+ const char *key,
+ enum lookup_type lookup_type)
{
struct strbuf rev = STRBUF_INIT;
size_t config_size;
@@ -718,7 +721,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
return entry->config;
}
- if (!gitmodule_oid_from_commit(treeish_name, &oid, &rev))
+ if (!gitmodule_oid_from_commit(repo, treeish_name, &oid, &rev))
goto out;
switch (lookup_type) {
@@ -732,7 +735,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
if (submodule)
goto out;
- config = odb_read_object(the_repository->objects, &oid,
+ config = odb_read_object(repo->objects, &oid,
&type, &config_size);
if (!config || type != OBJ_BLOB)
goto out;
@@ -843,21 +846,22 @@ void repo_read_gitmodules(struct repository *repo, int skip_if_read)
repo->submodule_cache->gitmodules_read = 1;
}
-void gitmodules_config_oid(const struct object_id *commit_oid)
+void gitmodules_config_oid(struct repository *repo,
+ const struct object_id *commit_oid)
{
struct strbuf rev = STRBUF_INIT;
struct object_id oid;
- submodule_cache_check_init(the_repository);
+ submodule_cache_check_init(repo);
- if (gitmodule_oid_from_commit(commit_oid, &oid, &rev)) {
+ if (gitmodule_oid_from_commit(repo, commit_oid, &oid, &rev)) {
git_config_from_blob_oid(gitmodules_cb, rev.buf,
- the_repository, &oid, the_repository,
+ repo, &oid, repo,
CONFIG_SCOPE_UNKNOWN);
}
strbuf_release(&rev);
- the_repository->submodule_cache->gitmodules_read = 1;
+ repo->submodule_cache->gitmodules_read = 1;
}
const struct submodule *submodule_from_name(struct repository *r,
@@ -865,7 +869,7 @@ const struct submodule *submodule_from_name(struct repository *r,
const char *name)
{
repo_read_gitmodules(r, 1);
- return config_from(r->submodule_cache, treeish_name, name, lookup_name);
+ return config_from(r, r->submodule_cache, treeish_name, name, lookup_name);
}
const struct submodule *submodule_from_path(struct repository *r,
@@ -873,7 +877,7 @@ const struct submodule *submodule_from_path(struct repository *r,
const char *path)
{
repo_read_gitmodules(r, 1);
- return config_from(r->submodule_cache, treeish_name, path, lookup_path);
+ return config_from(r, r->submodule_cache, treeish_name, path, lookup_path);
}
/**
@@ -980,11 +984,12 @@ int print_config_from_gitmodules(struct repository *repo, const char *key)
return 0;
}
-int config_set_in_gitmodules_file_gently(const char *key, const char *value)
+int config_set_in_gitmodules_file_gently(struct repository *repo,
+ const char *key, const char *value)
{
int ret;
- ret = repo_config_set_in_file_gently(the_repository, GITMODULES_FILE, key, NULL, value);
+ ret = repo_config_set_in_file_gently(repo, GITMODULES_FILE, key, NULL, value);
if (ret < 0)
/* Maybe the user already did that, don't error out here */
warning(_("Could not update .gitmodules entry %s"), key);
@@ -1017,13 +1022,15 @@ static int gitmodules_fetch_config(const char *var, const char *value,
return 0;
}
-void fetch_config_from_gitmodules(int *max_children, int *recurse_submodules)
+void fetch_config_from_gitmodules(struct repository *repo,
+ int *max_children,
+ int *recurse_submodules)
{
struct fetch_config config = {
.max_children = max_children,
.recurse_submodules = recurse_submodules
};
- config_from_gitmodules(gitmodules_fetch_config, the_repository, &config);
+ config_from_gitmodules(gitmodules_fetch_config, repo, &config);
}
static int gitmodules_update_clone_config(const char *var, const char *value,
@@ -1036,7 +1043,7 @@ static int gitmodules_update_clone_config(const char *var, const char *value,
return 0;
}
-void update_clone_config_from_gitmodules(int *max_jobs)
+void update_clone_config_from_gitmodules(struct repository *repo, int *max_jobs)
{
- config_from_gitmodules(gitmodules_update_clone_config, the_repository, max_jobs);
+ config_from_gitmodules(gitmodules_update_clone_config, repo, max_jobs);
}
diff --git a/submodule-config.h b/submodule-config.h
index f55d4e3b61..755570d5d1 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -57,7 +57,8 @@ int option_fetch_parse_recurse_submodules(const struct option *opt,
int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
int parse_push_recurse_submodules_arg(const char *opt, const char *arg);
void repo_read_gitmodules(struct repository *repo, int skip_if_read);
-void gitmodules_config_oid(const struct object_id *commit_oid);
+void gitmodules_config_oid(struct repository *repo,
+ const struct object_id *commit_oid);
/**
* Same as submodule_from_path but lookup by name.
@@ -80,7 +81,8 @@ const struct submodule *submodule_from_path(struct repository *r,
void submodule_free(struct repository *r);
int print_config_from_gitmodules(struct repository *repo, const char *key);
-int config_set_in_gitmodules_file_gently(const char *key, const char *value);
+int config_set_in_gitmodules_file_gently(struct repository *repo,
+ const char *key, const char *value);
/*
* Returns 0 if the name is syntactically acceptable as a submodule "name"
@@ -100,8 +102,10 @@ int check_submodule_url(const char *url);
* New helpers to retrieve arbitrary configuration from the '.gitmodules' file
* should NOT be added.
*/
-void fetch_config_from_gitmodules(int *max_children, int *recurse_submodules);
-void update_clone_config_from_gitmodules(int *max_jobs);
+void fetch_config_from_gitmodules(struct repository *repo,
+ int *max_children,
+ int *recurse_submodules);
+void update_clone_config_from_gitmodules(struct repository *repo, int *max_jobs);
/*
* Submodule entry that contains relevant information about a
diff --git a/submodule.c b/submodule.c
index 5c92575888..6fcb606f7e 100644
--- a/submodule.c
+++ b/submodule.c
@@ -133,7 +133,7 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)
strbuf_addstr(&entry, "submodule.");
strbuf_addstr(&entry, submodule->name);
strbuf_addstr(&entry, ".path");
- ret = config_set_in_gitmodules_file_gently(entry.buf, newpath);
+ ret = config_set_in_gitmodules_file_gently(the_repository, entry.buf, newpath);
strbuf_release(&entry);
return ret;
}
diff --git a/t/helper/test-submodule.c b/t/helper/test-submodule.c
index 3c5c4c4a09..ea9bef0904 100644
--- a/t/helper/test-submodule.c
+++ b/t/helper/test-submodule.c
@@ -168,7 +168,7 @@ static int cmd__submodule_config_set(int argc, const char **argv)
if (!is_writing_gitmodules_ok())
die("please make sure that the .gitmodules file is in the working tree");
- return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
+ return config_set_in_gitmodules_file_gently(the_repository, argv[1], argv[2]);
}
usage_with_options(usage, options);
}
@@ -188,7 +188,7 @@ static int cmd__submodule_config_unset(int argc, const char **argv)
if (argc == 2) {
if (!is_writing_gitmodules_ok())
die("please make sure that the .gitmodules file is in the working tree");
- return config_set_in_gitmodules_file_gently(argv[1], NULL);
+ return config_set_in_gitmodules_file_gently(the_repository, argv[1], NULL);
}
usage_with_options(usage, options);
}
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v2 04/13] submodule-config: stop using `the_hash_algo`
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
` (2 preceding siblings ...)
2026-09-02 13:34 ` [PATCH v2 03/13] submodule-config: remove uses of `the_repository` Patrick Steinhardt
@ 2026-09-02 13:34 ` Patrick Steinhardt
2026-09-02 13:34 ` [PATCH v2 05/13] submodule-config: stop registering submodule sources Patrick Steinhardt
` (9 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-02 13:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
We have two uses of `the_hash_algo` in "submodule-config.c":
- One trivial use in `gitmodules_cb`, which we can convert to use the
hash algorithm of the repository that's already available in the
caller's context.
- One use where we compute the hashmap key of an object ID. We should
only ever get valid, populated object IDs here, and consequently we
can easily adapt that function to use the hash algorithm of the
passed-in object ID.
Adapt both sites accordingly. Safeguard us against the case where the
passed-in object ID is _not_ properly initialized. While this case
shouldn't ever happen, it doesn't hurt to be defensive.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
submodule-config.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index f8c2cf7a93..7c73fa108b 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -133,7 +133,9 @@ void submodule_cache_free(struct submodule_cache *cache)
static unsigned int hash_oid_string(const struct object_id *oid,
const char *string)
{
- return memhash(oid->hash, the_hash_algo->rawsz) + strhash(string);
+ if (oid->algo == GIT_HASH_UNKNOWN)
+ BUG("hashing an object ID with unknown algorithm");
+ return memhash(oid->hash, hash_algos[oid->algo].rawsz) + strhash(string);
}
static void cache_put_path(struct submodule_cache *cache,
@@ -824,7 +826,7 @@ static int gitmodules_cb(const char *var, const char *value,
parameter.cache = repo->submodule_cache;
parameter.treeish_name = NULL;
- parameter.gitmodules_oid = null_oid(the_hash_algo);
+ parameter.gitmodules_oid = null_oid(repo->hash_algo);
parameter.overwrite = 1;
return parse_config(var, value, ctx, ¶meter);
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v2 05/13] submodule-config: stop registering submodule sources
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
` (3 preceding siblings ...)
2026-09-02 13:34 ` [PATCH v2 04/13] submodule-config: stop using `the_hash_algo` Patrick Steinhardt
@ 2026-09-02 13:34 ` Patrick Steinhardt
2026-09-06 18:38 ` Justin Tobler
2026-09-02 13:34 ` [PATCH v2 06/13] builtin/grep: stop registering submodule ODB as source Patrick Steinhardt
` (8 subsequent siblings)
13 siblings, 1 reply; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-02 13:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
When reading the ".gitmodules" file from a blob in a repository other
than `the_repository`, we register the repository's object database as
an in-memory source of `the_repository`'s object database. This call has
its origins in d9b8b8f896 (submodule-config.c: use repo_get_oid for
reading .gitmodules, 2019-04-16): back then, `config_with_options()` was
not able to read a blob from an arbitrary repository, but would always
read it via `the_repository`. So even though the blob could be resolved
in the submodule repository via `repo_get_oid()`, the submodule's object
database had to be registered as an in-memory source of `the_repository`
so that the subsequent object read was able to find the blob at all.
That need went away with e3e8bf046e (submodule-config: pass repo
upon blob config read, 2021-08-16), which taught the config machinery
to read the blob from the repository we pass to it. The same series
converted the eager submodule source registration into a lazy mechanism
that only registers submodule sources with the object database when an
object lookup failed. The intent though was that we don't ever have to
fall back to this mechanism in the first place, and to verify that this
is the case we introduced GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB. If set,
then any such lazy registration would cause us to BUG.
At the beginning of this series, we still triggered this bug in t1092.
But now that we have converted the "cache-tree" subsystem to not depend
on `the_repository` anymore it also knows to properly access objects via
the submodule. With that change, GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
does not cause any failures anymore.
Remove the call to `odb_add_submodule_source_by_path()`. This removes
the last user of `the_repository`, so at the same time we can also get
rid of `USE_THE_REPOSITORY_VARIABLE`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
submodule-config.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index 7c73fa108b..37c3be377b 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -803,9 +802,6 @@ static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void
} else if (repo_get_oid(repo, GITMODULES_INDEX, &oid) >= 0 ||
repo_get_oid(repo, GITMODULES_HEAD, &oid) >= 0) {
config_source.blob = oidstr = xstrdup(oid_to_hex(&oid));
- if (repo != the_repository)
- odb_add_submodule_source_by_path(the_repository->objects,
- repo->objects->sources->path);
} else {
goto out;
}
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v2 05/13] submodule-config: stop registering submodule sources
2026-09-02 13:34 ` [PATCH v2 05/13] submodule-config: stop registering submodule sources Patrick Steinhardt
@ 2026-09-06 18:38 ` Justin Tobler
2026-09-07 7:50 ` Patrick Steinhardt
0 siblings, 1 reply; 54+ messages in thread
From: Justin Tobler @ 2026-09-06 18:38 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano
On 26/09/02 03:34PM, Patrick Steinhardt wrote:
> When reading the ".gitmodules" file from a blob in a repository other
> than `the_repository`, we register the repository's object database as
> an in-memory source of `the_repository`'s object database. This call has
> its origins in d9b8b8f896 (submodule-config.c: use repo_get_oid for
> reading .gitmodules, 2019-04-16): back then, `config_with_options()` was
> not able to read a blob from an arbitrary repository, but would always
> read it via `the_repository`. So even though the blob could be resolved
> in the submodule repository via `repo_get_oid()`, the submodule's object
> database had to be registered as an in-memory source of `the_repository`
> so that the subsequent object read was able to find the blob at all.
Ok, so IIUC the problem was that a gitmodule blob from a repository that
is not "the_repository" would fail to be read by `config_with_options()`
because it would only read objects from "the_repository". The workaround
was to add the other repositories as another source to ensure the
gitmodule blob could be read. I had to reread the commit message a
couple of times to follow, but I think it makes sense now.
> That need went away with e3e8bf046e (submodule-config: pass repo
> upon blob config read, 2021-08-16), which taught the config machinery
> to read the blob from the repository we pass to it. The same series
> converted the eager submodule source registration into a lazy mechanism
> that only registers submodule sources with the object database when an
> object lookup failed. The intent though was that we don't ever have to
> fall back to this mechanism in the first place, and to verify that this
> is the case we introduced GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB. If set,
> then any such lazy registration would cause us to BUG.
>
> At the beginning of this series, we still triggered this bug in t1092.
> But now that we have converted the "cache-tree" subsystem to not depend
> on `the_repository` anymore it also knows to properly access objects via
> the submodule. With that change, GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
> does not cause any failures anymore.
Ok, now we can properly read objects from their respective repository
source and no longer neeed the workaround.
> Remove the call to `odb_add_submodule_source_by_path()`. This removes
> the last user of `the_repository`, so at the same time we can also get
> rid of `USE_THE_REPOSITORY_VARIABLE`.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> submodule-config.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/submodule-config.c b/submodule-config.c
> index 7c73fa108b..37c3be377b 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -1,4 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
> #define DISABLE_SIGN_COMPARE_WARNINGS
>
> #include "git-compat-util.h"
> @@ -803,9 +802,6 @@ static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void
> } else if (repo_get_oid(repo, GITMODULES_INDEX, &oid) >= 0 ||
> repo_get_oid(repo, GITMODULES_HEAD, &oid) >= 0) {
> config_source.blob = oidstr = xstrdup(oid_to_hex(&oid));
> - if (repo != the_repository)
> - odb_add_submodule_source_by_path(the_repository->objects,
> - repo->objects->sources->path);
Nice to have one less workaround. :)
-Justin
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PATCH v2 05/13] submodule-config: stop registering submodule sources
2026-09-06 18:38 ` Justin Tobler
@ 2026-09-07 7:50 ` Patrick Steinhardt
2026-09-08 23:04 ` Justin Tobler
0 siblings, 1 reply; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-07 7:50 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, Junio C Hamano
On Sun, Sep 06, 2026 at 01:38:15PM -0500, Justin Tobler wrote:
> On 26/09/02 03:34PM, Patrick Steinhardt wrote:
> > When reading the ".gitmodules" file from a blob in a repository other
> > than `the_repository`, we register the repository's object database as
> > an in-memory source of `the_repository`'s object database. This call has
> > its origins in d9b8b8f896 (submodule-config.c: use repo_get_oid for
> > reading .gitmodules, 2019-04-16): back then, `config_with_options()` was
> > not able to read a blob from an arbitrary repository, but would always
> > read it via `the_repository`. So even though the blob could be resolved
> > in the submodule repository via `repo_get_oid()`, the submodule's object
> > database had to be registered as an in-memory source of `the_repository`
> > so that the subsequent object read was able to find the blob at all.
>
> Ok, so IIUC the problem was that a gitmodule blob from a repository that
> is not "the_repository" would fail to be read by `config_with_options()`
> because it would only read objects from "the_repository". The workaround
> was to add the other repositories as another source to ensure the
> gitmodule blob could be read. I had to reread the commit message a
> couple of times to follow, but I think it makes sense now.
Yes. Do you think I should rewrite the commit message to make it a bit
less dense? Otherwise I'll leave it as-is for now.
Patrick
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2 05/13] submodule-config: stop registering submodule sources
2026-09-07 7:50 ` Patrick Steinhardt
@ 2026-09-08 23:04 ` Justin Tobler
2026-09-09 5:52 ` Patrick Steinhardt
0 siblings, 1 reply; 54+ messages in thread
From: Justin Tobler @ 2026-09-08 23:04 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano
On 26/09/07 09:50AM, Patrick Steinhardt wrote:
> On Sun, Sep 06, 2026 at 01:38:15PM -0500, Justin Tobler wrote:
> > On 26/09/02 03:34PM, Patrick Steinhardt wrote:
> > > When reading the ".gitmodules" file from a blob in a repository other
> > > than `the_repository`, we register the repository's object database as
> > > an in-memory source of `the_repository`'s object database. This call has
> > > its origins in d9b8b8f896 (submodule-config.c: use repo_get_oid for
> > > reading .gitmodules, 2019-04-16): back then, `config_with_options()` was
> > > not able to read a blob from an arbitrary repository, but would always
> > > read it via `the_repository`. So even though the blob could be resolved
> > > in the submodule repository via `repo_get_oid()`, the submodule's object
> > > database had to be registered as an in-memory source of `the_repository`
> > > so that the subsequent object read was able to find the blob at all.
> >
> > Ok, so IIUC the problem was that a gitmodule blob from a repository that
> > is not "the_repository" would fail to be read by `config_with_options()`
> > because it would only read objects from "the_repository". The workaround
> > was to add the other repositories as another source to ensure the
> > gitmodule blob could be read. I had to reread the commit message a
> > couple of times to follow, but I think it makes sense now.
>
> Yes. Do you think I should rewrite the commit message to make it a bit
> less dense? Otherwise I'll leave it as-is for now.
It is a little dense. I think it is a bit confusing which "repository"
we are talking about in the first paragraph. I think:
...we register _the_ repository's object database...
is a bit ambiguous since "the repository" is actually a repository that
is _not_ `the_repository`. At least for me, I was getting a bit tripped
up. XD
-Justin
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2 05/13] submodule-config: stop registering submodule sources
2026-09-08 23:04 ` Justin Tobler
@ 2026-09-09 5:52 ` Patrick Steinhardt
0 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-09 5:52 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, Junio C Hamano
On Tue, Sep 08, 2026 at 06:04:54PM -0500, Justin Tobler wrote:
> On 26/09/07 09:50AM, Patrick Steinhardt wrote:
> > On Sun, Sep 06, 2026 at 01:38:15PM -0500, Justin Tobler wrote:
> > > On 26/09/02 03:34PM, Patrick Steinhardt wrote:
> > > > When reading the ".gitmodules" file from a blob in a repository other
> > > > than `the_repository`, we register the repository's object database as
> > > > an in-memory source of `the_repository`'s object database. This call has
> > > > its origins in d9b8b8f896 (submodule-config.c: use repo_get_oid for
> > > > reading .gitmodules, 2019-04-16): back then, `config_with_options()` was
> > > > not able to read a blob from an arbitrary repository, but would always
> > > > read it via `the_repository`. So even though the blob could be resolved
> > > > in the submodule repository via `repo_get_oid()`, the submodule's object
> > > > database had to be registered as an in-memory source of `the_repository`
> > > > so that the subsequent object read was able to find the blob at all.
> > >
> > > Ok, so IIUC the problem was that a gitmodule blob from a repository that
> > > is not "the_repository" would fail to be read by `config_with_options()`
> > > because it would only read objects from "the_repository". The workaround
> > > was to add the other repositories as another source to ensure the
> > > gitmodule blob could be read. I had to reread the commit message a
> > > couple of times to follow, but I think it makes sense now.
> >
> > Yes. Do you think I should rewrite the commit message to make it a bit
> > less dense? Otherwise I'll leave it as-is for now.
>
> It is a little dense. I think it is a bit confusing which "repository"
> we are talking about in the first paragraph. I think:
>
> ...we register _the_ repository's object database...
>
> is a bit ambiguous since "the repository" is actually a repository that
> is _not_ `the_repository`. At least for me, I was getting a bit tripped
> up. XD
How about we say "we register _that_ repository's object database"
instead then? I've queued that change locally, but I'll refrain from
sending a new version only to swap out that one word.
Thanks!
Patrick
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v2 06/13] builtin/grep: stop registering submodule ODB as source
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
` (4 preceding siblings ...)
2026-09-02 13:34 ` [PATCH v2 05/13] submodule-config: stop registering submodule sources Patrick Steinhardt
@ 2026-09-02 13:34 ` Patrick Steinhardt
2026-09-02 13:34 ` [PATCH v2 07/13] odb: remove infrastructure to register submodule sources Patrick Steinhardt
` (7 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-02 13:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Same as with the preceding commit, git-grep(1) registers each
submodule's object database as an in-memory source of the main object
database before grepping it. This was introduced as an eager alternate
registration and converted into the lazy mechanism via 8d33c3af0b (grep:
use submodule-ODB-as-alternate lazy-addition, 2021-08-16).
Starting with 0693806bf8 (grep: add repository to OID grep sources,
2021-08-16), the command instead knows to pass submodule repositories to
our workers, which means that those now use that repository to look up
objects, too. As a consequence, registering submodule sources as
alternates is not required anymore.
Remove the logic to register submodule sources. Unfortunately, this does
not allow us to get rid of the object read lock as initializing the
subrepository is still racy.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/grep.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 073dfaaf45..b045f8a488 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -463,16 +463,6 @@ static int grep_submodule(struct grep_opt *opt,
ALLOC_GROW(repos_to_free, repos_to_free_nr + 1, repos_to_free_alloc);
repos_to_free[repos_to_free_nr++] = subrepo;
- /*
- * NEEDSWORK: repo_read_gitmodules() might call
- * odb_add_to_alternates_memory() via config_from_gitmodules(). This
- * operation causes a race condition with concurrent object readings
- * performed by the worker threads. That's why we need obj_read_lock()
- * here. It should be removed once it's no longer necessary to add the
- * subrepo's odbs to the in-memory alternates list.
- */
- obj_read_lock();
-
/*
* NEEDSWORK: when reading a submodule, the sparsity settings in the
* superproject are incorrectly forgotten or misused. For example:
@@ -498,18 +488,14 @@ static int grep_submodule(struct grep_opt *opt,
* ditto.
*
* Note that this list is not exhaustive.
+ *
+ * NEEDSWORK: initializing the subrepository is not thread-safe,
+ * either, as it may cause us to race around `get_main_ref_store()`. We
+ * thus need to hold the object-read lock to serialize all readers with
+ * one another.
*/
+ obj_read_lock();
repo_read_gitmodules(subrepo, 0);
-
- /*
- * All code paths tested by test code no longer need submodule ODBs to
- * be added as alternates, but add it to the list just in case.
- * Submodule ODBs added through add_submodule_odb_by_path() will be
- * lazily registered as alternates when needed (and except in an
- * unexpected code interaction, it won't be needed).
- */
- odb_add_submodule_source_by_path(the_repository->objects,
- subrepo->objects->sources->path);
obj_read_unlock();
memcpy(&subopt, opt, sizeof(subopt));
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v2 07/13] odb: remove infrastructure to register submodule sources
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
` (5 preceding siblings ...)
2026-09-02 13:34 ` [PATCH v2 06/13] builtin/grep: stop registering submodule ODB as source Patrick Steinhardt
@ 2026-09-02 13:34 ` Patrick Steinhardt
2026-09-04 22:36 ` Karthik Nayak
2026-09-02 13:34 ` [PATCH v2 08/13] tmp-objdir: drop unused function to register alternate Patrick Steinhardt
` (6 subsequent siblings)
13 siblings, 1 reply; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-02 13:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
The preceding commits have removed the last two users of
`odb_add_submodule_source_by_path()`. The mechanism was only ever
meant as a transitional crutch while migrating submodule object
access away from "add the submodule ODB as an alternate of
the_repository" towards explicitly passing the submodule repository,
see a35e03dee0 (submodule: lazily add submodule ODBs as alternates,
2021-08-16). Remove it.
As GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB is now a no-op, remove its
documentation and the exports from the test suite, as well.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb.c | 36 ----------------------------------
odb.h | 14 -------------
t/README | 7 -------
t/t5526-fetch-submodules.sh | 3 ---
t/t5531-deep-submodule-push.sh | 3 ---
t/t5545-push-options.sh | 3 ---
t/t5572-pull-submodule.sh | 3 ---
t/t6437-submodule-merge.sh | 3 ---
t/t7418-submodule-sparse-gitmodules.sh | 3 ---
t/t7814-grep-recurse-submodules.sh | 3 ---
10 files changed, 78 deletions(-)
diff --git a/odb.c b/odb.c
index 6d5943e5ea..2f8a70a90c 100644
--- a/odb.c
+++ b/odb.c
@@ -388,12 +388,6 @@ struct odb_source *odb_find_source_or_die(struct object_database *odb, const cha
return source;
}
-void odb_add_submodule_source_by_path(struct object_database *odb,
- const char *path)
-{
- string_list_insert(&odb->submodule_source_paths, path);
-}
-
static void fill_alternate_refs_command(struct repository *repo,
struct child_process *cmd,
const char *repo_path)
@@ -549,23 +543,6 @@ void disable_obj_read_lock(void)
pthread_mutex_destroy(&obj_read_mutex);
}
-static int register_all_submodule_sources(struct object_database *odb)
-{
- int ret = odb->submodule_source_paths.nr;
-
- for (size_t i = 0; i < odb->submodule_source_paths.nr; i++)
- odb_add_to_alternates_memory(odb,
- odb->submodule_source_paths.items[i].string);
- if (ret) {
- string_list_clear(&odb->submodule_source_paths, 0);
- trace2_data_intmax("submodule", odb->repo,
- "register_all_submodule_sources/registered", ret);
- if (git_env_bool("GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB", 0))
- BUG("register_all_submodule_sources() called");
- }
- return ret;
-}
-
static enum odb_read_status do_oid_object_info_extended(struct object_database *odb,
const struct object_id *oid,
struct object_info *oi, unsigned flags)
@@ -614,16 +591,6 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database *
}
}
- /*
- * This might be an attempt at accessing a submodule object as
- * if it were in main object store (having called
- * `odb_add_submodule_source_by_path()` on that submodule's
- * ODB). If any such ODBs exist, register them and try again.
- */
- if (register_all_submodule_sources(odb))
- /* We added some alternates; retry */
- continue;
-
/* Check if it is a missing object */
if (odb->repo->fetch_if_missing && repo_has_promisor_remote(odb->repo) &&
!already_retried &&
@@ -1109,7 +1076,6 @@ struct object_database *odb_new(struct repository *repo,
CALLOC_ARRAY(o, 1);
o->repo = repo;
pthread_mutex_init(&o->replace_mutex, NULL);
- string_list_init_dup(&o->submodule_source_paths);
hashmap_init(&o->source_by_path, odb_source_by_path_cmp, o, 0);
o->source_paths_icase = -1;
@@ -1166,8 +1132,6 @@ void odb_free(struct object_database *o)
odb_close(o);
odb_free_sources(o);
- string_list_clear(&o->submodule_source_paths, 0);
-
free(o);
}
diff --git a/odb.h b/odb.h
index 248ee9cdfa..54548efc55 100644
--- a/odb.h
+++ b/odb.h
@@ -89,12 +89,6 @@ struct object_database {
unsigned long object_count;
unsigned object_count_flags;
unsigned object_count_valid : 1;
-
- /*
- * Submodule source paths that will be added as additional sources to
- * allow lookup of submodule objects via the main object database.
- */
- struct string_list submodule_source_paths;
};
enum odb_new_flags {
@@ -224,14 +218,6 @@ void odb_restore_primary_source(struct object_database *odb,
struct odb_source *restore_source,
const char *old_path);
-/*
- * Call odb_add_submodule_source_by_path() to add the submodule at the given
- * path to a list. The object stores of all submodules in that list will be
- * added as additional sources in the object store when looking up objects.
- */
-void odb_add_submodule_source_by_path(struct object_database *odb,
- const char *path);
-
/*
* Iterate through all alternates of the database and execute the provided
* callback function for each of them. Stop iterating once the callback
diff --git a/t/README b/t/README
index 9a9daaf2af..f831c5355b 100644
--- a/t/README
+++ b/t/README
@@ -462,13 +462,6 @@ GIT_TEST_CHECKOUT_WORKERS=<n> overrides the 'checkout.workers' setting
to <n> and 'checkout.thresholdForParallelism' to 0, forcing the
execution of the parallel-checkout code.
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=<boolean>, when true, makes
-registering submodule ODBs as alternates a fatal action. Support for
-this environment variable can be removed once the migration to
-explicitly providing repositories when accessing submodule objects is
-complete or needs to be abandoned for whatever reason (in which case the
-migrated codepaths still retain their performance benefits).
-
GIT_TEST_REQUIRE_PREREQ=<list> allows specifying a space separated list of
prereqs that are required to succeed. If a prereq in this list is triggered by
a test and then fails then the whole test run will abort. This can help to make
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 7b3b7359da..37d7373b36 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -3,9 +3,6 @@
test_description='Recursive "git fetch" for submodules'
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
pwd=$(pwd)
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 7d239dd31f..73429ec6e3 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -5,9 +5,6 @@ test_description='test push with submodules'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
test_expect_success setup '
diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh
index fb13549da7..239edd7d62 100755
--- a/t/t5545-push-options.sh
+++ b/t/t5545-push-options.sh
@@ -5,9 +5,6 @@ test_description='pushing to a repository using push options'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
mk_repo_pair () {
diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh
index 42d14328b6..9969a3294e 100755
--- a/t/t5572-pull-submodule.sh
+++ b/t/t5572-pull-submodule.sh
@@ -2,9 +2,6 @@
test_description='pull can handle submodules'
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-submodule-update.sh
diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh
index 107e13afbc..1546d5f773 100755
--- a/t/t6437-submodule-merge.sh
+++ b/t/t6437-submodule-merge.sh
@@ -5,9 +5,6 @@ test_description='merging with submodules'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
#
diff --git a/t/t7418-submodule-sparse-gitmodules.sh b/t/t7418-submodule-sparse-gitmodules.sh
index dde11ecce8..cf94e30e78 100755
--- a/t/t7418-submodule-sparse-gitmodules.sh
+++ b/t/t7418-submodule-sparse-gitmodules.sh
@@ -12,9 +12,6 @@ The test setup uses a sparse checkout, however the same scenario can be set up
also by committing .gitmodules and then just removing it from the filesystem.
'
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
test_expect_success 'setup' '
diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh
index e1cf53dc9e..3d149d34c1 100755
--- a/t/t7814-grep-recurse-submodules.sh
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -9,9 +9,6 @@ submodules.
TEST_CREATE_REPO_NO_TEMPLATE=1
. ./test-lib.sh
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
test_expect_success 'setup directory structure and submodule' '
echo "(1|2)d(3|4)" >a &&
mkdir b &&
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v2 07/13] odb: remove infrastructure to register submodule sources
2026-09-02 13:34 ` [PATCH v2 07/13] odb: remove infrastructure to register submodule sources Patrick Steinhardt
@ 2026-09-04 22:36 ` Karthik Nayak
0 siblings, 0 replies; 54+ messages in thread
From: Karthik Nayak @ 2026-09-04 22:36 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Junio C Hamano
[-- Attachment #1: Type: text/plain, Size: 3522 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> The preceding commits have removed the last two users of
> `odb_add_submodule_source_by_path()`. The mechanism was only ever
> meant as a transitional crutch while migrating submodule object
> access away from "add the submodule ODB as an alternate of
> the_repository" towards explicitly passing the submodule repository,
> see a35e03dee0 (submodule: lazily add submodule ODBs as alternates,
> 2021-08-16). Remove it.
>
> As GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB is now a no-op, remove its
> documentation and the exports from the test suite, as well.
>
Okay so the last two commits removed the last two sources which
registered the submodule as an in-memory odb.
That leaves us to cleanup all the code around it.
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> odb.c | 36 ----------------------------------
> odb.h | 14 -------------
> t/README | 7 -------
> t/t5526-fetch-submodules.sh | 3 ---
> t/t5531-deep-submodule-push.sh | 3 ---
> t/t5545-push-options.sh | 3 ---
> t/t5572-pull-submodule.sh | 3 ---
> t/t6437-submodule-merge.sh | 3 ---
> t/t7418-submodule-sparse-gitmodules.sh | 3 ---
> t/t7814-grep-recurse-submodules.sh | 3 ---
> 10 files changed, 78 deletions(-)
>
> diff --git a/odb.c b/odb.c
> index 6d5943e5ea..2f8a70a90c 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -388,12 +388,6 @@ struct odb_source *odb_find_source_or_die(struct object_database *odb, const cha
> return source;
> }
>
> -void odb_add_submodule_source_by_path(struct object_database *odb,
> - const char *path)
> -{
> - string_list_insert(&odb->submodule_source_paths, path);
> -}
> -
> static void fill_alternate_refs_command(struct repository *repo,
> struct child_process *cmd,
> const char *repo_path)
> @@ -549,23 +543,6 @@ void disable_obj_read_lock(void)
> pthread_mutex_destroy(&obj_read_mutex);
> }
>
> -static int register_all_submodule_sources(struct object_database *odb)
> -{
> - int ret = odb->submodule_source_paths.nr;
> -
> - for (size_t i = 0; i < odb->submodule_source_paths.nr; i++)
> - odb_add_to_alternates_memory(odb,
> - odb->submodule_source_paths.items[i].string);
> - if (ret) {
> - string_list_clear(&odb->submodule_source_paths, 0);
> - trace2_data_intmax("submodule", odb->repo,
> - "register_all_submodule_sources/registered", ret);
> - if (git_env_bool("GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB", 0))
> - BUG("register_all_submodule_sources() called");
> - }
> - return ret;
> -}
> -
Nice, so this remove the code to add the submodules as alternates.
> static enum odb_read_status do_oid_object_info_extended(struct object_database *odb,
> const struct object_id *oid,
> struct object_info *oi, unsigned flags)
> @@ -614,16 +591,6 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database *
> }
> }
>
> - /*
> - * This might be an attempt at accessing a submodule object as
> - * if it were in main object store (having called
> - * `odb_add_submodule_source_by_path()` on that submodule's
> - * ODB). If any such ODBs exist, register them and try again.
> - */
> - if (register_all_submodule_sources(odb))
> - /* We added some alternates; retry */
> - continue;
> -
Right so this is the retry mechanism when readin an object fails.
[snip]
The rest look in order.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v2 08/13] tmp-objdir: drop unused function to register alternate
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
` (6 preceding siblings ...)
2026-09-02 13:34 ` [PATCH v2 07/13] odb: remove infrastructure to register submodule sources Patrick Steinhardt
@ 2026-09-02 13:34 ` Patrick Steinhardt
2026-09-02 13:34 ` [PATCH v2 09/13] odb/packed: fix memory leaks when freeing source Patrick Steinhardt
` (5 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-02 13:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
The last caller of `tmp_objdir_add_as_alternate()` went away in
bdee7b3013 (builtin/receive-pack: stage incoming objects via ODB
transactions, 2026-07-10) and is unused now. Remove the function.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
tmp-objdir.c | 5 -----
tmp-objdir.h | 6 ------
2 files changed, 11 deletions(-)
diff --git a/tmp-objdir.c b/tmp-objdir.c
index 0eaa79ffd7..deaaf6ba2e 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -321,11 +321,6 @@ const char **tmp_objdir_env(const struct tmp_objdir *t)
return t->env.v;
}
-void tmp_objdir_add_as_alternate(const struct tmp_objdir *t)
-{
- odb_add_to_alternates_memory(t->repo->objects, t->path.buf);
-}
-
struct odb_source *tmp_objdir_replace_primary_odb(struct tmp_objdir *t,
int will_destroy)
{
diff --git a/tmp-objdir.h b/tmp-objdir.h
index 81eb927413..05f0d08d10 100644
--- a/tmp-objdir.h
+++ b/tmp-objdir.h
@@ -55,12 +55,6 @@ int tmp_objdir_destroy(struct tmp_objdir *);
*/
void tmp_objdir_discard_objects(struct tmp_objdir *);
-/*
- * Add the temporary object directory as an alternate object store in the
- * current process.
- */
-void tmp_objdir_add_as_alternate(const struct tmp_objdir *);
-
/*
* Replaces the writable object store in the current process with the temporary
* object directory and makes the former main object store an alternate.
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v2 09/13] odb/packed: fix memory leaks when freeing source
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
` (7 preceding siblings ...)
2026-09-02 13:34 ` [PATCH v2 08/13] tmp-objdir: drop unused function to register alternate Patrick Steinhardt
@ 2026-09-02 13:34 ` Patrick Steinhardt
2026-09-02 13:34 ` [PATCH v2 10/13] builtin/multi-pack-index: refuse unknown sources with "--object-dir=" Patrick Steinhardt
` (4 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-02 13:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
When freeing a "packed" source we don't close either its packs nor its
multi-pack indices. This can cause memory leaks in case we create an
ad-hoc packed source. As we used to always link packed sources to the
main object database we never noticed this issue until now, but it's
going to surface in subsequent commits where we stop linking them.
Plug the memory leaks by closing the source first.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-packed.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 1d90e714e6..166e76e2d6 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -844,6 +844,7 @@ static void odb_source_packed_free(struct odb_source *source)
chdir_notify_unregister(odb_source_packed_reparent, packed);
+ odb_source_close(source);
for (struct packfile_list_entry *e = packed->packs.head; e; e = e->next)
free(e->pack);
packfile_list_clear(&packed->packs);
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v2 10/13] builtin/multi-pack-index: refuse unknown sources with "--object-dir="
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
` (8 preceding siblings ...)
2026-09-02 13:34 ` [PATCH v2 09/13] odb/packed: fix memory leaks when freeing source Patrick Steinhardt
@ 2026-09-02 13:34 ` Patrick Steinhardt
2026-09-08 23:12 ` Justin Tobler
2026-09-02 13:34 ` [PATCH v2 11/13] t/helper: adapt read-midx to not link ad-hoc source anymore Patrick Steinhardt
` (3 subsequent siblings)
13 siblings, 1 reply; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-02 13:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Users can tell git-multi-pack-index(1) to access multi-pack indices that
are stored in a different object directory via the "--object-dir="
option. This allows them to for example write or verify a multi-pack
index other than the one located in the main object directory in case a
repository has alternates with multiple multi-pack indices.
But while the documentation explicitly points out that the specified
object directory must be an alternate of the current repository, we
never verify that property. Instead, starting with 017db7bb14 (midx:
load multi-pack indices via their source, 2025-08-11), we now construct
an ad-hoc source and link it to the main object directory.
Besides contradicting the documentation, it's dubious that this really
ought to work in the first place: creating a multi-pack index (and
potentially a bitmap) for a completely foreign object directory is of
questionable value, as bitmap commit selection operates on the invoking
repository's refs. Furthermore, this is the only remaining caller
outside of our test helpers that constructs an ad-hoc source and links
it to the database, and we want to get rid of this mechanism as part of
this series.
Stop constructing the ad-hoc source and instead refuse the operation.
While this results in a change in behaviour, this restriction has been
documented as such ever since f57a739691 (midx: avoid opening multiple
MIDXs when writing, 2021-09-01).
Note that this change requires us to adapt one test chain in t5319, as
it creates an object directory that is not connected to any repository
and then uses it via "--object-dir=". The setup itself already documents
this and does the necessary gymnastics to link the object directory to a
temporary repository, but subsequent tests don't. Adapt those tests to
retain and reuse the temporary repository.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/multi-pack-index.c | 3 ++-
t/t5319-multi-pack-index.sh | 9 ++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index 6e73c85cde..753bd53a70 100644
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -90,7 +90,8 @@ static struct odb_source_files *handle_object_dir_option(struct repository *repo
{
struct odb_source *source = odb_find_source(repo->objects, opts.object_dir);
if (!source)
- source = odb_add_to_alternates_memory(repo->objects, opts.object_dir);
+ die(_("object directory is not an alternate of the current repository: '%s'"),
+ opts.object_dir);
return odb_source_files_downcast(source);
}
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 68143cb5b7..00e90f163f 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -698,10 +698,9 @@ test_expect_success 'force some 64-bit offsets with pack-objects' '
corrupt_data $idx64 $(test_oid idxoff) "\02" &&
# objects64 is not a real repository, but can serve as an alternate
# anyway so we can write a MIDX into it
- git init repo &&
- test_when_finished "rm -fr repo" &&
+ git init repo64 &&
(
- cd repo &&
+ cd repo64 &&
( cd ../objects64 && pwd ) >.git/objects/info/alternates &&
midx64=$(git multi-pack-index --object-dir=../objects64 write)
) &&
@@ -709,7 +708,7 @@ test_expect_success 'force some 64-bit offsets with pack-objects' '
'
test_expect_success 'verify multi-pack-index with 64-bit offsets' '
- git multi-pack-index verify --object-dir=objects64
+ git -C repo64 multi-pack-index verify --object-dir=../objects64
'
NUM_OBJECTS=63
@@ -721,7 +720,7 @@ MIDX_BYTE_LARGE_OFFSET=$(($MIDX_OFFSET_LARGE_OFFSETS + 3))
test_expect_success 'verify incorrect 64-bit offset' '
corrupt_midx_and_verify $MIDX_BYTE_LARGE_OFFSET "\07" objects64 \
- "incorrect object offset"
+ "incorrect object offset" "git -C repo64 multi-pack-index verify --object-dir=../objects64"
'
test_expect_success 'setup expire tests' '
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v2 10/13] builtin/multi-pack-index: refuse unknown sources with "--object-dir="
2026-09-02 13:34 ` [PATCH v2 10/13] builtin/multi-pack-index: refuse unknown sources with "--object-dir=" Patrick Steinhardt
@ 2026-09-08 23:12 ` Justin Tobler
0 siblings, 0 replies; 54+ messages in thread
From: Justin Tobler @ 2026-09-08 23:12 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano
On 26/09/02 03:34PM, Patrick Steinhardt wrote:
> Users can tell git-multi-pack-index(1) to access multi-pack indices that
> are stored in a different object directory via the "--object-dir="
> option. This allows them to for example write or verify a multi-pack
> index other than the one located in the main object directory in case a
> repository has alternates with multiple multi-pack indices.
>
> But while the documentation explicitly points out that the specified
> object directory must be an alternate of the current repository, we
> never verify that property. Instead, starting with 017db7bb14 (midx:
> load multi-pack indices via their source, 2025-08-11), we now construct
> an ad-hoc source and link it to the main object directory.
>
> Besides contradicting the documentation, it's dubious that this really
> ought to work in the first place: creating a multi-pack index (and
> potentially a bitmap) for a completely foreign object directory is of
> questionable value, as bitmap commit selection operates on the invoking
> repository's refs. Furthermore, this is the only remaining caller
> outside of our test helpers that constructs an ad-hoc source and links
> it to the database, and we want to get rid of this mechanism as part of
> this series.
I was curious if there was any intentional reason that 017db7bb14
started added these as an alternate source. I assume though the reason
was just to address the tests when we started loading multi-pack indexes
via sources. So aligning with the prexisting documentation makes sense
to me.
> Stop constructing the ad-hoc source and instead refuse the operation.
> While this results in a change in behaviour, this restriction has been
> documented as such ever since f57a739691 (midx: avoid opening multiple
> MIDXs when writing, 2021-09-01).
>
> Note that this change requires us to adapt one test chain in t5319, as
> it creates an object directory that is not connected to any repository
> and then uses it via "--object-dir=". The setup itself already documents
> this and does the necessary gymnastics to link the object directory to a
> temporary repository, but subsequent tests don't. Adapt those tests to
> retain and reuse the temporary repository.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/multi-pack-index.c | 3 ++-
> t/t5319-multi-pack-index.sh | 9 ++++-----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
> index 6e73c85cde..753bd53a70 100644
> --- a/builtin/multi-pack-index.c
> +++ b/builtin/multi-pack-index.c
> @@ -90,7 +90,8 @@ static struct odb_source_files *handle_object_dir_option(struct repository *repo
> {
> struct odb_source *source = odb_find_source(repo->objects, opts.object_dir);
> if (!source)
> - source = odb_add_to_alternates_memory(repo->objects, opts.object_dir);
> + die(_("object directory is not an alternate of the current repository: '%s'"),
> + opts.object_dir);
Now we no longer add these as in-memory alternates sources. Looks good.
> return odb_source_files_downcast(source);
> }
>
> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
> index 68143cb5b7..00e90f163f 100755
> --- a/t/t5319-multi-pack-index.sh
> +++ b/t/t5319-multi-pack-index.sh
> @@ -698,10 +698,9 @@ test_expect_success 'force some 64-bit offsets with pack-objects' '
> corrupt_data $idx64 $(test_oid idxoff) "\02" &&
> # objects64 is not a real repository, but can serve as an alternate
> # anyway so we can write a MIDX into it
> - git init repo &&
> - test_when_finished "rm -fr repo" &&
Ok, now we just reuse the properly set up repo. Make sense.
> + git init repo64 &&
> (
> - cd repo &&
> + cd repo64 &&
> ( cd ../objects64 && pwd ) >.git/objects/info/alternates &&
> midx64=$(git multi-pack-index --object-dir=../objects64 write)
> ) &&
> @@ -709,7 +708,7 @@ test_expect_success 'force some 64-bit offsets with pack-objects' '
> '
>
> test_expect_success 'verify multi-pack-index with 64-bit offsets' '
> - git multi-pack-index verify --object-dir=objects64
> + git -C repo64 multi-pack-index verify --object-dir=../objects64
> '
>
> NUM_OBJECTS=63
> @@ -721,7 +720,7 @@ MIDX_BYTE_LARGE_OFFSET=$(($MIDX_OFFSET_LARGE_OFFSETS + 3))
>
> test_expect_success 'verify incorrect 64-bit offset' '
> corrupt_midx_and_verify $MIDX_BYTE_LARGE_OFFSET "\07" objects64 \
> - "incorrect object offset"
> + "incorrect object offset" "git -C repo64 multi-pack-index verify --object-dir=../objects64"
> '
>
> test_expect_success 'setup expire tests' '
>
> --
> 2.55.0.979.g7e5102b832.dirty
>
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v2 11/13] t/helper: adapt read-midx to not link ad-hoc source anymore
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
` (9 preceding siblings ...)
2026-09-02 13:34 ` [PATCH v2 10/13] builtin/multi-pack-index: refuse unknown sources with "--object-dir=" Patrick Steinhardt
@ 2026-09-02 13:34 ` Patrick Steinhardt
2026-09-02 13:35 ` [PATCH v2 12/13] t/helper: stop registering alternates in "ref-store" command Patrick Steinhardt
` (2 subsequent siblings)
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-02 13:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Same as in the preceding commit, refactor the setup of ad-hoc object
database sources when accessing a multi-pack index in an arbitrary
location to not link the newly created source into the main object
database anymore.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/helper/test-read-midx.c | 43 +++++++++++++++++++++++++++++++------------
1 file changed, 31 insertions(+), 12 deletions(-)
diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c
index 27a05da957..1f7a1927e4 100644
--- a/t/helper/test-read-midx.c
+++ b/t/helper/test-read-midx.c
@@ -5,34 +5,42 @@
#include "midx.h"
#include "repository.h"
#include "odb.h"
+#include "odb/source-packed.h"
#include "pack-bitmap.h"
#include "packfile.h"
#include "setup.h"
#include "gettext.h"
#include "pack-revindex.h"
-static struct multi_pack_index *setup_midx(const char *object_dir)
+static struct multi_pack_index *setup_midx(const char *object_dir,
+ struct odb_source_packed **out)
{
- struct odb_source_files *files;
+ struct odb_source_packed *packed;
struct odb_source *source;
+
setup_git_directory(the_repository);
+
source = odb_find_source(the_repository->objects, object_dir);
- if (!source)
- source = odb_add_to_alternates_memory(the_repository->objects,
- object_dir);
- files = odb_source_files_downcast(source);
+ if (source) {
+ packed = odb_source_files_downcast(source)->packed;
+ } else {
+ packed = odb_source_packed_new(the_repository->objects,
+ object_dir, false);
+ *out = packed;
+ }
- return load_multi_pack_index(files->packed);
+ return load_multi_pack_index(packed);
}
static int read_midx_file(const char *object_dir, const char *checksum,
int show_objects)
{
+ struct odb_source_packed *packed = NULL;
uint32_t i;
struct multi_pack_index *m, *tip;
int ret = 0;
- m = tip = setup_midx(object_dir);
+ m = tip = setup_midx(object_dir, &packed);
if (!m)
return 1;
@@ -91,29 +99,35 @@ static int read_midx_file(const char *object_dir, const char *checksum,
out:
close_midx(tip);
+ if (packed)
+ odb_source_free(&packed->base);
return ret;
}
static int read_midx_checksum(const char *object_dir)
{
+ struct odb_source_packed *packed = NULL;
struct multi_pack_index *m;
- m = setup_midx(object_dir);
+ m = setup_midx(object_dir, &packed);
if (!m)
return 1;
printf("%s\n", midx_get_checksum_hex(m));
close_midx(m);
+ if (packed)
+ odb_source_free(&packed->base);
return 0;
}
static int read_midx_preferred_pack(const char *object_dir)
{
+ struct odb_source_packed *packed = NULL;
struct multi_pack_index *midx = NULL;
uint32_t preferred_pack;
- midx = setup_midx(object_dir);
+ midx = setup_midx(object_dir, &packed);
if (!midx)
return 1;
@@ -124,17 +138,21 @@ static int read_midx_preferred_pack(const char *object_dir)
}
printf("%s\n", midx->pack_names[preferred_pack]);
+
close_midx(midx);
+ if (packed)
+ odb_source_free(&packed->base);
return 0;
}
static int read_midx_bitmapped_packs(const char *object_dir)
{
+ struct odb_source_packed *packed = NULL;
struct multi_pack_index *midx = NULL;
struct bitmapped_pack pack;
uint32_t i;
- midx = setup_midx(object_dir);
+ midx = setup_midx(object_dir, &packed);
if (!midx)
return 1;
@@ -150,7 +168,8 @@ static int read_midx_bitmapped_packs(const char *object_dir)
}
close_midx(midx);
-
+ if (packed)
+ odb_source_free(&packed->base);
return 0;
}
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v2 12/13] t/helper: stop registering alternates in "ref-store" command
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
` (10 preceding siblings ...)
2026-09-02 13:34 ` [PATCH v2 11/13] t/helper: adapt read-midx to not link ad-hoc source anymore Patrick Steinhardt
@ 2026-09-02 13:35 ` Patrick Steinhardt
2026-09-02 13:35 ` [PATCH v2 13/13] odb: remove the ability to link sources ad-hoc Patrick Steinhardt
2026-09-04 22:45 ` [PATCH v2 00/13] odb: stop registering in-memory sources Karthik Nayak
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-02 13:35 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
When using the "ref-store" command we support access to multiple
different reference stores. As part of that we allow the caller to
explicitly exercise stores of a submodule. This allows us to verify
low-level behaviour of submodule stores, which is exercised in t1406.
When doing so we also link the submodule's object database into the main
object database. The intent of this is that it allows us to access
objects of the submodule, too. But that functionality is not even
needed anymore: when creating a submodule reference store, we will first
initialize the submodule repository and then initialize the store with
that repository. And as the reference subsystem doesn't depend on
`the_repository` anymore all subsequent object lookups performed by the
reference store will be routed to the submodule repository.
It is thus not needed anymore to register the submodule object store
with the main object database. Remove the call.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/helper/test-ref-store.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 5a9a3053d9..db58f00589 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -74,14 +74,6 @@ static const char **get_store(const char **argv, struct ref_store **refs)
} else if (!strcmp(argv[0], "main")) {
*refs = get_main_ref_store(the_repository);
} else if (skip_prefix(argv[0], "submodule:", &gitdir)) {
- struct strbuf sb = STRBUF_INIT;
-
- if (!repo_submodule_path_append(the_repository,
- &sb, gitdir, "objects/"))
- die("computing submodule path failed");
- odb_add_to_alternates_memory(the_repository->objects, sb.buf);
- strbuf_release(&sb);
-
*refs = repo_get_submodule_ref_store(the_repository, gitdir);
} else if (skip_prefix(argv[0], "worktree:", &gitdir)) {
struct worktree **p, **worktrees = get_worktrees(the_repository);
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v2 13/13] odb: remove the ability to link sources ad-hoc
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
` (11 preceding siblings ...)
2026-09-02 13:35 ` [PATCH v2 12/13] t/helper: stop registering alternates in "ref-store" command Patrick Steinhardt
@ 2026-09-02 13:35 ` Patrick Steinhardt
2026-09-04 22:45 ` [PATCH v2 00/13] odb: stop registering in-memory sources Karthik Nayak
13 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-02 13:35 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Over the course of this patch series we have adapted all callers of
`odb_add_to_alternates_memory()` to not do so anymore. Remove the
function.
This series of refactorings doesn't only simplify our code base. More
importantly, with those changes in place we can now unconditionally
assume that the list of sources linked to the object database only
consists of the primary source and its alternates. This serves as the
foundation to eventually move handling of alternates into the "files"
backend itself.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb.c | 6 ------
odb.h | 8 --------
2 files changed, 14 deletions(-)
diff --git a/odb.c b/odb.c
index 2f8a70a90c..5fe081496f 100644
--- a/odb.c
+++ b/odb.c
@@ -247,12 +247,6 @@ void odb_add_to_alternates_file(struct object_database *odb,
odb_add_alternate_recursively(odb, dir, 0);
}
-struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
- const char *dir)
-{
- return odb_add_alternate_recursively(odb, dir, 0);
-}
-
struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
const char *dir, int will_destroy,
struct odb_source **prev_source)
diff --git a/odb.h b/odb.h
index 54548efc55..9025239df5 100644
--- a/odb.h
+++ b/odb.h
@@ -258,14 +258,6 @@ int odb_has_alternates(struct object_database *odb);
void odb_add_to_alternates_file(struct object_database *odb,
const char *dir);
-/*
- * Add the directory to the in-memory list of alternate sources (along with any
- * recursive alternates it points to), but do not modify the on-disk alternates
- * file.
- */
-struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
- const char *dir);
-
/*
* Read an object from the database. Returns the object data and assigns object
* type and size to the `type` and `size` pointers, if these pointers are
--
2.55.0.979.g7e5102b832.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v2 00/13] odb: stop registering in-memory sources
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
` (12 preceding siblings ...)
2026-09-02 13:35 ` [PATCH v2 13/13] odb: remove the ability to link sources ad-hoc Patrick Steinhardt
@ 2026-09-04 22:45 ` Karthik Nayak
13 siblings, 0 replies; 54+ messages in thread
From: Karthik Nayak @ 2026-09-04 22:45 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Junio C Hamano
[-- Attachment #1: Type: text/plain, Size: 3013 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> Hi,
>
> the object database has a list of sources that is used for two
> different purposes:
>
> - We use it to track the list of alternates.
>
> - We use it to track temporary in-memory sources that we create for
> various purposes. Most importantly, this is used to link object
> database sources from submodules into the main store.
>
> This dual-use is quite awkward, as it mixes two different levels of
> concerns and thus as a consequence makes both harder to reason about.
> It's also a source of bugs: we make assumptions about the ordering of
> sources all over the place, and we furthermore assume in other places
> that the sources only contain alternates in the first place. I don't
> think this surfaces in the form of real bugs, but I've long disliked
> this dual-use.
>
> Furthermore, we want to migrate handling of alternates into the "files"
> backend itself in a subsequent patch series. This is most importantly to
> fix a performance regression by making the backend own all of its
> alternates, but it also fixes a couple of longer-standing design issues
> that I've been struggling with [1].
>
> Most importantly though: this whole machinery is not even needed at all.
> A couple years ago we have already refactored our codebase so that
> submodule sources don't even have to be linked into the main object
> database anymore. And all the other use cases where we link sources into
> the main object database can be trivially converted, too.
>
> So this patch series does exactly that: it removes the mechanism to link
> ad-hoc sources into the object database entirely. This ensures that the
> list of sources is exactly the list of alternates, and that makes it
> easier to move them into the "files" backend in a subsequent patch
> series.
>
> There is one exception though: creating transactions still creates a
> temporary quarantine directory. This mechanism is left as-is for now,
> but as it's an implementation detail of the "files" backend anyway
> that's not conflicting with our above stated goals.
>
> This series is built on top of 1630431f32 (The 21st batch, 2026-08-31)
> with ty/repository-fetch-if-missing at 508ec9837c (repository: move
> fetch_if_missing into struct repository, 2026-08-15) merged into it.
> There's still two merge conflicts, but these are trivial to resolve: in
> "odb.c" and "odb.h" you simply remove both ours and theirs, and in
> "builtin/multi-pack-index.c" you only need to munge the parameters a
> bit.
>
This was a bit of a dense read for me, mostly as I'm not too familiar
with the code here. So I can not so confidently say that the series
looks good :)
> Changes in v2:
> - Adapt `cache_tree_fully_valid()` to take a `struct index_state` as
> input instead of taking both a repository and a cache tree, as
> suggested by Junio.
> - Link to v1: https://patch.msgid.link/20260901-pks-odb-registering-in-memory-sources-v1-0-97a312d5fa25@pks.im
>
> Thanks!
>
> Patrick
[snip]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 00/13] odb: stop registering in-memory sources
2026-09-01 11:08 [PATCH 00/12] odb: stop registering in-memory sources Patrick Steinhardt
` (12 preceding siblings ...)
2026-09-02 13:34 ` [PATCH v2 00/13] odb: stop registering in-memory sources Patrick Steinhardt
@ 2026-09-11 5:51 ` Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 01/13] cache-tree: drop `the_repository` in `cache_tree_fully_valid()` Patrick Steinhardt
` (12 more replies)
13 siblings, 13 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-11 5:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Justin Tobler
Hi,
the object database has a list of sources that is used for two
different purposes:
- We use it to track the list of alternates.
- We use it to track temporary in-memory sources that we create for
various purposes. Most importantly, this is used to link object
database sources from submodules into the main store.
This dual-use is quite awkward, as it mixes two different levels of
concerns and thus as a consequence makes both harder to reason about.
It's also a source of bugs: we make assumptions about the ordering of
sources all over the place, and we furthermore assume in other places
that the sources only contain alternates in the first place. I don't
think this surfaces in the form of real bugs, but I've long disliked
this dual-use.
Furthermore, we want to migrate handling of alternates into the "files"
backend itself in a subsequent patch series. This is most importantly to
fix a performance regression by making the backend own all of its
alternates, but it also fixes a couple of longer-standing design issues
that I've been struggling with [1].
Most importantly though: this whole machinery is not even needed at all.
A couple years ago we have already refactored our codebase so that
submodule sources don't even have to be linked into the main object
database anymore. And all the other use cases where we link sources into
the main object database can be trivially converted, too.
So this patch series does exactly that: it removes the mechanism to link
ad-hoc sources into the object database entirely. This ensures that the
list of sources is exactly the list of alternates, and that makes it
easier to move them into the "files" backend in a subsequent patch
series.
There is one exception though: creating transactions still creates a
temporary quarantine directory. This mechanism is left as-is for now,
but as it's an implementation detail of the "files" backend anyway
that's not conflicting with our above stated goals.
This series is built on top of 1630431f32 (The 21st batch, 2026-08-31)
with ty/repository-fetch-if-missing at 508ec9837c (repository: move
fetch_if_missing into struct repository, 2026-08-15) merged into it.
There's still two merge conflicts, but these are trivial to resolve: in
"odb.c" and "odb.h" you simply remove both ours and theirs, and in
"builtin/multi-pack-index.c" you only need to munge the parameters a
bit.
Changes in v3:
- Improve commit message clarity a tiny bit :)
- Link to v2: https://patch.msgid.link/20260902-pks-odb-registering-in-memory-sources-v2-0-c6ca12fdea4d@pks.im
Changes in v2:
- Adapt `cache_tree_fully_valid()` to take a `struct index_state` as
input instead of taking both a repository and a cache tree, as
suggested by Junio.
- Link to v1: https://patch.msgid.link/20260901-pks-odb-registering-in-memory-sources-v1-0-97a312d5fa25@pks.im
Thanks!
Patrick
[1]: <amLgMqkqxR8mKIbT@pks.im>
---
Patrick Steinhardt (13):
cache-tree: drop `the_repository` in `cache_tree_fully_valid()`
cache-tree: remove dependency on `the_repository`
submodule-config: remove uses of `the_repository`
submodule-config: stop using `the_hash_algo`
submodule-config: stop registering submodule sources
builtin/grep: stop registering submodule ODB as source
odb: remove infrastructure to register submodule sources
tmp-objdir: drop unused function to register alternate
odb/packed: fix memory leaks when freeing source
builtin/multi-pack-index: refuse unknown sources with "--object-dir="
t/helper: adapt read-midx to not link ad-hoc source anymore
t/helper: stop registering alternates in "ref-store" command
odb: remove the ability to link sources ad-hoc
builtin/checkout.c | 2 +-
builtin/commit.c | 2 +-
builtin/fetch.c | 2 +-
builtin/grep.c | 28 +++-------
builtin/multi-pack-index.c | 3 +-
builtin/submodule--helper.c | 8 +--
cache-tree.c | 95 +++++++++++++++++++---------------
cache-tree.h | 7 +--
odb.c | 42 ---------------
odb.h | 22 --------
odb/source-packed.c | 1 +
read-cache-ll.h | 5 +-
read-cache.c | 9 ++--
sequencer.c | 2 +-
sparse-index.c | 2 +-
submodule-config.c | 59 +++++++++++----------
submodule-config.h | 12 +++--
submodule.c | 2 +-
t/README | 7 ---
t/helper/test-read-midx.c | 43 ++++++++++-----
t/helper/test-ref-store.c | 8 ---
t/helper/test-submodule.c | 4 +-
t/t5319-multi-pack-index.sh | 9 ++--
t/t5526-fetch-submodules.sh | 3 --
t/t5531-deep-submodule-push.sh | 3 --
t/t5545-push-options.sh | 3 --
t/t5572-pull-submodule.sh | 3 --
t/t6437-submodule-merge.sh | 3 --
t/t7418-submodule-sparse-gitmodules.sh | 3 --
t/t7814-grep-recurse-submodules.sh | 3 --
tmp-objdir.c | 5 --
tmp-objdir.h | 6 ---
unpack-trees.c | 9 ++--
33 files changed, 168 insertions(+), 247 deletions(-)
Range-diff versus v2:
1: 95ff726928 = 1: a785e29859 cache-tree: drop `the_repository` in `cache_tree_fully_valid()`
2: 2b2cf592c4 = 2: 002cca01eb cache-tree: remove dependency on `the_repository`
3: 7727c40da8 = 3: 17ab3e4553 submodule-config: remove uses of `the_repository`
4: 6c2168af46 = 4: 586a786dc6 submodule-config: stop using `the_hash_algo`
5: 60c1c9558d ! 5: 539dd9f9d5 submodule-config: stop registering submodule sources
@@ Commit message
submodule-config: stop registering submodule sources
When reading the ".gitmodules" file from a blob in a repository other
- than `the_repository`, we register the repository's object database as
+ than `the_repository`, we register that repository's object database as
an in-memory source of `the_repository`'s object database. This call has
its origins in d9b8b8f896 (submodule-config.c: use repo_get_oid for
reading .gitmodules, 2019-04-16): back then, `config_with_options()` was
6: 7f5354c2bf = 6: 11db0ac853 builtin/grep: stop registering submodule ODB as source
7: 24e5a2a3b7 = 7: 5d4660657c odb: remove infrastructure to register submodule sources
8: c07db4254f = 8: 577e58d475 tmp-objdir: drop unused function to register alternate
9: e1022b26a6 = 9: 00bc8549a9 odb/packed: fix memory leaks when freeing source
10: 8c5c827595 = 10: d66457c73a builtin/multi-pack-index: refuse unknown sources with "--object-dir="
11: 7d7cb53936 = 11: 7d13925a93 t/helper: adapt read-midx to not link ad-hoc source anymore
12: 537a58d286 = 12: 6a0092d870 t/helper: stop registering alternates in "ref-store" command
13: d704faeb44 = 13: 4e446e8a51 odb: remove the ability to link sources ad-hoc
---
base-commit: e5d60560f61f520e9ea350645a6cc9770b0f1607
change-id: 20260811-pks-odb-registering-in-memory-sources-88648cd95735
^ permalink raw reply [flat|nested] 54+ messages in thread* [PATCH v3 01/13] cache-tree: drop `the_repository` in `cache_tree_fully_valid()`
2026-09-11 5:51 ` [PATCH v3 " Patrick Steinhardt
@ 2026-09-11 5:51 ` Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 02/13] cache-tree: remove dependency on `the_repository` Patrick Steinhardt
` (11 subsequent siblings)
12 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-11 5:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Justin Tobler
The function `cache_tree_fully_valid()` verifies whether the cache tree
owned by the index is valid or not. As part of that, the function checks
whether the objects referenced by the cache all exist. But because the
function has no repository available, it is using the object database of
`the_repository` instead.
We could of course adapt callers to pass in a repository as parameter
explicitly to get rid of this implicit dependency on global state. But
all of them pass the cache tree owned by a `struct index_state`, and
that structure already has a reference to its owning repository.
So instead, adapt the function to accept a `struct index_state`, which
ensures that callers will implicitly always pass the correct repository.
Adapt callers accordingly.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/checkout.c | 2 +-
builtin/commit.c | 2 +-
cache-tree.c | 17 ++++++++++++-----
cache-tree.h | 2 +-
sequencer.c | 2 +-
sparse-index.c | 2 +-
unpack-trees.c | 2 +-
7 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 55e3a89a85..505d3f7bf3 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -921,7 +921,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
}
}
- if (!cache_tree_fully_valid(the_repository->index->cache_tree))
+ if (!cache_tree_fully_valid(the_repository->index))
cache_tree_update(the_repository->index, WRITE_TREE_SILENT | WRITE_TREE_REPAIR);
if (write_locked_index(the_repository->index, &lock_file, COMMIT_LOCK))
diff --git a/builtin/commit.c b/builtin/commit.c
index 28f6174503..840b6b4083 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -484,7 +484,7 @@ static const char *prepare_index(const char **argv, const char *prefix,
LOCK_DIE_ON_ERROR);
refresh_cache_or_die(refresh_flags);
if (the_repository->index->cache_changed
- || !cache_tree_fully_valid(the_repository->index->cache_tree))
+ || !cache_tree_fully_valid(the_repository->index))
cache_tree_update(the_repository->index, WRITE_TREE_SILENT);
if (write_locked_index(the_repository->index, &index_lock,
COMMIT_LOCK | SKIP_IF_UNCHANGED))
diff --git a/cache-tree.c b/cache-tree.c
index a220372a42..6103b3fcb3 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -275,22 +275,29 @@ static void discard_unused_subtrees(struct cache_tree *it)
}
}
-int cache_tree_fully_valid(struct cache_tree *it)
+static int cache_tree_fully_valid_recursive(struct object_database *odb,
+ struct cache_tree *it)
{
int i;
if (!it)
return 0;
if (it->entry_count < 0 ||
- !odb_has_object(the_repository->objects, &it->oid,
+ !odb_has_object(odb, &it->oid,
ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR))
return 0;
for (i = 0; i < it->subtree_nr; i++) {
- if (!cache_tree_fully_valid(it->down[i]->cache_tree))
+ if (!cache_tree_fully_valid_recursive(odb, it->down[i]->cache_tree))
return 0;
}
return 1;
}
+int cache_tree_fully_valid(struct index_state *istate)
+{
+ return cache_tree_fully_valid_recursive(istate->repo->objects,
+ istate->cache_tree);
+}
+
static int must_check_existence(const struct cache_entry *ce)
{
return !(repo_has_promisor_remote(the_repository) && ce_skip_worktree(ce));
@@ -775,7 +782,7 @@ struct tree *write_in_core_index_as_tree(struct repository *repo,
int was_valid, ret;
was_valid = index_state->cache_tree &&
- cache_tree_fully_valid(index_state->cache_tree);
+ cache_tree_fully_valid(index_state);
ret = write_index_as_tree_internal(&o, index_state, was_valid, 0, NULL);
if (ret == WRITE_TREE_UNMERGED_INDEX) {
@@ -811,7 +818,7 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
was_valid = !(flags & WRITE_TREE_IGNORE_CACHE_TREE) &&
index_state->cache_tree &&
- cache_tree_fully_valid(index_state->cache_tree);
+ cache_tree_fully_valid(index_state);
ret = write_index_as_tree_internal(oid, index_state, was_valid, flags,
prefix);
diff --git a/cache-tree.h b/cache-tree.h
index f8bddae523..4b3f60d6db 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -31,7 +31,7 @@ int cache_tree_subtree_pos(struct cache_tree *it, const char *path, int pathlen)
void cache_tree_write(struct strbuf *, struct cache_tree *root);
struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);
-int cache_tree_fully_valid(struct cache_tree *);
+int cache_tree_fully_valid(struct index_state *);
int cache_tree_update(struct index_state *, int);
int cache_tree_verify(struct repository *, struct index_state *);
diff --git a/sequencer.c b/sequencer.c
index 65afd100d9..11a95c031b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -814,7 +814,7 @@ static int do_recursive_merge(struct repository *r,
static struct object_id *get_cache_tree_oid(struct index_state *istate)
{
- if (!cache_tree_fully_valid(istate->cache_tree))
+ if (!cache_tree_fully_valid(istate))
if (cache_tree_update(istate, 0)) {
error(_("unable to update cache tree"));
return NULL;
diff --git a/sparse-index.c b/sparse-index.c
index c1fa231a89..3d77dadae5 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -228,7 +228,7 @@ int convert_to_sparse(struct index_state *istate, int flags)
if (index_has_unmerged_entries(istate))
return 0;
- if (!cache_tree_fully_valid(istate->cache_tree)) {
+ if (!cache_tree_fully_valid(istate)) {
/* Clear and recompute the cache-tree */
cache_tree_free(&istate->cache_tree);
diff --git a/unpack-trees.c b/unpack-trees.c
index 154d6d40a1..f6bb1e6d2b 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -2086,7 +2086,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
}
if (!o->skip_cache_tree_update &&
- !cache_tree_fully_valid(o->internal.result.cache_tree))
+ !cache_tree_fully_valid(&o->internal.result))
cache_tree_update(&o->internal.result,
WRITE_TREE_SILENT |
WRITE_TREE_REPAIR);
--
2.55.0.1074.ge7621b4bad.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v3 02/13] cache-tree: remove dependency on `the_repository`
2026-09-11 5:51 ` [PATCH v3 " Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 01/13] cache-tree: drop `the_repository` in `cache_tree_fully_valid()` Patrick Steinhardt
@ 2026-09-11 5:51 ` Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 03/13] submodule-config: remove uses of `the_repository` Patrick Steinhardt
` (10 subsequent siblings)
12 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-11 5:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Justin Tobler
The "cache-tree" subsystem still depends on `the_repository`. Adapt it
to instead use repositories provided via the context, either as a new
parameter or the one passed in via `struct index_state`.
Besides getting rid of `the_repository`, this also removes the last
dependency on registering submodule sources with the main object
database. When reading gitmodules from a submodule's index we implicitly
read that object via `the_repository`'s object database, which is of
course wrong. This works though because we would then register the
submodule's object database with the main object database, but a later
patch is going to get rid of that mechanism.
You can verify that we indeed no longer depend on this mechanism by
running tests with `GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=true`. Without
this patch we fail in t1092, with this patch we never register submodule
object databases anymore.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
cache-tree.c | 78 +++++++++++++++++++++++++++++++--------------------------
cache-tree.h | 5 ++--
read-cache-ll.h | 5 ++--
read-cache.c | 9 ++++---
unpack-trees.c | 7 +++---
5 files changed, 57 insertions(+), 47 deletions(-)
diff --git a/cache-tree.c b/cache-tree.c
index 6103b3fcb3..b8cbb5da22 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -298,12 +297,14 @@ int cache_tree_fully_valid(struct index_state *istate)
istate->cache_tree);
}
-static int must_check_existence(const struct cache_entry *ce)
+static int must_check_existence(const struct cache_entry *ce, void *cb_data)
{
- return !(repo_has_promisor_remote(the_repository) && ce_skip_worktree(ce));
+ struct repository *repo = cb_data;
+ return !(repo_has_promisor_remote(repo) && ce_skip_worktree(ce));
}
-static int update_one(struct cache_tree *it,
+static int update_one(struct repository *repo,
+ struct cache_tree *it,
struct cache_entry **cache,
int entries,
const char *base,
@@ -341,7 +342,7 @@ static int update_one(struct cache_tree *it,
}
if (0 <= it->entry_count &&
- odb_has_object(the_repository->objects, &it->oid,
+ odb_has_object(repo->objects, &it->oid,
ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR))
return it->entry_count;
@@ -382,7 +383,8 @@ static int update_one(struct cache_tree *it,
sub = find_subtree(it, path + baselen, sublen, 1);
if (!sub->cache_tree)
sub->cache_tree = cache_tree();
- subcnt = update_one(sub->cache_tree,
+ subcnt = update_one(repo,
+ sub->cache_tree,
cache + i, entries - i,
path,
baselen + sublen + 1,
@@ -446,10 +448,10 @@ static int update_one(struct cache_tree *it,
}
ce_missing_ok = mode == S_IFGITLINK || missing_ok ||
- !must_check_existence(ce);
+ !must_check_existence(ce, repo);
if (is_null_oid(oid) ||
(!ce_missing_ok &&
- !odb_has_object(the_repository->objects, oid,
+ !odb_has_object(repo->objects, oid,
ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR))) {
strbuf_release(&buffer);
if (expected_missing)
@@ -481,12 +483,12 @@ static int update_one(struct cache_tree *it,
/*
* "sub" can be an empty tree if all subentries are i-t-a.
*/
- if (contains_ita && is_empty_tree_oid(oid, the_repository->hash_algo))
+ if (contains_ita && is_empty_tree_oid(oid, repo->hash_algo))
continue;
strbuf_grow(&buffer, entlen + 100);
strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
- strbuf_add(&buffer, oid->hash, the_hash_algo->rawsz);
+ strbuf_add(&buffer, oid->hash, repo->hash_algo->rawsz);
#if DEBUG_CACHE_TREE
fprintf(stderr, "cache-tree update-one %o %.*s\n",
@@ -496,16 +498,16 @@ static int update_one(struct cache_tree *it,
if (repair) {
struct object_id oid;
- hash_object_file(the_hash_algo, buffer.buf, buffer.len,
+ hash_object_file(repo->hash_algo, buffer.buf, buffer.len,
OBJ_TREE, &oid);
- if (odb_has_object(the_repository->objects, &oid, ODB_HAS_OBJECT_RECHECK_PACKED))
+ if (odb_has_object(repo->objects, &oid, ODB_HAS_OBJECT_RECHECK_PACKED))
oidcpy(&it->oid, &oid);
else
to_invalidate = 1;
} else if (dryrun) {
- hash_object_file(the_hash_algo, buffer.buf, buffer.len,
+ hash_object_file(repo->hash_algo, buffer.buf, buffer.len,
OBJ_TREE, &it->oid);
- } else if (odb_write_object_ext(the_repository->objects, buffer.buf, buffer.len, OBJ_TREE,
+ } else if (odb_write_object_ext(repo->objects, buffer.buf, buffer.len, OBJ_TREE,
&it->oid, NULL, flags & WRITE_TREE_SILENT ? ODB_WRITE_OBJECT_SILENT : 0)) {
strbuf_release(&buffer);
return -1;
@@ -523,7 +525,7 @@ static int update_one(struct cache_tree *it,
int cache_tree_update(struct index_state *istate, int flags)
{
- int inflight = !!the_repository->objects->transaction;
+ int inflight = !!istate->repo->objects->transaction;
struct odb_transaction *transaction;
int skip, i;
@@ -535,14 +537,14 @@ int cache_tree_update(struct index_state *istate, int flags)
if (!istate->cache_tree)
istate->cache_tree = cache_tree();
- if (!(flags & WRITE_TREE_MISSING_OK) && repo_has_promisor_remote(the_repository))
- prefetch_cache_entries(istate, must_check_existence);
+ if (!(flags & WRITE_TREE_MISSING_OK) && repo_has_promisor_remote(istate->repo))
+ prefetch_cache_entries(istate, must_check_existence, istate->repo);
trace_performance_enter();
trace2_region_enter("cache_tree", "update", istate->repo);
if (!inflight)
- odb_transaction_begin_or_die(the_repository->objects, &transaction, 0);
- i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
+ odb_transaction_begin_or_die(istate->repo->objects, &transaction, 0);
+ i = update_one(istate->repo, istate->cache_tree, istate->cache, istate->cache_nr,
"", 0, &skip, flags);
if (!inflight)
odb_transaction_commit_and_finalize_or_die(transaction);
@@ -554,7 +556,8 @@ int cache_tree_update(struct index_state *istate, int flags)
return 0;
}
-static void write_one(struct strbuf *buffer, struct cache_tree *it,
+static void write_one(struct repository *repo,
+ struct strbuf *buffer, struct cache_tree *it,
const char *path, int pathlen)
{
int i;
@@ -580,7 +583,7 @@ static void write_one(struct strbuf *buffer, struct cache_tree *it,
#endif
if (0 <= it->entry_count) {
- strbuf_add(buffer, it->oid.hash, the_hash_algo->rawsz);
+ strbuf_add(buffer, it->oid.hash, repo->hash_algo->rawsz);
}
for (i = 0; i < it->subtree_nr; i++) {
struct cache_tree_sub *down = it->down[i];
@@ -590,15 +593,16 @@ static void write_one(struct strbuf *buffer, struct cache_tree *it,
prev->name, prev->namelen) <= 0)
die("fatal - unsorted cache subtree");
}
- write_one(buffer, down->cache_tree, down->name, down->namelen);
+ write_one(repo, buffer, down->cache_tree, down->name, down->namelen);
}
}
-void cache_tree_write(struct strbuf *sb, struct cache_tree *root)
+void cache_tree_write(struct repository *repo,
+ struct strbuf *sb, struct cache_tree *root)
{
- trace2_region_enter("cache_tree", "write", the_repository);
- write_one(sb, root, "", 0);
- trace2_region_leave("cache_tree", "write", the_repository);
+ trace2_region_enter("cache_tree", "write", repo);
+ write_one(repo, sb, root, "", 0);
+ trace2_region_leave("cache_tree", "write", repo);
}
static int parse_int(const char **ptr, unsigned long *len_p, int *out)
@@ -632,13 +636,14 @@ static int parse_int(const char **ptr, unsigned long *len_p, int *out)
return 0;
}
-static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
+static struct cache_tree *read_one(struct repository *repo,
+ const char **buffer, unsigned long *size_p)
{
const char *buf = *buffer;
unsigned long size = *size_p;
struct cache_tree *it;
int i, subtree_nr;
- const unsigned rawsz = the_hash_algo->rawsz;
+ const unsigned rawsz = repo->hash_algo->rawsz;
it = NULL;
/* skip name, but make sure name exists */
@@ -665,7 +670,7 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
if (size < rawsz)
goto free_return;
oidread(&it->oid, (const unsigned char *)buf,
- the_repository->hash_algo);
+ repo->hash_algo);
buf += rawsz;
size -= rawsz;
}
@@ -693,7 +698,7 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
struct cache_tree_sub *subtree;
const char *name = buf;
- sub = read_one(&buf, &size);
+ sub = read_one(repo, &buf, &size);
if (!sub)
goto free_return;
subtree = cache_tree_sub(it, name);
@@ -710,16 +715,17 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
return NULL;
}
-struct cache_tree *cache_tree_read(const char *buffer, unsigned long size)
+struct cache_tree *cache_tree_read(struct repository *repo,
+ const char *buffer, unsigned long size)
{
struct cache_tree *result;
if (buffer[0])
return NULL; /* not the whole tree */
- trace2_region_enter("cache_tree", "read", the_repository);
- result = read_one(&buffer, &size);
- trace2_region_leave("cache_tree", "read", the_repository);
+ trace2_region_enter("cache_tree", "read", repo);
+ result = read_one(repo, &buffer, &size);
+ trace2_region_leave("cache_tree", "read", repo);
return result;
}
@@ -810,7 +816,7 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR);
entries = read_index_from(index_state, index_path,
- repo_get_git_dir(the_repository));
+ repo_get_git_dir(index_state->repo));
if (entries < 0) {
ret = WRITE_TREE_UNREADABLE_INDEX;
goto out;
@@ -866,7 +872,7 @@ static void prime_cache_tree_rec(struct repository *r,
struct cache_tree_sub *sub;
struct tree *subtree = lookup_tree(r, &entry.oid);
- if (repo_parse_tree(the_repository, subtree) < 0)
+ if (repo_parse_tree(r, subtree) < 0)
exit(128);
sub = cache_tree_sub(it, entry.path);
sub->cache_tree = cache_tree();
diff --git a/cache-tree.h b/cache-tree.h
index 4b3f60d6db..7a2177de83 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -28,8 +28,9 @@ struct cache_tree_sub *cache_tree_sub(struct cache_tree *, const char *);
int cache_tree_subtree_pos(struct cache_tree *it, const char *path, int pathlen);
-void cache_tree_write(struct strbuf *, struct cache_tree *root);
-struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);
+void cache_tree_write(struct repository *repo, struct strbuf *, struct cache_tree *root);
+struct cache_tree *cache_tree_read(struct repository *repo,
+ const char *buffer, unsigned long size);
int cache_tree_fully_valid(struct index_state *);
int cache_tree_update(struct index_state *, int);
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 8eb266cfd1..066dd8bc3b 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -269,9 +269,10 @@ void validate_cache_entries(const struct index_state *istate);
* the given predicate. This function should only be called if
* repo_has_promisor_remote() returns true.
*/
-typedef int (*must_prefetch_predicate)(const struct cache_entry *);
+typedef int (*must_prefetch_predicate)(const struct cache_entry *, void *cb_data);
void prefetch_cache_entries(const struct index_state *istate,
- must_prefetch_predicate must_prefetch);
+ must_prefetch_predicate must_prefetch,
+ void *cb_data);
/* Initialize and use the cache information */
struct lock_file;
diff --git a/read-cache.c b/read-cache.c
index 8044ff820b..e40f290bb3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1748,7 +1748,7 @@ static int read_index_extension(struct index_state *istate,
{
switch (CACHE_EXT(ext)) {
case CACHE_EXT_TREE:
- istate->cache_tree = cache_tree_read(data, sz);
+ istate->cache_tree = cache_tree_read(istate->repo, data, sz);
break;
case CACHE_EXT_RESOLVE_UNDO:
istate->resolve_undo = resolve_undo_read(data, sz, the_hash_algo);
@@ -3012,7 +3012,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
!drop_cache_tree && istate->cache_tree) {
strbuf_reset(&sb);
- cache_tree_write(&sb, istate->cache_tree);
+ cache_tree_write(istate->repo, &sb, istate->cache_tree);
err = write_index_ext_header(f, eoie_c, CACHE_EXT_TREE, sb.len) < 0;
hashwrite(f, sb.buf, sb.len);
if (err) {
@@ -3733,7 +3733,8 @@ static void write_ieot_extension(struct strbuf *sb, struct index_entry_offset_ta
}
void prefetch_cache_entries(const struct index_state *istate,
- must_prefetch_predicate must_prefetch)
+ must_prefetch_predicate must_prefetch,
+ void *cb_data)
{
int i;
struct oid_array to_fetch = OID_ARRAY_INIT;
@@ -3741,7 +3742,7 @@ void prefetch_cache_entries(const struct index_state *istate,
for (i = 0; i < istate->cache_nr; i++) {
struct cache_entry *ce = istate->cache[i];
- if (S_ISGITLINK(ce->ce_mode) || !must_prefetch(ce))
+ if (S_ISGITLINK(ce->ce_mode) || !must_prefetch(ce, cb_data))
continue;
if (!odb_read_object_info_extended(the_repository->objects,
&ce->oid, NULL,
diff --git a/unpack-trees.c b/unpack-trees.c
index f6bb1e6d2b..1802809ad3 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -416,7 +416,8 @@ static void report_collided_checkout(struct index_state *index)
string_list_clear(&list, 0);
}
-static int must_checkout(const struct cache_entry *ce)
+static int must_checkout(const struct cache_entry *ce,
+ void *cb_data UNUSED)
{
return ce->ce_flags & CE_UPDATE;
}
@@ -477,7 +478,7 @@ static int check_updates(struct unpack_trees_options *o,
* Prefetch the objects that are to be checked out in the loop
* below.
*/
- prefetch_cache_entries(index, must_checkout);
+ prefetch_cache_entries(index, must_checkout, NULL);
get_parallel_checkout_configs(&pc_workers, &pc_threshold);
@@ -487,7 +488,7 @@ static int check_updates(struct unpack_trees_options *o,
for (i = 0; i < index->cache_nr; i++) {
struct cache_entry *ce = index->cache[i];
- if (must_checkout(ce)) {
+ if (must_checkout(ce, NULL)) {
size_t last_pc_queue_size = pc_queue_size();
if (ce->ce_flags & CE_WT_REMOVE)
--
2.55.0.1074.ge7621b4bad.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v3 03/13] submodule-config: remove uses of `the_repository`
2026-09-11 5:51 ` [PATCH v3 " Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 01/13] cache-tree: drop `the_repository` in `cache_tree_fully_valid()` Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 02/13] cache-tree: remove dependency on `the_repository` Patrick Steinhardt
@ 2026-09-11 5:51 ` Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 04/13] submodule-config: stop using `the_hash_algo` Patrick Steinhardt
` (9 subsequent siblings)
12 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-11 5:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Justin Tobler
Several functions in the submodule-config subsystem implicitly depend
on `the_repository`. Refactor these to take a `struct repository` as
parameter and adapt callers accordingly.
Note that as usual with these refactorings, callers simply pass
`the_repository` even if they already have a different repository
available in the calling context. This simplifies the migration and
ensures that we don't have a change in behaviour.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/fetch.c | 2 +-
builtin/grep.c | 2 +-
builtin/submodule--helper.c | 8 ++++----
submodule-config.c | 49 ++++++++++++++++++++++++++-------------------
submodule-config.h | 12 +++++++----
submodule.c | 2 +-
t/helper/test-submodule.c | 4 ++--
7 files changed, 45 insertions(+), 34 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index ab7db2be06..533fdfe7d8 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -2681,7 +2681,7 @@ int cmd_fetch(int argc,
int *rs = config.recurse_submodules == RECURSE_SUBMODULES_DEFAULT
? &config.recurse_submodules : NULL;
- fetch_config_from_gitmodules(sfjc, rs);
+ fetch_config_from_gitmodules(the_repository, sfjc, rs);
}
diff --git a/builtin/grep.c b/builtin/grep.c
index d3d86abe01..073dfaaf45 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -897,7 +897,7 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,
if (recurse_submodules) {
submodule_free(opt->repo);
obj_read_lock();
- gitmodules_config_oid(&real_obj->oid);
+ gitmodules_config_oid(the_repository, &real_obj->oid);
obj_read_unlock();
}
if (grep_object(opt, pathspec, real_obj, list->objects[i].name,
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index e7cd3225fa..aaaa963fd8 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -3041,7 +3041,7 @@ static int module_update(int argc, const char **argv, const char *prefix,
NULL
};
- update_clone_config_from_gitmodules(&opt.max_jobs);
+ update_clone_config_from_gitmodules(the_repository, &opt.max_jobs);
repo_config(the_repository, git_update_clone_config, &opt.max_jobs);
argc = parse_options(argc, argv, prefix, module_update_options,
@@ -3255,7 +3255,7 @@ static int module_set_url(int argc, const char **argv, const char *prefix,
path);
config_name = xstrfmt("submodule.%s.url", sub->name);
- ret = config_set_in_gitmodules_file_gently(config_name, newurl);
+ ret = config_set_in_gitmodules_file_gently(the_repository, config_name, newurl);
if (!ret) {
repo_read_gitmodules(the_repository, 0);
@@ -3311,7 +3311,7 @@ static int module_set_branch(int argc, const char **argv, const char *prefix,
path);
config_name = xstrfmt("submodule.%s.branch", sub->name);
- ret = config_set_in_gitmodules_file_gently(config_name, opt_branch);
+ ret = config_set_in_gitmodules_file_gently(the_repository, config_name, opt_branch);
free(config_name);
return !!ret;
@@ -3510,7 +3510,7 @@ static int config_submodule_in_gitmodules(const char *name, const char *var, con
die(_("please make sure that the .gitmodules file is in the working tree"));
key = xstrfmt("submodule.%s.%s", name, var);
- ret = config_set_in_gitmodules_file_gently(key, value);
+ ret = config_set_in_gitmodules_file_gently(the_repository, key, value);
free(key);
return ret;
diff --git a/submodule-config.c b/submodule-config.c
index f75997402a..f8c2cf7a93 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -667,19 +667,20 @@ static int parse_config(const char *var, const char *value,
return ret;
}
-static int gitmodule_oid_from_commit(const struct object_id *treeish_name,
+static int gitmodule_oid_from_commit(struct repository *repo,
+ const struct object_id *treeish_name,
struct object_id *gitmodules_oid,
struct strbuf *rev)
{
int ret = 0;
if (is_null_oid(treeish_name)) {
- oidclr(gitmodules_oid, the_repository->hash_algo);
+ oidclr(gitmodules_oid, repo->hash_algo);
return 1;
}
strbuf_addf(rev, "%s:.gitmodules", oid_to_hex(treeish_name));
- if (repo_get_oid(the_repository, rev->buf, gitmodules_oid) >= 0)
+ if (repo_get_oid(repo, rev->buf, gitmodules_oid) >= 0)
ret = 1;
return ret;
@@ -689,9 +690,11 @@ static int gitmodule_oid_from_commit(const struct object_id *treeish_name,
* (key) with on-demand reading of the appropriate .gitmodules from
* revisions.
*/
-static const struct submodule *config_from(struct submodule_cache *cache,
- const struct object_id *treeish_name, const char *key,
- enum lookup_type lookup_type)
+static const struct submodule *config_from(struct repository *repo,
+ struct submodule_cache *cache,
+ const struct object_id *treeish_name,
+ const char *key,
+ enum lookup_type lookup_type)
{
struct strbuf rev = STRBUF_INIT;
size_t config_size;
@@ -718,7 +721,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
return entry->config;
}
- if (!gitmodule_oid_from_commit(treeish_name, &oid, &rev))
+ if (!gitmodule_oid_from_commit(repo, treeish_name, &oid, &rev))
goto out;
switch (lookup_type) {
@@ -732,7 +735,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
if (submodule)
goto out;
- config = odb_read_object(the_repository->objects, &oid,
+ config = odb_read_object(repo->objects, &oid,
&type, &config_size);
if (!config || type != OBJ_BLOB)
goto out;
@@ -843,21 +846,22 @@ void repo_read_gitmodules(struct repository *repo, int skip_if_read)
repo->submodule_cache->gitmodules_read = 1;
}
-void gitmodules_config_oid(const struct object_id *commit_oid)
+void gitmodules_config_oid(struct repository *repo,
+ const struct object_id *commit_oid)
{
struct strbuf rev = STRBUF_INIT;
struct object_id oid;
- submodule_cache_check_init(the_repository);
+ submodule_cache_check_init(repo);
- if (gitmodule_oid_from_commit(commit_oid, &oid, &rev)) {
+ if (gitmodule_oid_from_commit(repo, commit_oid, &oid, &rev)) {
git_config_from_blob_oid(gitmodules_cb, rev.buf,
- the_repository, &oid, the_repository,
+ repo, &oid, repo,
CONFIG_SCOPE_UNKNOWN);
}
strbuf_release(&rev);
- the_repository->submodule_cache->gitmodules_read = 1;
+ repo->submodule_cache->gitmodules_read = 1;
}
const struct submodule *submodule_from_name(struct repository *r,
@@ -865,7 +869,7 @@ const struct submodule *submodule_from_name(struct repository *r,
const char *name)
{
repo_read_gitmodules(r, 1);
- return config_from(r->submodule_cache, treeish_name, name, lookup_name);
+ return config_from(r, r->submodule_cache, treeish_name, name, lookup_name);
}
const struct submodule *submodule_from_path(struct repository *r,
@@ -873,7 +877,7 @@ const struct submodule *submodule_from_path(struct repository *r,
const char *path)
{
repo_read_gitmodules(r, 1);
- return config_from(r->submodule_cache, treeish_name, path, lookup_path);
+ return config_from(r, r->submodule_cache, treeish_name, path, lookup_path);
}
/**
@@ -980,11 +984,12 @@ int print_config_from_gitmodules(struct repository *repo, const char *key)
return 0;
}
-int config_set_in_gitmodules_file_gently(const char *key, const char *value)
+int config_set_in_gitmodules_file_gently(struct repository *repo,
+ const char *key, const char *value)
{
int ret;
- ret = repo_config_set_in_file_gently(the_repository, GITMODULES_FILE, key, NULL, value);
+ ret = repo_config_set_in_file_gently(repo, GITMODULES_FILE, key, NULL, value);
if (ret < 0)
/* Maybe the user already did that, don't error out here */
warning(_("Could not update .gitmodules entry %s"), key);
@@ -1017,13 +1022,15 @@ static int gitmodules_fetch_config(const char *var, const char *value,
return 0;
}
-void fetch_config_from_gitmodules(int *max_children, int *recurse_submodules)
+void fetch_config_from_gitmodules(struct repository *repo,
+ int *max_children,
+ int *recurse_submodules)
{
struct fetch_config config = {
.max_children = max_children,
.recurse_submodules = recurse_submodules
};
- config_from_gitmodules(gitmodules_fetch_config, the_repository, &config);
+ config_from_gitmodules(gitmodules_fetch_config, repo, &config);
}
static int gitmodules_update_clone_config(const char *var, const char *value,
@@ -1036,7 +1043,7 @@ static int gitmodules_update_clone_config(const char *var, const char *value,
return 0;
}
-void update_clone_config_from_gitmodules(int *max_jobs)
+void update_clone_config_from_gitmodules(struct repository *repo, int *max_jobs)
{
- config_from_gitmodules(gitmodules_update_clone_config, the_repository, max_jobs);
+ config_from_gitmodules(gitmodules_update_clone_config, repo, max_jobs);
}
diff --git a/submodule-config.h b/submodule-config.h
index f55d4e3b61..755570d5d1 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -57,7 +57,8 @@ int option_fetch_parse_recurse_submodules(const struct option *opt,
int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
int parse_push_recurse_submodules_arg(const char *opt, const char *arg);
void repo_read_gitmodules(struct repository *repo, int skip_if_read);
-void gitmodules_config_oid(const struct object_id *commit_oid);
+void gitmodules_config_oid(struct repository *repo,
+ const struct object_id *commit_oid);
/**
* Same as submodule_from_path but lookup by name.
@@ -80,7 +81,8 @@ const struct submodule *submodule_from_path(struct repository *r,
void submodule_free(struct repository *r);
int print_config_from_gitmodules(struct repository *repo, const char *key);
-int config_set_in_gitmodules_file_gently(const char *key, const char *value);
+int config_set_in_gitmodules_file_gently(struct repository *repo,
+ const char *key, const char *value);
/*
* Returns 0 if the name is syntactically acceptable as a submodule "name"
@@ -100,8 +102,10 @@ int check_submodule_url(const char *url);
* New helpers to retrieve arbitrary configuration from the '.gitmodules' file
* should NOT be added.
*/
-void fetch_config_from_gitmodules(int *max_children, int *recurse_submodules);
-void update_clone_config_from_gitmodules(int *max_jobs);
+void fetch_config_from_gitmodules(struct repository *repo,
+ int *max_children,
+ int *recurse_submodules);
+void update_clone_config_from_gitmodules(struct repository *repo, int *max_jobs);
/*
* Submodule entry that contains relevant information about a
diff --git a/submodule.c b/submodule.c
index 5c92575888..6fcb606f7e 100644
--- a/submodule.c
+++ b/submodule.c
@@ -133,7 +133,7 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)
strbuf_addstr(&entry, "submodule.");
strbuf_addstr(&entry, submodule->name);
strbuf_addstr(&entry, ".path");
- ret = config_set_in_gitmodules_file_gently(entry.buf, newpath);
+ ret = config_set_in_gitmodules_file_gently(the_repository, entry.buf, newpath);
strbuf_release(&entry);
return ret;
}
diff --git a/t/helper/test-submodule.c b/t/helper/test-submodule.c
index 3c5c4c4a09..ea9bef0904 100644
--- a/t/helper/test-submodule.c
+++ b/t/helper/test-submodule.c
@@ -168,7 +168,7 @@ static int cmd__submodule_config_set(int argc, const char **argv)
if (!is_writing_gitmodules_ok())
die("please make sure that the .gitmodules file is in the working tree");
- return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
+ return config_set_in_gitmodules_file_gently(the_repository, argv[1], argv[2]);
}
usage_with_options(usage, options);
}
@@ -188,7 +188,7 @@ static int cmd__submodule_config_unset(int argc, const char **argv)
if (argc == 2) {
if (!is_writing_gitmodules_ok())
die("please make sure that the .gitmodules file is in the working tree");
- return config_set_in_gitmodules_file_gently(argv[1], NULL);
+ return config_set_in_gitmodules_file_gently(the_repository, argv[1], NULL);
}
usage_with_options(usage, options);
}
--
2.55.0.1074.ge7621b4bad.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v3 04/13] submodule-config: stop using `the_hash_algo`
2026-09-11 5:51 ` [PATCH v3 " Patrick Steinhardt
` (2 preceding siblings ...)
2026-09-11 5:51 ` [PATCH v3 03/13] submodule-config: remove uses of `the_repository` Patrick Steinhardt
@ 2026-09-11 5:51 ` Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 05/13] submodule-config: stop registering submodule sources Patrick Steinhardt
` (8 subsequent siblings)
12 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-11 5:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Justin Tobler
We have two uses of `the_hash_algo` in "submodule-config.c":
- One trivial use in `gitmodules_cb`, which we can convert to use the
hash algorithm of the repository that's already available in the
caller's context.
- One use where we compute the hashmap key of an object ID. We should
only ever get valid, populated object IDs here, and consequently we
can easily adapt that function to use the hash algorithm of the
passed-in object ID.
Adapt both sites accordingly. Safeguard us against the case where the
passed-in object ID is _not_ properly initialized. While this case
shouldn't ever happen, it doesn't hurt to be defensive.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
submodule-config.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index f8c2cf7a93..7c73fa108b 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -133,7 +133,9 @@ void submodule_cache_free(struct submodule_cache *cache)
static unsigned int hash_oid_string(const struct object_id *oid,
const char *string)
{
- return memhash(oid->hash, the_hash_algo->rawsz) + strhash(string);
+ if (oid->algo == GIT_HASH_UNKNOWN)
+ BUG("hashing an object ID with unknown algorithm");
+ return memhash(oid->hash, hash_algos[oid->algo].rawsz) + strhash(string);
}
static void cache_put_path(struct submodule_cache *cache,
@@ -824,7 +826,7 @@ static int gitmodules_cb(const char *var, const char *value,
parameter.cache = repo->submodule_cache;
parameter.treeish_name = NULL;
- parameter.gitmodules_oid = null_oid(the_hash_algo);
+ parameter.gitmodules_oid = null_oid(repo->hash_algo);
parameter.overwrite = 1;
return parse_config(var, value, ctx, ¶meter);
--
2.55.0.1074.ge7621b4bad.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v3 05/13] submodule-config: stop registering submodule sources
2026-09-11 5:51 ` [PATCH v3 " Patrick Steinhardt
` (3 preceding siblings ...)
2026-09-11 5:51 ` [PATCH v3 04/13] submodule-config: stop using `the_hash_algo` Patrick Steinhardt
@ 2026-09-11 5:51 ` Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 06/13] builtin/grep: stop registering submodule ODB as source Patrick Steinhardt
` (7 subsequent siblings)
12 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-11 5:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Justin Tobler
When reading the ".gitmodules" file from a blob in a repository other
than `the_repository`, we register that repository's object database as
an in-memory source of `the_repository`'s object database. This call has
its origins in d9b8b8f896 (submodule-config.c: use repo_get_oid for
reading .gitmodules, 2019-04-16): back then, `config_with_options()` was
not able to read a blob from an arbitrary repository, but would always
read it via `the_repository`. So even though the blob could be resolved
in the submodule repository via `repo_get_oid()`, the submodule's object
database had to be registered as an in-memory source of `the_repository`
so that the subsequent object read was able to find the blob at all.
That need went away with e3e8bf046e (submodule-config: pass repo
upon blob config read, 2021-08-16), which taught the config machinery
to read the blob from the repository we pass to it. The same series
converted the eager submodule source registration into a lazy mechanism
that only registers submodule sources with the object database when an
object lookup failed. The intent though was that we don't ever have to
fall back to this mechanism in the first place, and to verify that this
is the case we introduced GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB. If set,
then any such lazy registration would cause us to BUG.
At the beginning of this series, we still triggered this bug in t1092.
But now that we have converted the "cache-tree" subsystem to not depend
on `the_repository` anymore it also knows to properly access objects via
the submodule. With that change, GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
does not cause any failures anymore.
Remove the call to `odb_add_submodule_source_by_path()`. This removes
the last user of `the_repository`, so at the same time we can also get
rid of `USE_THE_REPOSITORY_VARIABLE`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
submodule-config.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index 7c73fa108b..37c3be377b 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -803,9 +802,6 @@ static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void
} else if (repo_get_oid(repo, GITMODULES_INDEX, &oid) >= 0 ||
repo_get_oid(repo, GITMODULES_HEAD, &oid) >= 0) {
config_source.blob = oidstr = xstrdup(oid_to_hex(&oid));
- if (repo != the_repository)
- odb_add_submodule_source_by_path(the_repository->objects,
- repo->objects->sources->path);
} else {
goto out;
}
--
2.55.0.1074.ge7621b4bad.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v3 06/13] builtin/grep: stop registering submodule ODB as source
2026-09-11 5:51 ` [PATCH v3 " Patrick Steinhardt
` (4 preceding siblings ...)
2026-09-11 5:51 ` [PATCH v3 05/13] submodule-config: stop registering submodule sources Patrick Steinhardt
@ 2026-09-11 5:51 ` Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 07/13] odb: remove infrastructure to register submodule sources Patrick Steinhardt
` (6 subsequent siblings)
12 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-11 5:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Justin Tobler
Same as with the preceding commit, git-grep(1) registers each
submodule's object database as an in-memory source of the main object
database before grepping it. This was introduced as an eager alternate
registration and converted into the lazy mechanism via 8d33c3af0b (grep:
use submodule-ODB-as-alternate lazy-addition, 2021-08-16).
Starting with 0693806bf8 (grep: add repository to OID grep sources,
2021-08-16), the command instead knows to pass submodule repositories to
our workers, which means that those now use that repository to look up
objects, too. As a consequence, registering submodule sources as
alternates is not required anymore.
Remove the logic to register submodule sources. Unfortunately, this does
not allow us to get rid of the object read lock as initializing the
subrepository is still racy.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/grep.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 073dfaaf45..b045f8a488 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -463,16 +463,6 @@ static int grep_submodule(struct grep_opt *opt,
ALLOC_GROW(repos_to_free, repos_to_free_nr + 1, repos_to_free_alloc);
repos_to_free[repos_to_free_nr++] = subrepo;
- /*
- * NEEDSWORK: repo_read_gitmodules() might call
- * odb_add_to_alternates_memory() via config_from_gitmodules(). This
- * operation causes a race condition with concurrent object readings
- * performed by the worker threads. That's why we need obj_read_lock()
- * here. It should be removed once it's no longer necessary to add the
- * subrepo's odbs to the in-memory alternates list.
- */
- obj_read_lock();
-
/*
* NEEDSWORK: when reading a submodule, the sparsity settings in the
* superproject are incorrectly forgotten or misused. For example:
@@ -498,18 +488,14 @@ static int grep_submodule(struct grep_opt *opt,
* ditto.
*
* Note that this list is not exhaustive.
+ *
+ * NEEDSWORK: initializing the subrepository is not thread-safe,
+ * either, as it may cause us to race around `get_main_ref_store()`. We
+ * thus need to hold the object-read lock to serialize all readers with
+ * one another.
*/
+ obj_read_lock();
repo_read_gitmodules(subrepo, 0);
-
- /*
- * All code paths tested by test code no longer need submodule ODBs to
- * be added as alternates, but add it to the list just in case.
- * Submodule ODBs added through add_submodule_odb_by_path() will be
- * lazily registered as alternates when needed (and except in an
- * unexpected code interaction, it won't be needed).
- */
- odb_add_submodule_source_by_path(the_repository->objects,
- subrepo->objects->sources->path);
obj_read_unlock();
memcpy(&subopt, opt, sizeof(subopt));
--
2.55.0.1074.ge7621b4bad.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v3 07/13] odb: remove infrastructure to register submodule sources
2026-09-11 5:51 ` [PATCH v3 " Patrick Steinhardt
` (5 preceding siblings ...)
2026-09-11 5:51 ` [PATCH v3 06/13] builtin/grep: stop registering submodule ODB as source Patrick Steinhardt
@ 2026-09-11 5:51 ` Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 08/13] tmp-objdir: drop unused function to register alternate Patrick Steinhardt
` (5 subsequent siblings)
12 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-11 5:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Justin Tobler
The preceding commits have removed the last two users of
`odb_add_submodule_source_by_path()`. The mechanism was only ever
meant as a transitional crutch while migrating submodule object
access away from "add the submodule ODB as an alternate of
the_repository" towards explicitly passing the submodule repository,
see a35e03dee0 (submodule: lazily add submodule ODBs as alternates,
2021-08-16). Remove it.
As GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB is now a no-op, remove its
documentation and the exports from the test suite, as well.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb.c | 36 ----------------------------------
odb.h | 14 -------------
t/README | 7 -------
t/t5526-fetch-submodules.sh | 3 ---
t/t5531-deep-submodule-push.sh | 3 ---
t/t5545-push-options.sh | 3 ---
t/t5572-pull-submodule.sh | 3 ---
t/t6437-submodule-merge.sh | 3 ---
t/t7418-submodule-sparse-gitmodules.sh | 3 ---
t/t7814-grep-recurse-submodules.sh | 3 ---
10 files changed, 78 deletions(-)
diff --git a/odb.c b/odb.c
index 6d5943e5ea..2f8a70a90c 100644
--- a/odb.c
+++ b/odb.c
@@ -388,12 +388,6 @@ struct odb_source *odb_find_source_or_die(struct object_database *odb, const cha
return source;
}
-void odb_add_submodule_source_by_path(struct object_database *odb,
- const char *path)
-{
- string_list_insert(&odb->submodule_source_paths, path);
-}
-
static void fill_alternate_refs_command(struct repository *repo,
struct child_process *cmd,
const char *repo_path)
@@ -549,23 +543,6 @@ void disable_obj_read_lock(void)
pthread_mutex_destroy(&obj_read_mutex);
}
-static int register_all_submodule_sources(struct object_database *odb)
-{
- int ret = odb->submodule_source_paths.nr;
-
- for (size_t i = 0; i < odb->submodule_source_paths.nr; i++)
- odb_add_to_alternates_memory(odb,
- odb->submodule_source_paths.items[i].string);
- if (ret) {
- string_list_clear(&odb->submodule_source_paths, 0);
- trace2_data_intmax("submodule", odb->repo,
- "register_all_submodule_sources/registered", ret);
- if (git_env_bool("GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB", 0))
- BUG("register_all_submodule_sources() called");
- }
- return ret;
-}
-
static enum odb_read_status do_oid_object_info_extended(struct object_database *odb,
const struct object_id *oid,
struct object_info *oi, unsigned flags)
@@ -614,16 +591,6 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database *
}
}
- /*
- * This might be an attempt at accessing a submodule object as
- * if it were in main object store (having called
- * `odb_add_submodule_source_by_path()` on that submodule's
- * ODB). If any such ODBs exist, register them and try again.
- */
- if (register_all_submodule_sources(odb))
- /* We added some alternates; retry */
- continue;
-
/* Check if it is a missing object */
if (odb->repo->fetch_if_missing && repo_has_promisor_remote(odb->repo) &&
!already_retried &&
@@ -1109,7 +1076,6 @@ struct object_database *odb_new(struct repository *repo,
CALLOC_ARRAY(o, 1);
o->repo = repo;
pthread_mutex_init(&o->replace_mutex, NULL);
- string_list_init_dup(&o->submodule_source_paths);
hashmap_init(&o->source_by_path, odb_source_by_path_cmp, o, 0);
o->source_paths_icase = -1;
@@ -1166,8 +1132,6 @@ void odb_free(struct object_database *o)
odb_close(o);
odb_free_sources(o);
- string_list_clear(&o->submodule_source_paths, 0);
-
free(o);
}
diff --git a/odb.h b/odb.h
index 248ee9cdfa..54548efc55 100644
--- a/odb.h
+++ b/odb.h
@@ -89,12 +89,6 @@ struct object_database {
unsigned long object_count;
unsigned object_count_flags;
unsigned object_count_valid : 1;
-
- /*
- * Submodule source paths that will be added as additional sources to
- * allow lookup of submodule objects via the main object database.
- */
- struct string_list submodule_source_paths;
};
enum odb_new_flags {
@@ -224,14 +218,6 @@ void odb_restore_primary_source(struct object_database *odb,
struct odb_source *restore_source,
const char *old_path);
-/*
- * Call odb_add_submodule_source_by_path() to add the submodule at the given
- * path to a list. The object stores of all submodules in that list will be
- * added as additional sources in the object store when looking up objects.
- */
-void odb_add_submodule_source_by_path(struct object_database *odb,
- const char *path);
-
/*
* Iterate through all alternates of the database and execute the provided
* callback function for each of them. Stop iterating once the callback
diff --git a/t/README b/t/README
index 9a9daaf2af..f831c5355b 100644
--- a/t/README
+++ b/t/README
@@ -462,13 +462,6 @@ GIT_TEST_CHECKOUT_WORKERS=<n> overrides the 'checkout.workers' setting
to <n> and 'checkout.thresholdForParallelism' to 0, forcing the
execution of the parallel-checkout code.
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=<boolean>, when true, makes
-registering submodule ODBs as alternates a fatal action. Support for
-this environment variable can be removed once the migration to
-explicitly providing repositories when accessing submodule objects is
-complete or needs to be abandoned for whatever reason (in which case the
-migrated codepaths still retain their performance benefits).
-
GIT_TEST_REQUIRE_PREREQ=<list> allows specifying a space separated list of
prereqs that are required to succeed. If a prereq in this list is triggered by
a test and then fails then the whole test run will abort. This can help to make
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 7b3b7359da..37d7373b36 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -3,9 +3,6 @@
test_description='Recursive "git fetch" for submodules'
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
pwd=$(pwd)
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 7d239dd31f..73429ec6e3 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -5,9 +5,6 @@ test_description='test push with submodules'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
test_expect_success setup '
diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh
index fb13549da7..239edd7d62 100755
--- a/t/t5545-push-options.sh
+++ b/t/t5545-push-options.sh
@@ -5,9 +5,6 @@ test_description='pushing to a repository using push options'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
mk_repo_pair () {
diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh
index 42d14328b6..9969a3294e 100755
--- a/t/t5572-pull-submodule.sh
+++ b/t/t5572-pull-submodule.sh
@@ -2,9 +2,6 @@
test_description='pull can handle submodules'
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-submodule-update.sh
diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh
index 107e13afbc..1546d5f773 100755
--- a/t/t6437-submodule-merge.sh
+++ b/t/t6437-submodule-merge.sh
@@ -5,9 +5,6 @@ test_description='merging with submodules'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
#
diff --git a/t/t7418-submodule-sparse-gitmodules.sh b/t/t7418-submodule-sparse-gitmodules.sh
index dde11ecce8..cf94e30e78 100755
--- a/t/t7418-submodule-sparse-gitmodules.sh
+++ b/t/t7418-submodule-sparse-gitmodules.sh
@@ -12,9 +12,6 @@ The test setup uses a sparse checkout, however the same scenario can be set up
also by committing .gitmodules and then just removing it from the filesystem.
'
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
. ./test-lib.sh
test_expect_success 'setup' '
diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh
index e1cf53dc9e..3d149d34c1 100755
--- a/t/t7814-grep-recurse-submodules.sh
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -9,9 +9,6 @@ submodules.
TEST_CREATE_REPO_NO_TEMPLATE=1
. ./test-lib.sh
-GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
-export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
-
test_expect_success 'setup directory structure and submodule' '
echo "(1|2)d(3|4)" >a &&
mkdir b &&
--
2.55.0.1074.ge7621b4bad.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v3 08/13] tmp-objdir: drop unused function to register alternate
2026-09-11 5:51 ` [PATCH v3 " Patrick Steinhardt
` (6 preceding siblings ...)
2026-09-11 5:51 ` [PATCH v3 07/13] odb: remove infrastructure to register submodule sources Patrick Steinhardt
@ 2026-09-11 5:51 ` Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 09/13] odb/packed: fix memory leaks when freeing source Patrick Steinhardt
` (4 subsequent siblings)
12 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-11 5:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Justin Tobler
The last caller of `tmp_objdir_add_as_alternate()` went away in
bdee7b3013 (builtin/receive-pack: stage incoming objects via ODB
transactions, 2026-07-10) and is unused now. Remove the function.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
tmp-objdir.c | 5 -----
tmp-objdir.h | 6 ------
2 files changed, 11 deletions(-)
diff --git a/tmp-objdir.c b/tmp-objdir.c
index 0eaa79ffd7..deaaf6ba2e 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -321,11 +321,6 @@ const char **tmp_objdir_env(const struct tmp_objdir *t)
return t->env.v;
}
-void tmp_objdir_add_as_alternate(const struct tmp_objdir *t)
-{
- odb_add_to_alternates_memory(t->repo->objects, t->path.buf);
-}
-
struct odb_source *tmp_objdir_replace_primary_odb(struct tmp_objdir *t,
int will_destroy)
{
diff --git a/tmp-objdir.h b/tmp-objdir.h
index 81eb927413..05f0d08d10 100644
--- a/tmp-objdir.h
+++ b/tmp-objdir.h
@@ -55,12 +55,6 @@ int tmp_objdir_destroy(struct tmp_objdir *);
*/
void tmp_objdir_discard_objects(struct tmp_objdir *);
-/*
- * Add the temporary object directory as an alternate object store in the
- * current process.
- */
-void tmp_objdir_add_as_alternate(const struct tmp_objdir *);
-
/*
* Replaces the writable object store in the current process with the temporary
* object directory and makes the former main object store an alternate.
--
2.55.0.1074.ge7621b4bad.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v3 09/13] odb/packed: fix memory leaks when freeing source
2026-09-11 5:51 ` [PATCH v3 " Patrick Steinhardt
` (7 preceding siblings ...)
2026-09-11 5:51 ` [PATCH v3 08/13] tmp-objdir: drop unused function to register alternate Patrick Steinhardt
@ 2026-09-11 5:51 ` Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 10/13] builtin/multi-pack-index: refuse unknown sources with "--object-dir=" Patrick Steinhardt
` (3 subsequent siblings)
12 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-11 5:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Justin Tobler
When freeing a "packed" source we don't close either its packs nor its
multi-pack indices. This can cause memory leaks in case we create an
ad-hoc packed source. As we used to always link packed sources to the
main object database we never noticed this issue until now, but it's
going to surface in subsequent commits where we stop linking them.
Plug the memory leaks by closing the source first.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-packed.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 1d90e714e6..166e76e2d6 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -844,6 +844,7 @@ static void odb_source_packed_free(struct odb_source *source)
chdir_notify_unregister(odb_source_packed_reparent, packed);
+ odb_source_close(source);
for (struct packfile_list_entry *e = packed->packs.head; e; e = e->next)
free(e->pack);
packfile_list_clear(&packed->packs);
--
2.55.0.1074.ge7621b4bad.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v3 10/13] builtin/multi-pack-index: refuse unknown sources with "--object-dir="
2026-09-11 5:51 ` [PATCH v3 " Patrick Steinhardt
` (8 preceding siblings ...)
2026-09-11 5:51 ` [PATCH v3 09/13] odb/packed: fix memory leaks when freeing source Patrick Steinhardt
@ 2026-09-11 5:51 ` Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 11/13] t/helper: adapt read-midx to not link ad-hoc source anymore Patrick Steinhardt
` (2 subsequent siblings)
12 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-11 5:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Justin Tobler
Users can tell git-multi-pack-index(1) to access multi-pack indices that
are stored in a different object directory via the "--object-dir="
option. This allows them to for example write or verify a multi-pack
index other than the one located in the main object directory in case a
repository has alternates with multiple multi-pack indices.
But while the documentation explicitly points out that the specified
object directory must be an alternate of the current repository, we
never verify that property. Instead, starting with 017db7bb14 (midx:
load multi-pack indices via their source, 2025-08-11), we now construct
an ad-hoc source and link it to the main object directory.
Besides contradicting the documentation, it's dubious that this really
ought to work in the first place: creating a multi-pack index (and
potentially a bitmap) for a completely foreign object directory is of
questionable value, as bitmap commit selection operates on the invoking
repository's refs. Furthermore, this is the only remaining caller
outside of our test helpers that constructs an ad-hoc source and links
it to the database, and we want to get rid of this mechanism as part of
this series.
Stop constructing the ad-hoc source and instead refuse the operation.
While this results in a change in behaviour, this restriction has been
documented as such ever since f57a739691 (midx: avoid opening multiple
MIDXs when writing, 2021-09-01).
Note that this change requires us to adapt one test chain in t5319, as
it creates an object directory that is not connected to any repository
and then uses it via "--object-dir=". The setup itself already documents
this and does the necessary gymnastics to link the object directory to a
temporary repository, but subsequent tests don't. Adapt those tests to
retain and reuse the temporary repository.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/multi-pack-index.c | 3 ++-
t/t5319-multi-pack-index.sh | 9 ++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index 6e73c85cde..753bd53a70 100644
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -90,7 +90,8 @@ static struct odb_source_files *handle_object_dir_option(struct repository *repo
{
struct odb_source *source = odb_find_source(repo->objects, opts.object_dir);
if (!source)
- source = odb_add_to_alternates_memory(repo->objects, opts.object_dir);
+ die(_("object directory is not an alternate of the current repository: '%s'"),
+ opts.object_dir);
return odb_source_files_downcast(source);
}
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 68143cb5b7..00e90f163f 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -698,10 +698,9 @@ test_expect_success 'force some 64-bit offsets with pack-objects' '
corrupt_data $idx64 $(test_oid idxoff) "\02" &&
# objects64 is not a real repository, but can serve as an alternate
# anyway so we can write a MIDX into it
- git init repo &&
- test_when_finished "rm -fr repo" &&
+ git init repo64 &&
(
- cd repo &&
+ cd repo64 &&
( cd ../objects64 && pwd ) >.git/objects/info/alternates &&
midx64=$(git multi-pack-index --object-dir=../objects64 write)
) &&
@@ -709,7 +708,7 @@ test_expect_success 'force some 64-bit offsets with pack-objects' '
'
test_expect_success 'verify multi-pack-index with 64-bit offsets' '
- git multi-pack-index verify --object-dir=objects64
+ git -C repo64 multi-pack-index verify --object-dir=../objects64
'
NUM_OBJECTS=63
@@ -721,7 +720,7 @@ MIDX_BYTE_LARGE_OFFSET=$(($MIDX_OFFSET_LARGE_OFFSETS + 3))
test_expect_success 'verify incorrect 64-bit offset' '
corrupt_midx_and_verify $MIDX_BYTE_LARGE_OFFSET "\07" objects64 \
- "incorrect object offset"
+ "incorrect object offset" "git -C repo64 multi-pack-index verify --object-dir=../objects64"
'
test_expect_success 'setup expire tests' '
--
2.55.0.1074.ge7621b4bad.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v3 11/13] t/helper: adapt read-midx to not link ad-hoc source anymore
2026-09-11 5:51 ` [PATCH v3 " Patrick Steinhardt
` (9 preceding siblings ...)
2026-09-11 5:51 ` [PATCH v3 10/13] builtin/multi-pack-index: refuse unknown sources with "--object-dir=" Patrick Steinhardt
@ 2026-09-11 5:51 ` Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 12/13] t/helper: stop registering alternates in "ref-store" command Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 13/13] odb: remove the ability to link sources ad-hoc Patrick Steinhardt
12 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-11 5:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Justin Tobler
Same as in the preceding commit, refactor the setup of ad-hoc object
database sources when accessing a multi-pack index in an arbitrary
location to not link the newly created source into the main object
database anymore.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/helper/test-read-midx.c | 43 +++++++++++++++++++++++++++++++------------
1 file changed, 31 insertions(+), 12 deletions(-)
diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c
index 27a05da957..1f7a1927e4 100644
--- a/t/helper/test-read-midx.c
+++ b/t/helper/test-read-midx.c
@@ -5,34 +5,42 @@
#include "midx.h"
#include "repository.h"
#include "odb.h"
+#include "odb/source-packed.h"
#include "pack-bitmap.h"
#include "packfile.h"
#include "setup.h"
#include "gettext.h"
#include "pack-revindex.h"
-static struct multi_pack_index *setup_midx(const char *object_dir)
+static struct multi_pack_index *setup_midx(const char *object_dir,
+ struct odb_source_packed **out)
{
- struct odb_source_files *files;
+ struct odb_source_packed *packed;
struct odb_source *source;
+
setup_git_directory(the_repository);
+
source = odb_find_source(the_repository->objects, object_dir);
- if (!source)
- source = odb_add_to_alternates_memory(the_repository->objects,
- object_dir);
- files = odb_source_files_downcast(source);
+ if (source) {
+ packed = odb_source_files_downcast(source)->packed;
+ } else {
+ packed = odb_source_packed_new(the_repository->objects,
+ object_dir, false);
+ *out = packed;
+ }
- return load_multi_pack_index(files->packed);
+ return load_multi_pack_index(packed);
}
static int read_midx_file(const char *object_dir, const char *checksum,
int show_objects)
{
+ struct odb_source_packed *packed = NULL;
uint32_t i;
struct multi_pack_index *m, *tip;
int ret = 0;
- m = tip = setup_midx(object_dir);
+ m = tip = setup_midx(object_dir, &packed);
if (!m)
return 1;
@@ -91,29 +99,35 @@ static int read_midx_file(const char *object_dir, const char *checksum,
out:
close_midx(tip);
+ if (packed)
+ odb_source_free(&packed->base);
return ret;
}
static int read_midx_checksum(const char *object_dir)
{
+ struct odb_source_packed *packed = NULL;
struct multi_pack_index *m;
- m = setup_midx(object_dir);
+ m = setup_midx(object_dir, &packed);
if (!m)
return 1;
printf("%s\n", midx_get_checksum_hex(m));
close_midx(m);
+ if (packed)
+ odb_source_free(&packed->base);
return 0;
}
static int read_midx_preferred_pack(const char *object_dir)
{
+ struct odb_source_packed *packed = NULL;
struct multi_pack_index *midx = NULL;
uint32_t preferred_pack;
- midx = setup_midx(object_dir);
+ midx = setup_midx(object_dir, &packed);
if (!midx)
return 1;
@@ -124,17 +138,21 @@ static int read_midx_preferred_pack(const char *object_dir)
}
printf("%s\n", midx->pack_names[preferred_pack]);
+
close_midx(midx);
+ if (packed)
+ odb_source_free(&packed->base);
return 0;
}
static int read_midx_bitmapped_packs(const char *object_dir)
{
+ struct odb_source_packed *packed = NULL;
struct multi_pack_index *midx = NULL;
struct bitmapped_pack pack;
uint32_t i;
- midx = setup_midx(object_dir);
+ midx = setup_midx(object_dir, &packed);
if (!midx)
return 1;
@@ -150,7 +168,8 @@ static int read_midx_bitmapped_packs(const char *object_dir)
}
close_midx(midx);
-
+ if (packed)
+ odb_source_free(&packed->base);
return 0;
}
--
2.55.0.1074.ge7621b4bad.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v3 12/13] t/helper: stop registering alternates in "ref-store" command
2026-09-11 5:51 ` [PATCH v3 " Patrick Steinhardt
` (10 preceding siblings ...)
2026-09-11 5:51 ` [PATCH v3 11/13] t/helper: adapt read-midx to not link ad-hoc source anymore Patrick Steinhardt
@ 2026-09-11 5:51 ` Patrick Steinhardt
2026-09-11 5:51 ` [PATCH v3 13/13] odb: remove the ability to link sources ad-hoc Patrick Steinhardt
12 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-11 5:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Justin Tobler
When using the "ref-store" command we support access to multiple
different reference stores. As part of that we allow the caller to
explicitly exercise stores of a submodule. This allows us to verify
low-level behaviour of submodule stores, which is exercised in t1406.
When doing so we also link the submodule's object database into the main
object database. The intent of this is that it allows us to access
objects of the submodule, too. But that functionality is not even
needed anymore: when creating a submodule reference store, we will first
initialize the submodule repository and then initialize the store with
that repository. And as the reference subsystem doesn't depend on
`the_repository` anymore all subsequent object lookups performed by the
reference store will be routed to the submodule repository.
It is thus not needed anymore to register the submodule object store
with the main object database. Remove the call.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/helper/test-ref-store.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 5a9a3053d9..db58f00589 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -74,14 +74,6 @@ static const char **get_store(const char **argv, struct ref_store **refs)
} else if (!strcmp(argv[0], "main")) {
*refs = get_main_ref_store(the_repository);
} else if (skip_prefix(argv[0], "submodule:", &gitdir)) {
- struct strbuf sb = STRBUF_INIT;
-
- if (!repo_submodule_path_append(the_repository,
- &sb, gitdir, "objects/"))
- die("computing submodule path failed");
- odb_add_to_alternates_memory(the_repository->objects, sb.buf);
- strbuf_release(&sb);
-
*refs = repo_get_submodule_ref_store(the_repository, gitdir);
} else if (skip_prefix(argv[0], "worktree:", &gitdir)) {
struct worktree **p, **worktrees = get_worktrees(the_repository);
--
2.55.0.1074.ge7621b4bad.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v3 13/13] odb: remove the ability to link sources ad-hoc
2026-09-11 5:51 ` [PATCH v3 " Patrick Steinhardt
` (11 preceding siblings ...)
2026-09-11 5:51 ` [PATCH v3 12/13] t/helper: stop registering alternates in "ref-store" command Patrick Steinhardt
@ 2026-09-11 5:51 ` Patrick Steinhardt
12 siblings, 0 replies; 54+ messages in thread
From: Patrick Steinhardt @ 2026-09-11 5:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Justin Tobler
Over the course of this patch series we have adapted all callers of
`odb_add_to_alternates_memory()` to not do so anymore. Remove the
function.
This series of refactorings doesn't only simplify our code base. More
importantly, with those changes in place we can now unconditionally
assume that the list of sources linked to the object database only
consists of the primary source and its alternates. This serves as the
foundation to eventually move handling of alternates into the "files"
backend itself.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb.c | 6 ------
odb.h | 8 --------
2 files changed, 14 deletions(-)
diff --git a/odb.c b/odb.c
index 2f8a70a90c..5fe081496f 100644
--- a/odb.c
+++ b/odb.c
@@ -247,12 +247,6 @@ void odb_add_to_alternates_file(struct object_database *odb,
odb_add_alternate_recursively(odb, dir, 0);
}
-struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
- const char *dir)
-{
- return odb_add_alternate_recursively(odb, dir, 0);
-}
-
struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
const char *dir, int will_destroy,
struct odb_source **prev_source)
diff --git a/odb.h b/odb.h
index 54548efc55..9025239df5 100644
--- a/odb.h
+++ b/odb.h
@@ -258,14 +258,6 @@ int odb_has_alternates(struct object_database *odb);
void odb_add_to_alternates_file(struct object_database *odb,
const char *dir);
-/*
- * Add the directory to the in-memory list of alternate sources (along with any
- * recursive alternates it points to), but do not modify the on-disk alternates
- * file.
- */
-struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
- const char *dir);
-
/*
* Read an object from the database. Returns the object data and assigns object
* type and size to the `type` and `size` pointers, if these pointers are
--
2.55.0.1074.ge7621b4bad.dirty
^ permalink raw reply related [flat|nested] 54+ messages in thread