unpack-trees: lose USE_THE_REPOSITORY

PATCH In review Junio C Hamano 3 participants 15 messages Nov 7 – Nov 11

Share & tools

Junio C Hamano

author

2026-11-07 01:26 UTC #0 lore raw

PATCH v2 1/2 unpack-trees: lose USE_THE_REPOSITORY

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 <[email protected]>

---

* I wrote this change when I ranted in

https://lore.kernel.org/git/[email protected]/

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(-)

unpack-trees.c +76 ~~-18~~

index d6a859420f..79341d7fa6 100644
@@ -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);

Signature · 1 line

2.56.0-599-geab4b2c7f0

Junio C Hamano

author

2026-11-07 01:29 UTC #1 lore raw

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

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

Signed-off-by Junio C Hamano <[email protected]>

---
* 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(-)

unpack-trees.c +47 ~~-47~~

index 79341d7fa6..eedc1ce7a5 100644
@@ -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;

Signature · 1 line

2.56.0-599-geab4b2c7f0

Derrick Stolee

2026-11-09 13:46 UTC #2 lore raw

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

Quoted 7 lines · Junio C Hamano

There are some functions that call it index. Make them consistent.
Signed-off-by Junio C Hamano <[email protected]>
---
* 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

Patrick Steinhardt

2026-11-09 13:41 UTC #3 lore raw

In reply to Junio C Hamano

Quoted 8 lines · 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.

Okay.

Quoted 5 lines

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.

Quoted 51 lines

unpack-trees.c +38 -1

index d6a859420f..79341d7fa6 100644
@@ -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");

unpack-trees.c static const char *unpack_plumbing_errors[NB_UNPACK_TREES_WARNING_TYPES] = { · line 131

Quoted 10 lines

unpack-trees.c +10 -0

+			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.

unpack-trees.c static const char *unpack_plumbing_errors[NB_UNPACK_TREES_WARNING_TYPES] = { · line 133

Quoted 2 lines

unpack-trees.c +2 -0
+
+

Nit: two empty lines.

unpack-trees.c static void load_gitmodules_file(struct index_state *index, · line 397

Quoted 5 lines

unpack-trees.c +1 -0

@@ -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

Junio C Hamano

author

2026-11-09 19:30 UTC #4 lore raw

Quoted 9 lines · Patrick Steinhardt

Quoted 5 lines
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.

Quoted 24 lines

unpack-trees.c +24 -0

+	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;
+		}
+	}

Quoted 8 lines

unpack-trees.c static const char *unpack_plumbing_errors[NB_UNPACK_TREES_WARNING_TYPES] = { · line 131
Quoted 5 lines
unpack-trees.c +5 -0
+	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.

Quoted 11 lines

unpack-trees.c static void load_gitmodules_file(struct index_state *index, · line 397
Quoted 5 lines
unpack-trees.c +1 -0
@@ -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.

Patrick Steinhardt

2026-11-10 07:04 UTC #5 lore raw

Quoted 13 lines · Junio C Hamano

Quoted 11 lines · Patrick Steinhardt
unpack-trees.c static void load_gitmodules_file(struct index_state *index, · line 397
Quoted 5 lines
unpack-trees.c +1 -0
@@ -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.

Quoted 3 lines

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.

Quoted 3 lines

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

Junio C Hamano

author

2026-11-10 16:13 UTC #6 lore raw

Quoted 3 lines · Patrick Steinhardt

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.

Patrick Steinhardt

2026-11-10 16:40 UTC #7 lore raw

Quoted 6 lines · Junio C Hamano

Quoted 3 lines · Patrick Steinhardt
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.

Quoted 6 lines

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

Derrick Stolee

2026-11-09 13:45 UTC #8 lore raw

In reply to Junio C Hamano

Quoted 50 lines · 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 <[email protected]>
---
* I wrote this change when I ranted in
https://lore.kernel.org/git/[email protected]/
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(-)
unpack-trees.c +8 -1
index d6a859420f..79341d7fa6 100644
@@ -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

unpack-trees.c static const char *unpack_plumbing_errors[NB_UNPACK_TREES_WARNING_TYPES] = { · line 101

Quoted 10 lines

unpack-trees.c +10 -0

+ * 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.

unpack-trees.c static const char *unpack_plumbing_errors[NB_UNPACK_TREES_WARNING_TYPES] = { · line 131

Quoted 5 lines

unpack-trees.c +5 -0

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

nit: extra empty line.

Quoted 12 lines

unpack-trees.c +1 -0

 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) {

unpack-trees.c static void load_gitmodules_file(struct index_state *index, · line 408

Quoted 10 lines

unpack-trees.c +3 -3

-			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

Junio C Hamano

author

2026-11-09 20:54 UTC #9 lore raw

v2 cover unpack-trees.c w/o the_repository

In reply to 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/[email protected]/

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(-)

Signature · 1 line

2.56.0-626-gaba6dc7e00

Junio C Hamano

author

2026-11-09 20:54 UTC #10 lore raw

PATCH v2 1/2 unpack-trees: lose USE_THE_REPOSITORY

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.

Signed-off-by Junio C Hamano <[email protected]>

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

unpack-trees.c +74 ~~-18~~

index 1802809ad3..a1f7d8bd6a 100644
@@ -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);

Signature · 1 line

2.56.0-626-gaba6dc7e00

Junio C Hamano

author

2026-11-09 20:54 UTC #11 lore raw

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

In reply to Junio C Hamano

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

Signed-off-by Junio C Hamano <[email protected]>

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

unpack-trees.c +47 ~~-47~~

index a1f7d8bd6a..0d00f6120a 100644
@@ -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;

Signature · 1 line

2.56.0-626-gaba6dc7e00

Derrick Stolee

2026-11-10 13:22 UTC #12 lore raw

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

In reply to Junio C Hamano

Quoted 11 lines · 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/[email protected]/
The only changes since that version are removals of two blank lines.

LGTM.

Thanks,
-Stolee

Junio C Hamano

author

2026-11-10 17:28 UTC #13 lore raw

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

Quoted 9 lines · Derrick Stolee

Quoted 8 lines · 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

Quoted 7 lines

Quoted 3 lines
https://lore.kernel.org/git/[email protected]/
The only changes since that version are removals of two blank lines.
LGTM.
Thanks,
-Stolee

Thanks.

Patrick Steinhardt

2026-11-11 12:48 UTC #14 lore raw

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

In reply to Derrick Stolee

Quoted 7 lines · Derrick Stolee

Quoted 6 lines · 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.

Quoted 6 lines

Quoted 5 lines
This is almost the exact copy of what I have at
https://lore.kernel.org/git/[email protected]/
The only changes since that version are removals of two blank lines.
LGTM.

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

Patrick