* [PATCH v3 06/18] setup: stop using `the_repository` in `verify_filename()`
From: Patrick Steinhardt @ 2026-05-19 9:52 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260519-pks-setup-wo-the-repository-v3-0-a00d8ea8b07f@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 v3 07/18] setup: stop using `the_repository` in `verify_non_filename()`
From: Patrick Steinhardt @ 2026-05-19 9:52 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260519-pks-setup-wo-the-repository-v3-0-a00d8ea8b07f@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 v3 08/18] setup: stop using `the_repository` in `enter_repo()`
From: Patrick Steinhardt @ 2026-05-19 9:52 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260519-pks-setup-wo-the-repository-v3-0-a00d8ea8b07f@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 v3 09/18] setup: stop using `the_repository` in `setup_work_tree()`
From: Patrick Steinhardt @ 2026-05-19 9:52 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260519-pks-setup-wo-the-repository-v3-0-a00d8ea8b07f@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 v3 10/18] setup: stop using `the_repository` in `set_git_work_tree()`
From: Patrick Steinhardt @ 2026-05-19 9:52 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260519-pks-setup-wo-the-repository-v3-0-a00d8ea8b07f@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 v3 11/18] setup: stop using `the_repository` in `setup_git_env()`
From: Patrick Steinhardt @ 2026-05-19 9:52 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260519-pks-setup-wo-the-repository-v3-0-a00d8ea8b07f@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 v3 12/18] setup: stop using `the_repository` in `setup_git_directory_gently()`
From: Patrick Steinhardt @ 2026-05-19 9:52 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260519-pks-setup-wo-the-repository-v3-0-a00d8ea8b07f@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 v3 14/18] setup: stop using `the_repository` in `upgrade_repository_format()`
From: Patrick Steinhardt @ 2026-05-19 9:52 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260519-pks-setup-wo-the-repository-v3-0-a00d8ea8b07f@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 v3 13/18] setup: stop using `the_repository` in `setup_git_directory()`
From: Patrick Steinhardt @ 2026-05-19 9:52 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260519-pks-setup-wo-the-repository-v3-0-a00d8ea8b07f@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 v3 15/18] setup: stop using `the_repository` in `check_repository_format()`
From: Patrick Steinhardt @ 2026-05-19 9:52 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260519-pks-setup-wo-the-repository-v3-0-a00d8ea8b07f@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 v3 16/18] setup: stop using `the_repository` in `initialize_repository_version()`
From: Patrick Steinhardt @ 2026-05-19 9:52 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260519-pks-setup-wo-the-repository-v3-0-a00d8ea8b07f@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 v3 17/18] setup: stop using `the_repository` in `create_reference_database()`
From: Patrick Steinhardt @ 2026-05-19 9:52 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260519-pks-setup-wo-the-repository-v3-0-a00d8ea8b07f@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 v3 18/18] setup: stop using `the_repository` in `init_db()`
From: Patrick Steinhardt @ 2026-05-19 9:52 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Elijah Newren, Junio C Hamano, Tian Yuchen
In-Reply-To: <20260519-pks-setup-wo-the-repository-v3-0-a00d8ea8b07f@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: [PATCH v11] checkout: extend --track with a "fetch" mode to refresh start-point
From: Junio C Hamano @ 2026-05-19 10:34 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Ramsay Jones, D. Ben Knoble, Kristoffer Haugsbakk,
Marc Branchaud, Phillip Wood, Harald Nordgren
In-Reply-To: <pull.2281.v11.git.git.1779177508772.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> checkout: --track=fetch
>
> * Find the right remote by checking which remote's fetch refspec maps
> to the user's start-point, instead of assuming the start-point begins
> with the remote's name. This fixes cases where the user has a custom
> refspec mapping into a namespace whose name differs from the remote
> (e.g. fetching from origin into refs/remotes/upstream/*).
This comment is even before looking at the patch text. After
getting one issue pointed out, I'd expect you to think about related
issues before sending a new round out.
One. Have you considered the case where the remote-tracking refs
are overlapping, e.g., where "origin" and "upstream" point at
different URLs but they both store in "refs/remotes/upstream/*"?
Perhaps their URLs may textually be different but are pointing
logically at the same place (e.g., one ssh:// the other https:// for
example).
What should happen? What does happen after you apply this patch?
> * For a bare namespace name, follow <namespace>/HEAD first to figure
> out which branch to fetch.
What should happen if HEAD does not exist? What does happen after
you apply this patch?
Thanks.
^ permalink raw reply
* Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root
From: Chandra Pratap @ 2026-05-19 10:39 UTC (permalink / raw)
To: Pablo Sabater
Cc: phillip.wood, git, gitster, christian.couder, karthik.188,
jltobler, ayu.chandekar, siddharthasthana31
In-Reply-To: <CAN5EUNQoKRqt3FGLmzRGpPU1nO5jCAogP8Wm9gBZXuPbMNbQAw@mail.gmail.com>
On Mon, 18 May 2026 at 18:57, Pablo Sabater <pabloosabaterr@gmail.com> wrote:
>
> 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.
Agreed, let's keep the cascading.
> >
> > > > 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
I guess we're deciding between cleaner root (parentless) commits and
cleaner merge commits. I favour merge commits because they appear
much more frequently in most codebases.
> >
> >
> > Thanks,
> > Chandra.
>
> Regards
>
> --
> Pablo
^ permalink raw reply
* Re: [PATCH v7] revision.c: implement --max-count-oldest
From: Mirko Faina @ 2026-05-19 10:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jean-Noël Avila, Patrick Steinhardt,
Tian Yuchen, Ben Knoble, Johannes Sixt, Chris Torek, Mirko Faina
In-Reply-To: <xmqqse7n7w2i.fsf@gitster.g>
On Tue, May 19, 2026 at 03:27:49PM +0900, Junio C Hamano wrote:
> Mirko Faina <mroik@delayed.space> writes:
>
> > + * graph_update() as it doesn't do the actualy printing, we'd
>
> "actually"?
Whoops, it should be "actual".
Thank you
^ permalink raw reply
* [PATCH] stash: reuse cached index entries in --patch temporary index
From: Adam Johnson via GitGitGadget @ 2026-05-19 12:43 UTC (permalink / raw)
To: git
Cc: Thomas Gummerer, Elijah Newren, Phillip Wood, Victoria Dye,
Adam Johnson, Adam Johnson
From: Adam Johnson <me@adamj.eu>
`git stash -p` prepares the interactive selection by creating a
temporary index at HEAD, switching `GIT_INDEX_FILE` to it, and then
running the `add -p` machinery.
That temporary index was created by running `git read-tree HEAD`. The
resulting index had no useful cached stat data or fsmonitor-valid bits
from the real index. When `run_add_p()` refreshed that temporary index
before showing the first prompt, it could end up lstat(2)-ing every
tracked file, even in a repository where `git diff` and `git restore -p`
can use fsmonitor to avoid that work.
Create the temporary index in-process instead. Use `unpack_trees()` to
reset the real index contents to HEAD while writing the result to the
temporary index path. For paths whose index entries already match HEAD,
`oneway_merge()` reuses the existing cache entries, preserving their
cached stat data and `CE_FSMONITOR_VALID` state.
This makes the refresh performed by `run_add_p()` behave like the one
used by `git restore -p`: unchanged paths can be skipped via fsmonitor
instead of being scanned again.
In a 206k file repository with `core.fsmonitor` enabled and a one-line
change in one file, time to first prompt dropped from 34.774 seconds to
0.659 seconds.
Signed-off-by: Adam Johnson <me@adamj.eu>
---
stash: reuse cached index entries in --patch temporary index
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2306%2Fadamchainz%2Faj%2Foptimize-stash-patch-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2306/adamchainz/aj/optimize-stash-patch-v1
Pull-Request: https://github.com/git/git/pull/2306
builtin/stash.c | 71 ++++++++++++++++++++++++++++++++++++++----
t/t3904-stash-patch.sh | 18 +++++++++++
2 files changed, 83 insertions(+), 6 deletions(-)
diff --git a/builtin/stash.c b/builtin/stash.c
index 32dbc97b47..48189cb9f7 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -372,6 +372,57 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)
return 0;
}
+static int create_index_from_tree(const struct object_id *tree_id,
+ const char *index_path)
+{
+ int nr_trees = 1;
+ int ret = 0;
+ struct unpack_trees_options opts;
+ struct tree_desc t[MAX_UNPACK_TREES];
+ struct tree *tree;
+ struct index_state dst_istate = INDEX_STATE_INIT(the_repository);
+ struct lock_file lock_file = LOCK_INIT;
+
+ repo_read_index_preload(the_repository, NULL, 0);
+ if (refresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL, NULL))
+ return -1;
+
+ hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR);
+
+ memset(&opts, 0, sizeof(opts));
+
+ tree = repo_parse_tree_indirect(the_repository, tree_id);
+ if (!tree || repo_parse_tree(the_repository, tree)) {
+ ret = -1;
+ goto done;
+ }
+
+ init_tree_desc(t, &tree->object.oid, tree->buffer, tree->size);
+
+ opts.head_idx = 1;
+ opts.src_index = the_repository->index;
+ opts.dst_index = &dst_istate;
+ opts.merge = 1;
+ opts.reset = UNPACK_RESET_PROTECT_UNTRACKED;
+ opts.fn = oneway_merge;
+
+ if (unpack_trees(nr_trees, t, &opts)) {
+ ret = -1;
+ goto done;
+ }
+
+ if (write_locked_index(&dst_istate, &lock_file, COMMIT_LOCK)) {
+ ret = error(_("unable to write new index file"));
+ goto done;
+ }
+
+done:
+ release_index(&dst_istate);
+ if (ret)
+ rollback_lock_file(&lock_file);
+ return ret;
+}
+
static int diff_tree_binary(struct strbuf *out, struct object_id *w_commit)
{
struct child_process cp = CHILD_PROCESS_INIT;
@@ -1321,18 +1372,26 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,
struct interactive_options *interactive_opts)
{
int ret = 0;
- struct child_process cp_read_tree = CHILD_PROCESS_INIT;
struct child_process cp_diff_tree = CHILD_PROCESS_INIT;
+ struct commit *head_commit;
+ const struct object_id *head_tree;
struct index_state istate = INDEX_STATE_INIT(the_repository);
char *old_index_env = NULL, *old_repo_index_file;
remove_path(stash_index_path.buf);
- cp_read_tree.git_cmd = 1;
- strvec_pushl(&cp_read_tree.args, "read-tree", "HEAD", NULL);
- strvec_pushf(&cp_read_tree.env, "GIT_INDEX_FILE=%s",
- stash_index_path.buf);
- if (run_command(&cp_read_tree)) {
+ head_commit = lookup_commit(the_repository, &info->b_commit);
+ if (!head_commit || repo_parse_commit(the_repository, head_commit)) {
+ ret = -1;
+ goto done;
+ }
+ head_tree = get_commit_tree_oid(head_commit);
+ if (!head_tree) {
+ ret = -1;
+ goto done;
+ }
+
+ if (create_index_from_tree(head_tree, stash_index_path.buf)) {
ret = -1;
goto done;
}
diff --git a/t/t3904-stash-patch.sh b/t/t3904-stash-patch.sh
index 90a4ff2c10..4b3241c8cd 100755
--- a/t/t3904-stash-patch.sh
+++ b/t/t3904-stash-patch.sh
@@ -84,6 +84,24 @@ test_expect_success 'none of this moved HEAD' '
verify_saved_head
'
+test_expect_success 'stash -p with unmodified tracked files present' '
+ git reset --hard &&
+ echo line1 >alpha &&
+ echo line1 >beta &&
+ git add alpha beta &&
+ git commit -m "add alpha and beta" &&
+ echo line2 >>alpha &&
+ echo y | git stash -p &&
+ echo line1 >expect &&
+ test_cmp expect alpha &&
+ test_cmp expect beta &&
+ git stash pop &&
+ printf "line1\nline2\n" >expect &&
+ test_cmp expect alpha &&
+ echo line1 >expect &&
+ test_cmp expect beta
+'
+
test_expect_success 'stash -p with split hunk' '
git reset --hard &&
cat >test <<-\EOF &&
base-commit: 7bcaabddcf68bd0702697da5904c3b68c52f94cf
--
gitgitgadget
^ permalink raw reply related
* [PATCH] http: fix memory leak in fetch_and_setup_pack_index()
From: LorenzoPegorari @ 2026-05-19 14:54 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Patrick Steinhardt, fox, Jeff King
Inside the function `fetch_and_setup_pack_index()`, when the pack
obtained using `fetch_pack_index()` fails to be verified by
`parse_pack_index()`, the function returns without closing and freeing
said pack.
Fix this by calling `close_pack_index()` to munmap the index file for
the leaking pack (which might have been mmapped by `fetch_pack_index()`
or `verify_pack_index()`), and then free it.
Signed-off-by: LorenzoPegorari <lorenzo.pegorari2002@gmail.com>
---
http.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/http.c b/http.c
index 67c9c6fc60..c28be26ad9 100644
--- a/http.c
+++ b/http.c
@@ -2545,11 +2545,13 @@ static int fetch_and_setup_pack_index(struct packfile_list *packs,
}
ret = verify_pack_index(new_pack);
- if (!ret)
- close_pack_index(new_pack);
+
+ close_pack_index(new_pack);
free(tmp_idx);
- if (ret)
+ if (ret) {
+ free(new_pack);
return -1;
+ }
packfile_list_prepend(packs, new_pack);
return 0;
--
2.54.0.dirty
^ permalink raw reply related
* Re: [PATCH v5 0/8] fetch: rework negotiation tip options
From: Derrick Stolee @ 2026-05-19 15:08 UTC (permalink / raw)
To: Matthew John Cheetham, Derrick Stolee via GitGitGadget, git; +Cc: gitster, ps
In-Reply-To: <VI0PR03MB11634DBCD7ED97E3358AC9339C0002@VI0PR03MB11634.eurprd03.prod.outlook.com>
On 5/19/2026 5:13 AM, Matthew John Cheetham wrote:
> On 2026-05-18 21:19, Derrick Stolee via GitGitGadget wrote:
>
>> Range-diff vs v4:
>>
>> 1: 7409a479d6 ! 1: 538913a327 t5516: fix test order flakiness
>> @@ Commit message
>> Use 'sort -k 3' to match the actual number of columns in the output.
>> + Reviewed-by: Matthew John Cheetham <mcheetham@outlook.com>
>> Signed-off-by: Derrick Stolee <stolee@gmail.com>
>
> mjcheetham@outlook.com, not mcheetham@outlook.com.
OH NO! Sorry for this typo that I copied into every commit.
> LGTM
>
> Thanks,
> Matthew
I'll fix this reviewed-by tag and resend. Sorry :(
Thanks for the review!
^ permalink raw reply
* Re: [GSoC PATCH v6 0/6] preserve promisor files content after repack
From: Lorenzo Pegorari @ 2026-05-19 15:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Christian Couder, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Tian Yuchen, Eric Sunshine, Elijah Newren
In-Reply-To: <xmqqse7xm8av.fsf@gitster.g>
On Tue, May 12, 2026 at 03:49:28PM +0900, Junio C Hamano wrote:
> Lorenzo, it seems that not many people are reviewing this final
> round, and then I noticed that the list of CC addresses lacks a big
> name in the promisor remote topic, so I added Christian to the To:
> line of this message. Christian, you have no obligation to review
> these patches if they do not interest you, but just in case you
> weren't aware of this effort, I thought it might interest you; I am
> sure we all would benefit from your expertise.
>
> Thanks.
Yeah, it does seem that way. This patch series is really messy, and
generated way too much noise, so I don't blame them, and I'm sorry about
that.
I still learned a ton, so I'm happy about that. In the mean time I will
stick to solving simpler issues.
Thanks for your patience,
Lorenzo
^ permalink raw reply
* Re: [PATCH v2 6/8] promisor-remote: trust known remotes matching acceptFromServerUrl
From: Christian Couder @ 2026-05-19 15:24 UTC (permalink / raw)
To: Toon Claes
Cc: git, Junio C Hamano, Patrick Steinhardt, Taylor Blau,
Karthik Nayak, Elijah Newren, Christian Couder
In-Reply-To: <875x4yoys5.fsf@toon--20250203-5JQV3.mail-host-address-is-not-set>
On Fri, May 8, 2026 at 3:45 PM Toon Claes <toon@iotcl.com> wrote:
>
> Christian Couder <christian.couder@gmail.com> writes:
> > +static bool match_one_url(const struct url_info *pi, const struct url_info *ui)
> > +{
> > + const char *pat = pi->url;
> > + const char *url = ui->url;
> > + char *p_str, *u_str;
> > + bool res;
> > +
> > + /*
> > + * Schemes must match exactly. They are case-folded by
> > + * url_normalize(), so strncmp() suffices.
> > + */
> > + if (pi->scheme_len != ui->scheme_len || strncmp(pat, url, pi->scheme_len))
> > + return false;
> > +
> > + /*
> > + * Ports must match exactly. url_normalize() strips default
> > + * ports (like 443 for https), so length and content
> > + * comparisons are sufficient.
> > + */
> > + if (pi->port_len != ui->port_len ||
> > + strncmp(pat + pi->port_off, url + ui->port_off, pi->port_len))
> > + return false;
> > +
> > + /*
> > + * Match host and path separately to prevent a '*' in the host
> > + * portion of the pattern from matching across the '/'
> > + * boundary into the path. Use WM_PATHNAME for the host so '*'
> > + * cannot cross '/' there, and 0 for the path so '*' can still
> > + * match multi-level paths.
> > + */
>
> Do we actually need WM_PATHNAME, because we only xstrndup() the host
> part anyway?
Yeah, it's not really needed.
On one hand it doesn't hurt either, and it conveys the intent, which
is that no / boundary should be crossed.
But on the other hand I agree it could be confusing and it's simpler
to just remove it, so I have removed it in the v3 I will send very
soon.
> > +
> > + p_str = xstrndup(pat + pi->host_off, pi->host_len);
> > + u_str = xstrndup(url + ui->host_off, ui->host_len);
> > + res = !wildmatch(p_str, u_str, WM_PATHNAME);
> > + free(p_str);
> > + free(u_str);
> > +
> > + if (!res)
> > + return false;
> > +
> > + p_str = xstrndup(pat + pi->path_off, pi->path_len);
> > + u_str = xstrndup(url + ui->path_off, ui->path_len);
> > + res = !wildmatch(p_str, u_str, 0);
> > + free(p_str);
> > + free(u_str);
>
> Is it correct we intentionally do not compare the user and pass (at
> `user_off` and `passwd_off`)? I assume so, because this allows the
> server to update those?
Yes, we ignore them intentionally. Using the existing `token` field
should be prefered, but maybe some need a user and password part of
the URL.
Anyway I have documented that in v3.
> > static int should_accept_remote(enum accept_promisor accept,
> > struct promisor_info *advertised,
> > + struct string_list *accept_urls,
> > struct string_list *config_info)
> > {
> > struct promisor_info *p;
> > @@ -771,9 +846,6 @@ static int should_accept_remote(enum accept_promisor accept,
> > if (accept == ACCEPT_KNOWN_NAME)
> > return all_fields_match(advertised, config_info, p);
> >
> > - if (accept != ACCEPT_KNOWN_URL)
> > - BUG("Unhandled 'enum accept_promisor' value '%d'", accept);
> > -
> > if (strcmp(p->url, remote_url)) {
> > warning(_("known remote named '%s' but with URL '%s' instead of '%s', "
> > "ignoring this remote"),
> > @@ -781,7 +853,21 @@ static int should_accept_remote(enum accept_promisor accept,
> > return 0;
> > }
> >
> > - return all_fields_match(advertised, config_info, p);
> > + if (accept == ACCEPT_KNOWN_URL)
> > + return all_fields_match(advertised, config_info, p);
> > +
> > + if (accept != ACCEPT_NONE)
> > + BUG("Unhandled 'enum accept_promisor' value '%d'", accept);
> > +
> > + /*
> > + * Even if accept == ACCEPT_NONE, we MUST trust this known
> > + * remote to update its token or other such fields if its URL
> > + * matches the acceptFromServerUrl allowlist!
> > + */
> > + if (url_matches_accept_list(accept_urls, remote_url))
> > + return all_fields_match(advertised, config_info, p);
>
> I should verify in the following patches, but it seems to me only when
> promisor.AcceptFromServer is set to None it will store the advertised
> servers to the local .git/config, or not?
Right, it's better to check if the URL is in the allowlist as soon as
we can. So in the v3 I have moved as much as possible the
`promisor.acceptFromServerUrl` related checks before the other checks.
The idea is that `promisor.acceptFromServerUrl` takes precedence over
`promisor.acceptFromServer`, so having the
`promisor.acceptFromServerUrl` checks first makes sense.
Note that we should still not accept an advertised remote with an URL
that matches a pattern in `promisor.acceptFromServerUrl` if a remote
with the same name but a different URL exist on the client, unless the
user has explicitly set `promisor.AcceptFromServer` to either 'All' or
'knownName'.
In the v3 I have also added some documentation to be explicit about this.
> > +test_expect_success "clone with 'None' but URL allowlisted" '
> > + git -C server config promisor.advertise true &&
> > + test_when_finished "rm -rf client" &&
> > +
> > + GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \
> > + -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
> > + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
> > + -c promisor.acceptfromserver=None \
> > + -c promisor.acceptFromServerUrl="$ENCODED_TRASH_DIRECTORY_URL/*" \
> > + --no-local --filter="blob:limit=5k" server client &&
> > +
> > + # Check that the largest object is still missing on the server
> > + check_missing_objects server 1 "$oid"
> > +'
>
> Why do some tests end with `initialize_server 1 "$oid"` and this one
> not? Isn't it weird tests prepare for the next test?
It's more cleaning up after themselves than preparing for the next test.
Initializing the server with `initialize_server 1 "$oid"` is only
needed when some large objects end up on the server where they should
not be.
If we wanted to be sure that the state of the server is always clean
for the next test, then we should initialize the server at the end of
every test, using something like:
test_when_finished "initialize_server 1 \"$oid\""
just in case something went wrong and a large file was transferred to
the server. But I think that's quite expensive for not much gain.
It's also clearer for readers to have `initialize_server 1 "$oid"`
only where we think it's really needed.
So in the end I prefer to leave this as-is for now. We can still
address this later in a separate series for all the tests in
"t5710-promisor-remote-capability.sh" if we really think it's worth
addressing.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 6/8] promisor-remote: trust known remotes matching acceptFromServerUrl
From: Christian Couder @ 2026-05-19 15:25 UTC (permalink / raw)
To: Toon Claes
Cc: git, Junio C Hamano, Patrick Steinhardt, Taylor Blau,
Karthik Nayak, Elijah Newren, Christian Couder
In-Reply-To: <87qzninlb4.fsf@toon--20250203-5JQV3.mail-host-address-is-not-set>
On Mon, May 11, 2026 at 3:11 PM Toon Claes <toon@iotcl.com> wrote:
>
> Christian Couder <christian.couder@gmail.com> writes:
>
> > +static bool match_one_url(const struct url_info *pi, const struct url_info *ui)
> > +{
> > + const char *pat = pi->url;
> > + const char *url = ui->url;
> > + char *p_str, *u_str;
> > + bool res;
> > +
> > + /*
> > + * Schemes must match exactly. They are case-folded by
> > + * url_normalize(), so strncmp() suffices.
> > + */
> > + if (pi->scheme_len != ui->scheme_len || strncmp(pat, url, pi->scheme_len))
> > + return false;
> > +
> > + /*
> > + * Ports must match exactly. url_normalize() strips default
> > + * ports (like 443 for https), so length and content
> > + * comparisons are sufficient.
> > + */
> > + if (pi->port_len != ui->port_len ||
> > + strncmp(pat + pi->port_off, url + ui->port_off, pi->port_len))
> > + return false;
> > +
> > + /*
> > + * Match host and path separately to prevent a '*' in the host
> > + * portion of the pattern from matching across the '/'
> > + * boundary into the path. Use WM_PATHNAME for the host so '*'
> > + * cannot cross '/' there, and 0 for the path so '*' can still
> > + * match multi-level paths.
> > + */
> > +
> > + p_str = xstrndup(pat + pi->host_off, pi->host_len);
> > + u_str = xstrndup(url + ui->host_off, ui->host_len);
> > + res = !wildmatch(p_str, u_str, WM_PATHNAME);
> > + free(p_str);
> > + free(u_str);
> > +
> > + if (!res)
>
> I feel it's a bit confusing your negating the result from wildmatch()
> to negate it here again? Maybe keep using the int return value, or
> rename the variable to 'matches' ?
I have simplified this in the v3.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 7/8] promisor-remote: auto-configure unknown remotes
From: Christian Couder @ 2026-05-19 15:25 UTC (permalink / raw)
To: Toon Claes
Cc: git, Junio C Hamano, Patrick Steinhardt, Taylor Blau,
Karthik Nayak, Elijah Newren, Christian Couder
In-Reply-To: <87v7cunlid.fsf@toon--20250203-5JQV3.mail-host-address-is-not-set>
On Mon, May 11, 2026 at 3:06 PM Toon Claes <toon@iotcl.com> wrote:
> > +static char *promisor_remote_name_from_url(const char *url)
> > +{
> > + struct url_info url_info = { 0 };
> > + char *normalized = url_normalize(url, &url_info);
> > + struct strbuf buf = STRBUF_INIT;
> > +
> > + if (!normalized) {
> > + warning(_("couldn't normalize advertised url '%s', "
> > + "ignoring this remote"), url);
> > + return NULL;
> > + }
> > +
> > + if (url_info.host_len) {
> > + strbuf_add(&buf, normalized + url_info.host_off, url_info.host_len);
> > + strbuf_addch(&buf, '-');
> > + }
> > +
> > + if (url_info.port_len) {
> > + strbuf_add(&buf, normalized + url_info.port_off, url_info.port_len);
> > + strbuf_addch(&buf, '-');
>
> If the url doesn't have a path, this could lead to the name being
> `example-com-8443`. But we have a MAX_REMOTES_WITH_SIMILAR_NAMES at 20,
> would this be an issue for a second remote without configured name?
>
> As far as I can tell from handle_matching_allowed_url(), it's no issue,
> because the numeric `-%d` suffix is added and we never atoi() the number
> from existing remotes in the config.
Right.
[...]
> > +test_expect_success "clone with URL allowlisted and no remote already configured" '
> > + git -C server config promisor.advertise true &&
> > + test_when_finished "rm -rf client" &&
> > + test_when_finished "rm -f full_names" &&
> > +
> > + GIT_NO_LAZY_FETCH=0 git clone \
> > + -c promisor.acceptfromserver=None \
> > + -c promisor.acceptFromServerUrl="$ENCODED_TRASH_DIRECTORY_URL/*" \
> > + --no-local --filter="blob:limit=5k" server client &&
>
> So promisor.acceptFromServerUrl only works if promisor.acceptFromServer
> is "none"? I mean which one should precedence? If
> promisor.acceptFromServer is set to "all", the promisor remote is
> accepted by the client, but not saved to the config. Is that
> intentional? Should we document that?
Yeah, it was buggy in v2 for some values of
`promisor.acceptfromserver`, and things were not properly documented.
But I think it's correct and much clearer now in v3 as discussed in my
reply to your comments on the previous patch.
Thanks.
^ permalink raw reply
* [PATCH v3 0/8] Auto-configure advertised remotes via URL allowlist
From: Christian Couder @ 2026-05-19 15:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Toon Claes, Christian Couder
In-Reply-To: <20260427124108.3524129-1-christian.couder@gmail.com>
Currently, the "promisor-remote" protocol capability allows a server
to advertise promisor remotes (and their tokens/filters), but the
client's `promisor.acceptFromServer` mechanism requires these remotes
to already exist in the config.
This is a significant burden for users and administrators who have to
pre-configure remotes.
This patch series improves on this by introducing a new
`promisor.acceptFromServerUrl` config option, which provides an
additive, URL-based security allowlist.
Multiple `promisor.acceptFromServerUrl` config options can be provided
in different config files. Each one should contain a URL glob pattern
which can optionally be prefixed with a remote name in the
"[<name>=]<pattern>" format.
The goal is for something like a simple:
git config set --global promisor.acceptFromServerUrl "https://my-org.com/*"
to be all that is needed for internal work in many organizations.
With this new config option:
- The server can update fields (like tokens) for known remotes,
provided their URL matches the allowlist, even if
`acceptFromServer` is set to `None`.
- Unknown remotes advertised by the server can be automatically
configured on the client if their URL matches the allowlist.
- If there is no `<name>` prefix before the glob pattern matched, the
auto-configured remote is named using the
"promisor-auto-<sanitized-url>" format. So the same auto-configured
remote config entry will be reused for the same URL.
- If a `<name>` prefix is provided, it will be used for the
auto-configured remote config entry.
- If the chosen name (auto-generated or prefixed) already exists but
points to a different URL, overwriting the existing config is
prevented by appending a numeric suffix (e.g., -1, -2) to the name
and auto-configuring using that name.
- The server's originally advertised name is always saved in the
`remote.<name>.advertisedAs` config variable of the auto-configured
remote for tracing and debugging.
Security considerations:
- Advertised URLs and glob patterns are routed through
url_normalize() / url_normalize_pattern() before matching, to
prevent percent-encoding, case variation, or path-traversal (..)
bypasses.
- URL matching is done component by component: scheme and port
must match exactly (no wildcards), the host is matched with
WM_PATHNAME so a '*' cannot cross the '/' boundary into the
path, and the path is matched without WM_PATHNAME so '*' can
still span multi-level paths.
- Auto-generated remote names are sanitized (non-alphanumeric
characters are replaced with '-', runs of '-' are collapsed)
and prefixed with 'promisor-auto-'. User-supplied names (from
the 'name=<pattern>' syntax) are validated with
valid_remote_name(). Together, these prevent a server from
maliciously overwriting standard remotes (like 'origin').
- If the auto-generated or user-supplied name collides with an
existing remote configured to a different URL, a numeric
suffix ('-1', '-2', ...) is appended, up to a bounded limit,
so a server cannot hijack an existing remote by name.
- Known remotes are still subject to URL consistency checks:
even if an advertised URL matches the allowlist, it is only
accepted for a known remote if it matches the URL already
configured locally for that remote.
- The documentation explains in detail how to write secure glob
patterns in `promisor.acceptFromServerUrl`, and highlights the
risks of overly broad patterns on shared hosting platforms.
High level description of the patches
=====================================
- Patch 1/8 is a very small preparatory patch that simplifies some
tests a bit.
- Patches 2/8 and 3/8 expose and adapt a url_normalize_pattern()
helper function in the urlmatch API.
- Patch 4/8 adapts `struct promisor_info` by adding a new
`local_name` member to it to prepare for the next patches.
- Patches 5/8 to 7/8 implement the core feature. They introduce the
parsing machinery, add the additive allowlist for known remotes
(with url_normalize() security), and finally implement the
auto-creation and collision resolution for unknown remotes.
- Patch 8/8 cleans up and modernizes the existing
`promisor.acceptFromServer` documentation.
Changes compared to v2
======================
Thanks to Toon, Patrick and Junio for reviewing the previous versions
of this series and of the preparatory series.
This series has been rebased on top of master now that the preparatory
series has been merged in a19de4d24a (Merge branch
'cc/promisor-auto-config-url', 2026-05-11).
Only the following patches changed:
- Patch 6/8 (promisor-remote: trust known remotes matching acceptFromServerUrl)
- The WM_PATHNAME flag is not used anymore when calling wildmatch().
- The match_one_url() function has been refactored using a new
match_pattern_url() helper function. There is no double negation
anymore.
- The call to url_matches_accept_list() in should_accept_remote()
has been moved up to make sure `promisor.acceptFromServerUrl`
takes precedence over `promisor.acceptFromServer`.
- It's documented that the username and password components of the
URL are intentionally ignored during matching.
- The documentation now clarifies how
`promisor.acceptFromServerUrl` interacts with
`promisor.acceptFromServer`.
- Patch 7/8 (promisor-remote: auto-configure unknown remotes)
- The call to should_accept_new_remote_url() is now before the
`accept == ACCEPT_ALL` check.
- The documentation continues to clarify how
`promisor.acceptFromServerUrl` interacts with
`promisor.acceptFromServer`.
CI tests
========
They all pass, see:
https://github.com/chriscool/git/actions/runs/26103407562
Range diff since v2
===================
1: 44e9a16455 = 1: ab231c0896 t5710: simplify 'mkdir X' followed by 'git -C X init'
2: 42f174910c = 2: b3e66f329f urlmatch: change 'allow_globs' arg to bool
3: 8088374458 = 3: 813d748dd6 urlmatch: add url_normalize_pattern() helper
4: 6bfda89a79 = 4: e92863bee8 promisor-remote: add 'local_name' to 'struct promisor_info'
5: fefa17e6dd = 5: 7e1b106404 promisor-remote: introduce promisor.acceptFromServerUrl
6: 2f238d0a7a ! 6: f00eed4bf2 promisor-remote: trust known remotes matching acceptFromServerUrl
@@ Commit message
returns the first matching allowed_url entry (or NULL).
The URL matching is done component by component: scheme and port are
- compared exactly, the host is matched with wildmatch() using the
- WM_PATHNAME flag (so '*' cannot cross the '/' boundary into the path),
- and the path is matched with wildmatch() without WM_PATHNAME (so '*'
- can still match multi-level paths). Before matching, the advertised
- URL is passed through url_normalize() so that case variations in the
- scheme/host, percent-encoding tricks, and ".." path segments cannot
- bypass the allowlist.
+ compared exactly, the host and path are matched with wildmatch().
+ Before matching, the advertised URL is passed through url_normalize()
+ so that case variations in the scheme/host, percent-encoding tricks,
+ and ".." path segments cannot bypass the allowlist.
- Let's then use this helper at the tail of should_accept_remote() so
- that, when `accept == ACCEPT_NONE`, a known remote whose URL matches
- the allowlist is still accepted.
+ The username and password components of the URL are intentionally
+ ignored during matching to allow servers to rotate them, though using
+ the 'token' field of the capability is preferred over embedding
+ credentials in the URL.
+
+ Let's then use this helper in should_accept_remote() so that, a known
+ remote whose URL matches the allowlist is accepted.
To prepare for this new logic, let's also:
@@ Commit message
- Replace the BUG() guard in the ACCEPT_KNOWN_URL case with an
explicit 'if (accept == ACCEPT_KNOWN_URL) return' and a new
- BUG() guard in the ACCEPT_NONE case, so url_matches_accept_list()
- is only called in the ACCEPT_NONE case.
+ BUG() guard in the ACCEPT_NONE case.
- Call accept_from_server_url() from filter_promisor_remote()
and relax its early return so that the function is entered when
@@ Commit message
including the URL normalization behavior and the component-wise
matching, and let's mention it in "gitprotocol-v2.adoc".
+ Also let's clarify in the documentation how
+ `promisor.acceptFromServerUrl` interacts with
+ `promisor.acceptFromServer`:
+
+ - Precedence: when both options are set,
+ `promisor.acceptFromServerUrl` is consulted first. If a matching
+ pattern leads to acceptance, the remote is accepted regardless of
+ `promisor.acceptFromServer`. Otherwise the decision is left to
+ `promisor.acceptFromServer`.
+
+ - URL-mismatch guard: even when the advertised URL matches the
+ allowlist, an already-existing client-side remote whose configured
+ URL differs from the advertised one is not accepted through
+ `promisor.acceptFromServerUrl`. `promisor.acceptFromServer=all` and
+ `=knownName` keep their pre-existing, looser semantics.
+
+ The precedence paragraph is intentionally scoped here to known remotes
+ only (field updates). A following commit that introduces auto-creation
+ of unknown remotes will extend it to cover that case as well.
+
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
## Documentation/config/promisor.adoc ##
@@ Documentation/config/promisor.adoc: promisor.acceptFromServer::
+URL will be accepted if it matches _ANY_ glob pattern specified by
+this option in _ANY_ config file read by Git.
++
++When both `promisor.acceptFromServer` and `promisor.acceptFromServerUrl`
++are set, `promisor.acceptFromServerUrl` is consulted first and takes
++precedence: if a matching pattern leads to acceptance (by accepting
++field updates for a known remote whose URL matches both the local
++configuration and the allowlist), the advertised remote is accepted
++regardless of the `promisor.acceptFromServer` setting. If no pattern
++in `promisor.acceptFromServerUrl` triggers acceptance, the decision
++is left to `promisor.acceptFromServer`.
+++
++Note however that, even when an advertised URL matches a pattern in
++`promisor.acceptFromServerUrl`, an already-existing remote on the
++client whose name matches the advertised name but whose configured URL
++differs from the advertised one will _NOT_ be accepted through
++`promisor.acceptFromServerUrl`. This prevents a server from silently
++re-pointing an existing client-side remote at a different URL. (Such a
++remote may still be accepted through `promisor.acceptFromServer=all`
++or `=knownName`, which have their own, looser semantics; see the
++documentation of that option.)
+++
+Be _VERY_ careful with these patterns: `*` matches any sequence of
+characters within the 'host' and 'path' parts of a URL (but cannot
+cross part boundaries). An overly broad pattern is a major security
@@ Documentation/config/promisor.adoc: promisor.acceptFromServer::
+characters are decoded where possible, and path segments like `..`
+are resolved. The port must also match exactly (e.g.,
+`https://example.com:8080/*` will not match a URL advertised on
-+port 9999).
++port 9999). The username and password components of the URL are
++ignored during matching. Note that embedding credentials in URLs is
++discouraged. Passing authentication tokens via the `token` field of
++the `promisor-remote` capability is strongly preferred.
++
+For the security implications of accepting a promisor remote, see the
+documentation of `promisor.acceptFromServer`. For details on the
@@ promisor-remote.c: static void load_accept_from_server_url(struct repository *re
}
}
++static bool match_pattern_url(const char *pat, size_t pat_len,
++ const char *url, size_t url_len)
++{
++ char *p_str = xstrndup(pat, pat_len);
++ char *u_str = xstrndup(url, url_len);
++ bool res = !wildmatch(p_str, u_str, 0);
++
++ free(p_str);
++ free(u_str);
++
++ return res;
++}
++
+static bool match_one_url(const struct url_info *pi, const struct url_info *ui)
+{
+ const char *pat = pi->url;
+ const char *url = ui->url;
-+ char *p_str, *u_str;
-+ bool res;
+
+ /*
+ * Schemes must match exactly. They are case-folded by
@@ promisor-remote.c: static void load_accept_from_server_url(struct repository *re
+ /*
+ * Match host and path separately to prevent a '*' in the host
+ * portion of the pattern from matching across the '/'
-+ * boundary into the path. Use WM_PATHNAME for the host so '*'
-+ * cannot cross '/' there, and 0 for the path so '*' can still
-+ * match multi-level paths.
++ * boundary into the path.
+ */
+
-+ p_str = xstrndup(pat + pi->host_off, pi->host_len);
-+ u_str = xstrndup(url + ui->host_off, ui->host_len);
-+ res = !wildmatch(p_str, u_str, WM_PATHNAME);
-+ free(p_str);
-+ free(u_str);
-+
-+ if (!res)
-+ return false;
-+
-+ p_str = xstrndup(pat + pi->path_off, pi->path_len);
-+ u_str = xstrndup(url + ui->path_off, ui->path_len);
-+ res = !wildmatch(p_str, u_str, 0);
-+ free(p_str);
-+ free(u_str);
-+
-+ return res;
++ return match_pattern_url(pat + pi->host_off, pi->host_len,
++ url + ui->host_off, ui->host_len) &&
++ match_pattern_url(pat + pi->path_off, pi->path_len,
++ url + ui->path_off, ui->path_len);
+}
+
+static struct allowed_url *url_matches_accept_list(
@@ promisor-remote.c: static void load_accept_from_server_url(struct repository *re
{
struct promisor_info *p;
@@ promisor-remote.c: static int should_accept_remote(enum accept_promisor accept,
- if (accept == ACCEPT_KNOWN_NAME)
+ "this remote should have been rejected earlier",
+ remote_name);
+
+- if (accept == ACCEPT_ALL)
+- return all_fields_match(advertised, config_info, NULL);
+-
+ /* Get config info for that promisor remote */
+ item = string_list_lookup(config_info, remote_name);
+
+- if (!item)
++ if (!item) {
+ /* We don't know about that remote */
++ if (accept == ACCEPT_ALL)
++ return all_fields_match(advertised, config_info, NULL);
+ return 0;
++ }
+
+ p = item->util;
+
+- if (accept == ACCEPT_KNOWN_NAME)
++ /* Known remote in the allowlist? */
++ if (!strcmp(p->url, remote_url) && url_matches_accept_list(accept_urls, remote_url))
return all_fields_match(advertised, config_info, p);
- if (accept != ACCEPT_KNOWN_URL)
- BUG("Unhandled 'enum accept_promisor' value '%d'", accept);
--
++ if (accept == ACCEPT_ALL)
++ return all_fields_match(advertised, config_info, NULL);
++
++ if (accept == ACCEPT_KNOWN_NAME)
++ return all_fields_match(advertised, config_info, p);
+
if (strcmp(p->url, remote_url)) {
warning(_("known remote named '%s' but with URL '%s' instead of '%s', "
- "ignoring this remote"),
@@ promisor-remote.c: static int should_accept_remote(enum accept_promisor accept,
return 0;
}
@@ promisor-remote.c: static int should_accept_remote(enum accept_promisor accept,
+ if (accept != ACCEPT_NONE)
+ BUG("Unhandled 'enum accept_promisor' value '%d'", accept);
+
-+ /*
-+ * Even if accept == ACCEPT_NONE, we MUST trust this known
-+ * remote to update its token or other such fields if its URL
-+ * matches the acceptFromServerUrl allowlist!
-+ */
-+ if (url_matches_accept_list(accept_urls, remote_url))
-+ return all_fields_match(advertised, config_info, p);
-+
+ return 0;
}
7: a077f33df4 ! 7: af06fb31db promisor-remote: auto-configure unknown remotes
@@ Commit message
handling, and by
- adding a "remote.<name>.advertisedAs" entry to "remote.adoc".
+ Also let's extend the precedence paragraph added by a previous commit
+ to mention this new acceptance path: until now, the only way for
+ `promisor.acceptFromServerUrl` to trigger acceptance was to allow
+ field updates for a known remote. With this commit, it can also trigger
+ auto-creation of a previously-unknown remote whose advertised URL
+ matches the allowlist.
+
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
## Documentation/config/promisor.adoc ##
@@ Documentation/config/promisor.adoc: promisor.acceptFromServer::
tokens) from the server, even if `promisor.acceptFromServer`
is set to `none` (the default).
@@ Documentation/config/promisor.adoc: this option in _ANY_ config file read by Git.
+ +
+ When both `promisor.acceptFromServer` and `promisor.acceptFromServerUrl`
+ are set, `promisor.acceptFromServerUrl` is consulted first and takes
+-precedence: if a matching pattern leads to acceptance (by accepting
+-field updates for a known remote whose URL matches both the local
+-configuration and the allowlist), the advertised remote is accepted
+-regardless of the `promisor.acceptFromServer` setting. If no pattern
+-in `promisor.acceptFromServerUrl` triggers acceptance, the decision
+-is left to `promisor.acceptFromServer`.
++precedence: if a matching pattern leads to acceptance (either by
++auto-configuring an unknown remote or by accepting field updates for
++a known remote whose URL matches both the local configuration and the
++allowlist), the advertised remote is accepted regardless of the
++`promisor.acceptFromServer` setting. If no pattern in
++`promisor.acceptFromServerUrl` triggers acceptance, the decision is
++left to `promisor.acceptFromServer`.
+ +
+ Note however that, even when an advertised URL matches a pattern in
+ `promisor.acceptFromServerUrl`, an already-existing remote on the
+@@ Documentation/config/promisor.adoc: documentation of that option.)
Be _VERY_ careful with these patterns: `*` matches any sequence of
characters within the 'host' and 'path' parts of a URL (but cannot
cross part boundaries). An overly broad pattern is a major security
@@ Documentation/config/promisor.adoc: this option in _ANY_ config file read by Git
+
1. Start with a secure protocol scheme, like `https://` or `ssh://`.
+
-@@ Documentation/config/promisor.adoc: are resolved. The port must also match exactly (e.g.,
- `https://example.com:8080/*` will not match a URL advertised on
- port 9999).
+@@ Documentation/config/promisor.adoc: ignored during matching. Note that embedding credentials in URLs is
+ discouraged. Passing authentication tokens via the `token` field of
+ the `promisor-remote` capability is strongly preferred.
+
+The glob pattern can optionally be prefixed with a remote name and an
+equals sign (e.g., `cdn=https://cdn.example.com/*`). If such a prefix
@@ promisor-remote.c: static struct allowed_url *url_matches_accept_list(
struct promisor_info *p;
struct string_list_item *item;
@@ promisor-remote.c: static int should_accept_remote(enum accept_promisor accept,
- /* Get config info for that promisor remote */
- item = string_list_lookup(config_info, remote_name);
-- if (!item)
-+ if (!item) {
+ if (!item) {
/* We don't know about that remote */
-- return 0;
++
+ int res = should_accept_new_remote_url(repo, accept_urls, advertised);
-+ if (res)
++ if (res) {
+ *reload_config = true;
-+ return res;
-+ }
-
- p = item->util;
-
++ return res;
++ }
++
+ if (accept == ACCEPT_ALL)
+ return all_fields_match(advertised, config_info, NULL);
+ return 0;
@@ promisor-remote.c: static void filter_promisor_remote(struct repository *repo,
string_list_sort(&config_info);
}
8: b68b9497aa = 8: 92075d88d8 doc: promisor: improve acceptFromServer entry
Christian Couder (8):
t5710: simplify 'mkdir X' followed by 'git -C X init'
urlmatch: change 'allow_globs' arg to bool
urlmatch: add url_normalize_pattern() helper
promisor-remote: add 'local_name' to 'struct promisor_info'
promisor-remote: introduce promisor.acceptFromServerUrl
promisor-remote: trust known remotes matching acceptFromServerUrl
promisor-remote: auto-configure unknown remotes
doc: promisor: improve acceptFromServer entry
Documentation/config/promisor.adoc | 146 +++++++--
Documentation/config/remote.adoc | 9 +
Documentation/gitprotocol-v2.adoc | 9 +-
promisor-remote.c | 413 ++++++++++++++++++++++++--
t/t5710-promisor-remote-capability.sh | 202 ++++++++++++-
urlmatch.c | 11 +-
urlmatch.h | 12 +
7 files changed, 754 insertions(+), 48 deletions(-)
--
2.54.0.134.gbbe8e27878.dirty
^ permalink raw reply
* [PATCH v3 1/8] t5710: simplify 'mkdir X' followed by 'git -C X init'
From: Christian Couder @ 2026-05-19 15:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Toon Claes, Christian Couder, Christian Couder
In-Reply-To: <20260519153808.494105-1-christian.couder@gmail.com>
It's simpler and more efficient to just use `git init client` instead
of `mkdir client && git -C client init`.
So let's replace the latter with the former.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t5710-promisor-remote-capability.sh | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
index b404ad9f0a..bf1cc54605 100755
--- a/t/t5710-promisor-remote-capability.sh
+++ b/t/t5710-promisor-remote-capability.sh
@@ -177,8 +177,7 @@ test_expect_success "init + fetch with promisor.advertise set to 'true'" '
git -C server config promisor.advertise true &&
test_when_finished "rm -rf client" &&
- mkdir client &&
- git -C client init &&
+ git init client &&
git -C client config remote.lop.promisor true &&
git -C client config remote.lop.fetch "+refs/heads/*:refs/remotes/lop/*" &&
git -C client config remote.lop.url "$TRASH_DIRECTORY_URL/lop" &&
@@ -231,8 +230,7 @@ test_expect_success "init + fetch two promisors but only one advertised" '
# Create a promisor that will be configured but not be used
git init --bare unused_lop &&
- mkdir client &&
- git -C client init &&
+ git init client &&
git -C client config remote.unused_lop.promisor true &&
git -C client config remote.unused_lop.fetch "+refs/heads/*:refs/remotes/unused_lop/*" &&
git -C client config remote.unused_lop.url "$TRASH_DIRECTORY_URL/unused_lop" &&
--
2.54.0.134.gbbe8e27878.dirty
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox