From 6e9d4f3029daa2c0068bb16939b943e7ac924222 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 24 Feb 2026 08:06:25 -0500 Subject: [PATCH 1/4] for-each-repo: test outside of repo context The 'git for-each-repo' tool is frequently run outside of a repo context in the real world. For example, it powers background maintenance. Despite this typical case, we have not been testing it without a local repository. Update t0068 to stop creating a test repo and to use global config everywhere. This has some subtle changes to test across the file. This was noticed because an earlier attempt to remove the_repository from builtin/for-each-repo.c did not catch a segmentation fault since the passed 'repo' is NULL. This use of the_repository will need to stay until we have a better way to handle config queries outside of a repo context. Similar use still exists in builtin/config.c for the same reason. Signed-off-by: Derrick Stolee --- t/t0068-for-each-repo.sh | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh index f2f3e500312916..512af34c82ecb8 100755 --- a/t/t0068-for-each-repo.sh +++ b/t/t0068-for-each-repo.sh @@ -2,6 +2,9 @@ test_description='git for-each-repo builtin' +# We need to test running 'git for-each-repo' outside of a repo context. +TEST_NO_CREATE_REPO=1 + . ./test-lib.sh test_expect_success 'run based on configured value' ' @@ -10,9 +13,10 @@ test_expect_success 'run based on configured value' ' git init three && git init ~/four && git -C two commit --allow-empty -m "DID NOT RUN" && - git config run.key "$TRASH_DIRECTORY/one" && - git config --add run.key "$TRASH_DIRECTORY/three" && - git config --add run.key "~/four" && + git config --global run.key "$TRASH_DIRECTORY/one" && + git config --global --add run.key "$TRASH_DIRECTORY/three" && + git config --global --add run.key "~/four" && + git for-each-repo --config=run.key commit --allow-empty -m "ran" && git -C one log -1 --pretty=format:%s >message && grep ran message && @@ -22,6 +26,7 @@ test_expect_success 'run based on configured value' ' grep ran message && git -C ~/four log -1 --pretty=format:%s >message && grep ran message && + git for-each-repo --config=run.key -- commit --allow-empty -m "ran again" && git -C one log -1 --pretty=format:%s >message && grep again message && @@ -46,7 +51,7 @@ test_expect_success 'error on bad config keys' ' ' test_expect_success 'error on NULL value for config keys' ' - cat >>.git/config <<-\EOF && + cat >>.gitconfig <<-\EOF && [empty] key EOF @@ -59,8 +64,8 @@ test_expect_success 'error on NULL value for config keys' ' ' test_expect_success '--keep-going' ' - git config keep.going non-existing && - git config --add keep.going . && + git config --global keep.going non-existing && + git config --global --add keep.going one && test_must_fail git for-each-repo --config=keep.going \ -- branch >out 2>err && @@ -70,7 +75,7 @@ test_expect_success '--keep-going' ' test_must_fail git for-each-repo --config=keep.going --keep-going \ -- branch >out 2>err && test_grep "cannot change to .*non-existing" err && - git branch >expect && + git -C one branch >expect && test_cmp expect out ' From 24398664c2009cc1fe94f8cebec145d062d96abd Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 2 Mar 2026 09:42:30 -0500 Subject: [PATCH 2/4] run-command: extract sanitize_repo_env helper The current prepare_other_repo_env() does two distinct things: 1. Strip certain known environment variables that should be set by a child process based on a different repository. 2. Set the GIT_DIR variable to avoid repository discovery. The second item is valuable for child processes that operate on submodules, where the repo discovery could be mistaken for the parent repository. In the next change, we will see an important case where only the first item is required as the GIT_DIR discovery should happen naturally from the '-C' parameter in the child process. Helped-by: Jeff King Signed-off-by: Derrick Stolee --- run-command.c | 7 ++++++- run-command.h | 15 ++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/run-command.c b/run-command.c index e3e02475ccec50..89dbe62ab842e9 100644 --- a/run-command.c +++ b/run-command.c @@ -1847,7 +1847,7 @@ int run_auto_maintenance(int quiet) return run_command(&maint); } -void prepare_other_repo_env(struct strvec *env, const char *new_git_dir) +void sanitize_repo_env(struct strvec *env) { const char * const *var; @@ -1856,6 +1856,11 @@ void prepare_other_repo_env(struct strvec *env, const char *new_git_dir) strcmp(*var, CONFIG_COUNT_ENVIRONMENT)) strvec_push(env, *var); } +} + +void prepare_other_repo_env(struct strvec *env, const char *new_git_dir) +{ + sanitize_repo_env(env); strvec_pushf(env, "%s=%s", GIT_DIR_ENVIRONMENT, new_git_dir); } diff --git a/run-command.h b/run-command.h index 0df25e445f001c..7e5a263ee6560d 100644 --- a/run-command.h +++ b/run-command.h @@ -509,13 +509,18 @@ struct run_process_parallel_opts */ void run_processes_parallel(const struct run_process_parallel_opts *opts); +/** + * Unset all local-repo GIT_* variables in env; see local_repo_env in + * environment.h. GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT are preserved + * to pass -c and --config-env options from the parent process. + */ +void sanitize_repo_env(struct strvec *env); + /** * Convenience function which prepares env for a command to be run in a - * new repo. This adds all GIT_* environment variables to env with the - * exception of GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT (which cause the - * corresponding environment variables to be unset in the subprocess) and adds - * an environment variable pointing to new_git_dir. See local_repo_env in - * environment.h for more information. + * new repo. This removes variables pointing to the local repository (using + * sanitize_repo_env() above), and adds an environment variable pointing to + * new_git_dir. */ void prepare_other_repo_env(struct strvec *env, const char *new_git_dir); From e759b350692360e968a61e2f9744138104e77ba6 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 23 Feb 2026 20:46:52 -0500 Subject: [PATCH 3/4] for-each-repo: work correctly in a worktree When run in a worktree, the GIT_DIR directory is set in a different way than in a typical repository. Show this by updating t0068 to include a worktree and add a test that runs from that worktree. This requires moving the repo.key config into a global config instead of the base test repository's local config (demonstrating that it worked with non-worktree Git repositories). We need to be careful to unset the local Git environment variables and let the child process rediscover them, while also reinstating those variables in the parent process afterwards. Update run_command_on_repo() to use the new sanitize_repo_env() helper method to erase these environment variables. During review of this bug fix, there were several incorrect patches demonstrating different bad behaviors. Most of these are covered by tests, when it is not too expensive to set it up. One case that would be expensive to set up is the GIT_NO_REPLACE_OBJECTS environment variable, but we trust that using sanitize_repo_env() will be sufficient to capture these uncovered cases by using the common code for resetting environment variables. Reported-by: Matthew Gabeler-Lee Signed-off-by: Derrick Stolee --- builtin/for-each-repo.c | 3 +++ t/t0068-for-each-repo.sh | 48 +++++++++++++++++++++++++++++++++++----- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c index 325a7925f1fdb5..82727c4aa2d015 100644 --- a/builtin/for-each-repo.c +++ b/builtin/for-each-repo.c @@ -2,6 +2,7 @@ #include "builtin.h" #include "config.h" +#include "environment.h" #include "gettext.h" #include "parse-options.h" #include "path.h" @@ -19,6 +20,8 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv) struct child_process child = CHILD_PROCESS_INIT; char *abspath = interpolate_path(path, 0); + sanitize_repo_env(&child.env); + child.git_cmd = 1; strvec_pushl(&child.args, "-C", abspath, NULL); diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh index 512af34c82ecb8..80b163ea99bd75 100755 --- a/t/t0068-for-each-repo.sh +++ b/t/t0068-for-each-repo.sh @@ -8,10 +8,12 @@ TEST_NO_CREATE_REPO=1 . ./test-lib.sh test_expect_success 'run based on configured value' ' - git init one && - git init two && - git init three && - git init ~/four && + git init --initial-branch=one one && + git init --initial-branch=two two && + git -C two worktree add --orphan ../three && + git -C three checkout -b three && + git init --initial-branch=four ~/four && + git -C two commit --allow-empty -m "DID NOT RUN" && git config --global run.key "$TRASH_DIRECTORY/one" && git config --global --add run.key "$TRASH_DIRECTORY/three" && @@ -35,7 +37,43 @@ test_expect_success 'run based on configured value' ' git -C three log -1 --pretty=format:%s >message && grep again message && git -C ~/four log -1 --pretty=format:%s >message && - grep again message + grep again message && + + git -C three for-each-repo --config=run.key -- \ + commit --allow-empty -m "ran from worktree" && + git -C one log -1 --pretty=format:%s >message && + test_grep "ran from worktree" message && + git -C two log -1 --pretty=format:%s >message && + test_grep ! "ran from worktree" message && + git -C three log -1 --pretty=format:%s >message && + test_grep "ran from worktree" message && + git -C ~/four log -1 --pretty=format:%s >message && + test_grep "ran from worktree" message && + + # Test running with config values set by environment + cat >expect <<-EOF && + ran from worktree (HEAD -> refs/heads/one) + ran from worktree (HEAD -> refs/heads/three) + ran from worktree (HEAD -> refs/heads/four) + EOF + + GIT_CONFIG_PARAMETERS="${SQ}log.decorate=full${SQ}" \ + git -C three for-each-repo --config=run.key -- log --format="%s%d" -1 >out && + test_cmp expect out && + + cat >test-config <<-EOF && + [run] + key = $(pwd)/one + key = $(pwd)/three + key = $(pwd)/four + + [log] + decorate = full + EOF + + GIT_CONFIG_GLOBAL="$(pwd)/test-config" \ + git -C three for-each-repo --config=run.key -- log --format="%s%d" -1 >out && + test_cmp expect out ' test_expect_success 'do nothing on empty config' ' From 8b1f083da1c91c3b3dff8a3af4fc0d57c1cb5ab9 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Sun, 1 Mar 2026 12:26:56 -0500 Subject: [PATCH 4/4] for-each-repo: simplify passing of parameters This change simplifies the code somewhat from its original implementation. Signed-off-by: Derrick Stolee --- builtin/for-each-repo.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c index 82727c4aa2d015..927d3d92da5732 100644 --- a/builtin/for-each-repo.c +++ b/builtin/for-each-repo.c @@ -14,9 +14,8 @@ static const char * const for_each_repo_usage[] = { NULL }; -static int run_command_on_repo(const char *path, int argc, const char ** argv) +static int run_command_on_repo(const char *path, const char **argv) { - int i; struct child_process child = CHILD_PROCESS_INIT; char *abspath = interpolate_path(path, 0); @@ -24,9 +23,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv) child.git_cmd = 1; strvec_pushl(&child.args, "-C", abspath, NULL); - - for (i = 0; i < argc; i++) - strvec_push(&child.args, argv[i]); + strvec_pushv(&child.args, argv); free(abspath); @@ -66,7 +63,7 @@ int cmd_for_each_repo(int argc, return 0; for (size_t i = 0; i < values->nr; i++) { - int ret = run_command_on_repo(values->items[i].string, argc, argv); + int ret = run_command_on_repo(values->items[i].string, argv); if (ret) { if (!keep_going) return ret;