Git development
 help / color / mirror / Atom feed
* [PATCH v2 04/18] setup: stop using `the_repository` in `prefix_path()`
From: Patrick Steinhardt @ 2026-05-18  9:30 UTC (permalink / raw)
  To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-0-6933c0f1d568@pks.im>

Stop using `the_repository` in `prefix_path()` and instead accept the
repository as a parameter. The injection of `the_repository` is thus
bumped one level higher, where callers now pass it in explicitly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/blame.c            |  2 +-
 builtin/check-attr.c       |  2 +-
 builtin/checkout-index.c   |  4 ++--
 builtin/mv.c               |  5 +++--
 builtin/sparse-checkout.c  |  3 ++-
 builtin/update-index.c     |  6 +++---
 line-log.c                 |  2 +-
 object-name.c              |  2 +-
 pathspec.c                 |  2 +-
 setup.c                    | 15 ++++++++-------
 setup.h                    |  4 ++--
 t/helper/test-path-utils.c |  2 +-
 12 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index f3a11eff44..ffbd3ce5c5 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -708,7 +708,7 @@ static unsigned parse_score(const char *arg)
 
 static char *add_prefix(const char *prefix, const char *path)
 {
-	return prefix_path(prefix, prefix ? strlen(prefix) : 0, path);
+	return prefix_path(the_repository, prefix, prefix ? strlen(prefix) : 0, path);
 }
 
 static int git_blame_config(const char *var, const char *value,
diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 51ed48ce43..04b86e42ae 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -67,7 +67,7 @@ static void check_attr(const char *prefix, struct attr_check *check,
 
 {
 	char *full_path =
-		prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
+		prefix_path(the_repository, prefix, prefix ? strlen(prefix) : 0, file);
 
 	if (collect_all) {
 		git_all_attrs(the_repository->index, full_path, check);
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 188128aebd..311b94ff31 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -303,7 +303,7 @@ int cmd_checkout_index(int argc,
 			die("git checkout-index: don't mix '--all' and explicit filenames");
 		if (read_from_stdin)
 			die("git checkout-index: don't mix '--stdin' and explicit filenames");
-		p = prefix_path(prefix, prefix_length, arg);
+		p = prefix_path(repo, prefix, prefix_length, arg);
 		err |= checkout_file(repo->index, p, prefix);
 		free(p);
 	}
@@ -325,7 +325,7 @@ int cmd_checkout_index(int argc,
 					die("line is badly quoted");
 				strbuf_swap(&buf, &unquoted);
 			}
-			p = prefix_path(prefix, prefix_length, buf.buf);
+			p = prefix_path(repo, prefix, prefix_length, buf.buf);
 			err |= checkout_file(repo->index, p, prefix);
 			free(p);
 		}
diff --git a/builtin/mv.c b/builtin/mv.c
index 2215d34e31..948b330639 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -71,7 +71,7 @@ static void internal_prefix_pathspec(struct strvec *out,
 
 		trimmed = xmemdupz(pathspec[i], to_copy);
 		maybe_basename = (flags & DUP_BASENAME) ? basename(trimmed) : trimmed;
-		prefixed_path = prefix_path(prefix, prefixlen, maybe_basename);
+		prefixed_path = prefix_path(the_repository, prefix, prefixlen, maybe_basename);
 		strvec_push(out, prefixed_path);
 
 		free(prefixed_path);
@@ -394,7 +394,8 @@ int cmd_mv(int argc,
 			for (j = 0; j < last - first; j++) {
 				const struct cache_entry *ce = the_repository->index->cache[first + j];
 				const char *path = ce->name;
-				char *prefixed_path = prefix_path(dst_with_slash, dst_with_slash_len, path + length + 1);
+				char *prefixed_path = prefix_path(the_repository, dst_with_slash,
+								  dst_with_slash_len, path + length + 1);
 
 				strvec_push(&sources, path);
 				strvec_push(&destinations, prefixed_path);
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index f4aa405da9..2af50fb2f9 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -735,7 +735,8 @@ static void sanitize_paths(struct repository *repo,
 		int prefix_len = strlen(prefix);
 
 		for (i = 0; i < args->nr; i++) {
-			char *prefixed_path = prefix_path(prefix, prefix_len, args->v[i]);
+			char *prefixed_path = prefix_path(the_repository, prefix,
+							  prefix_len, args->v[i]);
 			strvec_replace(args, i, prefixed_path);
 			free(prefixed_path);
 		}
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 8a5907767b..7434112b8e 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -655,7 +655,7 @@ static int do_unresolve(int ac, const char **av,
 
 	for (i = 1; i < ac; i++) {
 		const char *arg = av[i];
-		char *p = prefix_path(prefix, prefix_length, arg);
+		char *p = prefix_path(the_repository, prefix, prefix_length, arg);
 		err |= unresolve_one(p);
 		free(p);
 	}
@@ -1158,7 +1158,7 @@ int cmd_update_index(int argc,
 			}
 
 			setup_work_tree();
-			p = prefix_path(prefix, prefix_length, path);
+			p = prefix_path(the_repository, prefix, prefix_length, path);
 			update_one(p);
 			if (set_executable_bit)
 				chmod_path(set_executable_bit, p);
@@ -1208,7 +1208,7 @@ int cmd_update_index(int argc,
 					die("line is badly quoted");
 				strbuf_swap(&buf, &unquoted);
 			}
-			p = prefix_path(prefix, prefix_length, buf.buf);
+			p = prefix_path(the_repository, prefix, prefix_length, buf.buf);
 			update_one(p);
 			if (set_executable_bit)
 				chmod_path(set_executable_bit, p);
diff --git a/line-log.c b/line-log.c
index 858a899cd2..346c60c554 100644
--- a/line-log.c
+++ b/line-log.c
@@ -589,7 +589,7 @@ parse_lines(struct repository *r, struct commit *commit,
 		range_part = xstrndup(item->string, name_part - item->string);
 		name_part++;
 
-		full_name = prefix_path(prefix, prefix ? strlen(prefix) : 0,
+		full_name = prefix_path(r, prefix, prefix ? strlen(prefix) : 0,
 					name_part);
 
 		spec = alloc_filespec(full_name);
diff --git a/object-name.c b/object-name.c
index 37a9ce8e87..9ac86f19c7 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1707,7 +1707,7 @@ static char *resolve_relative_path(struct repository *r, const char *rel)
 		die(_("relative path syntax can't be used outside working tree"));
 
 	/* die() inside prefix_path() if resolved path is outside worktree */
-	return prefix_path(startup_info->prefix,
+	return prefix_path(the_repository, startup_info->prefix,
 			   startup_info->prefix ? strlen(startup_info->prefix) : 0,
 			   rel);
 }
diff --git a/pathspec.c b/pathspec.c
index 5993c4afa0..f78b22709c 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -486,7 +486,7 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,
 		match = xstrdup(copyfrom);
 		prefixlen = 0;
 	} else {
-		match = prefix_path_gently(prefix, prefixlen,
+		match = prefix_path_gently(the_repository, prefix, prefixlen,
 					   &prefixlen, copyfrom);
 		if (!match) {
 			const char *hint_path;
diff --git a/setup.c b/setup.c
index 041e08b98d..adad6ceec0 100644
--- a/setup.c
+++ b/setup.c
@@ -117,7 +117,8 @@ static int abspath_part_inside_repo(struct repository *repo, char *path)
  *  ../../sub1/sub2/foo -> sub1/sub2/foo (but no remaining prefix)
  *  `pwd`/../bar -> sub1/bar       (no remaining prefix)
  */
-char *prefix_path_gently(const char *prefix, int len,
+char *prefix_path_gently(struct repository *repo,
+			 const char *prefix, int len,
 			 int *remaining_prefix, const char *path)
 {
 	const char *orig = path;
@@ -130,7 +131,7 @@ char *prefix_path_gently(const char *prefix, int len,
 			free(sanitized);
 			return NULL;
 		}
-		if (abspath_part_inside_repo(the_repository, sanitized)) {
+		if (abspath_part_inside_repo(repo, sanitized)) {
 			free(sanitized);
 			return NULL;
 		}
@@ -146,13 +147,13 @@ char *prefix_path_gently(const char *prefix, int len,
 	return sanitized;
 }
 
-char *prefix_path(const char *prefix, int len, const char *path)
+char *prefix_path(struct repository *repo, const char *prefix, int len, const char *path)
 {
-	char *r = prefix_path_gently(prefix, len, NULL, path);
+	char *r = prefix_path_gently(repo, prefix, len, NULL, path);
 	if (!r) {
-		const char *hint_path = repo_get_work_tree(the_repository);
+		const char *hint_path = repo_get_work_tree(repo);
 		if (!hint_path)
-			hint_path = repo_get_git_dir(the_repository);
+			hint_path = repo_get_git_dir(repo);
 		die(_("'%s' is outside repository at '%s'"), path,
 		    absolute_path(hint_path));
 	}
@@ -162,7 +163,7 @@ char *prefix_path(const char *prefix, int len, const char *path)
 int path_inside_repo(const char *prefix, const char *path)
 {
 	int len = prefix ? strlen(prefix) : 0;
-	char *r = prefix_path_gently(prefix, len, NULL, path);
+	char *r = prefix_path_gently(the_repository, prefix, len, NULL, path);
 	if (r) {
 		free(r);
 		return 1;
diff --git a/setup.h b/setup.h
index 71d3f91883..24034572b1 100644
--- a/setup.h
+++ b/setup.h
@@ -138,8 +138,8 @@ const char *enter_repo(const char *path, unsigned flags);
 
 const char *setup_git_directory_gently(int *);
 const char *setup_git_directory(void);
-char *prefix_path(const char *prefix, int len, const char *path);
-char *prefix_path_gently(const char *prefix, int len, int *remaining, const char *path);
+char *prefix_path(struct repository *repo, const char *prefix, int len, const char *path);
+char *prefix_path_gently(struct repository *repo, const char *prefix, int len, int *remaining, const char *path);
 
 int check_filename(const char *prefix, const char *name);
 void verify_filename(const char *prefix,
diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
index 874542ec34..163fdeefb0 100644
--- a/t/helper/test-path-utils.c
+++ b/t/helper/test-path-utils.c
@@ -379,7 +379,7 @@ int cmd__path_utils(int argc, const char **argv)
 		int nongit_ok;
 		setup_git_directory_gently(&nongit_ok);
 		while (argc > 3) {
-			char *pfx = prefix_path(prefix, prefix_len, argv[3]);
+			char *pfx = prefix_path(the_repository, prefix, prefix_len, argv[3]);
 
 			puts(pfx);
 			free(pfx);

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* [PATCH v2 05/18] setup: stop using `the_repository` in `path_inside_repo()`
From: Patrick Steinhardt @ 2026-05-18  9:30 UTC (permalink / raw)
  To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-0-6933c0f1d568@pks.im>

Stop using `the_repository` in `path_inside_repo()` and instead accept
the repository as a parameter. The injection of `the_repository` is thus
bumped one level higher, where callers now pass it in explicitly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/diff.c | 4 ++--
 setup.c        | 4 ++--
 setup.h        | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index 0b23c41456..7ddebce2ac 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -471,8 +471,8 @@ int cmd_diff(int argc,
 		 * as a colourful "diff" replacement.
 		 */
 		if (nongit || ((argc == i + 2) &&
-			       (!path_inside_repo(prefix, argv[i]) ||
-				!path_inside_repo(prefix, argv[i + 1]))))
+			       (!path_inside_repo(the_repository, prefix, argv[i]) ||
+				!path_inside_repo(the_repository, prefix, argv[i + 1]))))
 			no_index = DIFF_NO_INDEX_IMPLICIT;
 	}
 
diff --git a/setup.c b/setup.c
index adad6ceec0..4ef6216e82 100644
--- a/setup.c
+++ b/setup.c
@@ -160,10 +160,10 @@ char *prefix_path(struct repository *repo, const char *prefix, int len, const ch
 	return r;
 }
 
-int path_inside_repo(const char *prefix, const char *path)
+int path_inside_repo(struct repository *repo, const char *prefix, const char *path)
 {
 	int len = prefix ? strlen(prefix) : 0;
-	char *r = prefix_path_gently(the_repository, prefix, len, NULL, path);
+	char *r = prefix_path_gently(repo, prefix, len, NULL, path);
 	if (r) {
 		free(r);
 		return 1;
diff --git a/setup.h b/setup.h
index 24034572b1..c3247d7fc8 100644
--- a/setup.h
+++ b/setup.h
@@ -146,7 +146,7 @@ void verify_filename(const char *prefix,
 		     const char *name,
 		     int diagnose_misspelt_rev);
 void verify_non_filename(const char *prefix, const char *name);
-int path_inside_repo(const char *prefix, const char *path);
+int path_inside_repo(struct repository *repo, const char *prefix, const char *path);
 
 void sanitize_stdfds(void);
 int daemonize(void);

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* [PATCH v2 06/18] setup: stop using `the_repository` in `verify_filename()`
From: Patrick Steinhardt @ 2026-05-18  9:30 UTC (permalink / raw)
  To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-0-6933c0f1d568@pks.im>

Stop using `the_repository` in `verify_filename()` and instead accept
the repository as a parameter. The injection of `the_repository` is thus
bumped one level higher, where callers now pass it in explicitly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/grep.c      | 2 +-
 builtin/reset.c     | 2 +-
 builtin/rev-parse.c | 4 ++--
 revision.c          | 2 +-
 setup.c             | 5 +++--
 setup.h             | 3 ++-
 6 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index e33285e5e6..b0e350cf89 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1163,7 +1163,7 @@ int cmd_grep(int argc,
 	if (!seen_dashdash) {
 		int j;
 		for (j = i; j < argc; j++)
-			verify_filename(prefix, argv[j], j == i && allow_revs);
+			verify_filename(the_repository, prefix, argv[j], j == i && allow_revs);
 	}
 
 	parse_pathspec(&pathspec, 0,
diff --git a/builtin/reset.c b/builtin/reset.c
index 3590be57a5..1ac374d31b 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -285,7 +285,7 @@ static void parse_args(struct pathspec *pathspec,
 			rev = *argv++;
 		} else {
 			/* Otherwise we treat this as a filename */
-			verify_filename(prefix, argv[0], 1);
+			verify_filename(the_repository, prefix, argv[0], 1);
 		}
 	}
 
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 2fcd6851d1..8fdb75413d 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -749,7 +749,7 @@ int cmd_rev_parse(int argc,
 
 		if (as_is) {
 			if (show_file(arg, output_prefix) && as_is < 2)
-				verify_filename(prefix, arg, 0);
+				verify_filename(the_repository, prefix, arg, 0);
 			continue;
 		}
 
@@ -1173,7 +1173,7 @@ int cmd_rev_parse(int argc,
 		as_is = 1;
 		if (!show_file(arg, output_prefix))
 			continue;
-		verify_filename(prefix, arg, 1);
+		verify_filename(the_repository, prefix, arg, 1);
 	}
 	strbuf_release(&buf);
 	if (verify) {
diff --git a/revision.c b/revision.c
index 599b3a66c3..5d53244379 100644
--- a/revision.c
+++ b/revision.c
@@ -3067,7 +3067,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 			 * but the latter we have checked in the main loop.
 			 */
 			for (j = i; j < argc; j++)
-				verify_filename(revs->prefix, argv[j], j == i);
+				verify_filename(the_repository, revs->prefix, argv[j], j == i);
 
 			strvec_pushv(&prune_data, argv + i);
 			break;
diff --git a/setup.c b/setup.c
index 4ef6216e82..e673663cab 100644
--- a/setup.c
+++ b/setup.c
@@ -280,7 +280,8 @@ static int looks_like_pathspec(const char *arg)
  * diagnose_misspelt_rev == 0 for the next ones (because we already
  * saw a filename, there's not ambiguity anymore).
  */
-void verify_filename(const char *prefix,
+void verify_filename(struct repository *repo,
+		     const char *prefix,
 		     const char *arg,
 		     int diagnose_misspelt_rev)
 {
@@ -288,7 +289,7 @@ void verify_filename(const char *prefix,
 		die(_("option '%s' must come before non-option arguments"), arg);
 	if (looks_like_pathspec(arg) || check_filename(prefix, arg))
 		return;
-	die_verify_filename(the_repository, prefix, arg, diagnose_misspelt_rev);
+	die_verify_filename(repo, prefix, arg, diagnose_misspelt_rev);
 }
 
 /*
diff --git a/setup.h b/setup.h
index c3247d7fc8..24a6f66629 100644
--- a/setup.h
+++ b/setup.h
@@ -142,7 +142,8 @@ char *prefix_path(struct repository *repo, const char *prefix, int len, const ch
 char *prefix_path_gently(struct repository *repo, const char *prefix, int len, int *remaining, const char *path);
 
 int check_filename(const char *prefix, const char *name);
-void verify_filename(const char *prefix,
+void verify_filename(struct repository *repo,
+		     const char *prefix,
 		     const char *name,
 		     int diagnose_misspelt_rev);
 void verify_non_filename(const char *prefix, const char *name);

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* [PATCH v2 07/18] setup: stop using `the_repository` in `verify_non_filename()`
From: Patrick Steinhardt @ 2026-05-18  9:30 UTC (permalink / raw)
  To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-0-6933c0f1d568@pks.im>

Stop using `the_repository` in `verify_non_filename()` and instead
accept the repository as a parameter. The injection of `the_repository`
is thus bumped one level higher, where callers now pass it in
explicitly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/checkout.c | 2 +-
 builtin/grep.c     | 2 +-
 builtin/reset.c    | 2 +-
 revision.c         | 4 ++--
 setup.c            | 4 ++--
 setup.h            | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index ac0186a33e..14cefa0199 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1492,7 +1492,7 @@ static int parse_branchname_arg(int argc, const char **argv,
 		 * it would be extremely annoying.
 		 */
 		if (argc)
-			verify_non_filename(opts->prefix, arg);
+			verify_non_filename(the_repository, opts->prefix, arg);
 	} else if (opts->accept_pathspec) {
 		argcount++;
 		argv++;
diff --git a/builtin/grep.c b/builtin/grep.c
index b0e350cf89..4ec0c016b1 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1151,7 +1151,7 @@ int cmd_grep(int argc,
 
 		object = parse_object_or_die(the_repository, &oid, arg);
 		if (!seen_dashdash)
-			verify_non_filename(prefix, arg);
+			verify_non_filename(the_repository, prefix, arg);
 		add_object_array_with_path(object, arg, &list, oc.mode, oc.path);
 		object_context_release(&oc);
 	}
diff --git a/builtin/reset.c b/builtin/reset.c
index 1ac374d31b..11f57605b5 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -281,7 +281,7 @@ static void parse_args(struct pathspec *pathspec,
 			 * Ok, argv[0] looks like a commit/tree; it should not
 			 * be a filename.
 			 */
-			verify_non_filename(prefix, argv[0]);
+			verify_non_filename(the_repository, prefix, argv[0]);
 			rev = *argv++;
 		} else {
 			/* Otherwise we treat this as a filename */
diff --git a/revision.c b/revision.c
index 5d53244379..b5fe3ef95d 100644
--- a/revision.c
+++ b/revision.c
@@ -2072,7 +2072,7 @@ static int handle_dotdot_1(const char *a_name, const char *b_name,
 		return -1;
 
 	if (!cant_be_filename) {
-		verify_non_filename(revs->prefix, full_name);
+		verify_non_filename(the_repository, revs->prefix, full_name);
 	}
 
 	a_obj = parse_object(revs->repo, &a_oid);
@@ -2225,7 +2225,7 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl
 		goto out;
 	}
 	if (!cant_be_filename)
-		verify_non_filename(revs->prefix, arg);
+		verify_non_filename(the_repository, revs->prefix, arg);
 	object = get_reference(revs, arg, &oid, flags ^ local_flags);
 	if (!object) {
 		ret = (revs->ignore_missing || revs->do_not_die_on_missing_objects) ? 0 : -1;
diff --git a/setup.c b/setup.c
index e673663cab..759aba4e2c 100644
--- a/setup.c
+++ b/setup.c
@@ -297,9 +297,9 @@ void verify_filename(struct repository *repo,
  * and we parsed the arg as a refname.  It should not be interpretable
  * as a filename.
  */
-void verify_non_filename(const char *prefix, const char *arg)
+void verify_non_filename(struct repository *repo, const char *prefix, const char *arg)
 {
-	if (!is_inside_work_tree(the_repository) || is_inside_git_dir(the_repository))
+	if (!is_inside_work_tree(repo) || is_inside_git_dir(repo))
 		return;
 	if (*arg == '-')
 		return; /* flag */
diff --git a/setup.h b/setup.h
index 24a6f66629..364c2c728a 100644
--- a/setup.h
+++ b/setup.h
@@ -146,7 +146,7 @@ void verify_filename(struct repository *repo,
 		     const char *prefix,
 		     const char *name,
 		     int diagnose_misspelt_rev);
-void verify_non_filename(const char *prefix, const char *name);
+void verify_non_filename(struct repository *repo, const char *prefix, const char *name);
 int path_inside_repo(struct repository *repo, const char *prefix, const char *path);
 
 void sanitize_stdfds(void);

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* [PATCH v2 08/18] setup: stop using `the_repository` in `enter_repo()`
From: Patrick Steinhardt @ 2026-05-18  9:30 UTC (permalink / raw)
  To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-0-6933c0f1d568@pks.im>

Stop using `the_repository` in `enter_repo()` and instead accept the
repository as a parameter. The injection of `the_repository` is thus
bumped one level higher, where callers now pass it in explicitly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/receive-pack.c   | 2 +-
 builtin/upload-archive.c | 2 +-
 builtin/upload-pack.c    | 2 +-
 daemon.c                 | 4 ++--
 http-backend.c           | 2 +-
 setup.c                  | 4 ++--
 setup.h                  | 2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f0771590a7..322d178c92 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2643,7 +2643,7 @@ int cmd_receive_pack(int argc,
 
 	setup_path();
 
-	if (!enter_repo(service_dir, 0))
+	if (!enter_repo(the_repository, service_dir, 0))
 		die("'%s' does not appear to be a git repository", service_dir);
 
 	repo_config(the_repository, receive_pack_config, NULL);
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index 25312bb2a5..718e74b3ac 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -31,7 +31,7 @@ int cmd_upload_archive_writer(int argc,
 	if (argc != 2)
 		usage(upload_archive_usage);
 
-	if (!enter_repo(argv[1], 0))
+	if (!enter_repo(the_repository, argv[1], 0))
 		die("'%s' does not appear to be a git repository", argv[1]);
 
 	init_archivers();
diff --git a/builtin/upload-pack.c b/builtin/upload-pack.c
index 30498fafea..32831fb879 100644
--- a/builtin/upload-pack.c
+++ b/builtin/upload-pack.c
@@ -59,7 +59,7 @@ int cmd_upload_pack(int argc,
 
 	if (strict)
 		enter_repo_flags |= ENTER_REPO_STRICT;
-	if (!enter_repo(dir, enter_repo_flags))
+	if (!enter_repo(the_repository, dir, enter_repo_flags))
 		die("'%s' does not appear to be a git repository", dir);
 
 	switch (determine_protocol_version_server()) {
diff --git a/daemon.c b/daemon.c
index 0a7b1aae44..947dd90655 100644
--- a/daemon.c
+++ b/daemon.c
@@ -244,14 +244,14 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)
 	}
 
 	enter_repo_flags = strict_paths ? ENTER_REPO_STRICT : 0;
-	path = enter_repo(dir, enter_repo_flags);
+	path = enter_repo(the_repository, dir, enter_repo_flags);
 	if (!path && base_path && base_path_relaxed) {
 		/*
 		 * if we fail and base_path_relaxed is enabled, try without
 		 * prefixing the base path
 		 */
 		dir = directory;
-		path = enter_repo(dir, enter_repo_flags);
+		path = enter_repo(the_repository, dir, enter_repo_flags);
 	}
 
 	if (!path) {
diff --git a/http-backend.c b/http-backend.c
index 1a171c5c5a..c7566b1d12 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -809,7 +809,7 @@ int cmd_main(int argc UNUSED, const char **argv UNUSED)
 		not_found(&hdr, "Request not supported: '%s'", dir);
 
 	setup_path();
-	if (!enter_repo(dir, 0))
+	if (!enter_repo(the_repository, dir, 0))
 		not_found(&hdr, "Not a git repository: '%s'", dir);
 	if (!getenv("GIT_HTTP_EXPORT_ALL") &&
 	    access("git-daemon-export-ok", F_OK) )
diff --git a/setup.c b/setup.c
index 759aba4e2c..cb479cd91a 100644
--- a/setup.c
+++ b/setup.c
@@ -1765,7 +1765,7 @@ enum discovery_result discover_git_directory_reason(struct strbuf *commondir,
 	return result;
 }
 
-const char *enter_repo(const char *path, unsigned flags)
+const char *enter_repo(struct repository *repo, const char *path, unsigned flags)
 {
 	static struct strbuf validated_path = STRBUF_INIT;
 	static struct strbuf used_path = STRBUF_INIT;
@@ -1838,7 +1838,7 @@ const char *enter_repo(const char *path, unsigned flags)
 	}
 
 	if (is_git_directory(".")) {
-		set_git_dir(the_repository, ".", 0);
+		set_git_dir(repo, ".", 0);
 		check_repository_format(NULL);
 		return path;
 	}
diff --git a/setup.h b/setup.h
index 364c2c728a..d0cfdfd44a 100644
--- a/setup.h
+++ b/setup.h
@@ -134,7 +134,7 @@ enum {
  * links.  User relative paths are also returned as they are given,
  * except DWIM suffixing.
  */
-const char *enter_repo(const char *path, unsigned flags);
+const char *enter_repo(struct repository *repo, const char *path, unsigned flags);
 
 const char *setup_git_directory_gently(int *);
 const char *setup_git_directory(void);

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* [PATCH v2 09/18] setup: stop using `the_repository` in `setup_work_tree()`
From: Patrick Steinhardt @ 2026-05-18  9:31 UTC (permalink / raw)
  To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-0-6933c0f1d568@pks.im>

Stop using `the_repository` in `setup_work_tree()` and instead accept
the repository as a parameter. The injection of `the_repository` is thus
bumped one level higher, where callers now pass it in explicitly.

Note that the function tracks two bits of information via global
variables. This of course doesn't make much sense anymore now that we
can set up worktrees for arbitrary repositories:

  - We track whether the worktree has already been initialized and, if
    so, we skip the call to `chdir_notify()` and setenv(3p). It does not
    make much sense to store this info in the repository, as we _would_
    want to update the environment when switching between worktrees back
    and forth.

    So instead of storing this info in the repository, we drop this
    state entirely and live with the fact that we may execute the logic
    twice. It should ultimately be idempotent though and thus not be
    much of a problem.

  - We track whether the worktree configuration is bogus. If so, and if
    later on some caller tries to setup the worktree, then we'll die
    instead. This is indeed information that we can move into the
    repository itself.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 blame.c                     |  2 +-
 builtin/check-attr.c        |  2 +-
 builtin/clone.c             |  2 +-
 builtin/describe.c          |  2 +-
 builtin/diff-index.c        |  2 +-
 builtin/diff.c              |  4 ++--
 builtin/difftool.c          |  2 +-
 builtin/grep.c              |  2 +-
 builtin/ls-files.c          |  2 +-
 builtin/read-tree.c         |  2 +-
 builtin/reset.c             |  2 +-
 builtin/rm.c                |  2 +-
 builtin/sparse-checkout.c   | 16 ++++++++--------
 builtin/submodule--helper.c |  2 +-
 builtin/update-index.c      | 10 +++++-----
 git.c                       |  2 +-
 repository.h                |  1 +
 setup.c                     | 15 ++++-----------
 setup.h                     |  2 +-
 t/helper/test-subprocess.c  |  4 +++-
 wt-status.c                 |  2 +-
 21 files changed, 38 insertions(+), 42 deletions(-)

diff --git a/blame.c b/blame.c
index a3c49d132e..977cbb7097 100644
--- a/blame.c
+++ b/blame.c
@@ -2813,7 +2813,7 @@ void setup_scoreboard(struct blame_scoreboard *sb,
 		}
 
 		if (!sb->contents_from)
-			setup_work_tree();
+			setup_work_tree(the_repository);
 
 		sb->final = fake_working_tree_commit(sb->repo,
 						     &sb->revs->diffopt,
diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 04b86e42ae..98f64d5b92 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -117,7 +117,7 @@ int cmd_check_attr(int argc,
 	int cnt, i, doubledash, filei;
 
 	if (!is_bare_repository())
-		setup_work_tree();
+		setup_work_tree(the_repository);
 
 	repo_config(the_repository, git_default_config, NULL);
 
diff --git a/builtin/clone.c b/builtin/clone.c
index d23b0cafcf..09f6d97658 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -669,7 +669,7 @@ static int checkout(int submodule_progress,
 	}
 
 	/* We need to be in the new work tree for the checkout */
-	setup_work_tree();
+	setup_work_tree(the_repository);
 
 	repo_hold_locked_index(the_repository, &lock_file, LOCK_DIE_ON_ERROR);
 
diff --git a/builtin/describe.c b/builtin/describe.c
index bffeed13a3..1c47d7c0b7 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -781,7 +781,7 @@ int cmd_describe(int argc,
 			struct rev_info revs;
 			int fd;
 
-			setup_work_tree();
+			setup_work_tree(the_repository);
 			prepare_repo_settings(the_repository);
 			the_repository->settings.command_requires_full_index = 0;
 			repo_read_index(the_repository);
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index 522dacfc4c..3db7cffede 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -69,7 +69,7 @@ int cmd_diff_index(int argc,
 	    rev.max_count != -1 || rev.min_age != -1 || rev.max_age != -1)
 		usage(diff_cache_usage);
 	if (!(option & DIFF_INDEX_CACHED)) {
-		setup_work_tree();
+		setup_work_tree(the_repository);
 		if (repo_read_index_preload(the_repository, &rev.diffopt.pathspec, 0) < 0) {
 			perror("repo_read_index_preload");
 			return -1;
diff --git a/builtin/diff.c b/builtin/diff.c
index 7ddebce2ac..1ede873ac1 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -159,7 +159,7 @@ static void builtin_diff_index(struct rev_info *revs,
 	    revs->max_age != -1)
 		usage(builtin_diff_usage);
 	if (!(option & DIFF_INDEX_CACHED)) {
-		setup_work_tree();
+		setup_work_tree(the_repository);
 		if (repo_read_index_preload(the_repository,
 					    &revs->diffopt.pathspec, 0) < 0) {
 			die_errno("repo_read_index_preload");
@@ -281,7 +281,7 @@ static void builtin_diff_files(struct rev_info *revs, int argc, const char **arg
 	    (revs->diffopt.output_format & DIFF_FORMAT_PATCH))
 		diff_merges_set_dense_combined_if_unset(revs);
 
-	setup_work_tree();
+	setup_work_tree(the_repository);
 	if (repo_read_index_preload(the_repository, &revs->diffopt.pathspec,
 				    0) < 0) {
 		die_errno("repo_read_index_preload");
diff --git a/builtin/difftool.c b/builtin/difftool.c
index e4bc1f8316..2a21005f2e 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -767,7 +767,7 @@ int cmd_difftool(int argc,
 		die(_("difftool requires worktree or --no-index"));
 
 	if (!no_index){
-		setup_work_tree();
+		setup_work_tree(repo);
 		setenv(GIT_DIR_ENVIRONMENT, absolute_path(repo_get_git_dir(repo)), 1);
 		setenv(GIT_WORK_TREE_ENVIRONMENT, absolute_path(repo_get_work_tree(repo)), 1);
 	} else if (dir_diff)
diff --git a/builtin/grep.c b/builtin/grep.c
index 4ec0c016b1..679f8b567a 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1272,7 +1272,7 @@ int cmd_grep(int argc,
 		die(_("--[no-]exclude-standard cannot be used for tracked contents"));
 	} else if (!list.nr) {
 		if (!cached)
-			setup_work_tree();
+			setup_work_tree(the_repository);
 
 		hit = grep_cache(&opt, &pathspec, cached);
 	} else {
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 09d95111b3..e1a22b41b9 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -704,7 +704,7 @@ int cmd_ls_files(int argc,
 		exc_given = 1;
 
 	if (require_work_tree && !is_inside_work_tree(repo))
-		setup_work_tree();
+		setup_work_tree(repo);
 
 	if (recurse_submodules &&
 	    (show_deleted || show_others || show_unmerged ||
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 460b21e40a..999a82ecdf 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -229,7 +229,7 @@ int cmd_read_tree(int argc,
 		opts.preserve_ignored = 0;
 	/* otherwise, opts.preserve_ignored is irrelevant */
 	if (opts.merge && !opts.index_only)
-		setup_work_tree();
+		setup_work_tree(the_repository);
 
 	if (opts.skip_sparse_checkout)
 		ensure_full_index(the_repository->index);
diff --git a/builtin/reset.c b/builtin/reset.c
index 11f57605b5..3be6bd0121 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -468,7 +468,7 @@ int cmd_reset(int argc,
 		trace2_cmd_mode(reset_type_names[reset_type]);
 
 	if (reset_type != SOFT && (reset_type != MIXED || repo_get_work_tree(the_repository)))
-		setup_work_tree();
+		setup_work_tree(the_repository);
 
 	if (reset_type == MIXED && is_bare_repository())
 		die(_("%s reset is not allowed in a bare repository"),
diff --git a/builtin/rm.c b/builtin/rm.c
index 05d89e98c3..081d0bc375 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -296,7 +296,7 @@ int cmd_rm(int argc,
 		die(_("No pathspec was given. Which files should I remove?"));
 
 	if (!index_only)
-		setup_work_tree();
+		setup_work_tree(the_repository);
 
 	prepare_repo_settings(the_repository);
 	the_repository->settings.command_requires_full_index = 0;
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 2af50fb2f9..d89acbeb53 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -63,7 +63,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,
 	int res;
 	struct repo_config_values *cfg = repo_config_values(the_repository);
 
-	setup_work_tree();
+	setup_work_tree(the_repository);
 	if (!cfg->apply_sparse_checkout)
 		die(_("this worktree is not sparse"));
 
@@ -229,7 +229,7 @@ static int update_working_directory(struct repository *r,
 	o.dst_index = r->index;
 	o.skip_sparse_checkout = 0;
 
-	setup_work_tree();
+	setup_work_tree(the_repository);
 
 	repo_hold_locked_index(r, &lock_file, LOCK_DIE_ON_ERROR);
 
@@ -468,7 +468,7 @@ static int sparse_checkout_init(int argc, const char **argv, const char *prefix,
 		OPT_END(),
 	};
 
-	setup_work_tree();
+	setup_work_tree(the_repository);
 	repo_read_index(repo);
 
 	init_opts.cone_mode = -1;
@@ -802,7 +802,7 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix,
 	int ret;
 	struct repo_config_values *cfg = repo_config_values(the_repository);
 
-	setup_work_tree();
+	setup_work_tree(the_repository);
 	if (!cfg->apply_sparse_checkout)
 		die(_("no sparse-checkout to add to"));
 
@@ -856,7 +856,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix,
 	struct strvec patterns = STRVEC_INIT;
 	int ret;
 
-	setup_work_tree();
+	setup_work_tree(the_repository);
 	repo_read_index(repo);
 
 	set_opts.cone_mode = -1;
@@ -912,7 +912,7 @@ static int sparse_checkout_reapply(int argc, const char **argv,
 	};
 	struct repo_config_values *cfg = repo_config_values(the_repository);
 
-	setup_work_tree();
+	setup_work_tree(the_repository);
 	if (!cfg->apply_sparse_checkout)
 		die(_("must be in a sparse-checkout to reapply sparsity patterns"));
 
@@ -975,7 +975,7 @@ static int sparse_checkout_clean(int argc, const char **argv,
 		OPT_END(),
 	};
 
-	setup_work_tree();
+	setup_work_tree(the_repository);
 	if (!cfg->apply_sparse_checkout)
 		die(_("must be in a sparse-checkout to clean directories"));
 	if (!core_sparse_checkout_cone)
@@ -1053,7 +1053,7 @@ static int sparse_checkout_disable(int argc, const char **argv,
 	 * forcibly return to a dense checkout regardless of initial state.
 	 */
 
-	setup_work_tree();
+	setup_work_tree(the_repository);
 	argc = parse_options(argc, argv, prefix,
 			     builtin_sparse_checkout_disable_options,
 			     builtin_sparse_checkout_disable_usage, 0);
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 2f589e3b37..1cc82a134d 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1250,7 +1250,7 @@ static int compute_summary_module_list(struct object_id *head_oid,
 
 	if (!info->cached) {
 		if (diff_cmd == DIFF_INDEX)
-			setup_work_tree();
+			setup_work_tree(the_repository);
 		if (repo_read_index_preload(the_repository, &rev.diffopt.pathspec, 0) < 0) {
 			perror("repo_read_index_preload");
 			ret = -1;
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 7434112b8e..d6dabacfd1 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -732,7 +732,7 @@ struct refresh_params {
 
 static int refresh(struct refresh_params *o, unsigned int flag)
 {
-	setup_work_tree();
+	setup_work_tree(the_repository);
 	repo_read_index(the_repository);
 	*o->has_errors |= refresh_index(the_repository->index, o->flags | flag, NULL,
 					NULL, NULL);
@@ -901,7 +901,7 @@ static enum parse_opt_result reupdate_callback(
 	BUG_ON_OPT_ARG(arg);
 
 	/* consume remaining arguments. */
-	setup_work_tree();
+	setup_work_tree(the_repository);
 	*has_errors = do_reupdate(ctx->argv + 1, prefix);
 	if (*has_errors)
 		the_repository->index->cache_changed = 0;
@@ -1157,7 +1157,7 @@ int cmd_update_index(int argc,
 				transaction = NULL;
 			}
 
-			setup_work_tree();
+			setup_work_tree(the_repository);
 			p = prefix_path(the_repository, prefix, prefix_length, path);
 			update_one(p);
 			if (set_executable_bit)
@@ -1199,7 +1199,7 @@ int cmd_update_index(int argc,
 		struct strbuf buf = STRBUF_INIT;
 		struct strbuf unquoted = STRBUF_INIT;
 
-		setup_work_tree();
+		setup_work_tree(the_repository);
 		while (getline_fn(&buf, stdin) != EOF) {
 			char *p;
 			if (!nul_term_line && buf.buf[0] == '"') {
@@ -1253,7 +1253,7 @@ int cmd_update_index(int argc,
 		report(_("Untracked cache disabled"));
 		break;
 	case UC_TEST:
-		setup_work_tree();
+		setup_work_tree(the_repository);
 		return !test_if_untracked_cache_is_supported();
 	case UC_ENABLE:
 	case UC_FORCE:
diff --git a/git.c b/git.c
index 5a40eab8a2..eaede42c4e 100644
--- a/git.c
+++ b/git.c
@@ -497,7 +497,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv, struct
 	commit_pager_choice();
 
 	if (!help && p->option & NEED_WORK_TREE)
-		setup_work_tree();
+		setup_work_tree(the_repository);
 
 	trace_argv_printf(argv, "trace: built-in: git");
 	trace2_cmd_name(p->cmd);
diff --git a/repository.h b/repository.h
index 4969d8b8eb..832451fc61 100644
--- a/repository.h
+++ b/repository.h
@@ -114,6 +114,7 @@ struct repository {
 	 * A NULL value indicates that there is no working directory.
 	 */
 	char *worktree;
+	bool worktree_config_is_bogus;
 
 	/*
 	 * Path from the root of the top-level superproject down to this
diff --git a/setup.c b/setup.c
index cb479cd91a..50324f8f37 100644
--- a/setup.c
+++ b/setup.c
@@ -26,7 +26,6 @@
 #include "trace2.h"
 #include "worktree.h"
 
-static int work_tree_config_is_bogus;
 enum allowed_bare_repo {
 	ALLOWED_BARE_REPO_EXPLICIT = 0,
 	ALLOWED_BARE_REPO_ALL,
@@ -494,18 +493,14 @@ int is_inside_work_tree(struct repository *repo)
 	return ret;
 }
 
-void setup_work_tree(void)
+void setup_work_tree(struct repository *repo)
 {
 	const char *work_tree;
-	static int initialized = 0;
 
-	if (initialized)
-		return;
-
-	if (work_tree_config_is_bogus)
+	if (repo->worktree_config_is_bogus)
 		die(_("unable to set up work tree using invalid config"));
 
-	work_tree = repo_get_work_tree(the_repository);
+	work_tree = repo_get_work_tree(repo);
 	if (!work_tree || chdir_notify(work_tree))
 		die(_("this operation must be run in a work tree"));
 
@@ -515,8 +510,6 @@ void setup_work_tree(void)
 	 */
 	if (getenv(GIT_WORK_TREE_ENVIRONMENT))
 		setenv(GIT_WORK_TREE_ENVIRONMENT, ".", 1);
-
-	initialized = 1;
 }
 
 static void setup_original_cwd(struct repository *repo)
@@ -1164,7 +1157,7 @@ static const char *setup_explicit_git_dir(struct repository *repo,
 		if (git_work_tree_cfg) {
 			/* #22.2, #30 */
 			warning("core.bare and core.worktree do not make sense");
-			work_tree_config_is_bogus = 1;
+			repo->worktree_config_is_bogus = true;
 		}
 
 		/* #18, #26 */
diff --git a/setup.h b/setup.h
index d0cfdfd44a..8fed365637 100644
--- a/setup.h
+++ b/setup.h
@@ -56,7 +56,7 @@ const char *resolve_gitdir_gently(const char *suspect, int *return_error_code);
 void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
 				const char *gitdir);
 
-void setup_work_tree(void);
+void setup_work_tree(struct repository *repo);
 
 /*
  * discover_git_directory_reason() is similar to discover_git_directory(),
diff --git a/t/helper/test-subprocess.c b/t/helper/test-subprocess.c
index c344f1694d..8a070e47cd 100644
--- a/t/helper/test-subprocess.c
+++ b/t/helper/test-subprocess.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "test-tool.h"
 #include "run-command.h"
 #include "setup.h"
@@ -11,7 +13,7 @@ int cmd__subprocess(int argc, const char **argv)
 	if (nogit)
 		die("No git repo found");
 	if (argc > 1 && !strcmp(argv[1], "--setup-work-tree")) {
-		setup_work_tree();
+		setup_work_tree(the_repository);
 		argv++;
 	}
 	cp.git_cmd = 1;
diff --git a/wt-status.c b/wt-status.c
index 479ccc3304..6cc77ba68c 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1206,7 +1206,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
 		status_printf_ln(s, c,
 			"--------------------------------------------------");
 		status_printf_ln(s, c, _("Changes not staged for commit:"));
-		setup_work_tree();
+		setup_work_tree(the_repository);
 		rev.diffopt.a_prefix = "i/";
 		rev.diffopt.b_prefix = "w/";
 		run_diff_files(&rev, 0);

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* [PATCH v2 10/18] setup: stop using `the_repository` in `set_git_work_tree()`
From: Patrick Steinhardt @ 2026-05-18  9:31 UTC (permalink / raw)
  To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-0-6933c0f1d568@pks.im>

Stop using `the_repository` in `set_git_work_tree()` and instead accept
the repository as a parameter. The injection of `the_repository` is thus
bumped one level higher, where callers now pass it in explicitly.

Similar as with the preceding commit, we track whether the worktree has
been initialized already via a global variable so that we can die in
case the repository is re-initialized with a different worktree path.
Store this info in the `struct repository` instead so that we correctly
handle this per repository.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/clone.c   |  2 +-
 builtin/init-db.c |  6 +++---
 repository.h      |  1 +
 setup.c           | 24 +++++++++++-------------
 setup.h           |  2 +-
 5 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 09f6d97658..8844e3d481 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1116,7 +1116,7 @@ int cmd_clone(int argc,
 			die_errno(_("could not create work tree dir '%s'"),
 				  work_tree);
 		junk_work_tree = work_tree;
-		set_git_work_tree(work_tree);
+		set_git_work_tree(the_repository, work_tree);
 	}
 
 	if (real_git_dir) {
diff --git a/builtin/init-db.c b/builtin/init-db.c
index bb853e69f5..e626b0d8b7 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -237,9 +237,9 @@ int cmd_init_db(int argc,
 		if (!git_work_tree_cfg)
 			git_work_tree_cfg = xgetcwd();
 		if (work_tree)
-			set_git_work_tree(work_tree);
+			set_git_work_tree(the_repository, work_tree);
 		else
-			set_git_work_tree(git_work_tree_cfg);
+			set_git_work_tree(the_repository, git_work_tree_cfg);
 		if (access(repo_get_work_tree(the_repository), X_OK))
 			die_errno (_("Cannot access work tree '%s'"),
 				   repo_get_work_tree(the_repository));
@@ -248,7 +248,7 @@ int cmd_init_db(int argc,
 		if (real_git_dir)
 			die(_("--separate-git-dir incompatible with bare repository"));
 		if (work_tree)
-			set_git_work_tree(work_tree);
+			set_git_work_tree(the_repository, work_tree);
 	}
 
 	flags |= INIT_DB_EXIST_OK;
diff --git a/repository.h b/repository.h
index 832451fc61..d391aff8ab 100644
--- a/repository.h
+++ b/repository.h
@@ -114,6 +114,7 @@ struct repository {
 	 * A NULL value indicates that there is no working directory.
 	 */
 	char *worktree;
+	bool worktree_initialized;
 	bool worktree_config_is_bogus;
 
 	/*
diff --git a/setup.c b/setup.c
index 50324f8f37..796ac5792f 100644
--- a/setup.c
+++ b/setup.c
@@ -1152,7 +1152,7 @@ static const char *setup_explicit_git_dir(struct repository *repo,
 
 	/* #3, #7, #11, #15, #19, #23, #27, #31 (see t1510) */
 	if (work_tree_env)
-		set_git_work_tree(work_tree_env);
+		set_git_work_tree(repo, work_tree_env);
 	else if (is_bare_repository_cfg > 0) {
 		if (git_work_tree_cfg) {
 			/* #22.2, #30 */
@@ -1167,7 +1167,7 @@ static const char *setup_explicit_git_dir(struct repository *repo,
 	}
 	else if (git_work_tree_cfg) { /* #6, #14 */
 		if (is_absolute_path(git_work_tree_cfg))
-			set_git_work_tree(git_work_tree_cfg);
+			set_git_work_tree(repo, git_work_tree_cfg);
 		else {
 			char *core_worktree;
 			if (chdir(gitdirenv))
@@ -1177,7 +1177,7 @@ static const char *setup_explicit_git_dir(struct repository *repo,
 			core_worktree = xgetcwd();
 			if (chdir(cwd->buf))
 				die_errno(_("cannot come back to cwd"));
-			set_git_work_tree(core_worktree);
+			set_git_work_tree(repo, core_worktree);
 			free(core_worktree);
 		}
 	}
@@ -1188,7 +1188,7 @@ static const char *setup_explicit_git_dir(struct repository *repo,
 		return NULL;
 	}
 	else /* #2, #10 */
-		set_git_work_tree(".");
+		set_git_work_tree(repo, ".");
 
 	/* set_git_work_tree() must have been called by now */
 	worktree = repo_get_work_tree(repo);
@@ -1248,7 +1248,7 @@ static const char *setup_discovered_git_dir(struct repository *repo,
 	}
 
 	/* #0, #1, #5, #8, #9, #12, #13 */
-	set_git_work_tree(".");
+	set_git_work_tree(repo, ".");
 	if (strcmp(gitdir, DEFAULT_GIT_DIR_ENVIRONMENT))
 		set_git_dir(repo, gitdir, 0);
 	if (offset >= cwd->len)
@@ -1839,29 +1839,27 @@ const char *enter_repo(struct repository *repo, const char *path, unsigned flags
 	return NULL;
 }
 
-static int git_work_tree_initialized;
-
 /*
  * Note.  This works only before you used a work tree.  This was added
  * primarily to support git-clone to work in a new repository it just
  * created, and is not meant to flip between different work trees.
  */
-void set_git_work_tree(const char *new_work_tree)
+void set_git_work_tree(struct repository *repo, const char *new_work_tree)
 {
-	if (git_work_tree_initialized) {
+	if (repo->worktree_initialized) {
 		struct strbuf realpath = STRBUF_INIT;
 
 		strbuf_realpath(&realpath, new_work_tree, 1);
 		new_work_tree = realpath.buf;
-		if (strcmp(new_work_tree, the_repository->worktree))
+		if (strcmp(new_work_tree, repo->worktree))
 			die("internal error: work tree has already been set\n"
 			    "Current worktree: %s\nNew worktree: %s",
-			    the_repository->worktree, new_work_tree);
+			    repo->worktree, new_work_tree);
 		strbuf_release(&realpath);
 		return;
 	}
-	git_work_tree_initialized = 1;
-	repo_set_worktree(the_repository, new_work_tree);
+	repo->worktree_initialized = true;
+	repo_set_worktree(repo, new_work_tree);
 }
 
 const char *setup_git_directory_gently(int *nongit_ok)
diff --git a/setup.h b/setup.h
index 8fed365637..1a37089fa0 100644
--- a/setup.h
+++ b/setup.h
@@ -96,7 +96,7 @@ static inline int discover_git_directory(struct strbuf *commondir,
 	return 0;
 }
 
-void set_git_work_tree(const char *tree);
+void set_git_work_tree(struct repository *repo, const char *tree);
 
 /* Flags that can be passed to `enter_repo()`. */
 enum {

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* [PATCH v2 11/18] setup: stop using `the_repository` in `setup_git_env()`
From: Patrick Steinhardt @ 2026-05-18  9:31 UTC (permalink / raw)
  To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-0-6933c0f1d568@pks.im>

Stop using `the_repository` in `setup_git_env()` and instead accept the
repository as a parameter. The injection of `the_repository` is thus
bumped one level higher, where callers now pass it in explicitly.

Furthermore, the function is never used outside of "setup.c". Drop the
declaration in "environment.h" and make it static.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 environment.h | 2 --
 setup.c       | 6 +++---
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/environment.h b/environment.h
index 123a71cdc8..9eb97b3869 100644
--- a/environment.h
+++ b/environment.h
@@ -147,8 +147,6 @@ void repo_config_values_init(struct repo_config_values *cfg);
  * Please do not add new global config variables here.
  */
 # ifdef USE_THE_REPOSITORY_VARIABLE
-void setup_git_env(const char *git_dir);
-
 /*
  * Returns true iff we have a configured git repository (either via
  * setup_git_directory, or in the environment via $GIT_DIR).
diff --git a/setup.c b/setup.c
index 796ac5792f..8965f8ccaf 100644
--- a/setup.c
+++ b/setup.c
@@ -1074,9 +1074,9 @@ static void setup_git_env_internal(struct repository *repo,
 		fetch_if_missing = 0;
 }
 
-void setup_git_env(const char *git_dir)
+static void setup_git_env(struct repository *repo, const char *git_dir)
 {
-	setup_git_env_internal(the_repository, git_dir, false);
+	setup_git_env_internal(repo, git_dir, false);
 }
 
 static void set_git_dir_1(struct repository *repo, const char *path, bool skip_initializing_odb)
@@ -1988,7 +1988,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			const char *gitdir = getenv(GIT_DIR_ENVIRONMENT);
 			if (!gitdir)
 				gitdir = DEFAULT_GIT_DIR_ENVIRONMENT;
-			setup_git_env(gitdir);
+			setup_git_env(the_repository, gitdir);
 		}
 		if (startup_info->have_repository) {
 			repo_set_hash_algo(the_repository, repo_fmt.hash_algo);

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* [PATCH v2 12/18] setup: stop using `the_repository` in `setup_git_directory_gently()`
From: Patrick Steinhardt @ 2026-05-18  9:31 UTC (permalink / raw)
  To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-0-6933c0f1d568@pks.im>

Stop using `the_repository` in `setup_git_directory_gently()` and
instead accept the repository as a parameter. The injection of
`the_repository` is thus bumped one level higher, where callers now pass
it in explicitly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/check-ref-format.c |  5 ++++-
 builtin/diff.c             |  2 +-
 builtin/hash-object.c      |  2 +-
 builtin/help.c             |  2 +-
 builtin/stripspace.c       |  2 +-
 git.c                      |  6 +++---
 http-fetch.c               |  2 +-
 imap-send.c                |  2 +-
 remote-curl.c              |  4 ++--
 setup.c                    | 36 ++++++++++++++++++------------------
 setup.h                    |  2 +-
 t/helper/test-path-utils.c |  2 +-
 t/helper/test-subprocess.c |  2 +-
 13 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 5d80afeec0..e42b0444ea 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -1,6 +1,9 @@
 /*
  * GIT - The information manager from hell
  */
+
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "builtin.h"
 #include "refs.h"
 #include "setup.h"
@@ -41,7 +44,7 @@ static int check_ref_format_branch(const char *arg)
 	const char *name;
 	int nongit;
 
-	setup_git_directory_gently(&nongit);
+	setup_git_directory_gently(the_repository, &nongit);
 	if (check_branch_ref(&sb, arg) ||
 	    !skip_prefix(sb.buf, "refs/heads/", &name))
 		die("'%s' is not a valid branch name", arg);
diff --git a/builtin/diff.c b/builtin/diff.c
index 1ede873ac1..4b46e394ce 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -455,7 +455,7 @@ int cmd_diff(int argc,
 			break;
 	}
 
-	prefix = setup_git_directory_gently(&nongit);
+	prefix = setup_git_directory_gently(the_repository, &nongit);
 
 	if (!nongit) {
 		prepare_repo_settings(the_repository);
diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 5d900a6b8c..d7905bedc2 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -102,7 +102,7 @@ int cmd_hash_object(int argc,
 	if (flags & INDEX_WRITE_OBJECT)
 		prefix = setup_git_directory();
 	else
-		prefix = setup_git_directory_gently(&nongit);
+		prefix = setup_git_directory_gently(the_repository, &nongit);
 
 	if (nongit && !the_hash_algo)
 		repo_set_hash_algo(the_repository, GIT_HASH_DEFAULT);
diff --git a/builtin/help.c b/builtin/help.c
index c0aece4da3..a140339999 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -740,7 +740,7 @@ int cmd_help(int argc,
 		return 0;
 	}
 
-	setup_git_directory_gently(&nongit);
+	setup_git_directory_gently(the_repository, &nongit);
 	repo_config(the_repository, git_help_config, NULL);
 
 	if (parsed_help_format != HELP_FORMAT_NONE)
diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index 4a566cbc5d..18705f1a5b 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -54,7 +54,7 @@ int cmd_stripspace(int argc,
 		usage_with_options(stripspace_usage, options);
 
 	if (mode == STRIP_COMMENTS || mode == COMMENT_LINES) {
-		setup_git_directory_gently(&nongit);
+		setup_git_directory_gently(the_repository, &nongit);
 		repo_config(the_repository, git_default_config, NULL);
 	}
 
diff --git a/git.c b/git.c
index eaede42c4e..2cc018fc5c 100644
--- a/git.c
+++ b/git.c
@@ -84,7 +84,7 @@ static int list_cmds(const char *spec)
 	* Set up the repository so we can pick up any repo-level config (like
 	* completion.commands).
 	*/
-	setup_git_directory_gently(&nongit);
+	setup_git_directory_gently(the_repository, &nongit);
 
 	while (*spec) {
 		const char *sep = strchrnul(spec, ',');
@@ -386,7 +386,7 @@ static int handle_alias(struct strvec *args, struct string_list *expanded_aliase
 			int nongit_ok;
 
 			/* Aliases expect GIT_PREFIX, GIT_DIR etc to be set */
-			setup_git_directory_gently(&nongit_ok);
+			setup_git_directory_gently(the_repository, &nongit_ok);
 
 			commit_pager_choice();
 
@@ -480,7 +480,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv, struct
 		prefix = setup_git_directory();
 		no_repo = 0;
 	} else if (run_setup & RUN_SETUP_GENTLY) {
-		prefix = setup_git_directory_gently(&no_repo);
+		prefix = setup_git_directory_gently(the_repository, &no_repo);
 	} else {
 		prefix = NULL;
 	}
diff --git a/http-fetch.c b/http-fetch.c
index 1922e23fcd..f9b6ecb061 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -109,7 +109,7 @@ int cmd_main(int argc, const char **argv)
 	struct strvec index_pack_args = STRVEC_INIT;
 	int ret;
 
-	setup_git_directory_gently(&nongit);
+	setup_git_directory_gently(the_repository, &nongit);
 
 	while (arg < argc && argv[arg][0] == '-') {
 		const char *p;
diff --git a/imap-send.c b/imap-send.c
index af02c6a689..cfd6a5120c 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1799,7 +1799,7 @@ int cmd_main(int argc, const char **argv)
 	int nongit_ok;
 	int ret;
 
-	setup_git_directory_gently(&nongit_ok);
+	setup_git_directory_gently(the_repository, &nongit_ok);
 	repo_config(the_repository, git_imap_config, &server);
 
 	argc = parse_options(argc, (const char **)argv, "", imap_send_options, imap_send_usage, 0);
diff --git a/remote-curl.c b/remote-curl.c
index aba60d5712..a84fc860ec 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -1557,7 +1557,7 @@ int cmd_main(int argc, const char **argv)
 	int nongit;
 	int ret = 1;
 
-	setup_git_directory_gently(&nongit);
+	setup_git_directory_gently(the_repository, &nongit);
 	if (argc < 2) {
 		error(_("remote-curl: usage: git remote-curl <remote> [<url>]"));
 		goto cleanup;
@@ -1605,7 +1605,7 @@ int cmd_main(int argc, const char **argv)
 			break;
 		if (starts_with(buf.buf, "fetch ")) {
 			if (nongit) {
-				setup_git_directory_gently(&nongit);
+				setup_git_directory_gently(the_repository, &nongit);
 				if (nongit)
 					die(_("remote-curl: fetch attempted without a local repo"));
 			}
diff --git a/setup.c b/setup.c
index 8965f8ccaf..c12c6cbda2 100644
--- a/setup.c
+++ b/setup.c
@@ -1862,7 +1862,7 @@ void set_git_work_tree(struct repository *repo, const char *new_work_tree)
 	repo_set_worktree(repo, new_work_tree);
 }
 
-const char *setup_git_directory_gently(int *nongit_ok)
+const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
 {
 	static struct strbuf cwd = STRBUF_INIT;
 	struct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT, report = STRBUF_INIT;
@@ -1877,7 +1877,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	 * configuration (including the per-repo config file that we
 	 * ignored previously).
 	 */
-	repo_config_clear(the_repository);
+	repo_config_clear(repo);
 
 	/*
 	 * Let's assume that we are in a git repository.
@@ -1893,18 +1893,18 @@ const char *setup_git_directory_gently(int *nongit_ok)
 
 	switch (setup_git_directory_gently_1(&dir, &gitdir, &report, 1)) {
 	case GIT_DIR_EXPLICIT:
-		prefix = setup_explicit_git_dir(the_repository, gitdir.buf, &cwd, &repo_fmt, nongit_ok);
+		prefix = setup_explicit_git_dir(repo, gitdir.buf, &cwd, &repo_fmt, nongit_ok);
 		break;
 	case GIT_DIR_DISCOVERED:
 		if (dir.len < cwd.len && chdir(dir.buf))
 			die(_("cannot change to '%s'"), dir.buf);
-		prefix = setup_discovered_git_dir(the_repository, gitdir.buf, &cwd, dir.len,
+		prefix = setup_discovered_git_dir(repo, gitdir.buf, &cwd, dir.len,
 						  &repo_fmt, nongit_ok);
 		break;
 	case GIT_DIR_BARE:
 		if (dir.len < cwd.len && chdir(dir.buf))
 			die(_("cannot change to '%s'"), dir.buf);
-		prefix = setup_bare_git_dir(the_repository, &cwd, dir.len, &repo_fmt, nongit_ok);
+		prefix = setup_bare_git_dir(repo, &cwd, dir.len, &repo_fmt, nongit_ok);
 		break;
 	case GIT_DIR_HIT_CEILING:
 		if (!nongit_ok)
@@ -1984,30 +1984,30 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	    startup_info->have_repository ||
 	    /* GIT_DIR_EXPLICIT */
 	    getenv(GIT_DIR_ENVIRONMENT)) {
-		if (!the_repository->gitdir) {
+		if (!repo->gitdir) {
 			const char *gitdir = getenv(GIT_DIR_ENVIRONMENT);
 			if (!gitdir)
 				gitdir = DEFAULT_GIT_DIR_ENVIRONMENT;
-			setup_git_env(the_repository, gitdir);
+			setup_git_env(repo, gitdir);
 		}
 		if (startup_info->have_repository) {
-			repo_set_hash_algo(the_repository, repo_fmt.hash_algo);
-			repo_set_compat_hash_algo(the_repository,
+			repo_set_hash_algo(repo, repo_fmt.hash_algo);
+			repo_set_compat_hash_algo(repo,
 						  repo_fmt.compat_hash_algo);
-			repo_set_ref_storage_format(the_repository,
+			repo_set_ref_storage_format(repo,
 						    repo_fmt.ref_storage_format,
 						    repo_fmt.ref_storage_payload);
-			the_repository->repository_format_worktree_config =
+			repo->repository_format_worktree_config =
 				repo_fmt.worktree_config;
-			the_repository->repository_format_relative_worktrees =
+			repo->repository_format_relative_worktrees =
 				repo_fmt.relative_worktrees;
-			the_repository->repository_format_submodule_path_cfg =
+			repo->repository_format_submodule_path_cfg =
 				repo_fmt.submodule_path_cfg;
 			/* take ownership of repo_fmt.partial_clone */
-			the_repository->repository_format_partial_clone =
+			repo->repository_format_partial_clone =
 				repo_fmt.partial_clone;
 			repo_fmt.partial_clone = NULL;
-			the_repository->repository_format_precious_objects =
+			repo->repository_format_precious_objects =
 				repo_fmt.precious_objects;
 		}
 	}
@@ -2040,13 +2040,13 @@ const char *setup_git_directory_gently(int *nongit_ok)
 		format = ref_storage_format_by_name(backend);
 		if (format == REF_STORAGE_FORMAT_UNKNOWN)
 			die(_("unknown ref storage format: '%s'"), backend);
-		repo_set_ref_storage_format(the_repository, format, payload);
+		repo_set_ref_storage_format(repo, format, payload);
 
 		free(backend);
 		free(payload);
 	}
 
-	setup_original_cwd(the_repository);
+	setup_original_cwd(repo);
 
 	strbuf_release(&dir);
 	strbuf_release(&gitdir);
@@ -2138,7 +2138,7 @@ void check_repository_format(struct repository_format *fmt)
  */
 const char *setup_git_directory(void)
 {
-	return setup_git_directory_gently(NULL);
+	return setup_git_directory_gently(the_repository, NULL);
 }
 
 const char *resolve_gitdir_gently(const char *suspect, int *return_error_code)
diff --git a/setup.h b/setup.h
index 1a37089fa0..18092fbf16 100644
--- a/setup.h
+++ b/setup.h
@@ -136,7 +136,7 @@ enum {
  */
 const char *enter_repo(struct repository *repo, const char *path, unsigned flags);
 
-const char *setup_git_directory_gently(int *);
+const char *setup_git_directory_gently(struct repository *repo, int *);
 const char *setup_git_directory(void);
 char *prefix_path(struct repository *repo, const char *prefix, int len, const char *path);
 char *prefix_path_gently(struct repository *repo, const char *prefix, int len, int *remaining, const char *path);
diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
index 163fdeefb0..15eb44485c 100644
--- a/t/helper/test-path-utils.c
+++ b/t/helper/test-path-utils.c
@@ -377,7 +377,7 @@ int cmd__path_utils(int argc, const char **argv)
 		const char *prefix = argv[2];
 		int prefix_len = strlen(prefix);
 		int nongit_ok;
-		setup_git_directory_gently(&nongit_ok);
+		setup_git_directory_gently(the_repository, &nongit_ok);
 		while (argc > 3) {
 			char *pfx = prefix_path(the_repository, prefix, prefix_len, argv[3]);
 
diff --git a/t/helper/test-subprocess.c b/t/helper/test-subprocess.c
index 8a070e47cd..a8194d24b3 100644
--- a/t/helper/test-subprocess.c
+++ b/t/helper/test-subprocess.c
@@ -9,7 +9,7 @@ int cmd__subprocess(int argc, const char **argv)
 	struct child_process cp = CHILD_PROCESS_INIT;
 	int nogit = 0;
 
-	setup_git_directory_gently(&nogit);
+	setup_git_directory_gently(the_repository, &nogit);
 	if (nogit)
 		die("No git repo found");
 	if (argc > 1 && !strcmp(argv[1], "--setup-work-tree")) {

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* [PATCH v2 13/18] setup: stop using `the_repository` in `setup_git_directory()`
From: Patrick Steinhardt @ 2026-05-18  9:31 UTC (permalink / raw)
  To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-0-6933c0f1d568@pks.im>

Stop using `the_repository` in `setup_git_directory()` and instead
accept the repository as a parameter. The injection of `the_repository`
is thus bumped one level higher, where callers now pass it in
explicitly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 archive.c                                    |  2 +-
 builtin/grep.c                               |  2 +-
 builtin/hash-object.c                        |  2 +-
 builtin/merge-file.c                         |  2 +-
 builtin/rev-parse.c                          |  4 ++--
 git.c                                        |  2 +-
 http-push.c                                  |  2 +-
 scalar.c                                     |  4 ++--
 setup.c                                      |  4 ++--
 setup.h                                      |  2 +-
 t/helper/test-advise.c                       |  2 +-
 t/helper/test-bitmap.c                       |  2 +-
 t/helper/test-bloom.c                        |  2 +-
 t/helper/test-cache-tree.c                   |  2 +-
 t/helper/test-config.c                       |  2 +-
 t/helper/test-dump-cache-tree.c              |  2 +-
 t/helper/test-dump-fsmonitor.c               |  2 +-
 t/helper/test-dump-split-index.c             |  2 +-
 t/helper/test-dump-untracked-cache.c         |  2 +-
 t/helper/test-find-pack.c                    |  2 +-
 t/helper/test-fsmonitor-client.c             |  2 +-
 t/helper/test-lazy-init-name-hash.c          |  2 +-
 t/helper/test-match-trees.c                  |  2 +-
 t/helper/test-pack-deltas.c                  |  2 +-
 t/helper/test-pack-mtimes.c                  |  2 +-
 t/helper/test-partial-clone.c                |  4 +++-
 t/helper/test-path-walk.c                    |  2 +-
 t/helper/test-reach.c                        |  2 +-
 t/helper/test-read-cache.c                   |  2 +-
 t/helper/test-read-graph.c                   |  2 +-
 t/helper/test-read-midx.c                    |  2 +-
 t/helper/test-ref-store.c                    |  2 +-
 t/helper/test-revision-walking.c             |  2 +-
 t/helper/test-scrap-cache-tree.c             |  2 +-
 t/helper/test-serve-v2.c                     |  2 +-
 t/helper/test-submodule-config.c             |  2 +-
 t/helper/test-submodule-nested-repo-config.c |  2 +-
 t/helper/test-submodule.c                    | 10 +++++-----
 t/helper/test-userdiff.c                     |  2 +-
 t/helper/test-write-cache.c                  |  2 +-
 40 files changed, 49 insertions(+), 47 deletions(-)

diff --git a/archive.c b/archive.c
index fcd474c682..51229107a5 100644
--- a/archive.c
+++ b/archive.c
@@ -786,7 +786,7 @@ int write_archive(int argc, const char **argv, const char *prefix,
 		 * die ourselves; but its error message will be more specific
 		 * than what we could write here.
 		 */
-		setup_git_directory();
+		setup_git_directory(the_repository);
 	}
 
 	parse_treeish_arg(argv, &args, remote);
diff --git a/builtin/grep.c b/builtin/grep.c
index 679f8b567a..560133feb8 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1064,7 +1064,7 @@ int cmd_grep(int argc,
 			use_index = 0;
 		else
 			/* die the same way as if we did it at the beginning */
-			setup_git_directory();
+			setup_git_directory(the_repository);
 	}
 	/* Ignore --recurse-submodules if --no-index is given or implied */
 	if (!use_index)
diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index d7905bedc2..f306b0643f 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -100,7 +100,7 @@ int cmd_hash_object(int argc,
 			     hash_object_usage, 0);
 
 	if (flags & INDEX_WRITE_OBJECT)
-		prefix = setup_git_directory();
+		prefix = setup_git_directory(the_repository);
 	else
 		prefix = setup_git_directory_gently(the_repository, &nongit);
 
diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index 59a9792208..8fa5765239 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -110,7 +110,7 @@ int cmd_merge_file(int argc,
 
 	if (!repo && object_id)
 		/* emit the correct "not a git repo" error in this case */
-		setup_git_directory();
+		setup_git_directory(the_repository);
 
 	for (i = 0; i < 3; i++) {
 		char *fname;
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 8fdb75413d..0a01ff7a75 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -739,7 +739,7 @@ int cmd_rev_parse(int argc,
 
 	/* No options; just report on whether we're in a git repo or not. */
 	if (argc == 1) {
-		setup_git_directory();
+		setup_git_directory(the_repository);
 		repo_config(the_repository, git_default_config, NULL);
 		return 0;
 	}
@@ -774,7 +774,7 @@ int cmd_rev_parse(int argc,
 
 		/* The rest of the options require a git repository. */
 		if (!did_repo_setup) {
-			prefix = setup_git_directory();
+			prefix = setup_git_directory(the_repository);
 			repo_config(the_repository, git_default_config, NULL);
 			did_repo_setup = 1;
 
diff --git a/git.c b/git.c
index 2cc018fc5c..42cd5f5b3c 100644
--- a/git.c
+++ b/git.c
@@ -477,7 +477,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv, struct
 		run_setup = RUN_SETUP_GENTLY;
 
 	if (run_setup & RUN_SETUP) {
-		prefix = setup_git_directory();
+		prefix = setup_git_directory(the_repository);
 		no_repo = 0;
 	} else if (run_setup & RUN_SETUP_GENTLY) {
 		prefix = setup_git_directory_gently(the_repository, &no_repo);
diff --git a/http-push.c b/http-push.c
index d143fe2845..520d6c3b6a 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1788,7 +1788,7 @@ int cmd_main(int argc, const char **argv)
 	if (delete_branch && rs.nr != 1)
 		die("You must specify only one branch name when deleting a remote branch");
 
-	gitdir = setup_git_directory();
+	gitdir = setup_git_directory(the_repository);
 
 	memset(remote_dir_exists, -1, 256);
 
diff --git a/scalar.c b/scalar.c
index 4efb6ac36d..a80d8ee3ff 100644
--- a/scalar.c
+++ b/scalar.c
@@ -58,7 +58,7 @@ static void setup_enlistment_directory(int argc, const char **argv,
 	}
 	strbuf_setlen(&path, len);
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	if (!the_repository->worktree)
 		die(_("Scalar enlistments require a worktree"));
@@ -514,7 +514,7 @@ static int cmd_clone(int argc, const char **argv)
 		goto cleanup;
 	}
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	/* common-main already logs `argv` */
 	trace2_def_repo(the_repository);
diff --git a/setup.c b/setup.c
index c12c6cbda2..5dc27caf15 100644
--- a/setup.c
+++ b/setup.c
@@ -2136,9 +2136,9 @@ void check_repository_format(struct repository_format *fmt)
  * directory is not a strict subdirectory of the work tree root. The
  * prefix always ends with a '/' character.
  */
-const char *setup_git_directory(void)
+const char *setup_git_directory(struct repository *repo)
 {
-	return setup_git_directory_gently(the_repository, NULL);
+	return setup_git_directory_gently(repo, NULL);
 }
 
 const char *resolve_gitdir_gently(const char *suspect, int *return_error_code)
diff --git a/setup.h b/setup.h
index 18092fbf16..b779661ce7 100644
--- a/setup.h
+++ b/setup.h
@@ -137,7 +137,7 @@ enum {
 const char *enter_repo(struct repository *repo, const char *path, unsigned flags);
 
 const char *setup_git_directory_gently(struct repository *repo, int *);
-const char *setup_git_directory(void);
+const char *setup_git_directory(struct repository *repo);
 char *prefix_path(struct repository *repo, const char *prefix, int len, const char *path);
 char *prefix_path_gently(struct repository *repo, const char *prefix, int len, int *remaining, const char *path);
 
diff --git a/t/helper/test-advise.c b/t/helper/test-advise.c
index 81ed93a05c..8f9db2693e 100644
--- a/t/helper/test-advise.c
+++ b/t/helper/test-advise.c
@@ -11,7 +11,7 @@ int cmd__advise_if_enabled(int argc, const char **argv)
 	if (argc != 2)
 		die("usage: %s <advice>", argv[0]);
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 	repo_config(the_repository, git_default_config, NULL);
 
 	/*
diff --git a/t/helper/test-bitmap.c b/t/helper/test-bitmap.c
index 16a01669e4..d9b9a83b8f 100644
--- a/t/helper/test-bitmap.c
+++ b/t/helper/test-bitmap.c
@@ -37,7 +37,7 @@ static int bitmap_dump_pseudo_merge_objects(uint32_t n)
 
 int cmd__bitmap(int argc, const char **argv)
 {
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	if (argc == 2 && !strcmp(argv[1], "list-commits"))
 		return bitmap_list_commits();
diff --git a/t/helper/test-bloom.c b/t/helper/test-bloom.c
index 3283544bd3..0c65befbf0 100644
--- a/t/helper/test-bloom.c
+++ b/t/helper/test-bloom.c
@@ -52,7 +52,7 @@ static const char *const bloom_usage = "\n"
 
 int cmd__bloom(int argc, const char **argv)
 {
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	if (argc < 2)
 		usage(bloom_usage);
diff --git a/t/helper/test-cache-tree.c b/t/helper/test-cache-tree.c
index ff61d0ca7e..d42e260092 100644
--- a/t/helper/test-cache-tree.c
+++ b/t/helper/test-cache-tree.c
@@ -33,7 +33,7 @@ int cmd__cache_tree(int argc, const char **argv)
 		OPT_END()
 	};
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	argc = parse_options(argc, argv, NULL, options, test_cache_tree_usage, 0);
 
diff --git a/t/helper/test-config.c b/t/helper/test-config.c
index 9f8cca7c48..cfb3f4b111 100644
--- a/t/helper/test-config.c
+++ b/t/helper/test-config.c
@@ -102,7 +102,7 @@ int cmd__config(int argc, const char **argv)
 		return 0;
 	}
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	git_configset_init(&cs);
 
diff --git a/t/helper/test-dump-cache-tree.c b/t/helper/test-dump-cache-tree.c
index 3f0c7d0ed0..ccb41a4239 100644
--- a/t/helper/test-dump-cache-tree.c
+++ b/t/helper/test-dump-cache-tree.c
@@ -66,7 +66,7 @@ int cmd__dump_cache_tree(int ac UNUSED, const char **av UNUSED)
 	struct cache_tree *another = cache_tree();
 	int ret;
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 	if (repo_read_index(the_repository) < 0)
 		die("unable to read index file");
 	istate = *the_repository->index;
diff --git a/t/helper/test-dump-fsmonitor.c b/t/helper/test-dump-fsmonitor.c
index efd017ca35..c991cbbb8a 100644
--- a/t/helper/test-dump-fsmonitor.c
+++ b/t/helper/test-dump-fsmonitor.c
@@ -9,7 +9,7 @@ int cmd__dump_fsmonitor(int ac UNUSED, const char **av UNUSED)
 {
 	struct index_state *istate = the_repository->index;
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 	if (do_read_index(istate, the_repository->index_file, 0) < 0)
 		die("unable to read index file");
 	if (!istate->fsmonitor_last_update) {
diff --git a/t/helper/test-dump-split-index.c b/t/helper/test-dump-split-index.c
index f855a3862c..aae0a40a74 100644
--- a/t/helper/test-dump-split-index.c
+++ b/t/helper/test-dump-split-index.c
@@ -17,7 +17,7 @@ int cmd__dump_split_index(int ac UNUSED, const char **av)
 {
 	struct split_index *si;
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	do_read_index(the_repository->index, av[1], 1);
 	printf("own %s\n", oid_to_hex(&the_repository->index->oid));
diff --git a/t/helper/test-dump-untracked-cache.c b/t/helper/test-dump-untracked-cache.c
index 01a109496b..24308bd371 100644
--- a/t/helper/test-dump-untracked-cache.c
+++ b/t/helper/test-dump-untracked-cache.c
@@ -54,7 +54,7 @@ int cmd__dump_untracked_cache(int ac UNUSED, const char **av UNUSED)
 	xsetenv("GIT_CONFIG_KEY_0", "core.untrackedCache", 1);
 	xsetenv("GIT_CONFIG_VALUE_0", "keep", 1);
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 	if (repo_read_index(the_repository) < 0)
 		die("unable to read index file");
 	uc = the_repository->index->untracked;
diff --git a/t/helper/test-find-pack.c b/t/helper/test-find-pack.c
index fc4b8a77b3..28d5b1fe09 100644
--- a/t/helper/test-find-pack.c
+++ b/t/helper/test-find-pack.c
@@ -25,7 +25,7 @@ int cmd__find_pack(int argc, const char **argv)
 	struct object_id oid;
 	struct packed_git *p;
 	int count = -1, actual_count = 0;
-	const char *prefix = setup_git_directory();
+	const char *prefix = setup_git_directory(the_repository);
 
 	struct option options[] = {
 		OPT_INTEGER('c', "check-count", &count, "expected number of packs"),
diff --git a/t/helper/test-fsmonitor-client.c b/t/helper/test-fsmonitor-client.c
index 02bfe92e8d..dc1dff23fb 100644
--- a/t/helper/test-fsmonitor-client.c
+++ b/t/helper/test-fsmonitor-client.c
@@ -210,7 +210,7 @@ int cmd__fsmonitor_client(int argc, const char **argv)
 
 	subcmd = argv[0];
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	if (!strcmp(subcmd, "query"))
 		return !!do_send_query(token);
diff --git a/t/helper/test-lazy-init-name-hash.c b/t/helper/test-lazy-init-name-hash.c
index 40f5df4412..e542985c94 100644
--- a/t/helper/test-lazy-init-name-hash.c
+++ b/t/helper/test-lazy-init-name-hash.c
@@ -211,7 +211,7 @@ int cmd__lazy_init_name_hash(int argc, const char **argv)
 	const char *prefix;
 	uint64_t avg_single, avg_multi;
 
-	prefix = setup_git_directory();
+	prefix = setup_git_directory(the_repository);
 
 	argc = parse_options(argc, argv, prefix, options, usage, 0);
 
diff --git a/t/helper/test-match-trees.c b/t/helper/test-match-trees.c
index 2ed064b971..006ce5278e 100644
--- a/t/helper/test-match-trees.c
+++ b/t/helper/test-match-trees.c
@@ -13,7 +13,7 @@ int cmd__match_trees(int ac UNUSED, const char **av)
 	struct object_id hash1, hash2, shifted;
 	struct tree *one, *two;
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	if (repo_get_oid(the_repository, av[1], &hash1))
 		die("cannot parse %s as an object name", av[1]);
diff --git a/t/helper/test-pack-deltas.c b/t/helper/test-pack-deltas.c
index 4981401eaa..c493b75e02 100644
--- a/t/helper/test-pack-deltas.c
+++ b/t/helper/test-pack-deltas.c
@@ -95,7 +95,7 @@ int cmd__pack_deltas(int argc, const char **argv)
 	if (argc || num_objects < 0)
 		usage_with_options(usage_str, options);
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	f = hashfd(the_repository->hash_algo, 1, "<stdout>");
 	write_pack_header(f, num_objects);
diff --git a/t/helper/test-pack-mtimes.c b/t/helper/test-pack-mtimes.c
index 7a8ee1de24..b774056799 100644
--- a/t/helper/test-pack-mtimes.c
+++ b/t/helper/test-pack-mtimes.c
@@ -32,7 +32,7 @@ int cmd__pack_mtimes(int argc, const char **argv)
 	struct strbuf buf = STRBUF_INIT;
 	struct packed_git *p;
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	if (argc != 2)
 		usage(pack_mtimes_usage);
diff --git a/t/helper/test-partial-clone.c b/t/helper/test-partial-clone.c
index d848800749..a7aab426d0 100644
--- a/t/helper/test-partial-clone.c
+++ b/t/helper/test-partial-clone.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "test-tool.h"
 #include "hex.h"
 #include "repository.h"
@@ -32,7 +34,7 @@ static void object_info(const char *gitdir, const char *oid_hex)
 
 int cmd__partial_clone(int argc, const char **argv)
 {
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	if (argc < 4)
 		die("too few arguments");
diff --git a/t/helper/test-path-walk.c b/t/helper/test-path-walk.c
index fe63002c2b..69676b15a5 100644
--- a/t/helper/test-path-walk.c
+++ b/t/helper/test-path-walk.c
@@ -89,7 +89,7 @@ int cmd__path_walk(int argc, const char **argv)
 		OPT_END(),
 	};
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 	revs.repo = the_repository;
 
 	argc = parse_options(argc, argv, NULL,
diff --git a/t/helper/test-reach.c b/t/helper/test-reach.c
index 3131b54a87..5d86a96c17 100644
--- a/t/helper/test-reach.c
+++ b/t/helper/test-reach.c
@@ -39,7 +39,7 @@ int cmd__reach(int ac, const char **av)
 	struct strbuf buf = STRBUF_INIT;
 	struct repository *r = the_repository;
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	if (ac < 2)
 		exit(1);
diff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c
index 9ae71cefb3..6b08ba8f07 100644
--- a/t/helper/test-read-cache.c
+++ b/t/helper/test-read-cache.c
@@ -19,7 +19,7 @@ int cmd__read_cache(int argc, const char **argv)
 
 	if (argc == 2)
 		cnt = strtol(argv[1], NULL, 0);
-	setup_git_directory();
+	setup_git_directory(the_repository);
 	repo_config(the_repository, git_default_config, NULL);
 
 	for (i = 0; i < cnt; i++) {
diff --git a/t/helper/test-read-graph.c b/t/helper/test-read-graph.c
index 6a5f64e473..9f07b9c25a 100644
--- a/t/helper/test-read-graph.c
+++ b/t/helper/test-read-graph.c
@@ -76,7 +76,7 @@ int cmd__read_graph(int argc, const char **argv)
 	struct odb_source *source;
 	int ret = 0;
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 	source = the_repository->objects->sources;
 
 	prepare_repo_settings(the_repository);
diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c
index 388d29e2b5..790000fb26 100644
--- a/t/helper/test-read-midx.c
+++ b/t/helper/test-read-midx.c
@@ -14,7 +14,7 @@
 static struct multi_pack_index *setup_midx(const char *object_dir)
 {
 	struct odb_source *source;
-	setup_git_directory();
+	setup_git_directory(the_repository);
 	source = odb_find_source(the_repository->objects, object_dir);
 	if (!source)
 		source = odb_add_to_alternates_memory(the_repository->objects,
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 74edf2029a..3866d0aca4 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -340,7 +340,7 @@ int cmd__ref_store(int argc UNUSED, const char **argv)
 	const char *func;
 	struct command *cmd;
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	argv = get_store(argv + 1, &refs);
 
diff --git a/t/helper/test-revision-walking.c b/t/helper/test-revision-walking.c
index 071f5bd1e2..70051eeaf8 100644
--- a/t/helper/test-revision-walking.c
+++ b/t/helper/test-revision-walking.c
@@ -56,7 +56,7 @@ int cmd__revision_walking(int argc, const char **argv)
 	if (argc < 2)
 		return 1;
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	if (!strcmp(argv[1], "run-twice")) {
 		printf("1st\n");
diff --git a/t/helper/test-scrap-cache-tree.c b/t/helper/test-scrap-cache-tree.c
index 64fff6e9e3..7b5ce501d9 100644
--- a/t/helper/test-scrap-cache-tree.c
+++ b/t/helper/test-scrap-cache-tree.c
@@ -12,7 +12,7 @@ int cmd__scrap_cache_tree(int ac UNUSED, const char **av UNUSED)
 {
 	struct lock_file index_lock = LOCK_INIT;
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 	repo_hold_locked_index(the_repository, &index_lock, LOCK_DIE_ON_ERROR);
 	if (repo_read_index(the_repository) < 0)
 		die("unable to read index file");
diff --git a/t/helper/test-serve-v2.c b/t/helper/test-serve-v2.c
index 63a200b8d4..27f3ed8947 100644
--- a/t/helper/test-serve-v2.c
+++ b/t/helper/test-serve-v2.c
@@ -23,7 +23,7 @@ int cmd__serve_v2(int argc, const char **argv)
 			 N_("exit immediately after advertising capabilities")),
 		OPT_END()
 	};
-	const char *prefix = setup_git_directory();
+	const char *prefix = setup_git_directory(the_repository);
 
 	/* ignore all unknown cmdline switches for now */
 	argc = parse_options(argc, argv, prefix, options, serve_usage,
diff --git a/t/helper/test-submodule-config.c b/t/helper/test-submodule-config.c
index cbe93f2f9e..3f30292179 100644
--- a/t/helper/test-submodule-config.c
+++ b/t/helper/test-submodule-config.c
@@ -34,7 +34,7 @@ int cmd__submodule_config(int argc, const char **argv)
 	if (my_argc % 2 != 0)
 		die_usage(argc, argv, "Wrong number of arguments.");
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	while (*arg) {
 		struct object_id commit_oid;
diff --git a/t/helper/test-submodule-nested-repo-config.c b/t/helper/test-submodule-nested-repo-config.c
index 2710341cd5..7e31d3fe47 100644
--- a/t/helper/test-submodule-nested-repo-config.c
+++ b/t/helper/test-submodule-nested-repo-config.c
@@ -19,7 +19,7 @@ int cmd__submodule_nested_repo_config(int argc, const char **argv)
 	if (argc < 3)
 		die_usage(argv, "Wrong number of arguments.");
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	if (repo_submodule_init(&subrepo, the_repository, argv[1], null_oid(the_hash_algo))) {
 		die_usage(argv, "Submodule not found.");
diff --git a/t/helper/test-submodule.c b/t/helper/test-submodule.c
index 0133852e1e..3c5c4c4a09 100644
--- a/t/helper/test-submodule.c
+++ b/t/helper/test-submodule.c
@@ -99,7 +99,7 @@ static int cmd__submodule_is_active(int argc, const char **argv)
 	if (argc != 1)
 		usage_with_options(submodule_is_active_usage, options);
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	return !is_submodule_active(the_repository, argv[0]);
 }
@@ -142,7 +142,7 @@ static int cmd__submodule_config_list(int argc, const char **argv)
 	argc = parse_options(argc, argv, "test-tools", options, usage,
 			     PARSE_OPT_KEEP_ARGV0);
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	if (argc == 2)
 		return print_config_from_gitmodules(the_repository, argv[1]);
@@ -161,7 +161,7 @@ static int cmd__submodule_config_set(int argc, const char **argv)
 	argc = parse_options(argc, argv, "test-tools", options, usage,
 			     PARSE_OPT_KEEP_ARGV0);
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	/* Equivalent to ACTION_SET in builtin/config.c */
 	if (argc == 3) {
@@ -183,7 +183,7 @@ static int cmd__submodule_config_unset(int argc, const char **argv)
 		NULL
 	};
 
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	if (argc == 2) {
 		if (!is_writing_gitmodules_ok())
@@ -202,7 +202,7 @@ static int cmd__submodule_config_writeable(int argc, const char **argv UNUSED)
 		"test-tool submodule config-writeable",
 		NULL
 	};
-	setup_git_directory();
+	setup_git_directory(the_repository);
 
 	if (argc == 1)
 		return is_writing_gitmodules_ok() ? 0 : -1;
diff --git a/t/helper/test-userdiff.c b/t/helper/test-userdiff.c
index aa3a9894d2..fc34c589b3 100644
--- a/t/helper/test-userdiff.c
+++ b/t/helper/test-userdiff.c
@@ -40,7 +40,7 @@ int cmd__userdiff(int argc, const char **argv)
 		return error("unknown argument %s", argv[1]);
 
 	if (want & USERDIFF_DRIVER_TYPE_CUSTOM) {
-		setup_git_directory();
+		setup_git_directory(the_repository);
 		repo_config(the_repository, cmd__userdiff_config, NULL);
 	}
 
diff --git a/t/helper/test-write-cache.c b/t/helper/test-write-cache.c
index b37dd2c5d6..98e1477c98 100644
--- a/t/helper/test-write-cache.c
+++ b/t/helper/test-write-cache.c
@@ -12,7 +12,7 @@ int cmd__write_cache(int argc, const char **argv)
 	int i, cnt = 1;
 	if (argc == 2)
 		cnt = strtol(argv[1], NULL, 0);
-	setup_git_directory();
+	setup_git_directory(the_repository);
 	repo_read_index(the_repository);
 	for (i = 0; i < cnt; i++) {
 		repo_hold_locked_index(the_repository, &index_lock,

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* [PATCH v2 14/18] setup: stop using `the_repository` in `upgrade_repository_format()`
From: Patrick Steinhardt @ 2026-05-18  9:31 UTC (permalink / raw)
  To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-0-6933c0f1d568@pks.im>

Stop using `the_repository` in `upgrade_repository_format()` and instead
accept the repository as a parameter. The injection of `the_repository`
is thus bumped one level higher, where callers now pass it in
explicitly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 list-objects-filter-options.c | 2 +-
 repository.h                  | 2 +-
 setup.c                       | 6 +++---
 worktree.c                    | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index cef67e5919..bc5d98f9e6 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -378,7 +378,7 @@ void partial_clone_register(
 			 */
 			return;
 	} else {
-		if (upgrade_repository_format(1) < 0)
+		if (upgrade_repository_format(the_repository, 1) < 0)
 			die(_("unable to upgrade repository format to support partial clone"));
 
 		/* Add promisor config for the remote */
diff --git a/repository.h b/repository.h
index d391aff8ab..c3ec0f4b79 100644
--- a/repository.h
+++ b/repository.h
@@ -281,6 +281,6 @@ void repo_update_index_if_able(struct repository *, struct lock_file *);
  * Return 1 if upgrade repository format to target_version succeeded,
  * 0 if no upgrade is necessary, and -1 when upgrade is not possible.
  */
-int upgrade_repository_format(int target_version);
+int upgrade_repository_format(struct repository *repo, int target_version);
 
 #endif /* REPOSITORY_H */
diff --git a/setup.c b/setup.c
index 5dc27caf15..ed0c14e98e 100644
--- a/setup.c
+++ b/setup.c
@@ -811,7 +811,7 @@ static int check_repository_format_gently(struct repository *repo,
 	return 0;
 }
 
-int upgrade_repository_format(int target_version)
+int upgrade_repository_format(struct repository *repo, int target_version)
 {
 	struct strbuf sb = STRBUF_INIT;
 	struct strbuf err = STRBUF_INIT;
@@ -819,7 +819,7 @@ int upgrade_repository_format(int target_version)
 	struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
 	int ret;
 
-	repo_common_path_append(the_repository, &sb, "config");
+	repo_common_path_append(repo, &sb, "config");
 	read_repository_format(&repo_fmt, sb.buf);
 	strbuf_release(&sb);
 
@@ -841,7 +841,7 @@ int upgrade_repository_format(int target_version)
 	}
 
 	strbuf_addf(&repo_version, "%d", target_version);
-	repo_config_set(the_repository, "core.repositoryformatversion", repo_version.buf);
+	repo_config_set(repo, "core.repositoryformatversion", repo_version.buf);
 
 	ret = 1;
 
diff --git a/worktree.c b/worktree.c
index d874e23b4e..988be84a30 100644
--- a/worktree.c
+++ b/worktree.c
@@ -1104,7 +1104,7 @@ void write_worktree_linking_files(const char *dotgit, const char *gitdir,
 	strbuf_realpath(&repo, repo.buf, 1);
 
 	if (use_relative_paths && !the_repository->repository_format_relative_worktrees) {
-		if (upgrade_repository_format(1) < 0)
+		if (upgrade_repository_format(the_repository, 1) < 0)
 			die(_("unable to upgrade repository format to support relative worktrees"));
 		if (repo_config_set_gently(the_repository, "extensions.relativeWorktrees", "true"))
 			die(_("unable to set extensions.relativeWorktrees setting"));

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* [PATCH v2 15/18] setup: stop using `the_repository` in `check_repository_format()`
From: Patrick Steinhardt @ 2026-05-18  9:31 UTC (permalink / raw)
  To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-0-6933c0f1d568@pks.im>

Stop using `the_repository` in `check_repository_format()` and instead
accept the repository as a parameter. The injection of `the_repository`
is thus bumped one level higher, where callers now pass it in
explicitly.

Furthermore, the function is never used outside "setup.c". Drop its
declaration in "setup.h" and make it static. Note that this requires us
to reorder the function.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 setup.c | 58 +++++++++++++++++++++++++++++++++-------------------------
 setup.h | 10 ----------
 2 files changed, 33 insertions(+), 35 deletions(-)

diff --git a/setup.c b/setup.c
index ed0c14e98e..406984b62c 100644
--- a/setup.c
+++ b/setup.c
@@ -1758,6 +1758,37 @@ enum discovery_result discover_git_directory_reason(struct strbuf *commondir,
 	return result;
 }
 
+/*
+ * Check the repository format version in the path found in repo_get_git_dir(repo),
+ * and die if it is a version we don't understand. Generally one would
+ * set_git_dir() before calling this, and use it only for "are we in a valid
+ * repo?".
+ *
+ * If successful and fmt is not NULL, fill fmt with data.
+ */
+static void check_repository_format(struct repository *repo, struct repository_format *fmt)
+{
+	struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
+	if (!fmt)
+		fmt = &repo_fmt;
+	check_repository_format_gently(repo, repo_get_git_dir(repo), fmt, NULL);
+	startup_info->have_repository = 1;
+	repo_set_hash_algo(repo, fmt->hash_algo);
+	repo_set_compat_hash_algo(repo, fmt->compat_hash_algo);
+	repo_set_ref_storage_format(repo,
+				    fmt->ref_storage_format,
+				    fmt->ref_storage_payload);
+	repo->repository_format_worktree_config =
+		fmt->worktree_config;
+	repo->repository_format_submodule_path_cfg =
+		fmt->submodule_path_cfg;
+	repo->repository_format_relative_worktrees =
+		fmt->relative_worktrees;
+	repo->repository_format_partial_clone =
+		xstrdup_or_null(fmt->partial_clone);
+	clear_repository_format(&repo_fmt);
+}
+
 const char *enter_repo(struct repository *repo, const char *path, unsigned flags)
 {
 	static struct strbuf validated_path = STRBUF_INIT;
@@ -1832,7 +1863,7 @@ const char *enter_repo(struct repository *repo, const char *path, unsigned flags
 
 	if (is_git_directory(".")) {
 		set_git_dir(repo, ".", 0);
-		check_repository_format(NULL);
+		check_repository_format(repo, NULL);
 		return path;
 	}
 
@@ -2107,29 +2138,6 @@ int git_config_perm(const char *var, const char *value)
 	return -(i & 0666);
 }
 
-void check_repository_format(struct repository_format *fmt)
-{
-	struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
-	if (!fmt)
-		fmt = &repo_fmt;
-	check_repository_format_gently(the_repository, repo_get_git_dir(the_repository), fmt, NULL);
-	startup_info->have_repository = 1;
-	repo_set_hash_algo(the_repository, fmt->hash_algo);
-	repo_set_compat_hash_algo(the_repository, fmt->compat_hash_algo);
-	repo_set_ref_storage_format(the_repository,
-				    fmt->ref_storage_format,
-				    fmt->ref_storage_payload);
-	the_repository->repository_format_worktree_config =
-		fmt->worktree_config;
-	the_repository->repository_format_submodule_path_cfg =
-		fmt->submodule_path_cfg;
-	the_repository->repository_format_relative_worktrees =
-		fmt->relative_worktrees;
-	the_repository->repository_format_partial_clone =
-		xstrdup_or_null(fmt->partial_clone);
-	clear_repository_format(&repo_fmt);
-}
-
 /*
  * Returns the "prefix", a path to the current working directory
  * relative to the work tree root, or NULL, if the current working
@@ -2804,7 +2812,7 @@ int init_db(const char *git_dir, const char *real_git_dir,
 	 * config file, so this will not fail.  What we are catching
 	 * is an attempt to reinitialize new repository with an old tool.
 	 */
-	check_repository_format(&repo_fmt);
+	check_repository_format(the_repository, &repo_fmt);
 
 	repository_format_configure(the_repository, &repo_fmt, hash, ref_storage_format);
 
diff --git a/setup.h b/setup.h
index b779661ce7..a820041af0 100644
--- a/setup.h
+++ b/setup.h
@@ -221,16 +221,6 @@ void clear_repository_format(struct repository_format *format);
 int verify_repository_format(const struct repository_format *format,
 			     struct strbuf *err);
 
-/*
- * Check the repository format version in the path found in repo_get_git_dir(the_repository),
- * and die if it is a version we don't understand. Generally one would
- * set_git_dir() before calling this, and use it only for "are we in a valid
- * repo?".
- *
- * If successful and fmt is not NULL, fill fmt with data.
- */
-void check_repository_format(struct repository_format *fmt);
-
 const char *get_template_dir(const char *option_template);
 
 #define INIT_DB_QUIET      (1 << 0)

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* [PATCH v2 16/18] setup: stop using `the_repository` in `initialize_repository_version()`
From: Patrick Steinhardt @ 2026-05-18  9:31 UTC (permalink / raw)
  To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-0-6933c0f1d568@pks.im>

Stop using `the_repository` in `initialize_repository_version()` and
instead accept the repository as a parameter. The injection of
`the_repository` is thus bumped one level higher, where callers now pass
it in explicitly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/clone.c |  4 ++--
 refs.c          |  2 +-
 setup.c         | 29 +++++++++++++++--------------
 setup.h         |  3 ++-
 4 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 8844e3d481..24fe0eead5 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1229,7 +1229,7 @@ int cmd_clone(int argc,
 	 *
 	 * This is sufficient for Git commands to discover the Git directory.
 	 */
-	initialize_repository_version(GIT_HASH_UNKNOWN,
+	initialize_repository_version(the_repository, GIT_HASH_UNKNOWN,
 				      the_repository->ref_storage_format, 1);
 
 	refs_create_refdir_stubs(the_repository, git_dir, NULL);
@@ -1442,7 +1442,7 @@ int cmd_clone(int argc,
 	 * ours to the same thing.
 	 */
 	hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport));
-	initialize_repository_version(hash_algo, the_repository->ref_storage_format, 1);
+	initialize_repository_version(the_repository, hash_algo, the_repository->ref_storage_format, 1);
 	repo_set_hash_algo(the_repository, hash_algo);
 	create_reference_database(NULL, 1);
 
diff --git a/refs.c b/refs.c
index 844785219d..c36a322f4c 100644
--- a/refs.c
+++ b/refs.c
@@ -3453,7 +3453,7 @@ int repo_migrate_ref_storage_format(struct repository *repo,
 	 * repository format so that clients will use the new ref store.
 	 * We also need to swap out the repository's main ref store.
 	 */
-	initialize_repository_version(hash_algo_by_ptr(repo->hash_algo), format, 1);
+	initialize_repository_version(the_repository, hash_algo_by_ptr(repo->hash_algo), format, 1);
 
 	/*
 	 * Unset the old ref store and release it. `get_main_ref_store()` will
diff --git a/setup.c b/setup.c
index 406984b62c..e09483ba34 100644
--- a/setup.c
+++ b/setup.c
@@ -2385,7 +2385,8 @@ static int needs_work_tree_config(const char *git_dir, const char *work_tree)
 	return 1;
 }
 
-void initialize_repository_version(int hash_algo,
+void initialize_repository_version(struct repository *repo,
+				   int hash_algo,
 				   enum ref_storage_format ref_storage_format,
 				   int reinit)
 {
@@ -2402,35 +2403,35 @@ void initialize_repository_version(int hash_algo,
 	 */
 	if (hash_algo != GIT_HASH_SHA1_LEGACY ||
 	    ref_storage_format != REF_STORAGE_FORMAT_FILES ||
-	    the_repository->ref_storage_payload)
+	    repo->ref_storage_payload)
 		target_version = GIT_REPO_VERSION_READ;
 
 	if (hash_algo != GIT_HASH_SHA1_LEGACY && hash_algo != GIT_HASH_UNKNOWN)
-		repo_config_set(the_repository, "extensions.objectformat",
+		repo_config_set(repo, "extensions.objectformat",
 				hash_algos[hash_algo].name);
 	else if (reinit)
-		repo_config_set_gently(the_repository, "extensions.objectformat", NULL);
+		repo_config_set_gently(repo, "extensions.objectformat", NULL);
 
-	if (the_repository->ref_storage_payload) {
+	if (repo->ref_storage_payload) {
 		struct strbuf ref_uri = STRBUF_INIT;
 
 		strbuf_addf(&ref_uri, "%s://%s",
 			    ref_storage_format_to_name(ref_storage_format),
-			    the_repository->ref_storage_payload);
-		repo_config_set(the_repository, "extensions.refstorage", ref_uri.buf);
+			    repo->ref_storage_payload);
+		repo_config_set(repo, "extensions.refstorage", ref_uri.buf);
 		strbuf_release(&ref_uri);
 	} else if (ref_storage_format != REF_STORAGE_FORMAT_FILES) {
-		repo_config_set(the_repository, "extensions.refstorage",
+		repo_config_set(repo, "extensions.refstorage",
 				ref_storage_format_to_name(ref_storage_format));
 	} else if (reinit) {
-		repo_config_set_gently(the_repository, "extensions.refstorage", NULL);
+		repo_config_set_gently(repo, "extensions.refstorage", NULL);
 	}
 
 	if (reinit) {
 		struct strbuf config = STRBUF_INIT;
 		struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
 
-		repo_common_path_append(the_repository, &config, "config");
+		repo_common_path_append(repo, &config, "config");
 		read_repository_format(&repo_fmt, config.buf);
 
 		if (repo_fmt.v1_only_extensions.nr)
@@ -2440,17 +2441,17 @@ void initialize_repository_version(int hash_algo,
 		clear_repository_format(&repo_fmt);
 	}
 
-	repo_config_get_bool(the_repository, "init.defaultSubmodulePathConfig",
+	repo_config_get_bool(repo, "init.defaultSubmodulePathConfig",
 			     &default_submodule_path_config);
 	if (default_submodule_path_config) {
 		/* extensions.submodulepathconfig requires at least version 1 */
 		if (target_version == 0)
 			target_version = 1;
-		repo_config_set(the_repository, "extensions.submodulepathconfig", "true");
+		repo_config_set(repo, "extensions.submodulepathconfig", "true");
 	}
 
 	strbuf_addf(&repo_version, "%d", target_version);
-	repo_config_set(the_repository, "core.repositoryformatversion", repo_version.buf);
+	repo_config_set(repo, "core.repositoryformatversion", repo_version.buf);
 
 	strbuf_release(&repo_version);
 }
@@ -2551,7 +2552,7 @@ static int create_default_files(struct repository *repo,
 		adjust_shared_perm(repo, repo_get_git_dir(repo));
 	}
 
-	initialize_repository_version(fmt->hash_algo, fmt->ref_storage_format, reinit);
+	initialize_repository_version(repo, fmt->hash_algo, fmt->ref_storage_format, reinit);
 
 	/* Check filemode trustability */
 	repo_git_path_replace(repo, &path, "config");
diff --git a/setup.h b/setup.h
index a820041af0..c33b675ccf 100644
--- a/setup.h
+++ b/setup.h
@@ -232,7 +232,8 @@ int init_db(const char *git_dir, const char *real_git_dir,
 	    enum ref_storage_format ref_storage_format,
 	    const char *initial_branch, int init_shared_repository,
 	    unsigned int flags);
-void initialize_repository_version(int hash_algo,
+void initialize_repository_version(struct repository *repo,
+				   int hash_algo,
 				   enum ref_storage_format ref_storage_format,
 				   int reinit);
 void create_reference_database(const char *initial_branch, int quiet);

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* [PATCH v2 17/18] setup: stop using `the_repository` in `create_reference_database()`
From: Patrick Steinhardt @ 2026-05-18  9:31 UTC (permalink / raw)
  To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-0-6933c0f1d568@pks.im>

Stop using `the_repository` in `create_reference_database()` and instead
accept the repository as a parameter. The injection of `the_repository`
is thus bumped one level higher, where callers now pass it in
explicitly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/clone.c |  2 +-
 setup.c         | 13 +++++++------
 setup.h         |  2 +-
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 24fe0eead5..53a41629e6 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1444,7 +1444,7 @@ int cmd_clone(int argc,
 	hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport));
 	initialize_repository_version(the_repository, hash_algo, the_repository->ref_storage_format, 1);
 	repo_set_hash_algo(the_repository, hash_algo);
-	create_reference_database(NULL, 1);
+	create_reference_database(the_repository, NULL, 1);
 
 	/*
 	 * Before fetching from the remote, download and install bundle
diff --git a/setup.c b/setup.c
index e09483ba34..9c49319568 100644
--- a/setup.c
+++ b/setup.c
@@ -2468,13 +2468,14 @@ static int is_reinit(struct repository *repo)
 	return ret;
 }
 
-void create_reference_database(const char *initial_branch, int quiet)
+void create_reference_database(struct repository *repo,
+			       const char *initial_branch, int quiet)
 {
 	struct strbuf err = STRBUF_INIT;
 	char *to_free = NULL;
-	int reinit = is_reinit(the_repository);
+	int reinit = is_reinit(repo);
 
-	if (ref_store_create_on_disk(get_main_ref_store(the_repository), 0, &err))
+	if (ref_store_create_on_disk(get_main_ref_store(repo), 0, &err))
 		die("failed to set up refs db: %s", err.buf);
 
 	/*
@@ -2486,14 +2487,14 @@ void create_reference_database(const char *initial_branch, int quiet)
 
 		if (!initial_branch)
 			initial_branch = to_free =
-				repo_default_branch_name(the_repository, quiet);
+				repo_default_branch_name(repo, quiet);
 
 		ref = xstrfmt("refs/heads/%s", initial_branch);
 		if (check_refname_format(ref, 0) < 0)
 			die(_("invalid initial branch name: '%s'"),
 			    initial_branch);
 
-		if (refs_update_symref(get_main_ref_store(the_repository), "HEAD", ref, NULL) < 0)
+		if (refs_update_symref(get_main_ref_store(repo), "HEAD", ref, NULL) < 0)
 			exit(1);
 		free(ref);
 	}
@@ -2830,7 +2831,7 @@ int init_db(const char *git_dir, const char *real_git_dir,
 				      &repo_fmt, init_shared_repository);
 
 	if (!(flags & INIT_DB_SKIP_REFDB))
-		create_reference_database(initial_branch, flags & INIT_DB_QUIET);
+		create_reference_database(the_repository, initial_branch, flags & INIT_DB_QUIET);
 	create_object_directory(the_repository);
 
 	if (repo_settings_get_shared_repository(the_repository)) {
diff --git a/setup.h b/setup.h
index c33b675ccf..21737e9bd6 100644
--- a/setup.h
+++ b/setup.h
@@ -236,7 +236,7 @@ void initialize_repository_version(struct repository *repo,
 				   int hash_algo,
 				   enum ref_storage_format ref_storage_format,
 				   int reinit);
-void create_reference_database(const char *initial_branch, int quiet);
+void create_reference_database(struct repository *repo, const char *initial_branch, int quiet);
 
 /*
  * NOTE NOTE NOTE!!

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* [PATCH v2 18/18] setup: stop using `the_repository` in `init_db()`
From: Patrick Steinhardt @ 2026-05-18  9:31 UTC (permalink / raw)
  To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260518-pks-setup-wo-the-repository-v2-0-6933c0f1d568@pks.im>

Stop using `the_repository` in `init_db()` and instead accept
the repository as a parameter. The injection of `the_repository` is thus
bumped one level higher, where callers now pass it in explicitly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/clone.c   |  2 +-
 builtin/init-db.c |  2 +-
 setup.c           | 43 ++++++++++++++++++++++---------------------
 setup.h           |  3 ++-
 4 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 53a41629e6..d60d1b60bc 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1186,7 +1186,7 @@ int cmd_clone(int argc,
 	 * repository, and reference backends may persist that information into
 	 * their on-disk data structures.
 	 */
-	init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN,
+	init_db(the_repository, git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN,
 		ref_storage_format, NULL,
 		do_not_override_repo_unix_permissions, INIT_DB_QUIET | INIT_DB_SKIP_REFDB);
 
diff --git a/builtin/init-db.c b/builtin/init-db.c
index e626b0d8b7..c55517ad94 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -252,7 +252,7 @@ int cmd_init_db(int argc,
 	}
 
 	flags |= INIT_DB_EXIST_OK;
-	ret = init_db(git_dir, real_git_dir, template_dir, hash_algo,
+	ret = init_db(the_repository, git_dir, real_git_dir, template_dir, hash_algo,
 		      ref_storage_format, initial_branch,
 		      init_shared_repository, flags);
 
diff --git a/setup.c b/setup.c
index 9c49319568..6aee839d8c 100644
--- a/setup.c
+++ b/setup.c
@@ -2778,7 +2778,8 @@ static void repository_format_configure(struct repository *repo,
 				    repo_fmt->ref_storage_payload);
 }
 
-int init_db(const char *git_dir, const char *real_git_dir,
+int init_db(struct repository *repo,
+	    const char *git_dir, const char *real_git_dir,
 	    const char *template_dir, int hash,
 	    enum ref_storage_format ref_storage_format,
 	    const char *initial_branch,
@@ -2798,13 +2799,13 @@ int init_db(const char *git_dir, const char *real_git_dir,
 		if (!exist_ok && !stat(real_git_dir, &st))
 			die(_("%s already exists"), real_git_dir);
 
-		set_git_dir(the_repository, real_git_dir, 1);
-		git_dir = repo_get_git_dir(the_repository);
+		set_git_dir(repo, real_git_dir, 1);
+		git_dir = repo_get_git_dir(repo);
 		separate_git_dir(git_dir, original_git_dir);
 	}
 	else {
-		set_git_dir(the_repository, git_dir, 1);
-		git_dir = repo_get_git_dir(the_repository);
+		set_git_dir(repo, git_dir, 1);
+		git_dir = repo_get_git_dir(repo);
 	}
 	startup_info->have_repository = 1;
 
@@ -2814,27 +2815,27 @@ int init_db(const char *git_dir, const char *real_git_dir,
 	 * config file, so this will not fail.  What we are catching
 	 * is an attempt to reinitialize new repository with an old tool.
 	 */
-	check_repository_format(the_repository, &repo_fmt);
+	check_repository_format(repo, &repo_fmt);
 
-	repository_format_configure(the_repository, &repo_fmt, hash, ref_storage_format);
+	repository_format_configure(repo, &repo_fmt, hash, ref_storage_format);
 
 	/*
 	 * Ensure `core.hidedotfiles` is processed. This must happen after we
 	 * have set up the repository format such that we can evaluate
 	 * includeIf conditions correctly in the case of re-initialization.
 	 */
-	repo_config(the_repository, git_default_core_config, NULL);
+	repo_config(repo, git_default_core_config, NULL);
 
-	safe_create_dir(the_repository, git_dir, 0);
+	safe_create_dir(repo, git_dir, 0);
 
-	reinit = create_default_files(the_repository, template_dir, original_git_dir,
+	reinit = create_default_files(repo, template_dir, original_git_dir,
 				      &repo_fmt, init_shared_repository);
 
 	if (!(flags & INIT_DB_SKIP_REFDB))
-		create_reference_database(the_repository, initial_branch, flags & INIT_DB_QUIET);
-	create_object_directory(the_repository);
+		create_reference_database(repo, initial_branch, flags & INIT_DB_QUIET);
+	create_object_directory(repo);
 
-	if (repo_settings_get_shared_repository(the_repository)) {
+	if (repo_settings_get_shared_repository(repo)) {
 		char buf[10];
 		/* We do not spell "group" and such, so that
 		 * the configuration can be read by older version
@@ -2842,29 +2843,29 @@ int init_db(const char *git_dir, const char *real_git_dir,
 		 * and compatibility values for PERM_GROUP and
 		 * PERM_EVERYBODY.
 		 */
-		if (repo_settings_get_shared_repository(the_repository) < 0)
+		if (repo_settings_get_shared_repository(repo) < 0)
 			/* force to the mode value */
-			xsnprintf(buf, sizeof(buf), "0%o", -repo_settings_get_shared_repository(the_repository));
-		else if (repo_settings_get_shared_repository(the_repository) == PERM_GROUP)
+			xsnprintf(buf, sizeof(buf), "0%o", -repo_settings_get_shared_repository(repo));
+		else if (repo_settings_get_shared_repository(repo) == PERM_GROUP)
 			xsnprintf(buf, sizeof(buf), "%d", OLD_PERM_GROUP);
-		else if (repo_settings_get_shared_repository(the_repository) == PERM_EVERYBODY)
+		else if (repo_settings_get_shared_repository(repo) == PERM_EVERYBODY)
 			xsnprintf(buf, sizeof(buf), "%d", OLD_PERM_EVERYBODY);
 		else
 			BUG("invalid value for shared_repository");
-		repo_config_set(the_repository, "core.sharedrepository", buf);
-		repo_config_set(the_repository, "receive.denyNonFastforwards", "true");
+		repo_config_set(repo, "core.sharedrepository", buf);
+		repo_config_set(repo, "receive.denyNonFastforwards", "true");
 	}
 
 	if (!(flags & INIT_DB_QUIET)) {
 		int len = strlen(git_dir);
 
 		if (reinit)
-			printf(repo_settings_get_shared_repository(the_repository)
+			printf(repo_settings_get_shared_repository(repo)
 			       ? _("Reinitialized existing shared Git repository in %s%s\n")
 			       : _("Reinitialized existing Git repository in %s%s\n"),
 			       git_dir, len && git_dir[len-1] != '/' ? "/" : "");
 		else
-			printf(repo_settings_get_shared_repository(the_repository)
+			printf(repo_settings_get_shared_repository(repo)
 			       ? _("Initialized empty shared Git repository in %s%s\n")
 			       : _("Initialized empty Git repository in %s%s\n"),
 			       git_dir, len && git_dir[len-1] != '/' ? "/" : "");
diff --git a/setup.h b/setup.h
index 21737e9bd6..9409326fe4 100644
--- a/setup.h
+++ b/setup.h
@@ -227,7 +227,8 @@ const char *get_template_dir(const char *option_template);
 #define INIT_DB_EXIST_OK   (1 << 1)
 #define INIT_DB_SKIP_REFDB (1 << 2)
 
-int init_db(const char *git_dir, const char *real_git_dir,
+int init_db(struct repository *repo,
+	    const char *git_dir, const char *real_git_dir,
 	    const char *template_dir, int hash_algo,
 	    enum ref_storage_format ref_storage_format,
 	    const char *initial_branch, int init_shared_repository,

-- 
2.54.0.771.g3ed373ac14.dirty


^ permalink raw reply related

* Re: git rebase --continue segfault
From: Phillip Wood @ 2026-05-18 10:02 UTC (permalink / raw)
  To: Alex Naidenkov, git, Junio C Hamano; +Cc: Patrick Steinhardt
In-Reply-To: <eaa03980-fbce-4402-88b8-0f260f2927ab@leshe4ka.ru>

Hi Alex

On 17/05/2026 10:19, Alex Naidenkov wrote:
> Hi, ive encountered on segfault when ran `git rebase --continue`. 
> Hopefully this would help
> 
> - i was in the middle of big rebase
> 
> - entered pin for signing commit
> - segfault happened

Thanks for reporting this, if you run

	ls -l "$(git rev-parse --git-path objects/pack)"/*.pack

How many pack files does it show? If there is only a single pack file 
with a modification time matching when the rebase failed then I suspect 
you've been bitten by the bug fixed in [1] which corrupted my repository 
over the weekend (ironically I was rebasing my local git to include that 
bug fix, but accidentally started the rebase with a build including that 
bug). "git rebase" runs "git commit" to reword commits and commit 
conflict resolutions and each time "git commit" calls 
run_auto_maintenance() so with that bug it stands a good chance of 
starting parallel repacks.

Hopefully there will be a 2.54.1 release containing the fix soon

Phillip

[1] 
https://lore.kernel.org/20260513-pks-maintenance-fix-lock-with-detach-v3-0-f27a1ac82891@pks.im

> ```
> 
> Debuginfo Build ID: d98c557aaa4baa2e6da8a12cf5a76d241c5af104
> ```
> 
> ```
> 
> gef➤  bt
> #0  repo_parse_tree_gently (r=0x557e86b43780 <the_repo.lto_priv>, 
> item=0x0, quiet_on_missing=0x0) at /usr/src/debug/git/git/tree.c:193
> #1  0x0000557e869161bd in repo_parse_tree (r=<optimized out>, 
> item=<optimized out>) at /usr/src/debug/git/git/tree.h:28
> #2  collect_merge_info (opt=0x557e9bf793b0, merge_base=<optimized out>, 
> side1=<optimized out>, side2=<optimized out>) at /usr/src/debug/git/git/ 
> merge-ort.c:1745
> #3  merge_ort_nonrecursive_internal (opt=opt@entry=0x7ffe20ae9eb0, 
> merge_base=<optimized out>, merge_base@entry=0x557e9bf793b0, 
> side1=side1@entry=0x557e9bf79430, side2=<optimized out>,
>      side2@entry=0x0, result=result@entry=0x7ffe20ae9e80) at /usr/src/ 
> debug/git/git/merge-ort.c:5256
> #4  0x0000557e8691a6b8 in merge_incore_nonrecursive (opt=0x7ffe20ae9eb0, 
> merge_base=0x557e9bf793b0, side1=0x557e9bf79430, side2=0x0, 
> result=0x7ffe20ae9e80)
>      at /usr/src/debug/git/git/merge-ort.c:5419
> #5  0x0000557e869e0e8a in do_recursive_merge (r=r@entry=0x557e86b43780 
> <the_repo.lto_priv>, base=base@entry=0x557e9bf8b800, 
> next=next@entry=0x557e9bf8b850,
>      base_label=base_label@entry=0x557e9bf627e0 "parent of db33c0f 
> (fix)", next_label=next_label@entry=0x557e9bfab130 "db33c0f (fix)", 
> head=head@entry=0x7ffe20aea160,
>      msgbuf=0x557e9bf1b710, opts=0x7ffe20aeb990) at /usr/src/debug/git/ 
> git/sequencer.c:782
> #6  0x0000557e869e355e in do_pick_commit (r=0x557e86b43780 
> <the_repo.lto_priv>, item=<optimized out>, opts=0x7ffe20aeb990, 
> final_fixup=0x0, check_todo=0x7ffe20aea35c)
>      at /usr/src/debug/git/git/sequencer.c:2445
> #7  0x0000557e869ebe75 in pick_one_commit (r=<optimized out>, 
> todo_list=0x7ffe20aeb330, opts=<optimized out>, 
> check_todo=0x7ffe20aea35c, reschedule=<synthetic pointer>)
>      at /usr/src/debug/git/git/sequencer.c:4921
> #8  pick_commits (r=0x557e86b43780 <the_repo.lto_priv>, 
> todo_list=<optimized out>, opts=0x7ffe20aeb990) at /usr/src/debug/git/ 
> git/sequencer.c:5030
> #9  0x0000557e869ef336 in sequencer_continue (r=<optimized out>, 
> opts=<optimized out>) at /usr/src/debug/git/git/sequencer.c:5487
> #10 0x0000557e867d536e in run_sequencer_rebase (opts=0x7ffe20aeb7a0) at 
> builtin/rebase.c:376
> #11 run_specific_rebase (opts=0x7ffe20aeb7a0) at builtin/rebase.c:755
> #12 cmd_rebase (argc=<optimized out>, argv=<optimized out>, 
> prefix=<optimized out>, repo=<optimized out>) at builtin/rebase.c:1910
> #13 0x0000557e866e9e65 in run_builtin (p=0x557e86b35530 
> <commands.lto_priv+2352>, argc=<optimized out>, argv=<optimized out>, 
> repo=0x557e86b43780 <the_repo.lto_priv>)
>      at /usr/src/debug/git/git/git.c:506
> #14 handle_builtin (args=args@entry=0x7ffe20aed760) at /usr/src/debug/ 
> git/git/git.c:780
> #15 0x0000557e866eb30c in run_argv (args=0x7ffe20aed760) at /usr/src/ 
> debug/git/git/git.c:863
> #16 cmd_main (argc=<optimized out>, argv=<optimized out>) at /usr/src/ 
> debug/git/git/git.c:984
> #17 0x0000557e866e77e4 in main (argc=0x3, argv=0x7ffe20aeda58) at /usr/ 
> src/debug/git/git/common-main.c:9
> 
> ```
> ```
> 
> [System Info]
> git version:
> git version 2.54.0
> cpu: x86_64
> built from commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
> sizeof-long: 8
> sizeof-size_t: 8
> shell-path: /bin/sh
> rust: enabled
> gettext: enabled
> libcurl: 8.19.0
> OpenSSL: OpenSSL 3.6.2 7 Apr 2026
> zlib-ng: 2.3.3
> SHA-1: SHA1_DC
> SHA-256: SHA256_BLK
> default-ref-format: files
> default-hash: sha1
> uname: Linux 7.0.5-arch1-1 #1 SMP PREEMPT_DYNAMIC Fri, 08 May 2026 
> 09:29:23 +0000 x86_64
> compiler info: gnuc: 15.2
> libc info: glibc: 2.43
> $SHELL (typically, interactive shell): /usr/bin/zsh
> 
> 
> [Enabled Hooks]
> pre-commit
> ```
> 
> 


^ permalink raw reply

* [PATCH 0/1] commit: allow -m/-F with --fixup=amend: or reword:
From: erik @ 2026-05-18 11:22 UTC (permalink / raw)
  To: git; +Cc: gitster, charvi077, Erik Cervin-Edin

From: Erik Cervin-Edin <erik@cervined.in>

The commit --fixup=reword: (and --fixup:amend) options are powerful but
currently not well-suited for non-interactive workflows.

I often find myself hacking away on a branch and the last thing I do is
finalize and formulate the commit messages. One of the current ways of
doing this is running an interactive rebase and picking the commits in
your branch to reword. However, doing this requires you to linearly go
through the messages and edit them one by one. The other options which
allows more flexible editing is to generate linear patches -- but this
trades editing freedom for branch topology freedom and has its own
drawbacks.

The --fixup=reword: flag introduced in 494d314a05 (commit: add
amend suboption to --fixup to create amend! commit, 2021-03-15),
adds a third workflow which allows rewording commits without initiating
a rebase and from the comfort of the HEAD of the branch. However, doing
such editing is only possible using $EDITOR, which restricts its use in
some workflows.

When amend:/reword: were introduced in Charvi's series, -m support
for amend fixups was discussed but not pursued
(xmqqwnuvsw0d.fsf@gitster.g and xmqqczwmsjzl.fsf@gitster.g):

On Fri, 26 Feb 2021 11:32:30 -0800, Junio C Hamano wrote:
> >> > +                     if (have_option_m)
> >> > +                             die(_("cannot combine -m with --fixup:%s"), fixup_message);
> >> > +                     else
> >> > +                             prepare_amend_commit(commit, &sb, &ctx);
> >>
> >> Hmph, why is -m so special?  Should we allow --fixup=amend:<cmd>
> >> with -F (or -c/-C for that matter), or are these other options
> >> caught at a lot higher layer already and we do not have to check
> >> them here?
> >
> > yes, those options are caught earlier and give the error as below:
> > "Only one of -c/-C/-F/--fixup can be used."
> > and only `-m` is checked over here.
>
> And the reason why -m cannot be checked early is because we do not
> recognize which kind of "fixup" we are doing when "only one of
> -c/-C/-F/--fixup" check is made before this function is called?
>
> OK.  I wonder if we can tell which kind of fixup we are doing much
> earlier, though.  Then we could extend it to say "Only one of
> -c/-C/-F/-m/--fixup=amend:<commit> can be used", etc., and we do not
> have to have this "only -m is checked here, everything else is
> checked earlier" curiosity.  But I do not know if such a change is
> necessarily an improvement.  I guess a better "fix" would probably
> be to add a comment to this function where it only checks for "-m"
> and tell readers why -c/-C/-F do not have to be checked here.

This patch picks up that thread by allowing both -m and -F for
amend/reword fixups, bypassing the need for an interactive editor.
This makes it practical to, for example, write replacement messages in
files and batch-apply them as reword fixups without stepping through
each one interactively. It's also friendly to AI agents who have a hard time
editing text using a non-interactive $EDITOR.

Allowing -c/-C was also considered but left out of this patch -- it can
be added in a re-roll if reviewers think it's worthwhile. I could see it
being useful, for example if you want to use git notes as a re-write
commit message channel. Since this is my first patch I intentionally
thought it best to start small.

Erik Cervin-Edin (1):
  commit: allow -m/-F with --fixup=amend: or reword:

 Documentation/git-commit.adoc             | 13 +++--
 builtin/commit.c                          | 41 ++++++++++----
 t/t7500-commit-template-squash-signoff.sh | 67 +++++++++++++++++++----
 3 files changed, 92 insertions(+), 29 deletions(-)

-- 
2.54.0.772.g683d7313b1


^ permalink raw reply

* [PATCH 1/1] commit: allow -m/-F with --fixup=amend: or reword:
From: erik @ 2026-05-18 11:22 UTC (permalink / raw)
  To: git; +Cc: gitster, charvi077, Erik Cervin-Edin
In-Reply-To: <20260518112225.73172-2-erik@cervined.in>

From: Erik Cervin-Edin <erik@cervined.in>

--fixup=amend: and --fixup=reword: require an editor to supply the
replacement commit message. The -m and -F flags are rejected: -m is
caught by a die() in prepare_to_commit(), and -F is caught by
die_for_incompatible_opt4() which groups -F with --fixup as mutually
exclusive. This makes these modes unusable in non-interactive
workflows -- notably AI coding agents.

When the amend suboption was introduced in 494d314a05 (commit: add
amend suboption to --fixup to create amend! commit, 2021-03-15),
-m support for amend fixups was discussed but not pursued, and -F
was already caught by the higher-layer incompatibility check grouping
it with --fixup.

Allow -m and -F to supply the replacement message body for amend and
reword fixups. When provided, bypass the editor and directly use the
user's message as the body, replacing the original commit's message. For
-F, the file contents are read into the message strbuf and then handled
identically to -m.

Plain --fixup (without amend: or reword:) continues to reject -F but
still accepts -m (even though it's practically a no-op).

Signed-off-by: Erik Cervin Edin <erik@cervined.in>
---
 Documentation/git-commit.adoc             | 13 +++--
 builtin/commit.c                          | 41 ++++++++++----
 t/t7500-commit-template-squash-signoff.sh | 67 +++++++++++++++++++----
 3 files changed, 92 insertions(+), 29 deletions(-)

diff --git a/Documentation/git-commit.adoc b/Documentation/git-commit.adoc
index 8329c1034b..9478d5d265 100644
--- a/Documentation/git-commit.adoc
+++ b/Documentation/git-commit.adoc
@@ -111,12 +111,13 @@ commit, but the additional commentary will be thrown away once the
 The commit created by `--fixup=amend:<commit>` is similar but its
 title is instead prefixed with "amend!". The log message of
 _<commit>_ is copied into the log message of the "amend!" commit and
-opened in an editor so it can be refined. When `git rebase
---autosquash` squashes the "amend!" commit into _<commit>_, the
-log message of _<commit>_ is replaced by the refined log message
-from the "amend!" commit. It is an error for the "amend!" commit's
-log message to be empty unless `--allow-empty-message` is
-specified.
+opened in an editor so it can be refined. The replacement message may
+also be supplied directly using `-m` or `-F`, bypassing the need to open
+an editor. When `git rebase --autosquash` squashes the "amend!" commit
+into _<commit>_, the log message of _<commit>_ is replaced by the
+refined log message from the "amend!" commit. It is an error for the
+"amend!" commit's log message to be empty unless `--allow-empty-message`
+is specified.
 +
 `--fixup=reword:<commit>` is shorthand for `--fixup=amend:<commit>
  --only`. It creates an "amend!" commit with only a log message
diff --git a/builtin/commit.c b/builtin/commit.c
index 28f6174503..269c2d782b 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -837,21 +837,19 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		hook_arg1 = "message";
 
 		/*
-		 * Only `-m` commit message option is checked here, as
-		 * it supports `--fixup` to append the commit message.
-		 *
-		 * The other commit message options `-c`/`-C`/`-F` are
-		 * incompatible with all the forms of `--fixup` and
-		 * have already errored out while parsing the `git commit`
-		 * options.
+		 * `-m` (and `-F`, converted to `-m` earlier for
+		 * amend/reword) appends the message body here.
+		 * `-c`/`-C` are still incompatible with all forms
+		 * of `--fixup`.
 		 */
 		if (have_option_m && !strcmp(fixup_prefix, "fixup"))
 			strbuf_addbuf(&sb, &message);
 
 		if (!strcmp(fixup_prefix, "amend")) {
 			if (have_option_m)
-				die(_("options '%s' and '%s:%s' cannot be used together"), "-m", "--fixup", fixup_message);
-			prepare_amend_commit(commit, &sb, &ctx);
+				strbuf_addbuf(&sb, &message);
+			else
+				prepare_amend_commit(commit, &sb, &ctx);
 		}
 	} else if (!stat(git_path_merge_msg(the_repository), &statbuf)) {
 		size_t merge_msg_start;
@@ -1338,10 +1336,12 @@ static int parse_and_validate_options(int argc, const char *argv[],
 	}
 	if (fixup_message && squash_message)
 		die(_("options '%s' and '%s' cannot be used together"), "--squash", "--fixup");
-	die_for_incompatible_opt4(!!use_message, "-C",
+	die_for_incompatible_opt3(!!use_message, "-C",
 				  !!edit_message, "-c",
-				  !!logfile, "-F",
 				  !!fixup_message, "--fixup");
+	die_for_incompatible_opt3(!!use_message, "-C",
+				  !!edit_message, "-c",
+				  !!logfile, "-F");
 	die_for_incompatible_opt4(have_option_m, "-m",
 				  !!edit_message, "-c",
 				  !!use_message, "-C",
@@ -1410,6 +1410,9 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		}
 	}
 
+	if (logfile && fixup_message && !strcmp(fixup_prefix, "fixup"))
+		die(_("options '%s' and '%s' cannot be used together"), "-F", "--fixup");
+
 	if (0 <= edit_flag)
 		use_editor = edit_flag;
 
@@ -1821,6 +1824,22 @@ int cmd_commit(int argc,
 	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
 					  builtin_commit_usage,
 					  prefix, current_head, &s);
+
+	if (logfile && fixup_message && !strcmp(fixup_prefix, "amend")) {
+		if (!strcmp(logfile, "-")) {
+			if (isatty(0))
+				fprintf(stderr, _("(reading log message from standard input)\n"));
+			if (strbuf_read(&message, 0, 0) < 0)
+				die_errno(_("could not read log from standard input"));
+		} else {
+			if (strbuf_read_file(&message, logfile, 0) < 0)
+				die_errno(_("could not read log file '%s'"), logfile);
+		}
+		strbuf_complete_line(&message);
+		have_option_m = 1;
+		FREE_AND_NULL(logfile);
+	}
+
 	if (trailer_args.nr)
 		trailer_config_init();
 
diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh
index 66aff8e097..b7579ad789 100755
--- a/t/t7500-commit-template-squash-signoff.sh
+++ b/t/t7500-commit-template-squash-signoff.sh
@@ -384,18 +384,28 @@ test_expect_success '--fixup=reword: ignores staged changes' '
 	test_cmp foo actual
 '
 
-test_expect_success '--fixup=reword: error out with -m option' '
+test_expect_success '--fixup=amend: with -m option' '
 	commit_for_rebase_autosquash_setup &&
-	echo "fatal: options '\''-m'\'' and '\''--fixup:reword'\'' cannot be used together" >expect &&
-	test_must_fail git commit --fixup=reword:HEAD~ -m "reword commit message" 2>actual &&
-	test_cmp expect actual
+	cat >expected <<-EOF &&
+	amend! $(git log -1 --format=%s HEAD~)
+
+	amend commit message
+	EOF
+	git commit --fixup=amend:HEAD~ -m "amend commit message" &&
+	get_commit_msg HEAD >actual &&
+	test_cmp expected actual
 '
 
-test_expect_success '--fixup=amend: error out with -m option' '
+test_expect_success '--fixup=reword: with -m option' '
 	commit_for_rebase_autosquash_setup &&
-	echo "fatal: options '\''-m'\'' and '\''--fixup:amend'\'' cannot be used together" >expect &&
-	test_must_fail git commit --fixup=amend:HEAD~ -m "amend commit message" 2>actual &&
-	test_cmp expect actual
+	cat >expected <<-EOF &&
+	amend! $(git log -1 --format=%s HEAD~)
+
+	reword commit message
+	EOF
+	git commit --fixup=reword:HEAD~ -m "reword commit message" &&
+	get_commit_msg HEAD >actual &&
+	test_cmp expected actual
 '
 
 test_expect_success 'consecutive amend! commits remove amend! line from commit msg body' '
@@ -432,6 +442,12 @@ test_expect_success 'deny to create amend! commit if its commit msg body is empt
 	test_cmp expected actual
 '
 
+test_expect_success '--fixup=amend: -m with empty message aborts' '
+	commit_for_rebase_autosquash_setup &&
+	test_must_fail git commit --fixup=amend:HEAD~ -m "" 2>err &&
+	test_grep "empty commit message body" err
+'
+
 test_expect_success 'amend! commit allows empty commit msg body with --allow-empty-message' '
 	commit_for_rebase_autosquash_setup &&
 	cat >expected <<-EOF &&
@@ -468,10 +484,37 @@ test_expect_success '--fixup=reword: give error with pathsec' '
 	test_cmp expect actual
 '
 
-test_expect_success '--fixup=reword: -F give error message' '
-	echo "fatal: options '\''-F'\'' and '\''--fixup'\'' cannot be used together" >expect &&
-	test_must_fail git commit --fixup=reword:HEAD~ -F msg  2>actual &&
-	test_cmp expect actual
+test_expect_success '--fixup=reword: with -F option' '
+	commit_for_rebase_autosquash_setup &&
+	echo "message from file" >msgfile &&
+	cat >expected <<-EOF &&
+	amend! $(git log -1 --format=%s HEAD~)
+
+	message from file
+	EOF
+	git commit --fixup=reword:HEAD~ -F msgfile &&
+	get_commit_msg HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '--fixup=amend: with -F option' '
+	commit_for_rebase_autosquash_setup &&
+	echo "amend message from file" >msgfile &&
+	cat >expected <<-EOF &&
+	amend! $(git log -1 --format=%s HEAD~)
+
+	amend message from file
+	EOF
+	git commit --fixup=amend:HEAD~ -F msgfile &&
+	get_commit_msg HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '-F with plain --fixup still errors' '
+	commit_for_rebase_autosquash_setup &&
+	echo "message" >msgfile &&
+	test_must_fail git commit --fixup HEAD~ -F msgfile 2>err &&
+	test_grep "cannot be used together" err
 '
 
 test_expect_success 'commit --squash works with -F' '
-- 
2.54.0.772.g683d7313b1


^ permalink raw reply related

* Re: [PATCH v2 2/3] builtin/log: prefetch necessary blobs for `git cherry`
From: Derrick Stolee @ 2026-05-18 12:14 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Elijah Newren via GitGitGadget, git
In-Reply-To: <CABPp-BGpXgDfJeDEB91U-h092-8L6Q_MLrzSLFg9HotPDZ-m-g@mail.gmail.com>

On 5/13/2026 7:17 PM, Elijah Newren wrote:
> On Mon, Apr 27, 2026 at 6:17 AM Derrick Stolee <stolee@gmail.com> wrote:
>>
>> On 4/17/2026 8:32 PM, Elijah Newren via GitGitGadget wrote:
>>> From: Elijah Newren <newren@gmail.com>
>>> +static void collect_diff_blob_oids(struct commit *commit,
>>> +                                struct diff_options *opts,
>>> +                                struct oidset *blobs)
>>
>> I think that this is generally a good idea, though I worry that
>> having this hidden in builtin/log.c may not be the right long-
>> term home.
>>
>> I expect that we'll find more and more examples where we want to
>> prefetch blobs in different operations, those that exist now and
>> those that may be created in the future. It would be preferred if
>> they could automatically take advantage of the logic already in
>> diff_queued_diff_prefetch() within diffcore_std() in diff.c.
>>
>> Ultimately, _this_ patch cares about a diff.
> 
> I read this patch a bit differently -- could you say more about what
> you have in mind?
> 
> The body of collect_diff_blob_oids() really is just diff_tree_oid() +
> diffcore_std() + process each pair, so at the per-commit level I am
> already leaning on the diff library.  One of the things this patch
> adds is accumulation across many commits: the containing loop (in
> prefetch_cherry_blobs) is over a commit range, not over a single diff.
> 
> Concretely, the motivating case was a patch touching a few files where
> upstream had tens of thousands of commits in <limit>..<head>, several
> hundred of which modified the same set of files.  A per-diff prefetch
> like diff.c uses would turn that into hundreds of small fetches of 1-3
> blobs each; what this series gives you is one fetch.  So the win
> really does live above the diff library, not inside it.

My initial thought was about finding what we can abstract into the
diff API for later reuse. Upon rereading, it's clear that this is
tied very closely with the --cherry feature and wouldn't make a lot
of sense in the API layer.

> There are two further wrinkles in cherry that are filters layered on
> top of the cross-commit accumulation, and they're cherry-specific in a
> way that I don't think belongs in the diff library:
> 
>    1. For most commits in <limit>..<head>, cherry doesn't care about
> the diff at all -- if the list of files modified doesn't exactly match
> the commit of interest, the commit is skipped before patch-id is even
> computed.  Prefetching for those would be wasted.
> 
>    2. We skip prefetching content for binary files (because patch-id
> uses oid_to_hex() for such files instead of the diff contents).
> 
>> Could we compute a
>> "diff prep" computation using the core diff library instead of
>> inventing a second queue of results for diffing?
> 
> To check this concretely I looked at each of the existing
> promisor_remote_get_direct() callsites for a similar producer.  The
> closest cousin of collect_diff_blob_oids() (the only part of this
> patch that looks like it might be close to the right shape to put in a
> core diff library) is diff.c's diff_queued_diff_prefetch() -- but it
> operates on the already-populated global diff_queued_diff and fetches
> immediately, rather than setting up the diff itself and returning an
> oidset for the caller to accumulate.  Reshaping it to match cherry's
> needs would either break its current caller in diffcore_std() or
> introduce a parallel function whose only consumer is cherry.  None of
> the other sites (path-walk in backfill, index walk in read-cache,
> three-way state in merge-ort, etc.) do anything resembling "diff two
> trees and harvest oids."
> 
> And even if we did factor a helper out, cherry's filter is
> patch-id-specific: commit_patch_id() substitutes oid_to_hex() for
> files marked binary by their userdiff driver, so we deliberately skip
> prefetching those.  That isn't a generic "diff prep" consideration --
> it only makes sense because the caller is patch-id.  We could express
> it as a predicate parameter, but with one caller that would feel to me
> like it's just pushing cherry's policy across an API boundary for no
> gain.

Thanks for the additional context. I agree with your assessment.

>> Patch 3 cares about a "scan prep" which cares about loading all
>> blobs for a given tree with respect to a pathspec. This is very
>> similar to what a checkout would do, though it ultimately uses
>> a form of diff to find out what change should be applied to the
>> working directory. Perhaps 'git archive' is a better matching
>> example.
> 
> Agreed that archive is the closer analog -- both grep and archive do a
> pathspec-filtered single-tree walk, whereas checkout's prefetch is
> tied to the index and optimizes to the subset of paths that are
> different since the previous version checked out.  Retrofitting that
> to grep would mean materializing an index for the target revision just
> to throw it away, which feels like more machinery to bridge the
> abstractions than the walk itself would take.

Makes sense.

>> By implementing things in a
>> common location, then we can have later integrations add to the
>> confidence in the feature through tests covering each user-facing
>> use.
> 
> Sounds great...but what common user-facing uses exist?
> 
> Looking at the existing 11 callsites of promisor_remote_get_direct()
> after this series [1], each has pretty specialized data needs --
> index-driven (read-cache), index-pack & pack-objects internals,
> path-walk batches (backfill), merge-ort's three-way logic,
> diffcore-rename's two independent rename-detection paths, plain old
> diffs, collection across a subset of commits (cherry),
> pathspec-filtered tree walk (grep), and
> on-demand-single-blob-at-a-time (odb.c) -- so I don't see a natural
> shared layer above the primitive itself (which is already
> promisor_remote_get_direct).
> 
> archive, if it had prefetch logic, would be the first match.  But it's
> not clear where the shared logic between grep and archive would live,
> if archive even had any prefetch logic to share.
> 
> So I'm inclined to leave both new producers local to their builtins
> for now, and factor a tree-walk helper when archive (or a third
> caller) actually wants one.  But I'm happy to be told I've missed the
> boat.

No, clearly I missed the boat. Thanks for giving me insight to your
deep understanding of this area. You executed on a good design based
on the right amount of specialization required for this need.

Thanks,
-Stolee



^ permalink raw reply

* Re: [PATCH v3 0/4] Batch prefetching
From: Derrick Stolee @ 2026-05-18 12:17 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget, git; +Cc: Elijah Newren, Phillip Wood
In-Reply-To: <pull.2089.v3.git.1778775928.gitgitgadget@gmail.com>

On 5/14/2026 12:25 PM, Elijah Newren via GitGitGadget wrote:
> Changes since v2:
> 
>  * Modified the final patch as suggested by Stolee to include pathspec usage
>    in the testcase
>  * Modified the last two patches to not re-download blobs we already have
>    locally, and adjusted the tests to verify
>  * Inserted a new first patch, containing a documentation addition that
>    would have helped me avoid making the above mistake in the first place.

Thank you for these changes. I reviewed the updates and documentation and
think this version is good to go. 
> Note: Stolee also suggest some code sharing or code movement in his review
> of v2 2/3, but possibly based on a misunderstanding of v2 2/3 (that patch
> isn't about a diff) and it's not clear to me what could be shared or moved,
> so that's not part of this round.

Your detailed responses in the v2 thread helped me understand that my thought
was misguided. Thanks for giving me extra confidence in your approach here.

-Stolee


^ permalink raw reply

* Re: [PATCH] merge: use repo_in_merge_bases for octopus up-to-date check
From: Derrick Stolee @ 2026-05-18 12:20 UTC (permalink / raw)
  To: Kristofer Karlsson via GitGitGadget, git; +Cc: Kristofer Karlsson
In-Reply-To: <pull.2110.git.1778566286543.gitgitgadget@gmail.com>

On 5/12/2026 2:11 AM, Kristofer Karlsson via GitGitGadget wrote:
> From: Kristofer Karlsson <krka@spotify.com>
> 
> The octopus merge path checks whether each remote head is already
> an ancestor of HEAD by computing all merge-bases via
> repo_get_merge_bases() and comparing the first result's OID to
> the remote head.  This is more expensive than necessary:
> repo_get_merge_bases() calls paint_down_to_common() with
> min_generation=0, performs the full STALE drain, and may run
> remove_redundant(), when all we need is a yes/no reachability
> answer.
> 
> Replace this with repo_in_merge_bases(), which answers the
> is-ancestor question directly.  When generation numbers are
> available, repo_in_merge_bases() uses can_all_from_reach() -- a
> DFS bounded by generation number that stops as soon as the target
> is found or ruled out, without entering paint_down_to_common() at
> all.  Without generation numbers, it still benefits from a tighter
> min_generation floor.
> 
> Signed-off-by: Kristofer Karlsson <krka@spotify.com>
> ---
>     merge: use repo_in_merge_bases for octopus up-to-date check
>     
>     While reviewing callers of repo_get_merge_bases() for a different patch,
>     I noticed the octopus up-to-date loop in builtin/merge.c computes full
>     merge-bases only to check whether each remote head is an ancestor of
>     HEAD.
>     
>     The existing code calls repo_get_merge_bases(), takes the first result,
>     frees the list, and compares the OID to the remote head. This is
>     equivalent to an is-ancestor check, which repo_in_merge_bases() answers
>     directly.
>     
>     Using repo_in_merge_bases() simplifies the code (-14/+4 lines) and
>     avoids unnecessary work: with generation numbers it uses
>     can_all_from_reach() instead of paint_down_to_common(), and without
>     generation numbers it still benefits from a tighter min_generation
>     floor. In practice this only matters for octopus merges on repos with
>     deep history, so the main value here is the simplification.

The code change looks right to me. Do you have any performance numbers
to share? Or was this motivated mostly as an opportunity for code cleanup?

Thanks,
-Stolee



^ permalink raw reply

* Re: [PATCH 1/1] commit: allow -m/-F with --fixup=amend: or reword:
From: Junio C Hamano @ 2026-05-18 12:39 UTC (permalink / raw)
  To: erik; +Cc: git, charvi077
In-Reply-To: <20260518112225.73172-4-erik@cervined.in>

erik@cervined.in writes:

> From: Erik Cervin-Edin <erik@cervined.in>

The name on this overriding in-body From: line, and the name on
Signed-off-by: line below, must match.  Please pick a name with or
without hyphen and stick to it.

> --fixup=amend: and --fixup=reword: require an editor to supply the
> replacement commit message. The -m and -F flags are rejected: -m is
> caught by a die() in prepare_to_commit(), and -F is caught by
> die_for_incompatible_opt4() which groups -F with --fixup as mutually
> exclusive. This makes these modes unusable in non-interactive
> workflows -- notably AI coding agents.

"Unusable" may be stronger than reality, as you can make creatie use
of GIT_EDITOR to achieve what you want.  "awkward" or "poorly suited"
would be more fitting.

> Plain --fixup (without amend: or reword:) continues to reject -F but
> still accepts -m (even though it's practically a no-op).

Is it "practically a no-op"?  Wouldn't

   $ git commit --fixup <commit> -m "message body"

be useful to leave a message in the resulting commit, which is later
to be squashed into the named <commit>?  Actually squashing with "fixup!"
may lose the message supplied here, but wouldn't people use this
facility to more easily identify what each of the fixups are about?

For the same reason, "-F" would be just as useful as "-m" in this context,
and it feels a bit inconsistent to allow one while rejecting the other.

> diff --git a/builtin/commit.c b/builtin/commit.c
> index 28f6174503..269c2d782b 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -837,21 +837,19 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  		hook_arg1 = "message";
>  
>  		/*
> -		 * Only `-m` commit message option is checked here, as
> -		 * it supports `--fixup` to append the commit message.
> -		 *
> -		 * The other commit message options `-c`/`-C`/`-F` are
> -		 * incompatible with all the forms of `--fixup` and
> -		 * have already errored out while parsing the `git commit`
> -		 * options.
> +		 * `-m` (and `-F`, converted to `-m` earlier for
> +		 * amend/reword) appends the message body here.
> +		 * `-c`/`-C` are still incompatible with all forms
> +		 * of `--fixup`.
>  		 */
>  		if (have_option_m && !strcmp(fixup_prefix, "fixup"))
>  			strbuf_addbuf(&sb, &message);
>  
>  		if (!strcmp(fixup_prefix, "amend")) {
>  			if (have_option_m)
> -				die(_("options '%s' and '%s:%s' cannot be used together"), "-m", "--fixup", fixup_message);

Good that you got rid of this overly long die() message line.

> -			prepare_amend_commit(commit, &sb, &ctx);
> +				strbuf_addbuf(&sb, &message);
> +			else
> +				prepare_amend_commit(commit, &sb, &ctx);
>  		}
>  	} else if (!stat(git_path_merge_msg(the_repository), &statbuf)) {
>  		size_t merge_msg_start;
> @@ -1338,10 +1336,12 @@ static int parse_and_validate_options(int argc, const char *argv[],
>  	}
>  	if (fixup_message && squash_message)
>  		die(_("options '%s' and '%s' cannot be used together"), "--squash", "--fixup");
> -	die_for_incompatible_opt4(!!use_message, "-C",
> +	die_for_incompatible_opt3(!!use_message, "-C",
>  				  !!edit_message, "-c",
> -				  !!logfile, "-F",
>  				  !!fixup_message, "--fixup");
> +	die_for_incompatible_opt3(!!use_message, "-C",
> +				  !!edit_message, "-c",
> +				  !!logfile, "-F");
>  	die_for_incompatible_opt4(have_option_m, "-m",
>  				  !!edit_message, "-c",
>  				  !!use_message, "-C",
> @@ -1410,6 +1410,9 @@ static int parse_and_validate_options(int argc, const char *argv[],
>  		}
>  	}
>  
> +	if (logfile && fixup_message && !strcmp(fixup_prefix, "fixup"))
> +		die(_("options '%s' and '%s' cannot be used together"), "-F", "--fixup");
> +
>  	if (0 <= edit_flag)
>  		use_editor = edit_flag;
>  
> @@ -1821,6 +1824,22 @@ int cmd_commit(int argc,
>  	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
>  					  builtin_commit_usage,
>  					  prefix, current_head, &s);
> +
> +	if (logfile && fixup_message && !strcmp(fixup_prefix, "amend")) {
> +		if (!strcmp(logfile, "-")) {
> +			if (isatty(0))
> +				fprintf(stderr, _("(reading log message from standard input)\n"));
> +			if (strbuf_read(&message, 0, 0) < 0)
> +				die_errno(_("could not read log from standard input"));
> +		} else {
> +			if (strbuf_read_file(&message, logfile, 0) < 0)
> +				die_errno(_("could not read log file '%s'"), logfile);
> +		}
> +		strbuf_complete_line(&message);
> +		have_option_m = 1;
> +		FREE_AND_NULL(logfile);
> +	}
> +

It is curious that for this new feature alone, but not the other
existing code paths, "-m" and "-F" options reads from file in the
new code here, instead of letting the existing code for "-F" to read
(which happens inside prepare_to_commit(), I presume?).

A potential problem of the above code is if we find something wrong
in message and complain later in the control flow, we have long lost
where the message came from, as the point of the above code is
exactly to pretend that "--fixup:amend/reword -F" message did *not*
come from a file with the "-F" option, but from the command line via
the "-m" option.

> +test_expect_success '--fixup=amend: with -m option' '
>  	commit_for_rebase_autosquash_setup &&
> -	echo "fatal: options '\''-m'\'' and '\''--fixup:reword'\'' cannot be used together" >expect &&
> -	test_must_fail git commit --fixup=reword:HEAD~ -m "reword commit message" 2>actual &&
> -	test_cmp expect actual
> +	cat >expected <<-EOF &&

This comment is not about the added logic, but I notice that among
86 hits with string "expect" in this file in today's "master", only
14 hits are with string "expected", i.e., the prevalent name for the
"golden copy result" that is compared with the actula result (called
"actual") is "expect", not "expected".  Please do not make the
situation worse.

> -	test_cmp expect actual
> +	cat >expected <<-EOF &&

Ditto.

^ permalink raw reply

* Re: [PATCH v3 0/4] Batch prefetching
From: Junio C Hamano @ 2026-05-18 12:44 UTC (permalink / raw)
  To: Derrick Stolee
  Cc: Elijah Newren via GitGitGadget, git, Elijah Newren, Phillip Wood
In-Reply-To: <0da4f159-8d4b-49e2-93c1-25aa0bf69371@gmail.com>

Derrick Stolee <stolee@gmail.com> writes:

> On 5/14/2026 12:25 PM, Elijah Newren via GitGitGadget wrote:
>> Changes since v2:
>> 
>>  * Modified the final patch as suggested by Stolee to include pathspec usage
>>    in the testcase
>>  * Modified the last two patches to not re-download blobs we already have
>>    locally, and adjusted the tests to verify
>>  * Inserted a new first patch, containing a documentation addition that
>>    would have helped me avoid making the above mistake in the first place.
>
> Thank you for these changes. I reviewed the updates and documentation and
> think this version is good to go. 
>> Note: Stolee also suggest some code sharing or code movement in his review
>> of v2 2/3, but possibly based on a misunderstanding of v2 2/3 (that patch
>> isn't about a diff) and it's not clear to me what could be shared or moved,
>> so that's not part of this round.
>
> Your detailed responses in the v2 thread helped me understand that my thought
> was misguided. Thanks for giving me extra confidence in your approach here.

Thanks, both.  I do agree that the series is in a good shape.  Let
me mark the topic for 'next'.

^ permalink raw reply

* Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root
From: Pablo Sabater @ 2026-05-18 13:26 UTC (permalink / raw)
  To: Chandra Pratap
  Cc: phillip.wood, git, gitster, christian.couder, karthik.188,
	jltobler, ayu.chandekar, siddharthasthana31
In-Reply-To: <CA+J6zkTGgeNuH0eusTy+t8LO3bjygSz4svJB=K4R5ASmBdd0uQ@mail.gmail.com>

Hi Chandra, Phillip,

> > >
> > > I have mixed feelings about which approach to choose.
> > > The idea of a blank line was thought at
> > > https://lore.kernel.org/git/xmqq8s8vvw9m.fsf@gitster.c.googlers.com/
> > > but Junio argued against it for having an extra row because the
> > > indentation he proposed didn't collapse, however I find indentation +
> > > no collapse the most confusing one.
> > > I'd say that I'm fine with both approaches, blank line or indentation
> > > + collapse.
> >
> > I'm afraid I don't understand this - what does it mean for the
> > indentation to collapse, or not collapse.

Collapsing would be when branches move to the left, eg:

  *
  |\     <- merge
  | *
  |/     <- collapse
  *
> > Looking at the examples Junio
> > gave they look quite nice to me, though I'd find it clearer if
> >
> >
> >   | | *  12345678 2021-01-14 merge xxxxx@xxxx into the history
> >   | | |\
> >   | | | \
> >   | | *  \  23456789 2021-01-12 merge citest into the main history
> >   | | |\  * 5505e019c2 2014-07-09 initial xxxxxx@xxxx
> >   | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest)
> > Added defau
> >   | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from
> > f7daf088)
> >
> > was rendered as
> >
> >
> >   | | *  12345678 2021-01-14 merge xxxxx@xxxx into the history
> >   | | |\
> >   | | | *  5505e019c2 2014-07-09 initial xxxxxx@xxxx
> >   | | *    23456789 2021-01-12 merge citest into the main history
> >   | | |\
> >   | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest)
> > Added defau
> >   | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from
> > f7daf088)
>
> It probably *does* look clearer here, but I have the same reservations
> against this as Junio: the break won't be as noticeable when --graph is
> *not* used with --oneline.
>
> > >>> without the patch:
> > >>>
> > >>>     * A root
> > >>>     * B root
> > >>>     * C root
> > >>>     * D1 child
> > >>>     * D root
> > >>>
> > >>> with the patch, the indentation cascades:
> > >>>
> > >>>     * A root
> > >>>       * B root
> > >>>         * C root
> > >>>           * D1 child
> > >>>        _ /
> > >>>       /
> > >>>      /
> > >>>     * D root
> > >
> > >    * A root
> > >
> > >    * B root
> > >
> > >    * C root
> > >
> > >    * D1 child
> > >
> > >    * D root
> > >
> > > Here I think a blank line looks worse, too much space for just 5
> > > commits and becomes one extra line which if this were like up to 7 or
> > > more parentless commits one after the other would be more noticeable.
> >
> > But there shouldn't be a blank line between D and D1 so the two
> > alternatives take up the same amount of vertical space, the main
> > difference being whether D1 appears next to D
> >
> >      * A root     * A root
> >                     * B root
> >      * B root         * C root
> >                         * D1 child
> >      * C root         _/
> >                     /
> >      * D1 child    /
> >      * D root     * D root
> >
> > Of course if the indentation was smarter it would take up less room and
> > look better than having blank lines
> >
> >      * A root
> >        * B root
> >          * C root
> >      * D1 child
> >      * D root
>
> Right, this would be ideal but that would require too much change to the
> existing graphing logic, and should be its own patch.

For the examples I'll use the term parentless instead of root, as
boundary commits are excluded even if they are roots.
By having is_parentless as a flag in 'git_graph' that every stage can
access we could modify the rendering and maybe completely drop the
commit placeholders, working on it for v4 but currently renders like
this

    * A parentless
      * B parentless
        * C parentless
  * D1 child
  * D parentless

(A has indentation when it could not have, but that would require a
lookahead if the next commit is also parentless)
But definitely a step forward.

Do we want cascading or just a fixed indentation?

    * A parentless
    * B parentless
    * C parentless
  * D1 child
  * D parentless

By being indented it indicates that it is parentless and that the one
below doesn't relate to it, but cascading looks clearer.

>
> > > But there are cases that blank line might be better:
> > >
> > >    * 10_A2
> > >    * 10_A1
> > >    * 10_A
> > >      *   10_M
> > >     /|\
> > >    | | * 10_D
> > >    | * 10_C
> > >    * 10_B
> > >
> > > Feels like a shower of commits instead of an indented merge.
> >
> > Yes, that is a bit confusing. I think the thing I find confusing with
> > this approach is that we're treating the commit rendered below the root
> > commit specially, rather than treating the root commit itself specially.
> > To me it is the root commit that's the odd one out because it does not
> > have any parents, but we treat the commit that's rendered below as the
> > odd one by indenting it relative to its parents.
>
> I guess that would make the examples look something like this:
>
>   * A root
>   * B root
>   * C root
> * D1 child
> * D root
>
> No cascading, and no need for that massive _ / collapse line.
>
> * 10_A2
> * 10_A1
>  \
>   * 10_A
> *   10_M
> | \ \
> | | * 10_D
> | * 10_C
> * 10_B
>
> I say it looks better than the alternatives, but I'm not sure if this will
> be easy to implement. The diagonal connection line (\) will need to
> be printed before printing the actual root commit, which will require
> lookahead logic.
>
> I'd prefer to avoid major surgery on the codebase.

Octopus merges need a pre-commit phase where an additional row
increases the space around a commit with multiple parents to make room
for it.
A new phase can be created similarly to pre-commit as pre-root where
the connection edge (\) can be printed before the indented commit.

So far this is the comparison:

indentation at root:

    * A parentless
  * B1 child
   \
    * B parentless
  * C1 child
  * C parentless

indentation AFTER the root (current v3):

  * A parentless
    * B1 child
   /
  * B parentless
    * C1 child
   /
  * C parentless

Karthik mentioned that by indenting the parentless, we lose the
consistency of having the roots on their real column and now some are
indented and some are not.
The biggest winner having the parentless indented are the merge commits:

  * A child
  * A child
    \
      * A parentless
  *-.   B child
  | \ \
  | |  * C parentless
  | * D parentless
  * E parentless

which IMO looks clearer than the commit shower:

  * A child
  * A child
  * A parentless
    *   B child
   /|\
  | | * C parentless
  | * D parentless
  * E parentless
>
>
> Thanks,
> Chandra.

Regards

--
Pablo

^ permalink raw reply

* Re: [PATCH] merge: use repo_in_merge_bases for octopus up-to-date check
From: Kristofer Karlsson @ 2026-05-18 14:38 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <c5b333f1-0db6-4aec-a369-6503cb924e7f@gmail.com>

Good question! No, this was intended only as a code cleanup and
semantic simplification.
The code path seems like an edge case, so benchmarking it did not seem
worthwhile.
The main win as I see it is clearer semantics for what it's doing and
the optimization is just a bonus.

Checking if repo_get_merge_bases(HEAD, J) == J is better expressed as
repo_in_merge_bases(J, HEAD)

repo_in_merge_bases should probably be renamed to something like
repo_is_ancestor, since its comment says:
Is "commit" an ancestor of (i.e. reachable from) the "reference"?
but I can understand that it may be painful to rename it in practice.

If I do a mental rename, this becomes simpler to reason about:

Checking if repo_get_merge_bases(HEAD, J) == J is better expressed as
repo_is_ancestor(J, HEAD)

- Kristofer

On Mon, 18 May 2026 at 14:20, Derrick Stolee <stolee@gmail.com> wrote:
>
> On 5/12/2026 2:11 AM, Kristofer Karlsson via GitGitGadget wrote:
> > From: Kristofer Karlsson <krka@spotify.com>
> >
> > The octopus merge path checks whether each remote head is already
> > an ancestor of HEAD by computing all merge-bases via
> > repo_get_merge_bases() and comparing the first result's OID to
> > the remote head.  This is more expensive than necessary:
> > repo_get_merge_bases() calls paint_down_to_common() with
> > min_generation=0, performs the full STALE drain, and may run
> > remove_redundant(), when all we need is a yes/no reachability
> > answer.
> >
> > Replace this with repo_in_merge_bases(), which answers the
> > is-ancestor question directly.  When generation numbers are
> > available, repo_in_merge_bases() uses can_all_from_reach() -- a
> > DFS bounded by generation number that stops as soon as the target
> > is found or ruled out, without entering paint_down_to_common() at
> > all.  Without generation numbers, it still benefits from a tighter
> > min_generation floor.
> >
> > Signed-off-by: Kristofer Karlsson <krka@spotify.com>
> > ---
> >     merge: use repo_in_merge_bases for octopus up-to-date check
> >
> >     While reviewing callers of repo_get_merge_bases() for a different patch,
> >     I noticed the octopus up-to-date loop in builtin/merge.c computes full
> >     merge-bases only to check whether each remote head is an ancestor of
> >     HEAD.
> >
> >     The existing code calls repo_get_merge_bases(), takes the first result,
> >     frees the list, and compares the OID to the remote head. This is
> >     equivalent to an is-ancestor check, which repo_in_merge_bases() answers
> >     directly.
> >
> >     Using repo_in_merge_bases() simplifies the code (-14/+4 lines) and
> >     avoids unnecessary work: with generation numbers it uses
> >     can_all_from_reach() instead of paint_down_to_common(), and without
> >     generation numbers it still benefits from a tighter min_generation
> >     floor. In practice this only matters for octopus merges on repos with
> >     deep history, so the main value here is the simplification.
>
> The code change looks right to me. Do you have any performance numbers
> to share? Or was this motivated mostly as an opportunity for code cleanup?
>
> Thanks,
> -Stolee
>
>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox