[PATCH v3 01/13] cache-tree: drop `the_repository` in `cache_tree_fully_valid()`
From: Patrick Steinhardt <hidden>
Date: 2026-09-11 05:51:52
Subsystem:
the rest · Maintainer:
Linus Torvalds
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 <redacted> Signed-off-by: Patrick Steinhardt <redacted> --- 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