[PATCH 1/2] unpack-trees: lose USE_THE_REPOSITORY

[PATCH 1/2] unpack-trees: lose USE_THE_REPOSITORY

[Junio C Hamano]

This file defines a set of fairly low level functions to implement
merge and checkout behaviour, many of which depended on and can only
work with the_repository.

Two key data structures passed around in the code paths, "struct
unpack_trees_options" and "struct index_state", both have a pointer
to the repository instance, and we can use it to make it work on an
arbitrary repository, not just the_repository.

In any correctly written clients of this library file, the
repository instance that is pointed at by these two structs MUST be
the same one.  For now, make sure that is the case (and otherwise
BUG out) by introducing a unpack_repo() helper function that takes
these two structures (when available) and returns the repository.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * I wrote this change when I ranted in

   https://lore.kernel.org/git/xmqq33tdcuub.fsf@gitster.g/

   on 'seen', and "git checkout -m --detach master" was sufficient
   to build and start a new branch based on 'master' and there
   wasn't any need for adjustment.  All tests seem to pass.

 unpack-trees.c | 94 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 76 insertions(+), 18 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index d6a859420f..79341d7fa6 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "git-compat-util.h"
@@ -82,6 +81,56 @@ static const char *unpack_plumbing_errors[NB_UNPACK_TREES_WARNING_TYPES] = {
 	  ? ((o)->internal.msgs[(type)])      \
 	  : (unpack_plumbing_errors[(type)]) )
 
+
+/*
+ * Some functions in these code paths get unpack_trees_options, which
+ * has up to two istates, and some get an istate.  These istates can
+ * be used to find the repository to be used for this operation.
+ *
+ * As these are all working on the same repository, they must match.
+ * For now, with a super paranoid check, make sure all potential
+ * sources of the information match before using it.
+ *
+ * Once this proves working well, we could just use the first one we
+ * find in a production setting.
+ */
+static struct repository *unpack_repo(struct index_state *istate,
+				      struct unpack_trees_options *o)
+{
+	struct repository *repo = NULL;
+	const char *camefrom = NULL;
+
+	if (istate) {
+		if (!istate->repo)
+			BUG("istate->repo == NULL");
+		repo = istate->repo;
+		camefrom = "istate->repo";
+	}
+	if (o) {
+		if (o->src_index) {
+			if (!o->src_index->repo)
+				BUG("o->src_index->repo == NULL");
+			if (repo && repo != o->src_index->repo)
+				BUG("%s != o->src_index->repo", camefrom);
+			repo = o->src_index->repo;
+			camefrom = "o->src_index->repo";
+		}
+
+		if (o->dst_index) {
+			if (!o->dst_index->repo)
+				BUG("o->dst_index->repo == NULL");
+			if (repo && repo != o->dst_index->repo)
+				BUG("%s != o->dst_index->repo", camefrom);
+			repo = o->dst_index->repo;
+		}
+	}
+	if (!repo)
+		BUG("no repository for unpack_trees()?");
+	return repo;
+
+}
+
+
 static const char *super_prefixed(const char *path, const char *super_prefix)
 {
 	/*
@@ -345,15 +394,16 @@ static void load_gitmodules_file(struct index_state *index,
 				 struct checkout *state)
 {
 	int pos = index_name_pos(index, GITMODULES_FILE, strlen(GITMODULES_FILE));
+	struct repository *repo = unpack_repo(index, NULL);
 
 	if (pos >= 0) {
 		struct cache_entry *ce = index->cache[pos];
 		if (!state && ce->ce_flags & CE_WT_REMOVE) {
-			repo_read_gitmodules(the_repository, 0);
+			repo_read_gitmodules(repo, 0);
 		} else if (state && (ce->ce_flags & CE_UPDATE)) {
-			submodule_free(the_repository);
+			submodule_free(repo);
 			checkout_entry(ce, state, NULL, NULL);
-			repo_read_gitmodules(the_repository, 0);
+			repo_read_gitmodules(repo, 0);
 		}
 	}
 }
@@ -362,6 +412,7 @@ static struct progress *get_progress(struct unpack_trees_options *o,
 				     struct index_state *index)
 {
 	unsigned cnt = 0, total = 0;
+	struct repository *repo = unpack_repo(index, o);
 
 	if (!o->update || !o->verbose_update)
 		return NULL;
@@ -372,7 +423,7 @@ static struct progress *get_progress(struct unpack_trees_options *o,
 			total++;
 	}
 
-	return start_delayed_progress(the_repository,
+	return start_delayed_progress(repo,
 				      _("Updating files"), total);
 }
 
@@ -430,6 +481,7 @@ static int check_updates(struct unpack_trees_options *o,
 	struct progress *progress;
 	struct checkout state = CHECKOUT_INIT;
 	int i, pc_workers, pc_threshold;
+	struct repository *repo = unpack_repo(index, o);
 
 	trace_performance_enter();
 	state.super_prefix = o->super_prefix;
@@ -473,7 +525,7 @@ static int check_updates(struct unpack_trees_options *o,
 	if (should_update_submodules())
 		load_gitmodules_file(index, &state);
 
-	if (repo_has_promisor_remote(the_repository))
+	if (repo_has_promisor_remote(repo))
 		/*
 		 * Prefetch the objects that are to be checked out in the loop
 		 * below.
@@ -879,6 +931,7 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,
 	struct traverse_info newinfo;
 	struct name_entry *p;
 	int nr_entries;
+	struct repository *repo = unpack_repo(NULL, o);
 
 	nr_entries = all_trees_same_as_cache_tree(n, dirmask, names, info);
 	if (nr_entries > 0) {
@@ -940,7 +993,7 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,
 			const struct object_id *oid = NULL;
 			if (dirmask & 1)
 				oid = &names[i].oid;
-			buf[nr_buf++] = fill_tree_descriptor(the_repository, t + i, oid);
+			buf[nr_buf++] = fill_tree_descriptor(repo, t + i, oid);
 		}
 	}
 
@@ -1773,24 +1826,25 @@ static int clear_ce_flags(struct index_state *istate,
 	static struct strbuf prefix = STRBUF_INIT;
 	char label[100];
 	int rval;
+	struct repository *repo = unpack_repo(istate, NULL);
 
 	strbuf_reset(&prefix);
 	if (show_progress)
 		istate->progress = start_delayed_progress(
-					the_repository,
+					repo,
 					_("Updating index flags"),
 					istate->cache_nr);
 
 	xsnprintf(label, sizeof(label), "clear_ce_flags(0x%08lx,0x%08lx)",
 		  (unsigned long)select_mask, (unsigned long)clear_mask);
-	trace2_region_enter("unpack_trees", label, istate->repo);
+	trace2_region_enter("unpack_trees", label, repo);
 	rval = clear_ce_flags_1(istate,
 				istate->cache,
 				istate->cache_nr,
 				&prefix,
 				select_mask, clear_mask,
 				pl, 0, 0);
-	trace2_region_leave("unpack_trees", label, istate->repo);
+	trace2_region_leave("unpack_trees", label, repo);
 
 	stop_progress(&istate->progress);
 	return rval;
@@ -1885,13 +1939,13 @@ static int verify_absent(const struct cache_entry *,
  */
 int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)
 {
-	struct repository *repo = o->src_index->repo;
+	struct repository *repo = unpack_repo(NULL, o);
 	int i, ret;
 	static struct cache_entry *dfc;
 	struct pattern_list pl;
 	int free_pattern_list = 0;
 	struct dir_struct dir = DIR_INIT;
-	struct repo_config_values *cfg = repo_config_values(the_repository);
+	struct repo_config_values *cfg = repo_config_values(repo);
 
 	if (o->reset == UNPACK_RESET_INVALID)
 		BUG("o->reset had a value of 1; should be UNPACK_TREES_*_UNTRACKED");
@@ -1936,7 +1990,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 		populate_from_existing_patterns(o, &pl);
 	}
 
-	index_state_init(&o->internal.result, o->src_index->repo);
+	index_state_init(&o->internal.result, repo);
 	o->internal.result.initialized = 1;
 	o->internal.result.timestamp.sec = o->src_index->timestamp.sec;
 	o->internal.result.timestamp.nsec = o->src_index->timestamp.nsec;
@@ -2081,7 +2135,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 		move_index_extensions(&o->internal.result, o->src_index);
 		if (!ret) {
 			if (git_env_bool("GIT_TEST_CHECK_CACHE_TREE", 0) &&
-			    cache_tree_verify(the_repository,
+			    cache_tree_verify(repo,
 					      &o->internal.result) < 0) {
 				ret = -1;
 				goto done;
@@ -2137,10 +2191,11 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o,
 	int i;
 	unsigned old_show_all_errors;
 	int free_pattern_list = 0;
+	struct repository *repo = unpack_repo(NULL, o);
 
 	old_show_all_errors = o->internal.show_all_errors;
 	o->internal.show_all_errors = 1;
-	index_state_init(&o->internal.result, o->src_index->repo);
+	index_state_init(&o->internal.result, repo);
 
 	/* Sanity checks */
 	if (!o->update || o->index_only || o->skip_sparse_checkout)
@@ -2333,10 +2388,11 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,
 	struct dir_struct d;
 	char *pathbuf;
 	int cnt = 0;
+	struct repository *repo = unpack_repo(NULL, o);
 
 	if (S_ISGITLINK(ce->ce_mode)) {
 		struct object_id oid;
-		int sub_head = repo_resolve_gitlink_ref(the_repository, ce->name,
+		int sub_head = repo_resolve_gitlink_ref(repo, ce->name,
 							"HEAD", &oid);
 		/*
 		 * If we are not going to update the submodule, then
@@ -2424,6 +2480,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
 			      struct unpack_trees_options *o)
 {
 	const struct cache_entry *result;
+	struct repository *repo = unpack_repo(NULL, o);
 
 	/*
 	 * It may be that the 'lstat()' succeeded even though
@@ -2432,7 +2489,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
 	 *
 	 * Ignore that lstat() if it matches.
 	 */
-	if (repo_ignore_case(the_repository) && icase_exists(o, name, len, st))
+	if (repo_ignore_case(repo) && icase_exists(o, name, len, st))
 		return 0;
 
 	if (o->internal.dir &&
@@ -2652,6 +2709,7 @@ static int merged_sparse_dir(const struct cache_entry * const *src, int n,
 	void * tree_bufs[MAX_UNPACK_TREES + 1];
 	struct traverse_info info;
 	int i, ret;
+	struct repository *repo = unpack_repo(NULL, o);
 
 	/*
 	 * Create the tree traversal information for traversing into *only* the
@@ -2665,7 +2723,7 @@ static int merged_sparse_dir(const struct cache_entry * const *src, int n,
 
 	/* Get the tree descriptors of the sparse directory in each of the merging trees */
 	for (i = 0; i < n; i++)
-		tree_bufs[i] = fill_tree_descriptor(o->src_index->repo, &t[i],
+		tree_bufs[i] = fill_tree_descriptor(repo, &t[i],
 						    src[i] && !is_null_oid(&src[i]->oid) ? &src[i]->oid : NULL);
 
 	ret = traverse_trees(o->src_index, n, t, &info);
-- 
2.56.0-599-geab4b2c7f0

[PATCH 2/2] unpack-trees: consistently use "struct index_state *istate"

[Junio C Hamano]

There are some functions that call it index.  Make them consistent.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * This change is not necessary for functioning of anything, but
   while writing the previous patch, the inconsistency was
   irritating enough.

 unpack-trees.c | 94 +++++++++++++++++++++++++-------------------------
 1 file changed, 47 insertions(+), 47 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 79341d7fa6..eedc1ce7a5 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -390,14 +390,14 @@ static int check_submodule_move_head(const struct cache_entry *ce,
  * (2) before checking out entries to the working tree if the gitmodules file
  *     has been marked for update.  This situation is specified by 'state' != NULL.
  */
-static void load_gitmodules_file(struct index_state *index,
+static void load_gitmodules_file(struct index_state *istate,
 				 struct checkout *state)
 {
-	int pos = index_name_pos(index, GITMODULES_FILE, strlen(GITMODULES_FILE));
-	struct repository *repo = unpack_repo(index, NULL);
+	int pos = index_name_pos(istate, GITMODULES_FILE, strlen(GITMODULES_FILE));
+	struct repository *repo = unpack_repo(istate, NULL);
 
 	if (pos >= 0) {
-		struct cache_entry *ce = index->cache[pos];
+		struct cache_entry *ce = istate->cache[pos];
 		if (!state && ce->ce_flags & CE_WT_REMOVE) {
 			repo_read_gitmodules(repo, 0);
 		} else if (state && (ce->ce_flags & CE_UPDATE)) {
@@ -409,16 +409,16 @@ static void load_gitmodules_file(struct index_state *index,
 }
 
 static struct progress *get_progress(struct unpack_trees_options *o,
-				     struct index_state *index)
+				     struct index_state *istate)
 {
 	unsigned cnt = 0, total = 0;
-	struct repository *repo = unpack_repo(index, o);
+	struct repository *repo = unpack_repo(istate, o);
 
 	if (!o->update || !o->verbose_update)
 		return NULL;
 
-	for (; cnt < index->cache_nr; cnt++) {
-		const struct cache_entry *ce = index->cache[cnt];
+	for (; cnt < istate->cache_nr; cnt++) {
+		const struct cache_entry *ce = istate->cache[cnt];
 		if (ce->ce_flags & (CE_UPDATE | CE_WT_REMOVE))
 			total++;
 	}
@@ -428,22 +428,22 @@ static struct progress *get_progress(struct unpack_trees_options *o,
 }
 
 static void setup_collided_checkout_detection(struct checkout *state,
-					      struct index_state *index)
+					      struct index_state *istate)
 {
 	int i;
 
 	state->clone = 1;
-	for (i = 0; i < index->cache_nr; i++)
-		index->cache[i]->ce_flags &= ~CE_MATCHED;
+	for (i = 0; i < istate->cache_nr; i++)
+		istate->cache[i]->ce_flags &= ~CE_MATCHED;
 }
 
-static void report_collided_checkout(struct index_state *index)
+static void report_collided_checkout(struct index_state *istate)
 {
 	struct string_list list = STRING_LIST_INIT_NODUP;
 	int i;
 
-	for (i = 0; i < index->cache_nr; i++) {
-		struct cache_entry *ce = index->cache[i];
+	for (i = 0; i < istate->cache_nr; i++) {
+		struct cache_entry *ce = istate->cache[i];
 
 		if (!(ce->ce_flags & CE_MATCHED))
 			continue;
@@ -474,33 +474,33 @@ static int must_checkout(const struct cache_entry *ce,
 }
 
 static int check_updates(struct unpack_trees_options *o,
-			 struct index_state *index)
+			 struct index_state *istate)
 {
 	unsigned cnt = 0;
 	int errs = 0;
 	struct progress *progress;
 	struct checkout state = CHECKOUT_INIT;
 	int i, pc_workers, pc_threshold;
-	struct repository *repo = unpack_repo(index, o);
+	struct repository *repo = unpack_repo(istate, o);
 
 	trace_performance_enter();
 	state.super_prefix = o->super_prefix;
 	state.force = 1;
 	state.quiet = 1;
 	state.refresh_cache = 1;
-	state.istate = index;
+	state.istate = istate;
 	clone_checkout_metadata(&state.meta, &o->meta, NULL);
 
 	if (!o->update || o->dry_run) {
-		remove_marked_cache_entries(index, 0);
+		remove_marked_cache_entries(istate, 0);
 		trace_performance_leave("check_updates");
 		return 0;
 	}
 
 	if (o->clone)
-		setup_collided_checkout_detection(&state, index);
+		setup_collided_checkout_detection(&state, istate);
 
-	progress = get_progress(o, index);
+	progress = get_progress(o, istate);
 
 	/* Start with clean cache to avoid using any possibly outdated info. */
 	invalidate_lstat_cache();
@@ -508,10 +508,10 @@ static int check_updates(struct unpack_trees_options *o,
 	git_attr_set_direction(GIT_ATTR_CHECKOUT);
 
 	if (should_update_submodules())
-		load_gitmodules_file(index, NULL);
+		load_gitmodules_file(istate, NULL);
 
-	for (i = 0; i < index->cache_nr; i++) {
-		const struct cache_entry *ce = index->cache[i];
+	for (i = 0; i < istate->cache_nr; i++) {
+		const struct cache_entry *ce = istate->cache[i];
 
 		if (ce->ce_flags & CE_WT_REMOVE) {
 			display_progress(progress, ++cnt);
@@ -519,26 +519,26 @@ static int check_updates(struct unpack_trees_options *o,
 		}
 	}
 
-	remove_marked_cache_entries(index, 0);
+	remove_marked_cache_entries(istate, 0);
 	remove_scheduled_dirs();
 
 	if (should_update_submodules())
-		load_gitmodules_file(index, &state);
+		load_gitmodules_file(istate, &state);
 
 	if (repo_has_promisor_remote(repo))
 		/*
 		 * Prefetch the objects that are to be checked out in the loop
 		 * below.
 		 */
-		prefetch_cache_entries(index, must_checkout, NULL);
+		prefetch_cache_entries(istate, must_checkout, NULL);
 
 	get_parallel_checkout_configs(&pc_workers, &pc_threshold);
 
 	enable_delayed_checkout(&state);
 	if (pc_workers > 1)
 		init_parallel_checkout();
-	for (i = 0; i < index->cache_nr; i++) {
-		struct cache_entry *ce = index->cache[i];
+	for (i = 0; i < istate->cache_nr; i++) {
+		struct cache_entry *ce = istate->cache[i];
 
 		if (must_checkout(ce, NULL)) {
 			size_t last_pc_queue_size = pc_queue_size();
@@ -561,7 +561,7 @@ static int check_updates(struct unpack_trees_options *o,
 	git_attr_set_direction(GIT_ATTR_CHECKIN);
 
 	if (o->clone)
-		report_collided_checkout(index);
+		report_collided_checkout(istate);
 
 	trace_performance_leave("check_updates");
 	return errs != 0;
@@ -678,19 +678,19 @@ static void mark_ce_used(struct cache_entry *ce, struct unpack_trees_options *o)
 	}
 }
 
-static void mark_all_ce_unused(struct index_state *index)
+static void mark_all_ce_unused(struct index_state *istate)
 {
 	int i;
-	for (i = 0; i < index->cache_nr; i++)
-		index->cache[i]->ce_flags &= ~(CE_UNPACKED | CE_ADDED | CE_NEW_SKIP_WORKTREE);
+	for (i = 0; i < istate->cache_nr; i++)
+		istate->cache[i]->ce_flags &= ~(CE_UNPACKED | CE_ADDED | CE_NEW_SKIP_WORKTREE);
 }
 
 static int locate_in_src_index(const struct cache_entry *ce,
 			       struct unpack_trees_options *o)
 {
-	struct index_state *index = o->src_index;
+	struct index_state *istate = o->src_index;
 	int len = ce_namelen(ce);
-	int pos = index_name_pos(index, ce->name, len);
+	int pos = index_name_pos(istate, ce->name, len);
 	if (pos < 0)
 		pos = -1 - pos;
 	return pos;
@@ -704,12 +704,12 @@ static int locate_in_src_index(const struct cache_entry *ce,
 static void mark_ce_used_same_name(struct cache_entry *ce,
 				   struct unpack_trees_options *o)
 {
-	struct index_state *index = o->src_index;
+	struct index_state *istate = o->src_index;
 	int len = ce_namelen(ce);
 	int pos;
 
-	for (pos = locate_in_src_index(ce, o); pos < index->cache_nr; pos++) {
-		struct cache_entry *next = index->cache[pos];
+	for (pos = locate_in_src_index(ce, o); pos < istate->cache_nr; pos++) {
+		struct cache_entry *next = istate->cache[pos];
 		if (len != ce_namelen(next) ||
 		    memcmp(ce->name, next->name, len))
 			break;
@@ -719,11 +719,11 @@ static void mark_ce_used_same_name(struct cache_entry *ce,
 
 static struct cache_entry *next_cache_entry(struct unpack_trees_options *o)
 {
-	const struct index_state *index = o->src_index;
+	const struct index_state *istate = o->src_index;
 	int pos = o->internal.cache_bottom;
 
-	while (pos < index->cache_nr) {
-		struct cache_entry *ce = index->cache[pos];
+	while (pos < istate->cache_nr) {
+		struct cache_entry *ce = istate->cache[pos];
 		if (!(ce->ce_flags & CE_UNPACKED)) {
 			o->internal.cache_bottom = pos;
 			return ce;
@@ -736,14 +736,14 @@ static struct cache_entry *next_cache_entry(struct unpack_trees_options *o)
 static void add_same_unmerged(const struct cache_entry *ce,
 			      struct unpack_trees_options *o)
 {
-	struct index_state *index = o->src_index;
+	struct index_state *istate = o->src_index;
 	int len = ce_namelen(ce);
-	int pos = index_name_pos(index, ce->name, len);
+	int pos = index_name_pos(istate, ce->name, len);
 
 	if (0 <= pos)
 		die("programming error in a caller of mark_ce_used_same_name");
-	for (pos = -pos - 1; pos < index->cache_nr; pos++) {
-		struct cache_entry *next = index->cache[pos];
+	for (pos = -pos - 1; pos < istate->cache_nr; pos++) {
+		struct cache_entry *next = istate->cache[pos];
 		if (len != ce_namelen(next) ||
 		    memcmp(ce->name, next->name, len))
 			break;
@@ -1335,11 +1335,11 @@ static int find_cache_pos(struct traverse_info *info,
 {
 	int pos;
 	struct unpack_trees_options *o = info->data;
-	struct index_state *index = o->src_index;
+	struct index_state *istate = o->src_index;
 	int pfxlen = info->pathlen;
 
-	for (pos = o->internal.cache_bottom; pos < index->cache_nr; pos++) {
-		const struct cache_entry *ce = index->cache[pos];
+	for (pos = o->internal.cache_bottom; pos < istate->cache_nr; pos++) {
+		const struct cache_entry *ce = istate->cache[pos];
 		const char *ce_name, *ce_slash;
 		int cmp, ce_len;
 
-- 
2.56.0-599-geab4b2c7f0

Re: [PATCH 2/2] unpack-trees: consistently use "struct index_state *istate"

[Derrick Stolee]

On 11/6/2026 8:29 PM, Junio C Hamano wrote:
> There are some functions that call it index.  Make them consistent.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  * This change is not necessary for functioning of anything, but
>    while writing the previous patch, the inconsistency was
>    irritating enough.
I agree that making this consistent will help a lot when mutating
the code in the future. Doing this cleanup next to the_repository
cleanup makes sense.

Thanks,
-Stolee

Re: [PATCH 1/2] unpack-trees: lose USE_THE_REPOSITORY

[Patrick Steinhardt]

On Fri, Nov 06, 2026 at 05:26:52PM -0800, Junio C Hamano wrote:
> This file defines a set of fairly low level functions to implement
> merge and checkout behaviour, many of which depended on and can only
> work with the_repository.
> 
> Two key data structures passed around in the code paths, "struct
> unpack_trees_options" and "struct index_state", both have a pointer
> to the repository instance, and we can use it to make it work on an
> arbitrary repository, not just the_repository.

Okay.

> In any correctly written clients of this library file, the
> repository instance that is pointed at by these two structs MUST be
> the same one.  For now, make sure that is the case (and otherwise
> BUG out) by introducing a unpack_repo() helper function that takes
> these two structures (when available) and returns the repository.

Fair. It's a bit curious of an interface to have this kind of ambiguity.
But as an intermediate step I think the proposed solution is okayish,
and I do like that we at least verify that it's the same repository.

> diff --git a/unpack-trees.c b/unpack-trees.c
> index d6a859420f..79341d7fa6 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1,4 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
>  #define DISABLE_SIGN_COMPARE_WARNINGS
>  
>  #include "git-compat-util.h"
> @@ -82,6 +81,56 @@ static const char *unpack_plumbing_errors[NB_UNPACK_TREES_WARNING_TYPES] = {
>  	  ? ((o)->internal.msgs[(type)])      \
>  	  : (unpack_plumbing_errors[(type)]) )
>  
> +
> +/*
> + * Some functions in these code paths get unpack_trees_options, which
> + * has up to two istates, and some get an istate.  These istates can
> + * be used to find the repository to be used for this operation.
> + *
> + * As these are all working on the same repository, they must match.
> + * For now, with a super paranoid check, make sure all potential
> + * sources of the information match before using it.
> + *
> + * Once this proves working well, we could just use the first one we
> + * find in a production setting.
> + */
> +static struct repository *unpack_repo(struct index_state *istate,
> +				      struct unpack_trees_options *o)
> +{
> +	struct repository *repo = NULL;
> +	const char *camefrom = NULL;
> +
> +	if (istate) {
> +		if (!istate->repo)
> +			BUG("istate->repo == NULL");
> +		repo = istate->repo;
> +		camefrom = "istate->repo";
> +	}
> +	if (o) {
> +		if (o->src_index) {
> +			if (!o->src_index->repo)
> +				BUG("o->src_index->repo == NULL");
> +			if (repo && repo != o->src_index->repo)
> +				BUG("%s != o->src_index->repo", camefrom);
> +			repo = o->src_index->repo;
> +			camefrom = "o->src_index->repo";
> +		}
> +
> +		if (o->dst_index) {
> +			if (!o->dst_index->repo)
> +				BUG("o->dst_index->repo == NULL");
> +			if (repo && repo != o->dst_index->repo)
> +				BUG("%s != o->dst_index->repo", camefrom);
> +			repo = o->dst_index->repo;
> +		}
> +	}
> +	if (!repo)
> +		BUG("no repository for unpack_trees()?");
> +	return repo;
> +
> +}

So this overall surprisingly complex, but I think it's sensible to be
defensive.

> +
> +

Nit: two empty lines.

> @@ -345,15 +394,16 @@ static void load_gitmodules_file(struct index_state *index,
>  				 struct checkout *state)
>  {
>  	int pos = index_name_pos(index, GITMODULES_FILE, strlen(GITMODULES_FILE));
> +	struct repository *repo = unpack_repo(index, NULL);

This conversion here raises the question what we want to do in the case
where the caller has both structures available, but we don't end up
passing them to the pointer. This could in theory hide a bug where we
have mismatching repositories when the entry points don't verify that
property.

I guess that's ultimately not much of a problem. But an alternative take
could ensure the propert only at the borders of this subsystem, so any
externally-visible function. And internally, we'd then only ever pass
around a repository.

But in the end I don't really think this matters much, so please only
take this as me thinking out loud.

Patrick

Re: [PATCH 1/2] unpack-trees: lose USE_THE_REPOSITORY

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

>> In any correctly written clients of this library file, the
>> repository instance that is pointed at by these two structs MUST be
>> the same one.  For now, make sure that is the case (and otherwise
>> BUG out) by introducing a unpack_repo() helper function that takes
>> these two structures (when available) and returns the repository.
>
> Fair. It's a bit curious of an interface to have this kind of ambiguity.
> But as an intermediate step I think the proposed solution is okayish,
> and I do like that we at least verify that it's the same repository.

Yeah, while I am reasonably sure that all three (when they exist)
must always be pointing at the same repository, I may have missed a
legitimate use case where that assumption does not hold.  And that
is why the initial implementation is checking them with paranoia to
easily detect cases that I missed.


>> +	if (istate) {
>> +		if (!istate->repo)
>> +			BUG("istate->repo == NULL");
>> +		repo = istate->repo;
>> +		camefrom = "istate->repo";
>> +	}
>> +	if (o) {
>> +		if (o->src_index) {
>> +			if (!o->src_index->repo)
>> +				BUG("o->src_index->repo == NULL");
>> +			if (repo && repo != o->src_index->repo)
>> +				BUG("%s != o->src_index->repo", camefrom);
>> +			repo = o->src_index->repo;
>> +			camefrom = "o->src_index->repo";
>> +		}
>> +
>> +		if (o->dst_index) {
>> +			if (!o->dst_index->repo)
>> +				BUG("o->dst_index->repo == NULL");
>> +			if (repo && repo != o->dst_index->repo)
>> +				BUG("%s != o->dst_index->repo", camefrom);
>> +			repo = o->dst_index->repo;
>> +		}
>> +	}
>> +	if (!repo)
>> +		BUG("no repository for unpack_trees()?");
>> +	return repo;
>> +
>> +}
>
> So this overall surprisingly complex, but I think it's sensible to be
> defensive.

Yes, we could do

	if (istate && istate->repo)
		return itate->repo;
	if (o && o->src_index && o->src_index->repo)
		return o->src_index->repo;
	if (o && o->dst_index && o->dst_index->repo)
		return o->dst_index->repo;
	BUG();

but then you'd need to start a debugger to tell which assumption was
violated.

>> @@ -345,15 +394,16 @@ static void load_gitmodules_file(struct index_state *index,
>>  				 struct checkout *state)
>>  {
>>  	int pos = index_name_pos(index, GITMODULES_FILE, strlen(GITMODULES_FILE));
>> +	struct repository *repo = unpack_repo(index, NULL);
>
> This conversion here raises the question what we want to do in the case
> where the caller has both structures available, but we don't end up
> passing them to the pointer. This could in theory hide a bug where we
> have mismatching repositories when the entry points don't verify that
> property.

Puzzled.  Here we do have an istate but have no unpack_trees_options
available, so it is natural to pass NULL for the latter to tell the
validator "we do not have unpack_trees_options".  Are you worried
about a case where a programmer who writes an unpack_repo() call
passes NULL by mistake efven when istate or unpack_trees_options are
available?

Thanks.

Re: [PATCH 1/2] unpack-trees: lose USE_THE_REPOSITORY

[Patrick Steinhardt]

On Mon, Nov 09, 2026 at 11:30:06AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> >> @@ -345,15 +394,16 @@ static void load_gitmodules_file(struct index_state *index,
> >>  				 struct checkout *state)
> >>  {
> >>  	int pos = index_name_pos(index, GITMODULES_FILE, strlen(GITMODULES_FILE));
> >> +	struct repository *repo = unpack_repo(index, NULL);
> >
> > This conversion here raises the question what we want to do in the case
> > where the caller has both structures available, but we don't end up
> > passing them to the pointer. This could in theory hide a bug where we
> > have mismatching repositories when the entry points don't verify that
> > property.
> 
> Puzzled.  Here we do have an istate but have no unpack_trees_options
> available, so it is natural to pass NULL for the latter to tell the
> validator "we do not have unpack_trees_options".

It is natural, but the calling context _does_ have options available.
It's just that the caller of this function here decided to not pass it
down because we previously didn't need it. In fact, every new callsite
of `unpack_repo()` does have options available in their call stack.

> Are you worried about a case where a programmer who writes an
> unpack_repo() call passes NULL by mistake efven when istate or
> unpack_trees_options are available?

All I'm pointing out is that there are cases where we could be as
defensive as everywhere else if we started passing down the options to
calling contexts that don't have it yet. I wouldn't say I'm "worried"
though. The proposed patch is very defensive overall, and I'm not overly
concerned about us missing that defense in some cases.

Patrick

Re: [PATCH 1/2] unpack-trees: lose USE_THE_REPOSITORY

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> All I'm pointing out is that there are cases where we could be as
> defensive as everywhere else if we started passing down the options to
> calling contexts that don't have it yet.

OK, that does make sense, but I suspect the current helper functions
that do not get unpack_opt do not because they do not have a reason
to access anything in it.  We could throw "struct repository *" into
that struct and use that one (ignoring the one in the istates).

I however think that would be a separate clean-up.

Thanks.

Re: [PATCH 1/2] unpack-trees: lose USE_THE_REPOSITORY

[Patrick Steinhardt]

On Tue, Nov 10, 2026 at 08:13:39AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > All I'm pointing out is that there are cases where we could be as
> > defensive as everywhere else if we started passing down the options to
> > calling contexts that don't have it yet.
> 
> OK, that does make sense, but I suspect the current helper functions
> that do not get unpack_opt do not because they do not have a reason
> to access anything in it.  We could throw "struct repository *" into
> that struct and use that one (ignoring the one in the istates).
> 
> I however think that would be a separate clean-up.

Yup, that's reasonable. Thanks!

Patrick

Re: [PATCH 1/2] unpack-trees: lose USE_THE_REPOSITORY

[Derrick Stolee]

On 11/6/2026 8:26 PM, Junio C Hamano wrote:
> This file defines a set of fairly low level functions to implement
> merge and checkout behaviour, many of which depended on and can only
> work with the_repository.
> 
> Two key data structures passed around in the code paths, "struct
> unpack_trees_options" and "struct index_state", both have a pointer
> to the repository instance, and we can use it to make it work on an
> arbitrary repository, not just the_repository.
> 
> In any correctly written clients of this library file, the
> repository instance that is pointed at by these two structs MUST be
> the same one.  For now, make sure that is the case (and otherwise
> BUG out) by introducing a unpack_repo() helper function that takes
> these two structures (when available) and returns the repository.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
>  * I wrote this change when I ranted in
> 
>    https://lore.kernel.org/git/xmqq33tdcuub.fsf@gitster.g/
> 
>    on 'seen', and "git checkout -m --detach master" was sufficient
>    to build and start a new branch based on 'master' and there
>    wasn't any need for adjustment.  All tests seem to pass.
> 
>  unpack-trees.c | 94 ++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 76 insertions(+), 18 deletions(-)
> 
> diff --git a/unpack-trees.c b/unpack-trees.c
> index d6a859420f..79341d7fa6 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1,4 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
>  #define DISABLE_SIGN_COMPARE_WARNINGS
>  
>  #include "git-compat-util.h"
> @@ -82,6 +81,56 @@ static const char *unpack_plumbing_errors[NB_UNPACK_TREES_WARNING_TYPES] = {
>  	  ? ((o)->internal.msgs[(type)])      \
>  	  : (unpack_plumbing_errors[(type)]) )
>  
> +
> +/*
> + * Some functions in these code paths get unpack_trees_options, which
> + * has up to two istates, and some get an istate.  These istates can
> + * be used to find the repository to be used for this operation.
> + *
> + * As these are all working on the same repository, they must match.
> + * For now, with a super paranoid check, make sure all potential
> + * sources of the information match before using it.
> + *
> + * Once this proves working well, we could just use the first one we
> + * find in a production setting.
> + */
> +static struct repository *unpack_repo(struct index_state *istate,
> +				      struct unpack_trees_options *o)
> +{
> +	struct repository *repo = NULL;
> +	const char *camefrom = NULL;
Overall, this is a good helper implementation.

> +	if (!repo)
> +		BUG("no repository for unpack_trees()?");
> +	return repo;
> +
> +}

nit: extra empty line.

>  static const char *super_prefixed(const char *path, const char *super_prefix)
>  {
>  	/*
> @@ -345,15 +394,16 @@ static void load_gitmodules_file(struct index_state *index,
>  				 struct checkout *state)
>  {
>  	int pos = index_name_pos(index, GITMODULES_FILE, strlen(GITMODULES_FILE));
> +	struct repository *repo = unpack_repo(index, NULL);
>  
>  	if (pos >= 0) {
>  		struct cache_entry *ce = index->cache[pos];
>  		if (!state && ce->ce_flags & CE_WT_REMOVE) {
> -			repo_read_gitmodules(the_repository, 0);
> +			repo_read_gitmodules(repo, 0);
>  		} else if (state && (ce->ce_flags & CE_UPDATE)) {
> -			submodule_free(the_repository);
> +			submodule_free(repo);
>  			checkout_entry(ce, state, NULL, NULL);
> -			repo_read_gitmodules(the_repository, 0);
> +			repo_read_gitmodules(repo, 0);
>  		}
>  	}

This sample of how it is used makes it clear how this is a
valuable abstraction.

Thanks,
-Stolee

[PATCH v2 0/2] unpack-trees.c w/o the_repository

[Junio C Hamano]

unpack_trees() API serves as low-level functions to implement merge
and checkout.  It heavily relies on the_repository, making it clear
it only works with that repository instance.  We however can use the
repository instance pointed at by key data structures passed
throughout the callchain.

This is almost the exact copy of what I have at 

    https://lore.kernel.org/git/xmqqtsltbfdv.fsf@gitster.g/

The only changes since that version are removals of two blank lines.

 1/2: unpack-trees: lose USE_THE_REPOSITORY
 2/2: unpack-trees: consistently use "struct index_state *istate"

 unpack-trees.c | 180 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 118 insertions(+), 62 deletions(-)

-- 
2.56.0-626-gaba6dc7e00

[PATCH v2 1/2] unpack-trees: lose USE_THE_REPOSITORY

[Junio C Hamano]

This file defines a set of fairly low level functions to implement
merge and checkout behaviour, many of which depended on and can only
work with the_repository.

Two key data structures passed around in the code paths, "struct
unpack_trees_options" and "struct index_state", both have a pointer
to the repository instance, and we can use it to make it work on an
arbitrary repository, not just the_repository.

In any correctly written clients of this library file, the
repository instance that is pointed at by these two structs MUST be
the same one.  For now, make sure that is the case (and otherwise
BUG out) by introducing a unpack_repo() helper function that takes
these two structures (when available) and returns the repository.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 unpack-trees.c | 92 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 74 insertions(+), 18 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 1802809ad3..a1f7d8bd6a 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "git-compat-util.h"
@@ -82,6 +81,54 @@ static const char *unpack_plumbing_errors[NB_UNPACK_TREES_WARNING_TYPES] = {
 	  ? ((o)->internal.msgs[(type)])      \
 	  : (unpack_plumbing_errors[(type)]) )
 
+
+/*
+ * Some functions in these code paths get unpack_trees_options, which
+ * has up to two istates, and some get an istate.  These istates can
+ * be used to find the repository to be used for this operation.
+ *
+ * As these are all working on the same repository, they must match.
+ * For now, with a super paranoid check, make sure all potential
+ * sources of the information match before using it.
+ *
+ * Once this proves working well, we could just use the first one we
+ * find in a production setting.
+ */
+static struct repository *unpack_repo(struct index_state *istate,
+				      struct unpack_trees_options *o)
+{
+	struct repository *repo = NULL;
+	const char *camefrom = NULL;
+
+	if (istate) {
+		if (!istate->repo)
+			BUG("istate->repo == NULL");
+		repo = istate->repo;
+		camefrom = "istate->repo";
+	}
+	if (o) {
+		if (o->src_index) {
+			if (!o->src_index->repo)
+				BUG("o->src_index->repo == NULL");
+			if (repo && repo != o->src_index->repo)
+				BUG("%s != o->src_index->repo", camefrom);
+			repo = o->src_index->repo;
+			camefrom = "o->src_index->repo";
+		}
+
+		if (o->dst_index) {
+			if (!o->dst_index->repo)
+				BUG("o->dst_index->repo == NULL");
+			if (repo && repo != o->dst_index->repo)
+				BUG("%s != o->dst_index->repo", camefrom);
+			repo = o->dst_index->repo;
+		}
+	}
+	if (!repo)
+		BUG("no repository for unpack_trees()?");
+	return repo;
+}
+
 static const char *super_prefixed(const char *path, const char *super_prefix)
 {
 	/*
@@ -345,15 +392,16 @@ static void load_gitmodules_file(struct index_state *index,
 				 struct checkout *state)
 {
 	int pos = index_name_pos(index, GITMODULES_FILE, strlen(GITMODULES_FILE));
+	struct repository *repo = unpack_repo(index, NULL);
 
 	if (pos >= 0) {
 		struct cache_entry *ce = index->cache[pos];
 		if (!state && ce->ce_flags & CE_WT_REMOVE) {
-			repo_read_gitmodules(the_repository, 0);
+			repo_read_gitmodules(repo, 0);
 		} else if (state && (ce->ce_flags & CE_UPDATE)) {
-			submodule_free(the_repository);
+			submodule_free(repo);
 			checkout_entry(ce, state, NULL, NULL);
-			repo_read_gitmodules(the_repository, 0);
+			repo_read_gitmodules(repo, 0);
 		}
 	}
 }
@@ -362,6 +410,7 @@ static struct progress *get_progress(struct unpack_trees_options *o,
 				     struct index_state *index)
 {
 	unsigned cnt = 0, total = 0;
+	struct repository *repo = unpack_repo(index, o);
 
 	if (!o->update || !o->verbose_update)
 		return NULL;
@@ -372,7 +421,7 @@ static struct progress *get_progress(struct unpack_trees_options *o,
 			total++;
 	}
 
-	return start_delayed_progress(the_repository,
+	return start_delayed_progress(repo,
 				      _("Updating files"), total);
 }
 
@@ -430,6 +479,7 @@ static int check_updates(struct unpack_trees_options *o,
 	struct progress *progress;
 	struct checkout state = CHECKOUT_INIT;
 	int i, pc_workers, pc_threshold;
+	struct repository *repo = unpack_repo(index, o);
 
 	trace_performance_enter();
 	state.super_prefix = o->super_prefix;
@@ -473,7 +523,7 @@ static int check_updates(struct unpack_trees_options *o,
 	if (should_update_submodules())
 		load_gitmodules_file(index, &state);
 
-	if (repo_has_promisor_remote(the_repository))
+	if (repo_has_promisor_remote(repo))
 		/*
 		 * Prefetch the objects that are to be checked out in the loop
 		 * below.
@@ -879,6 +929,7 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,
 	struct traverse_info newinfo;
 	struct name_entry *p;
 	int nr_entries;
+	struct repository *repo = unpack_repo(NULL, o);
 
 	nr_entries = all_trees_same_as_cache_tree(n, dirmask, names, info);
 	if (nr_entries > 0) {
@@ -940,7 +991,7 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,
 			const struct object_id *oid = NULL;
 			if (dirmask & 1)
 				oid = &names[i].oid;
-			buf[nr_buf++] = fill_tree_descriptor(the_repository, t + i, oid);
+			buf[nr_buf++] = fill_tree_descriptor(repo, t + i, oid);
 		}
 	}
 
@@ -1773,24 +1824,25 @@ static int clear_ce_flags(struct index_state *istate,
 	static struct strbuf prefix = STRBUF_INIT;
 	char label[100];
 	int rval;
+	struct repository *repo = unpack_repo(istate, NULL);
 
 	strbuf_reset(&prefix);
 	if (show_progress)
 		istate->progress = start_delayed_progress(
-					the_repository,
+					repo,
 					_("Updating index flags"),
 					istate->cache_nr);
 
 	xsnprintf(label, sizeof(label), "clear_ce_flags(0x%08lx,0x%08lx)",
 		  (unsigned long)select_mask, (unsigned long)clear_mask);
-	trace2_region_enter("unpack_trees", label, istate->repo);
+	trace2_region_enter("unpack_trees", label, repo);
 	rval = clear_ce_flags_1(istate,
 				istate->cache,
 				istate->cache_nr,
 				&prefix,
 				select_mask, clear_mask,
 				pl, 0, 0);
-	trace2_region_leave("unpack_trees", label, istate->repo);
+	trace2_region_leave("unpack_trees", label, repo);
 
 	stop_progress(&istate->progress);
 	return rval;
@@ -1885,13 +1937,13 @@ static int verify_absent(const struct cache_entry *,
  */
 int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)
 {
-	struct repository *repo = o->src_index->repo;
+	struct repository *repo = unpack_repo(NULL, o);
 	int i, ret;
 	static struct cache_entry *dfc;
 	struct pattern_list pl;
 	int free_pattern_list = 0;
 	struct dir_struct dir = DIR_INIT;
-	struct repo_config_values *cfg = repo_config_values(the_repository);
+	struct repo_config_values *cfg = repo_config_values(repo);
 
 	if (o->reset == UNPACK_RESET_INVALID)
 		BUG("o->reset had a value of 1; should be UNPACK_TREES_*_UNTRACKED");
@@ -1936,7 +1988,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 		populate_from_existing_patterns(o, &pl);
 	}
 
-	index_state_init(&o->internal.result, o->src_index->repo);
+	index_state_init(&o->internal.result, repo);
 	o->internal.result.initialized = 1;
 	o->internal.result.timestamp.sec = o->src_index->timestamp.sec;
 	o->internal.result.timestamp.nsec = o->src_index->timestamp.nsec;
@@ -2080,7 +2132,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 		move_index_extensions(&o->internal.result, o->src_index);
 		if (!ret) {
 			if (git_env_bool("GIT_TEST_CHECK_CACHE_TREE", 0) &&
-			    cache_tree_verify(the_repository,
+			    cache_tree_verify(repo,
 					      &o->internal.result) < 0) {
 				ret = -1;
 				goto done;
@@ -2136,10 +2188,11 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o,
 	int i;
 	unsigned old_show_all_errors;
 	int free_pattern_list = 0;
+	struct repository *repo = unpack_repo(NULL, o);
 
 	old_show_all_errors = o->internal.show_all_errors;
 	o->internal.show_all_errors = 1;
-	index_state_init(&o->internal.result, o->src_index->repo);
+	index_state_init(&o->internal.result, repo);
 
 	/* Sanity checks */
 	if (!o->update || o->index_only || o->skip_sparse_checkout)
@@ -2332,10 +2385,11 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,
 	struct dir_struct d;
 	char *pathbuf;
 	int cnt = 0;
+	struct repository *repo = unpack_repo(NULL, o);
 
 	if (S_ISGITLINK(ce->ce_mode)) {
 		struct object_id oid;
-		int sub_head = repo_resolve_gitlink_ref(the_repository, ce->name,
+		int sub_head = repo_resolve_gitlink_ref(repo, ce->name,
 							"HEAD", &oid);
 		/*
 		 * If we are not going to update the submodule, then
@@ -2423,6 +2477,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
 			      struct unpack_trees_options *o)
 {
 	const struct cache_entry *result;
+	struct repository *repo = unpack_repo(NULL, o);
 
 	/*
 	 * It may be that the 'lstat()' succeeded even though
@@ -2431,7 +2486,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
 	 *
 	 * Ignore that lstat() if it matches.
 	 */
-	if (repo_ignore_case(the_repository) && icase_exists(o, name, len, st))
+	if (repo_ignore_case(repo) && icase_exists(o, name, len, st))
 		return 0;
 
 	if (o->internal.dir &&
@@ -2651,6 +2706,7 @@ static int merged_sparse_dir(const struct cache_entry * const *src, int n,
 	void * tree_bufs[MAX_UNPACK_TREES + 1];
 	struct traverse_info info;
 	int i, ret;
+	struct repository *repo = unpack_repo(NULL, o);
 
 	/*
 	 * Create the tree traversal information for traversing into *only* the
@@ -2664,7 +2720,7 @@ static int merged_sparse_dir(const struct cache_entry * const *src, int n,
 
 	/* Get the tree descriptors of the sparse directory in each of the merging trees */
 	for (i = 0; i < n; i++)
-		tree_bufs[i] = fill_tree_descriptor(o->src_index->repo, &t[i],
+		tree_bufs[i] = fill_tree_descriptor(repo, &t[i],
 						    src[i] && !is_null_oid(&src[i]->oid) ? &src[i]->oid : NULL);
 
 	ret = traverse_trees(o->src_index, n, t, &info);
-- 
2.56.0-626-gaba6dc7e00

[PATCH v2 2/2] unpack-trees: consistently use "struct index_state *istate"

[Junio C Hamano]

There are some functions that call it index.  Make them consistent.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 unpack-trees.c | 94 +++++++++++++++++++++++++-------------------------
 1 file changed, 47 insertions(+), 47 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index a1f7d8bd6a..0d00f6120a 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -388,14 +388,14 @@ static int check_submodule_move_head(const struct cache_entry *ce,
  * (2) before checking out entries to the working tree if the gitmodules file
  *     has been marked for update.  This situation is specified by 'state' != NULL.
  */
-static void load_gitmodules_file(struct index_state *index,
+static void load_gitmodules_file(struct index_state *istate,
 				 struct checkout *state)
 {
-	int pos = index_name_pos(index, GITMODULES_FILE, strlen(GITMODULES_FILE));
-	struct repository *repo = unpack_repo(index, NULL);
+	int pos = index_name_pos(istate, GITMODULES_FILE, strlen(GITMODULES_FILE));
+	struct repository *repo = unpack_repo(istate, NULL);
 
 	if (pos >= 0) {
-		struct cache_entry *ce = index->cache[pos];
+		struct cache_entry *ce = istate->cache[pos];
 		if (!state && ce->ce_flags & CE_WT_REMOVE) {
 			repo_read_gitmodules(repo, 0);
 		} else if (state && (ce->ce_flags & CE_UPDATE)) {
@@ -407,16 +407,16 @@ static void load_gitmodules_file(struct index_state *index,
 }
 
 static struct progress *get_progress(struct unpack_trees_options *o,
-				     struct index_state *index)
+				     struct index_state *istate)
 {
 	unsigned cnt = 0, total = 0;
-	struct repository *repo = unpack_repo(index, o);
+	struct repository *repo = unpack_repo(istate, o);
 
 	if (!o->update || !o->verbose_update)
 		return NULL;
 
-	for (; cnt < index->cache_nr; cnt++) {
-		const struct cache_entry *ce = index->cache[cnt];
+	for (; cnt < istate->cache_nr; cnt++) {
+		const struct cache_entry *ce = istate->cache[cnt];
 		if (ce->ce_flags & (CE_UPDATE | CE_WT_REMOVE))
 			total++;
 	}
@@ -426,22 +426,22 @@ static struct progress *get_progress(struct unpack_trees_options *o,
 }
 
 static void setup_collided_checkout_detection(struct checkout *state,
-					      struct index_state *index)
+					      struct index_state *istate)
 {
 	int i;
 
 	state->clone = 1;
-	for (i = 0; i < index->cache_nr; i++)
-		index->cache[i]->ce_flags &= ~CE_MATCHED;
+	for (i = 0; i < istate->cache_nr; i++)
+		istate->cache[i]->ce_flags &= ~CE_MATCHED;
 }
 
-static void report_collided_checkout(struct index_state *index)
+static void report_collided_checkout(struct index_state *istate)
 {
 	struct string_list list = STRING_LIST_INIT_NODUP;
 	int i;
 
-	for (i = 0; i < index->cache_nr; i++) {
-		struct cache_entry *ce = index->cache[i];
+	for (i = 0; i < istate->cache_nr; i++) {
+		struct cache_entry *ce = istate->cache[i];
 
 		if (!(ce->ce_flags & CE_MATCHED))
 			continue;
@@ -472,33 +472,33 @@ static int must_checkout(const struct cache_entry *ce,
 }
 
 static int check_updates(struct unpack_trees_options *o,
-			 struct index_state *index)
+			 struct index_state *istate)
 {
 	unsigned cnt = 0;
 	int errs = 0;
 	struct progress *progress;
 	struct checkout state = CHECKOUT_INIT;
 	int i, pc_workers, pc_threshold;
-	struct repository *repo = unpack_repo(index, o);
+	struct repository *repo = unpack_repo(istate, o);
 
 	trace_performance_enter();
 	state.super_prefix = o->super_prefix;
 	state.force = 1;
 	state.quiet = 1;
 	state.refresh_cache = 1;
-	state.istate = index;
+	state.istate = istate;
 	clone_checkout_metadata(&state.meta, &o->meta, NULL);
 
 	if (!o->update || o->dry_run) {
-		remove_marked_cache_entries(index, 0);
+		remove_marked_cache_entries(istate, 0);
 		trace_performance_leave("check_updates");
 		return 0;
 	}
 
 	if (o->clone)
-		setup_collided_checkout_detection(&state, index);
+		setup_collided_checkout_detection(&state, istate);
 
-	progress = get_progress(o, index);
+	progress = get_progress(o, istate);
 
 	/* Start with clean cache to avoid using any possibly outdated info. */
 	invalidate_lstat_cache();
@@ -506,10 +506,10 @@ static int check_updates(struct unpack_trees_options *o,
 	git_attr_set_direction(GIT_ATTR_CHECKOUT);
 
 	if (should_update_submodules())
-		load_gitmodules_file(index, NULL);
+		load_gitmodules_file(istate, NULL);
 
-	for (i = 0; i < index->cache_nr; i++) {
-		const struct cache_entry *ce = index->cache[i];
+	for (i = 0; i < istate->cache_nr; i++) {
+		const struct cache_entry *ce = istate->cache[i];
 
 		if (ce->ce_flags & CE_WT_REMOVE) {
 			display_progress(progress, ++cnt);
@@ -517,26 +517,26 @@ static int check_updates(struct unpack_trees_options *o,
 		}
 	}
 
-	remove_marked_cache_entries(index, 0);
+	remove_marked_cache_entries(istate, 0);
 	remove_scheduled_dirs();
 
 	if (should_update_submodules())
-		load_gitmodules_file(index, &state);
+		load_gitmodules_file(istate, &state);
 
 	if (repo_has_promisor_remote(repo))
 		/*
 		 * Prefetch the objects that are to be checked out in the loop
 		 * below.
 		 */
-		prefetch_cache_entries(index, must_checkout, NULL);
+		prefetch_cache_entries(istate, must_checkout, NULL);
 
 	get_parallel_checkout_configs(&pc_workers, &pc_threshold);
 
 	enable_delayed_checkout(&state);
 	if (pc_workers > 1)
 		init_parallel_checkout();
-	for (i = 0; i < index->cache_nr; i++) {
-		struct cache_entry *ce = index->cache[i];
+	for (i = 0; i < istate->cache_nr; i++) {
+		struct cache_entry *ce = istate->cache[i];
 
 		if (must_checkout(ce, NULL)) {
 			size_t last_pc_queue_size = pc_queue_size();
@@ -559,7 +559,7 @@ static int check_updates(struct unpack_trees_options *o,
 	git_attr_set_direction(GIT_ATTR_CHECKIN);
 
 	if (o->clone)
-		report_collided_checkout(index);
+		report_collided_checkout(istate);
 
 	trace_performance_leave("check_updates");
 	return errs != 0;
@@ -676,19 +676,19 @@ static void mark_ce_used(struct cache_entry *ce, struct unpack_trees_options *o)
 	}
 }
 
-static void mark_all_ce_unused(struct index_state *index)
+static void mark_all_ce_unused(struct index_state *istate)
 {
 	int i;
-	for (i = 0; i < index->cache_nr; i++)
-		index->cache[i]->ce_flags &= ~(CE_UNPACKED | CE_ADDED | CE_NEW_SKIP_WORKTREE);
+	for (i = 0; i < istate->cache_nr; i++)
+		istate->cache[i]->ce_flags &= ~(CE_UNPACKED | CE_ADDED | CE_NEW_SKIP_WORKTREE);
 }
 
 static int locate_in_src_index(const struct cache_entry *ce,
 			       struct unpack_trees_options *o)
 {
-	struct index_state *index = o->src_index;
+	struct index_state *istate = o->src_index;
 	int len = ce_namelen(ce);
-	int pos = index_name_pos(index, ce->name, len);
+	int pos = index_name_pos(istate, ce->name, len);
 	if (pos < 0)
 		pos = -1 - pos;
 	return pos;
@@ -702,12 +702,12 @@ static int locate_in_src_index(const struct cache_entry *ce,
 static void mark_ce_used_same_name(struct cache_entry *ce,
 				   struct unpack_trees_options *o)
 {
-	struct index_state *index = o->src_index;
+	struct index_state *istate = o->src_index;
 	int len = ce_namelen(ce);
 	int pos;
 
-	for (pos = locate_in_src_index(ce, o); pos < index->cache_nr; pos++) {
-		struct cache_entry *next = index->cache[pos];
+	for (pos = locate_in_src_index(ce, o); pos < istate->cache_nr; pos++) {
+		struct cache_entry *next = istate->cache[pos];
 		if (len != ce_namelen(next) ||
 		    memcmp(ce->name, next->name, len))
 			break;
@@ -717,11 +717,11 @@ static void mark_ce_used_same_name(struct cache_entry *ce,
 
 static struct cache_entry *next_cache_entry(struct unpack_trees_options *o)
 {
-	const struct index_state *index = o->src_index;
+	const struct index_state *istate = o->src_index;
 	int pos = o->internal.cache_bottom;
 
-	while (pos < index->cache_nr) {
-		struct cache_entry *ce = index->cache[pos];
+	while (pos < istate->cache_nr) {
+		struct cache_entry *ce = istate->cache[pos];
 		if (!(ce->ce_flags & CE_UNPACKED)) {
 			o->internal.cache_bottom = pos;
 			return ce;
@@ -734,14 +734,14 @@ static struct cache_entry *next_cache_entry(struct unpack_trees_options *o)
 static void add_same_unmerged(const struct cache_entry *ce,
 			      struct unpack_trees_options *o)
 {
-	struct index_state *index = o->src_index;
+	struct index_state *istate = o->src_index;
 	int len = ce_namelen(ce);
-	int pos = index_name_pos(index, ce->name, len);
+	int pos = index_name_pos(istate, ce->name, len);
 
 	if (0 <= pos)
 		die("programming error in a caller of mark_ce_used_same_name");
-	for (pos = -pos - 1; pos < index->cache_nr; pos++) {
-		struct cache_entry *next = index->cache[pos];
+	for (pos = -pos - 1; pos < istate->cache_nr; pos++) {
+		struct cache_entry *next = istate->cache[pos];
 		if (len != ce_namelen(next) ||
 		    memcmp(ce->name, next->name, len))
 			break;
@@ -1333,11 +1333,11 @@ static int find_cache_pos(struct traverse_info *info,
 {
 	int pos;
 	struct unpack_trees_options *o = info->data;
-	struct index_state *index = o->src_index;
+	struct index_state *istate = o->src_index;
 	int pfxlen = info->pathlen;
 
-	for (pos = o->internal.cache_bottom; pos < index->cache_nr; pos++) {
-		const struct cache_entry *ce = index->cache[pos];
+	for (pos = o->internal.cache_bottom; pos < istate->cache_nr; pos++) {
+		const struct cache_entry *ce = istate->cache[pos];
 		const char *ce_name, *ce_slash;
 		int cmp, ce_len;
 
-- 
2.56.0-626-gaba6dc7e00

Re: [PATCH v2 0/2] unpack-trees.c w/o the_repository

[Derrick Stolee]

On 11/9/2026 3:54 PM, Junio C Hamano wrote:
> unpack_trees() API serves as low-level functions to implement merge
> and checkout.  It heavily relies on the_repository, making it clear
> it only works with that repository instance.  We however can use the
> repository instance pointed at by key data structures passed
> throughout the callchain.
> 
> This is almost the exact copy of what I have at 
> 
>     https://lore.kernel.org/git/xmqqtsltbfdv.fsf@gitster.g/
> 
> The only changes since that version are removals of two blank lines.
LGTM.

Thanks,
-Stolee

Re: [PATCH v2 0/2] unpack-trees.c w/o the_repository

[Junio C Hamano]

Derrick Stolee <stolee@gmail.com> writes:

> On 11/9/2026 3:54 PM, Junio C Hamano wrote:
>> unpack_trees() API serves as low-level functions to implement merge
>> and checkout.  It heavily relies on the_repository, making it clear
>> it only works with that repository instance.  We however can use the
>> repository instance pointed at by key data structures passed
>> throughout the callchain.
>> 
>> This is almost the exact copy of what I have at 
>> 
>>     https://lore.kernel.org/git/xmqqtsltbfdv.fsf@gitster.g/
>> 
>> The only changes since that version are removals of two blank lines.
> LGTM.
>
> Thanks,
> -Stolee

Thanks.

Re: [PATCH v2 0/2] unpack-trees.c w/o the_repository

[Patrick Steinhardt]

On Tue, Nov 10, 2026 at 08:22:26AM -0500, Derrick Stolee wrote:
> On 11/9/2026 3:54 PM, Junio C Hamano wrote:
> > unpack_trees() API serves as low-level functions to implement merge
> > and checkout.  It heavily relies on the_repository, making it clear
> > it only works with that repository instance.  We however can use the
> > repository instance pointed at by key data structures passed
> > throughout the callchain.
> > 
> > This is almost the exact copy of what I have at 
> > 
> >     https://lore.kernel.org/git/xmqqtsltbfdv.fsf@gitster.g/
> > 
> > The only changes since that version are removals of two blank lines.
> LGTM.

Likewise, I'm happy with this version. Thanks!

Patrick