{"thread":{"id":"65065","subject":"[PATCH 0/2] for-each-repo: work correctly in a worktree","startedAt":"2026-02-24T03:32:33Z","lastAt":"2026-03-05T17:23:47Z","messageCount":48,"participants":["Derrick Stolee via GitGitGadget","Eric Sunshine","Jeff King","Patrick Steinhardt","Derrick Stolee","Junio C Hamano","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"536901","messageId":"pull.2056.git.1771903950.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":null,"subject":"[PATCH 0/2] for-each-repo: work correctly in a worktree","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-02-24T03:32:28Z","receivedAt":"2026-02-24T03:32:33Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This was reported by Matthew [1] and is a quick fix.\n\n[1]\nhttps://lore.kernel.org/git/CABpCjbY=wpStuhxqRJ5TSNV3A-CmN-g-xZGJOQGSSv3GYhs2fQ@mail.gmail.com/\n\nI also took the liberty of removing the_repository as I wanted to make sure\nthat wasn't involved here.\n\nThanks, -Stolee\n\nDerrick Stolee (2):\n for-each-repo: stop using the_repository\n for-each-repo: work correctly in a worktree\n\n builtin/for-each-repo.c | 10 ++++++----\n t/t0068-for-each-repo.sh | 22 +++++++++++++++++-----\n 2 files changed, 23 insertions(+), 9 deletions(-)\n\n\nbase-commit: 67ad42147a7acc2af6074753ebd03d904476118f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2056%2Fderrickstolee%2Ffor-each-repo-in-gitdir-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2056/derrickstolee/for-each-repo-in-gitdir-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2056\n-- \ngitgitgadget\n"},{"id":"536902","messageId":"86cd83f65b30aab3233e27b3e5c4f03041e68766.1771903950.git.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":"pull.2056.git.1771903950.gitgitgadget@gmail.com","subject":"[PATCH 1/2] for-each-repo: stop using the_repository","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-02-24T03:32:29Z","receivedAt":"2026-02-24T03:32:34Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThis is a simple refactor before digging into a bug fix.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/for-each-repo.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\nindex 325a7925f1..478ccf1287 100644\n--- a/builtin/for-each-repo.c\n+++ b/builtin/for-each-repo.c\n@@ -1,5 +1,3 @@\n-#define USE_THE_REPOSITORY_VARIABLE\n-\n #include \"builtin.h\"\n #include \"config.h\"\n #include \"gettext.h\"\n@@ -33,7 +31,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)\n int cmd_for_each_repo(int argc,\n \t\t const char **argv,\n \t\t const char *prefix,\n-\t\t struct repository *repo UNUSED)\n+\t\t struct repository *repo)\n {\n \tstatic const char *config_key = NULL;\n \tint keep_going = 0;\n@@ -55,7 +53,7 @@ int cmd_for_each_repo(int argc,\n \tif (!config_key)\n \t\tdie(_(\"missing --config=\"));\n \n-\terr = repo_config_get_string_multi(the_repository, config_key, &values);\n+\terr = repo_config_get_string_multi(repo, config_key, &values);\n \tif (err < 0)\n \t\tusage_msg_optf(_(\"got bad config --config=%s\"),\n \t\t\t for_each_repo_usage, options, config_key);\n-- \ngitgitgadget\n\n"},{"id":"536903","messageId":"a47f9e9386badd83f0f5820f33f5eed68ca5fd82.1771903950.git.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":"pull.2056.git.1771903950.gitgitgadget@gmail.com","subject":"[PATCH 2/2] for-each-repo: work correctly in a worktree","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-02-24T03:32:30Z","receivedAt":"2026-02-24T03:32:35Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen run in a worktree, the GIT_DIR directory is set in a different way\nthan in a typical repository. Show this by updating t0068 to include a\nworktree and add a test that runs from that worktree. This requires\nmoving the repo.key config into a global config instead of the base test\nrepository's local config (demonstrating that it worked with\nnon-worktree Git repositories).\n\nThe fix is simple: unset the environment variable before looping over\nthe repos.\n\nReported-by: Matthew Gabeler-Lee \nSigned-off-by: Derrick Stolee \n---\n builtin/for-each-repo.c | 4 ++++\n t/t0068-for-each-repo.sh | 22 +++++++++++++++++-----\n 2 files changed, 21 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\nindex 478ccf1287..f39b085d7d 100644\n--- a/builtin/for-each-repo.c\n+++ b/builtin/for-each-repo.c\n@@ -1,5 +1,6 @@\n #include \"builtin.h\"\n #include \"config.h\"\n+#include \"environment.h\"\n #include \"gettext.h\"\n #include \"parse-options.h\"\n #include \"path.h\"\n@@ -60,6 +61,9 @@ int cmd_for_each_repo(int argc,\n \telse if (err)\n \t\treturn 0;\n \n+\t/* Be sure to not pass GIT_DIR to children. */\n+\tunsetenv(GIT_DIR_ENVIRONMENT);\n+\n \tfor (size_t i = 0; i < values->nr; i++) {\n \t\tint ret = run_command_on_repo(values->items[i].string, argc, argv);\n \t\tif (ret) {\ndiff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh\nindex f2f3e50031..00b72dcac1 100755\n--- a/t/t0068-for-each-repo.sh\n+++ b/t/t0068-for-each-repo.sh\n@@ -7,12 +7,13 @@ test_description='git for-each-repo builtin'\n test_expect_success 'run based on configured value' '\n \tgit init one &&\n \tgit init two &&\n-\tgit init three &&\n+\tgit -C two worktree add --orphan ../three &&\n \tgit init ~/four &&\n \tgit -C two commit --allow-empty -m \"DID NOT RUN\" &&\n-\tgit config run.key \"$TRASH_DIRECTORY/one\" &&\n-\tgit config --add run.key \"$TRASH_DIRECTORY/three\" &&\n-\tgit config --add run.key \"~/four\" &&\n+\tgit config --global run.key \"$TRASH_DIRECTORY/one\" &&\n+\tgit config --global --add run.key \"$TRASH_DIRECTORY/three\" &&\n+\tgit config --global --add run.key \"~/four\" &&\n+\n \tgit for-each-repo --config=run.key commit --allow-empty -m \"ran\" &&\n \tgit -C one log -1 --pretty=format:%s >message &&\n \tgrep ran message &&\n@@ -22,6 +23,7 @@ test_expect_success 'run based on configured value' '\n \tgrep ran message &&\n \tgit -C ~/four log -1 --pretty=format:%s >message &&\n \tgrep ran message &&\n+\n \tgit for-each-repo --config=run.key -- commit --allow-empty -m \"ran again\" &&\n \tgit -C one log -1 --pretty=format:%s >message &&\n \tgrep again message &&\n@@ -30,7 +32,17 @@ test_expect_success 'run based on configured value' '\n \tgit -C three log -1 --pretty=format:%s >message &&\n \tgrep again message &&\n \tgit -C ~/four log -1 --pretty=format:%s >message &&\n-\tgrep again message\n+\tgrep again message &&\n+\n+\tgit -C three for-each-repo --config=run.key -- commit --allow-empty -m \"ran from worktree\" &&\n+\tgit -C one log -1 --pretty=format:%s >message &&\n+\tgrep worktree message &&\n+\tgit -C two log -1 --pretty=format:%s >message &&\n+\t! grep worktree message &&\n+\tgit -C three log -1 --pretty=format:%s >message &&\n+\tgrep worktree message &&\n+\tgit -C ~/four log -1 --pretty=format:%s >message &&\n+\tgrep worktree message\n '\n \n test_expect_success 'do nothing on empty config' '\n-- \ngitgitgadget\n"},{"id":"536904","messageId":"CAPig+cQcpJu_Z6VXbn5cee2AHmPHQaOLG39HFRG1SGnnY1cWFA@mail.gmail.com","threadId":"65065","inReplyTo":"a47f9e9386badd83f0f5820f33f5eed68ca5fd82.1771903950.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] for-each-repo: work correctly in a worktree","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2026-02-24T03:34:30Z","receivedAt":"2026-02-24T03:34:43Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"[Cc:+peff]\n\nOn Mon, Feb 23, 2026 at 10:32 PM Derrick Stolee via GitGitGadget\n wrote:\n> When run in a worktree, the GIT_DIR directory is set in a different way\n> than in a typical repository. Show this by updating t0068 to include a\n> worktree and add a test that runs from that worktree. This requires\n> moving the repo.key config into a global config instead of the base test\n> repository's local config (demonstrating that it worked with\n> non-worktree Git repositories).\n>\n> The fix is simple: unset the environment variable before looping over\n> the repos.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\n> @@ -60,6 +61,9 @@ int cmd_for_each_repo(int argc,\n> + /* Be sure to not pass GIT_DIR to children. */\n> + unsetenv(GIT_DIR_ENVIRONMENT);\n\nThis only unsets GIT_DIR. Is that sufficient in the general case?\nElsewhere, we recommend[*] unsetting all of Git's local environment\nvariables.\n\n[*]: From the \"githooks\" man page: \"Environment variables, such as\nGIT_DIR, GIT_WORK_TREE, etc., are exported so that Git commands run by\nthe hook can correctly locate the repository. If your hook needs to\ninvoke Git commands in a foreign repository or in a different working\ntree of the same repository, then it should clear these environment\nvariables so they do not interfere with Git operations at the foreign\nlocation. For example: `unset $(git rev-parse --local-env-vars)`\"\n"},{"id":"536947","messageId":"20260224091806.GC986367@coredump.intra.peff.net","threadId":"65065","inReplyTo":"CAPig+cQcpJu_Z6VXbn5cee2AHmPHQaOLG39HFRG1SGnnY1cWFA@mail.gmail.com","subject":"Re: [PATCH 2/2] for-each-repo: work correctly in a worktree","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-02-24T09:18:06Z","receivedAt":"2026-02-24T09:18:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 23, 2026 at 10:34:30PM -0500, Eric Sunshine wrote:\n\n> > diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\n> > @@ -60,6 +61,9 @@ int cmd_for_each_repo(int argc,\n> > + /* Be sure to not pass GIT_DIR to children. */\n> > + unsetenv(GIT_DIR_ENVIRONMENT);\n> \n> This only unsets GIT_DIR. Is that sufficient in the general case?\n> Elsewhere, we recommend[*] unsetting all of Git's local environment\n> variables.\n> \n> [*]: From the \"githooks\" man page: \"Environment variables, such as\n> GIT_DIR, GIT_WORK_TREE, etc., are exported so that Git commands run by\n> the hook can correctly locate the repository. If your hook needs to\n> invoke Git commands in a foreign repository or in a different working\n> tree of the same repository, then it should clear these environment\n> variables so they do not interfere with Git operations at the foreign\n> location. For example: `unset $(git rev-parse --local-env-vars)`\"\n\nYeah, agreed. There's another subtle issue, which is that this is\nunsetting GIT_DIR in the parent process. So any other code we call that\nis meant to run in the original repo might get confused. I can well\nbelieve there isn't any such code for a command like for-each-repo, but\nas a general principle, the change should be made in the sub-process.\n\nYou can stick the elements of local_repo_env into the \"env\" list of the\nchild_process struct. If you grep around, you can find some instances of\nthis.\n\nThere's an open question there of how to handle config in the\nenvironment, though. Depending on the sub-process, you may or may not\nwant such config to pass down to it. For for-each-repo, I'd guess that\nyou'd want:\n\n git -c foo.bar=baz for-each-repo ...\n\nto pass that foo.bar value. We do have a helper to handle that in\nrun-command.h:\n\n /**\n * Convenience function which prepares env for a command to be run in a\n * new repo. This adds all GIT_* environment variables to env with the\n * exception of GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT (which cause the\n * corresponding environment variables to be unset in the subprocess) and adds\n * an environment variable pointing to new_git_dir. See local_repo_env in\n * environment.h for more information.\n */\n void prepare_other_repo_env(struct strvec *env, const char *new_git_dir);\n\nDo be careful using it here, though. It expects to set GIT_DIR itself to\npoint to the new repo (which is passed in). But I'm not sure that's 100%\ncompatible with how for-each-repo works, which is using \"git -C $repo\"\nunder the hood, and letting the usual discovery happen.\n\nSo for a bare repo, you'd want to pass the repo directory. But for a\nnon-bare one, you'd want $repo/.git. And there are even more weird\ncorner cases, like the fact that using \"/my/repo/but/inside/a/subdir\"\nwith for-each-repo will find \"/my/repo\".\n\nSo you might need to refactor prepare_other_repo_env() to split out the\n\"everything but the config\" logic versus the \"set GIT_DIR\" logic. Or\njust inline the former in run_command_on_repo(), though it probably is\nbetter to keep the logic in one place (it's not many lines, but it has\nto know about all of the env variables that affect config).\n\nAlternatively, for-each-repo could do repo discovery itself on the paths\nit is passed, before calling sub-programs. That's a bigger change, but\npossibly it could or should be flagging an error for some cases? I\ndunno.\n\n-Peff\n"},{"id":"536948","messageId":"aZ1s7tONvd9wiYZV@pks.im","threadId":"65065","inReplyTo":"86cd83f65b30aab3233e27b3e5c4f03041e68766.1771903950.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] for-each-repo: stop using the_repository","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-24T09:18:38Z","receivedAt":"2026-02-24T09:18:44Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Feb 24, 2026 at 03:32:29AM +0000, Derrick Stolee via GitGitGadget wrote:\n> diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\n> index 325a7925f1..478ccf1287 100644\n> --- a/builtin/for-each-repo.c\n> +++ b/builtin/for-each-repo.c\n> @@ -1,5 +1,3 @@\n> -#define USE_THE_REPOSITORY_VARIABLE\n> -\n> #include \"builtin.h\"\n> #include \"config.h\"\n> #include \"gettext.h\"\n> @@ -33,7 +31,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)\n> int cmd_for_each_repo(int argc,\n> \t\t const char **argv,\n> \t\t const char *prefix,\n> -\t\t struct repository *repo UNUSED)\n> +\t\t struct repository *repo)\n> {\n> \tstatic const char *config_key = NULL;\n> \tint keep_going = 0;\n> @@ -55,7 +53,7 @@ int cmd_for_each_repo(int argc,\n> \tif (!config_key)\n> \t\tdie(_(\"missing --config=\"));\n> \n> -\terr = repo_config_get_string_multi(the_repository, config_key, &values);\n> +\terr = repo_config_get_string_multi(repo, config_key, &values);\n> \tif (err < 0)\n> \t\tusage_msg_optf(_(\"got bad config --config=%s\"),\n> \t\t\t for_each_repo_usage, options, config_key);\n\nThe command is marked as `RUN_SETUP_GENTLY`, so it may run in a context\nwhere there is no repository. In such cases, `repo` would be `NULL`, and\nthat would cause the command to segfault here, wouldn't it?\n\nPatrick\n"},{"id":"536949","messageId":"aZ1s8y7f7PS7FVOG@pks.im","threadId":"65065","inReplyTo":"CAPig+cQcpJu_Z6VXbn5cee2AHmPHQaOLG39HFRG1SGnnY1cWFA@mail.gmail.com","subject":"Re: [PATCH 2/2] for-each-repo: work correctly in a worktree","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-24T09:18:43Z","receivedAt":"2026-02-24T09:18:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Feb 23, 2026 at 10:34:30PM -0500, Eric Sunshine wrote:\n> [Cc:+peff]\n> \n> On Mon, Feb 23, 2026 at 10:32 PM Derrick Stolee via GitGitGadget\n> wrote:\n> > When run in a worktree, the GIT_DIR directory is set in a different way\n> > than in a typical repository. Show this by updating t0068 to include a\n> > worktree and add a test that runs from that worktree. This requires\n> > moving the repo.key config into a global config instead of the base test\n> > repository's local config (demonstrating that it worked with\n> > non-worktree Git repositories).\n> >\n> > The fix is simple: unset the environment variable before looping over\n> > the repos.\n> >\n> > Signed-off-by: Derrick Stolee \n> > ---\n> > diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\n> > @@ -60,6 +61,9 @@ int cmd_for_each_repo(int argc,\n> > + /* Be sure to not pass GIT_DIR to children. */\n> > + unsetenv(GIT_DIR_ENVIRONMENT);\n> \n> This only unsets GIT_DIR. Is that sufficient in the general case?\n> Elsewhere, we recommend[*] unsetting all of Git's local environment\n> variables.\n\nGood question indeed. We have the `local_repo_env` array that contains\nall the environment variables that may influence repository discovery.\n\nPatrick\n"},{"id":"536965","messageId":"614c8072-347a-4ba5-8796-4742868389d3@gmail.com","threadId":"65065","inReplyTo":"aZ1s7tONvd9wiYZV@pks.im","subject":"Re: [PATCH 1/2] for-each-repo: stop using the_repository","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-02-24T12:07:57Z","receivedAt":"2026-02-24T12:08:00Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 2/24/26 4:18 AM, Patrick Steinhardt wrote:\n> On Tue, Feb 24, 2026 at 03:32:29AM +0000, Derrick Stolee via GitGitGadget wrote:\n>> diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\n>> index 325a7925f1..478ccf1287 100644\n>> --- a/builtin/for-each-repo.c\n>> +++ b/builtin/for-each-repo.c\n>> @@ -1,5 +1,3 @@\n>> -#define USE_THE_REPOSITORY_VARIABLE\n>> -\n>> #include \"builtin.h\"\n>> #include \"config.h\"\n>> #include \"gettext.h\"\n>> @@ -33,7 +31,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)\n>> int cmd_for_each_repo(int argc,\n>> \t\t const char **argv,\n>> \t\t const char *prefix,\n>> -\t\t struct repository *repo UNUSED)\n>> +\t\t struct repository *repo)\n>> {\n>> \tstatic const char *config_key = NULL;\n>> \tint keep_going = 0;\n>> @@ -55,7 +53,7 @@ int cmd_for_each_repo(int argc,\n>> \tif (!config_key)\n>> \t\tdie(_(\"missing --config=\"));\n>> \n>> -\terr = repo_config_get_string_multi(the_repository, config_key, &values);\n>> +\terr = repo_config_get_string_multi(repo, config_key, &values);\n>> \tif (err < 0)\n>> \t\tusage_msg_optf(_(\"got bad config --config=%s\"),\n>> \t\t\t for_each_repo_usage, options, config_key);\n> \n> The command is marked as `RUN_SETUP_GENTLY`, so it may run in a context\n> where there is no repository. In such cases, `repo` would be `NULL`, and\n> that would cause the command to segfault here, wouldn't it?\n\nAh. That's an interesting subtlety of the setup that I did not know.\n\nI'll make sure this is covered in tests, because the current tests run in\nthe default test repo but our expected use case should be outside a repo.\n\nThanks,\n-Stolee\n\n"},{"id":"536966","messageId":"fce7662f-d741-41e1-93dd-f82e65e04f41@gmail.com","threadId":"65065","inReplyTo":"20260224091806.GC986367@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] for-each-repo: work correctly in a worktree","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-02-24T12:11:13Z","receivedAt":"2026-02-24T12:11:15Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 2/24/26 4:18 AM, Jeff King wrote:\n> On Mon, Feb 23, 2026 at 10:34:30PM -0500, Eric Sunshine wrote:\n> \n>>> diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\n>>> @@ -60,6 +61,9 @@ int cmd_for_each_repo(int argc,\n>>> + /* Be sure to not pass GIT_DIR to children. */\n>>> + unsetenv(GIT_DIR_ENVIRONMENT);\n>>\n>> This only unsets GIT_DIR. Is that sufficient in the general case?\n>> Elsewhere, we recommend[*] unsetting all of Git's local environment\n>> variables.\n>>\n>> [*]: From the \"githooks\" man page: \"Environment variables, such as\n>> GIT_DIR, GIT_WORK_TREE, etc., are exported so that Git commands run by\n>> the hook can correctly locate the repository. If your hook needs to\n>> invoke Git commands in a foreign repository or in a different working\n>> tree of the same repository, then it should clear these environment\n>> variables so they do not interfere with Git operations at the foreign\n>> location. For example: `unset $(git rev-parse --local-env-vars)`\"\n> \n> Yeah, agreed. There's another subtle issue, which is that this is\n> unsetting GIT_DIR in the parent process. So any other code we call that\n> is meant to run in the original repo might get confused. I can well\n> believe there isn't any such code for a command like for-each-repo, but\n> as a general principle, the change should be made in the sub-process.\n> \n> You can stick the elements of local_repo_env into the \"env\" list of the\n> child_process struct. If you grep around, you can find some instances of\n> this.\n> \n> There's an open question there of how to handle config in the\n> environment, though. Depending on the sub-process, you may or may not\n> want such config to pass down to it. For for-each-repo, I'd guess that\n> you'd want:\n> \n> git -c foo.bar=baz for-each-repo ...\n> \n> to pass that foo.bar value. We do have a helper to handle that in\n> run-command.h:\n> \n> /**\n> * Convenience function which prepares env for a command to be run in a\n> * new repo. This adds all GIT_* environment variables to env with the\n> * exception of GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT (which cause the\n> * corresponding environment variables to be unset in the subprocess) and adds\n> * an environment variable pointing to new_git_dir. See local_repo_env in\n> * environment.h for more information.\n> */\n> void prepare_other_repo_env(struct strvec *env, const char *new_git_dir);\n> \n> Do be careful using it here, though. It expects to set GIT_DIR itself to\n> point to the new repo (which is passed in). But I'm not sure that's 100%\n> compatible with how for-each-repo works, which is using \"git -C $repo\"\n> under the hood, and letting the usual discovery happen.\n> \n> So for a bare repo, you'd want to pass the repo directory. But for a\n> non-bare one, you'd want $repo/.git. And there are even more weird\n> corner cases, like the fact that using \"/my/repo/but/inside/a/subdir\"\n> with for-each-repo will find \"/my/repo\".\n> \n> So you might need to refactor prepare_other_repo_env() to split out the\n> \"everything but the config\" logic versus the \"set GIT_DIR\" logic. Or\n> just inline the former in run_command_on_repo(), though it probably is\n> better to keep the logic in one place (it's not many lines, but it has\n> to know about all of the env variables that affect config).\n\nThanks for the recommendations. I'll come back with a more sophisticated\nv2 that handles these issues.\n\n> Alternatively, for-each-repo could do repo discovery itself on the paths\n> it is passed, before calling sub-programs. That's a bigger change, but\n> possibly it could or should be flagging an error for some cases? I\n> dunno.\nI'm surprised that passing '-C ' doesn't already overwrite these\nvariables but I suppose environment variables override arguments in this\ncase. (This is the root of the bug.)\n\nThanks,\n-Stolee\n\n"},{"id":"537020","messageId":"pull.2056.v2.git.1771968924.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":"pull.2056.git.1771903950.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] for-each-repo: work correctly in a worktree","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-02-24T21:35:22Z","receivedAt":"2026-02-24T21:35:27Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This was reported by Matthew [1] and is a quick fix.\n\n[1]\nhttps://lore.kernel.org/git/CABpCjbY=wpStuhxqRJ5TSNV3A-CmN-g-xZGJOQGSSv3GYhs2fQ@mail.gmail.com/\n\nI also took the liberty of removing the_repository as I wanted to make sure\nthat wasn't involved here.\n\nThanks, -Stolee\n\nDerrick Stolee (2):\n for-each-repo: test outside of repo context\n for-each-repo: work correctly in a worktree\n\n builtin/for-each-repo.c | 33 ++++++++++++++++++++++++++++++---\n t/t0068-for-each-repo.sh | 33 ++++++++++++++++++++++++---------\n 2 files changed, 54 insertions(+), 12 deletions(-)\n\n\nbase-commit: 67ad42147a7acc2af6074753ebd03d904476118f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2056%2Fderrickstolee%2Ffor-each-repo-in-gitdir-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2056/derrickstolee/for-each-repo-in-gitdir-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/2056\n\nRange-diff vs v1:\n\n 1: 86cd83f65b < -: ---------- for-each-repo: stop using the_repository\n -: ---------- > 1: 6e9d4f3029 for-each-repo: test outside of repo context\n 2: a47f9e9386 ! 2: 4e3f4aa6cd for-each-repo: work correctly in a worktree\n @@ Commit message\n repository's local config (demonstrating that it worked with\n non-worktree Git repositories).\n \n - The fix is simple: unset the environment variable before looping over\n - the repos.\n + We need to be careful to unset the local Git environment variables and\n + let the child process rediscover them, while also reinstating those\n + variables in the parent process afterwards. Update run_command_on_repo()\n + to store, unset, then reset the non-NULL variables.\n \n Reported-by: Matthew Gabeler-Lee \n Signed-off-by: Derrick Stolee \n \n ## builtin/for-each-repo.c ##\n @@\n + \n #include \"builtin.h\"\n #include \"config.h\"\n +#include \"environment.h\"\n #include \"gettext.h\"\n #include \"parse-options.h\"\n #include \"path.h\"\n -@@ builtin/for-each-repo.c: int cmd_for_each_repo(int argc,\n - \telse if (err)\n - \t\treturn 0;\n +@@ builtin/for-each-repo.c: static const char * const for_each_repo_usage[] = {\n + \n + static int run_command_on_repo(const char *path, int argc, const char ** argv)\n + {\n +-\tint i;\n ++\tint res;\n + \tstruct child_process child = CHILD_PROCESS_INIT;\n ++\tchar **envvars;\n ++\tsize_t envvar_nr = 0;\n + \tchar *abspath = interpolate_path(path, 0);\n + \n ++\twhile (local_repo_env[envvar_nr])\n ++\t\tenvvar_nr++;\n ++\n ++\tCALLOC_ARRAY(envvars, envvar_nr);\n ++\n ++\tfor (size_t i = 0; i < envvar_nr; i++) {\n ++\t\tenvvars[i] = getenv(local_repo_env[i]);\n ++\n ++\t\tif (envvars[i]) {\n ++\t\t\tunsetenv(local_repo_env[i]);\n ++\t\t\tenvvars[i] = xstrdup(envvars[i]);\n ++\t\t}\n ++\t}\n ++\n + \tchild.git_cmd = 1;\n + \tstrvec_pushl(&child.args, \"-C\", abspath, NULL);\n + \n +-\tfor (i = 0; i < argc; i++)\n ++\tfor (int i = 0; i < argc; i++)\n + \t\tstrvec_push(&child.args, argv[i]);\n \n -+\t/* Be sure to not pass GIT_DIR to children. */\n -+\tunsetenv(GIT_DIR_ENVIRONMENT);\n + \tfree(abspath);\n + \n +-\treturn run_command(&child);\n ++\tres = run_command(&child);\n ++\n ++\tfor (size_t i = 0; i < envvar_nr; i++) {\n ++\t\tif (envvars[i]) {\n ++\t\t\tsetenv(local_repo_env[i], envvars[i], 1);\n ++\t\t\tfree(envvars[i]);\n ++\t\t}\n ++\t}\n +\n - \tfor (size_t i = 0; i < values->nr; i++) {\n - \t\tint ret = run_command_on_repo(values->items[i].string, argc, argv);\n - \t\tif (ret) {\n ++\tfree(envvars);\n ++\treturn res;\n + }\n + \n + int cmd_for_each_repo(int argc,\n \n ## t/t0068-for-each-repo.sh ##\n -@@ t/t0068-for-each-repo.sh: test_description='git for-each-repo builtin'\n +@@ t/t0068-for-each-repo.sh: TEST_NO_CREATE_REPO=1\n test_expect_success 'run based on configured value' '\n \tgit init one &&\n \tgit init two &&\n @@ t/t0068-for-each-repo.sh: test_description='git for-each-repo builtin'\n +\tgit -C two worktree add --orphan ../three &&\n \tgit init ~/four &&\n \tgit -C two commit --allow-empty -m \"DID NOT RUN\" &&\n --\tgit config run.key \"$TRASH_DIRECTORY/one\" &&\n --\tgit config --add run.key \"$TRASH_DIRECTORY/three\" &&\n --\tgit config --add run.key \"~/four\" &&\n -+\tgit config --global run.key \"$TRASH_DIRECTORY/one\" &&\n -+\tgit config --global --add run.key \"$TRASH_DIRECTORY/three\" &&\n -+\tgit config --global --add run.key \"~/four\" &&\n -+\n - \tgit for-each-repo --config=run.key commit --allow-empty -m \"ran\" &&\n - \tgit -C one log -1 --pretty=format:%s >message &&\n - \tgrep ran message &&\n -@@ t/t0068-for-each-repo.sh: test_expect_success 'run based on configured value' '\n - \tgrep ran message &&\n - \tgit -C ~/four log -1 --pretty=format:%s >message &&\n - \tgrep ran message &&\n -+\n - \tgit for-each-repo --config=run.key -- commit --allow-empty -m \"ran again\" &&\n - \tgit -C one log -1 --pretty=format:%s >message &&\n - \tgrep again message &&\n + \tgit config --global run.key \"$TRASH_DIRECTORY/one\" &&\n @@ t/t0068-for-each-repo.sh: test_expect_success 'run based on configured value' '\n \tgit -C three log -1 --pretty=format:%s >message &&\n \tgrep again message &&\n\n-- \ngitgitgadget\n"},{"id":"537021","messageId":"6e9d4f3029daa2c0068bb16939b943e7ac924222.1771968924.git.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":"pull.2056.v2.git.1771968924.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] for-each-repo: test outside of repo context","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-02-24T21:35:23Z","receivedAt":"2026-02-24T21:35:28Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe 'git for-each-repo' tool is frequently run outside of a repo context\nin the real world. For example, it powers background maintenance.\nDespite this typical case, we have not been testing it without a local\nrepository.\n\nUpdate t0068 to stop creating a test repo and to use global config\neverywhere. This has some subtle changes to test across the file.\n\nThis was noticed because an earlier attempt to remove the_repository\nfrom builtin/for-each-repo.c did not catch a segmentation fault since\nthe passed 'repo' is NULL. This use of the_repository will need to stay\nuntil we have a better way to handle config queries outside of a repo\ncontext. Similar use still exists in builtin/config.c for the same\nreason.\n\nSigned-off-by: Derrick Stolee \n---\n t/t0068-for-each-repo.sh | 19 ++++++++++++-------\n 1 file changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh\nindex f2f3e50031..512af34c82 100755\n--- a/t/t0068-for-each-repo.sh\n+++ b/t/t0068-for-each-repo.sh\n@@ -2,6 +2,9 @@\n \n test_description='git for-each-repo builtin'\n \n+# We need to test running 'git for-each-repo' outside of a repo context.\n+TEST_NO_CREATE_REPO=1\n+\n . ./test-lib.sh\n \n test_expect_success 'run based on configured value' '\n@@ -10,9 +13,10 @@ test_expect_success 'run based on configured value' '\n \tgit init three &&\n \tgit init ~/four &&\n \tgit -C two commit --allow-empty -m \"DID NOT RUN\" &&\n-\tgit config run.key \"$TRASH_DIRECTORY/one\" &&\n-\tgit config --add run.key \"$TRASH_DIRECTORY/three\" &&\n-\tgit config --add run.key \"~/four\" &&\n+\tgit config --global run.key \"$TRASH_DIRECTORY/one\" &&\n+\tgit config --global --add run.key \"$TRASH_DIRECTORY/three\" &&\n+\tgit config --global --add run.key \"~/four\" &&\n+\n \tgit for-each-repo --config=run.key commit --allow-empty -m \"ran\" &&\n \tgit -C one log -1 --pretty=format:%s >message &&\n \tgrep ran message &&\n@@ -22,6 +26,7 @@ test_expect_success 'run based on configured value' '\n \tgrep ran message &&\n \tgit -C ~/four log -1 --pretty=format:%s >message &&\n \tgrep ran message &&\n+\n \tgit for-each-repo --config=run.key -- commit --allow-empty -m \"ran again\" &&\n \tgit -C one log -1 --pretty=format:%s >message &&\n \tgrep again message &&\n@@ -46,7 +51,7 @@ test_expect_success 'error on bad config keys' '\n '\n \n test_expect_success 'error on NULL value for config keys' '\n-\tcat >>.git/config <<-\\EOF &&\n+\tcat >>.gitconfig <<-\\EOF &&\n \t[empty]\n \t\tkey\n \tEOF\n@@ -59,8 +64,8 @@ test_expect_success 'error on NULL value for config keys' '\n '\n \n test_expect_success '--keep-going' '\n-\tgit config keep.going non-existing &&\n-\tgit config --add keep.going . &&\n+\tgit config --global keep.going non-existing &&\n+\tgit config --global --add keep.going one &&\n \n \ttest_must_fail git for-each-repo --config=keep.going \\\n \t\t-- branch >out 2>err &&\n@@ -70,7 +75,7 @@ test_expect_success '--keep-going' '\n \ttest_must_fail git for-each-repo --config=keep.going --keep-going \\\n \t\t-- branch >out 2>err &&\n \ttest_grep \"cannot change to .*non-existing\" err &&\n-\tgit branch >expect &&\n+\tgit -C one branch >expect &&\n \ttest_cmp expect out\n '\n \n-- \ngitgitgadget\n\n"},{"id":"537022","messageId":"4e3f4aa6cd36f779c6c1d6b4f30bb68ed807b9da.1771968924.git.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":"pull.2056.v2.git.1771968924.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] for-each-repo: work correctly in a worktree","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-02-24T21:35:24Z","receivedAt":"2026-02-24T21:35:29Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen run in a worktree, the GIT_DIR directory is set in a different way\nthan in a typical repository. Show this by updating t0068 to include a\nworktree and add a test that runs from that worktree. This requires\nmoving the repo.key config into a global config instead of the base test\nrepository's local config (demonstrating that it worked with\nnon-worktree Git repositories).\n\nWe need to be careful to unset the local Git environment variables and\nlet the child process rediscover them, while also reinstating those\nvariables in the parent process afterwards. Update run_command_on_repo()\nto store, unset, then reset the non-NULL variables.\n\nReported-by: Matthew Gabeler-Lee \nSigned-off-by: Derrick Stolee \n---\n builtin/for-each-repo.c | 33 ++++++++++++++++++++++++++++++---\n t/t0068-for-each-repo.sh | 14 ++++++++++++--\n 2 files changed, 42 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\nindex 325a7925f1..3f3e71979c 100644\n--- a/builtin/for-each-repo.c\n+++ b/builtin/for-each-repo.c\n@@ -2,6 +2,7 @@\n \n #include \"builtin.h\"\n #include \"config.h\"\n+#include \"environment.h\"\n #include \"gettext.h\"\n #include \"parse-options.h\"\n #include \"path.h\"\n@@ -15,19 +16,45 @@ static const char * const for_each_repo_usage[] = {\n \n static int run_command_on_repo(const char *path, int argc, const char ** argv)\n {\n-\tint i;\n+\tint res;\n \tstruct child_process child = CHILD_PROCESS_INIT;\n+\tchar **envvars;\n+\tsize_t envvar_nr = 0;\n \tchar *abspath = interpolate_path(path, 0);\n \n+\twhile (local_repo_env[envvar_nr])\n+\t\tenvvar_nr++;\n+\n+\tCALLOC_ARRAY(envvars, envvar_nr);\n+\n+\tfor (size_t i = 0; i < envvar_nr; i++) {\n+\t\tenvvars[i] = getenv(local_repo_env[i]);\n+\n+\t\tif (envvars[i]) {\n+\t\t\tunsetenv(local_repo_env[i]);\n+\t\t\tenvvars[i] = xstrdup(envvars[i]);\n+\t\t}\n+\t}\n+\n \tchild.git_cmd = 1;\n \tstrvec_pushl(&child.args, \"-C\", abspath, NULL);\n \n-\tfor (i = 0; i < argc; i++)\n+\tfor (int i = 0; i < argc; i++)\n \t\tstrvec_push(&child.args, argv[i]);\n \n \tfree(abspath);\n \n-\treturn run_command(&child);\n+\tres = run_command(&child);\n+\n+\tfor (size_t i = 0; i < envvar_nr; i++) {\n+\t\tif (envvars[i]) {\n+\t\t\tsetenv(local_repo_env[i], envvars[i], 1);\n+\t\t\tfree(envvars[i]);\n+\t\t}\n+\t}\n+\n+\tfree(envvars);\n+\treturn res;\n }\n \n int cmd_for_each_repo(int argc,\ndiff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh\nindex 512af34c82..d55557a934 100755\n--- a/t/t0068-for-each-repo.sh\n+++ b/t/t0068-for-each-repo.sh\n@@ -10,7 +10,7 @@ TEST_NO_CREATE_REPO=1\n test_expect_success 'run based on configured value' '\n \tgit init one &&\n \tgit init two &&\n-\tgit init three &&\n+\tgit -C two worktree add --orphan ../three &&\n \tgit init ~/four &&\n \tgit -C two commit --allow-empty -m \"DID NOT RUN\" &&\n \tgit config --global run.key \"$TRASH_DIRECTORY/one\" &&\n@@ -35,7 +35,17 @@ test_expect_success 'run based on configured value' '\n \tgit -C three log -1 --pretty=format:%s >message &&\n \tgrep again message &&\n \tgit -C ~/four log -1 --pretty=format:%s >message &&\n-\tgrep again message\n+\tgrep again message &&\n+\n+\tgit -C three for-each-repo --config=run.key -- commit --allow-empty -m \"ran from worktree\" &&\n+\tgit -C one log -1 --pretty=format:%s >message &&\n+\tgrep worktree message &&\n+\tgit -C two log -1 --pretty=format:%s >message &&\n+\t! grep worktree message &&\n+\tgit -C three log -1 --pretty=format:%s >message &&\n+\tgrep worktree message &&\n+\tgit -C ~/four log -1 --pretty=format:%s >message &&\n+\tgrep worktree message\n '\n \n test_expect_success 'do nothing on empty config' '\n-- \ngitgitgadget\n"},{"id":"537025","messageId":"xmqqv7flervq.fsf@gitster.g","threadId":"65065","inReplyTo":"4e3f4aa6cd36f779c6c1d6b4f30bb68ed807b9da.1771968924.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-24T21:47:05Z","receivedAt":"2026-02-24T21:47:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> static int run_command_on_repo(const char *path, int argc, const char ** argv)\n> {\n> -\tint i;\n> +\tint res;\n> \tstruct child_process child = CHILD_PROCESS_INIT;\n> +\tchar **envvars;\n> +\tsize_t envvar_nr = 0;\n> \tchar *abspath = interpolate_path(path, 0);\n> \n> +\twhile (local_repo_env[envvar_nr])\n> +\t\tenvvar_nr++;\n> +\n> +\tCALLOC_ARRAY(envvars, envvar_nr);\n> +\n> +\tfor (size_t i = 0; i < envvar_nr; i++) {\n> +\t\tenvvars[i] = getenv(local_repo_env[i]);\n> +\n> +\t\tif (envvars[i]) {\n> +\t\t\tunsetenv(local_repo_env[i]);\n> +\t\t\tenvvars[i] = xstrdup(envvars[i]);\n> +\t\t}\n> +\t}\n>\n> \tchild.git_cmd = 1;\n> \tstrvec_pushl(&child.args, \"-C\", abspath, NULL);\n> \n> -\tfor (i = 0; i < argc; i++)\n> +\tfor (int i = 0; i < argc; i++)\n> \t\tstrvec_push(&child.args, argv[i]);\n> \n> \tfree(abspath);\n> \n> -\treturn run_command(&child);\n> +\tres = run_command(&child);\n> +\n> +\tfor (size_t i = 0; i < envvar_nr; i++) {\n> +\t\tif (envvars[i]) {\n> +\t\t\tsetenv(local_repo_env[i], envvars[i], 1);\n> +\t\t\tfree(envvars[i]);\n> +\t\t}\n> +\t}\n> +\n> +\tfree(envvars);\n> +\treturn res;\n> }\n \n\nDoesn't run_command() let you unsetenv in the child without\naffecting the parent process?\n\nLooking at run-command.c:prep_childenv(), it seems that you can pass\n\"VAR=VAL\" to \"export VAR=VAL\" in the child, and pass \"VAR\" to \"unset\nVAR\" in the child.\n\nOr is it essential to unset in both parent and child while the child\nis working and that is why we unset in the parent and then restore\nlater? I find this highly confusing.\n\n\n> int cmd_for_each_repo(int argc,\n> diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh\n> index 512af34c82..d55557a934 100755\n> --- a/t/t0068-for-each-repo.sh\n> +++ b/t/t0068-for-each-repo.sh\n> @@ -10,7 +10,7 @@ TEST_NO_CREATE_REPO=1\n> test_expect_success 'run based on configured value' '\n> \tgit init one &&\n> \tgit init two &&\n> -\tgit init three &&\n> +\tgit -C two worktree add --orphan ../three &&\n> \tgit init ~/four &&\n> \tgit -C two commit --allow-empty -m \"DID NOT RUN\" &&\n> \tgit config --global run.key \"$TRASH_DIRECTORY/one\" &&\n> @@ -35,7 +35,17 @@ test_expect_success 'run based on configured value' '\n> \tgit -C three log -1 --pretty=format:%s >message &&\n> \tgrep again message &&\n> \tgit -C ~/four log -1 --pretty=format:%s >message &&\n> -\tgrep again message\n> +\tgrep again message &&\n> +\n> +\tgit -C three for-each-repo --config=run.key -- commit --allow-empty -m \"ran from worktree\" &&\n> +\tgit -C one log -1 --pretty=format:%s >message &&\n> +\tgrep worktree message &&\n> +\tgit -C two log -1 --pretty=format:%s >message &&\n> +\t! grep worktree message &&\n> +\tgit -C three log -1 --pretty=format:%s >message &&\n> +\tgrep worktree message &&\n> +\tgit -C ~/four log -1 --pretty=format:%s >message &&\n> +\tgrep worktree message\n> '\n> \n> test_expect_success 'do nothing on empty config' '\n"},{"id":"537081","messageId":"eeebc30a-40bf-40ac-a16b-ca5e128c3c01@gmail.com","threadId":"65065","inReplyTo":"xmqqv7flervq.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-02-25T11:44:51Z","receivedAt":"2026-02-25T11:44:54Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 2/24/26 4:47 PM, Junio C Hamano wrote:\n\n> Doesn't run_command() let you unsetenv in the child without\n> affecting the parent process?\n> \n> Looking at run-command.c:prep_childenv(), it seems that you can pass\n> \"VAR=VAL\" to \"export VAR=VAL\" in the child, and pass \"VAR\" to \"unset\n> VAR\" in the child.\n\nYou're right. Here's a much simpler implementation:\n\nstatic int run_command_on_repo(const char *path, int argc, const char ** argv)\n{\n\tint i = 0;\n\tstruct child_process child = CHILD_PROCESS_INIT;\n\tchar *abspath = interpolate_path(path, 0);\n\n\twhile (local_repo_env[i]) {\n\t\tstrvec_push(&child.env, local_repo_env[i]);\n\t\ti++;\n\t}\n\n\tchild.git_cmd = 1;\n\tstrvec_pushl(&child.args, \"-C\", abspath, NULL);\n\n\tfor (i = 0; i < argc; i++)\n\t\tstrvec_push(&child.args, argv[i]);\n\n\tfree(abspath);\n\n\treturn run_command(&child);\n}\n\n> Or is it essential to unset in both parent and child while the child\n> is working and that is why we unset in the parent and then restore\n> later? I find this highly confusing.\n\nNope, not necessary to adjust it in the parent. The simpler version\nabove works in my test case. I'll apply it to an upcoming v3, but\nwill wait a couple of days to see if there is any more feedback on\nthis v2.5 before doing so.\n\nThanks,\n-Stolee\n\n"},{"id":"537083","messageId":"20260225131344.GA2139176@coredump.intra.peff.net","threadId":"65065","inReplyTo":"eeebc30a-40bf-40ac-a16b-ca5e128c3c01@gmail.com","subject":"Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-02-25T13:13:44Z","receivedAt":"2026-02-25T13:13:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 25, 2026 at 06:44:51AM -0500, Derrick Stolee wrote:\n\n> > Looking at run-command.c:prep_childenv(), it seems that you can pass\n> > \"VAR=VAL\" to \"export VAR=VAL\" in the child, and pass \"VAR\" to \"unset\n> > VAR\" in the child.\n> \n> You're right. Here's a much simpler implementation:\n> \n> static int run_command_on_repo(const char *path, int argc, const char ** argv)\n> {\n> \tint i = 0;\n> \tstruct child_process child = CHILD_PROCESS_INIT;\n> \tchar *abspath = interpolate_path(path, 0);\n> \n> \twhile (local_repo_env[i]) {\n> \t\tstrvec_push(&child.env, local_repo_env[i]);\n> \t\ti++;\n> \t}\n\nYou can actually just use strvec_pushv() to do this as a one-liner\n(though annoyingly you need a cast because of how const works; you can\neasily find an example with grep).\n\nBut I really think you should consider keeping config-related variables\nin place, as prepare_other_repo_env() does. Otherwise something like:\n\n git -c pack.threads=1 for-each-repo repack -ad\n\nwill ignore that config in the sub-processes (whereas it currently is\nrespected).\n\nAnd for that, you do need to loop yourself.\n\n-Peff\n"},{"id":"537084","messageId":"20260225132326.GB2139176@coredump.intra.peff.net","threadId":"65065","inReplyTo":"fce7662f-d741-41e1-93dd-f82e65e04f41@gmail.com","subject":"Re: [PATCH 2/2] for-each-repo: work correctly in a worktree","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-02-25T13:23:26Z","receivedAt":"2026-02-25T13:23:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 24, 2026 at 07:11:13AM -0500, Derrick Stolee wrote:\n\n> > it is passed, before calling sub-programs. That's a bigger change, but\n> > possibly it could or should be flagging an error for some cases? I\n> > dunno.\n> I'm surprised that passing '-C ' doesn't already overwrite these\n> variables but I suppose environment variables override arguments in this\n> case. (This is the root of the bug.)\n\nI can see why you'd be surprised if you think of \"-C\" as \"change to this\ngit repo\". But it really is \"change to this directory\". It is perfectly\nOK to \"git -C\" into a non-toplevel directory of a repo (and continue\nrespecting any repo discovery that happened already and is in the\nenvironment), or even weird stuff like:\n\n GIT_DIR=/some/repo.git git -C /some/worktree add foo\n\nWhat you almost kind-of want is \"--git-dir\", except it puts the onus on\nthe caller to find the actual repo directory (so detecting bare vs\ndiscovering the .git). Part of the point of introducing -C long ago was\nbecause --git-dir was so annoying to use.\n\nProbably there is room for some middle-ground option, which is \"do repo\ndetection starting in this directory and use that as the --git-dir\" (and\nI guess also do worktree discovery in the same way). But I don't think\nwe would ever switch -C to that. It would almost certainly break lots of\npeople if we changed it now.\n\n-Peff\n"},{"id":"537199","messageId":"08c6e203-3444-45c7-9bc9-cc2590be30c3@gmail.com","threadId":"65065","inReplyTo":"20260225131344.GA2139176@coredump.intra.peff.net","subject":"Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-02-26T15:29:47Z","receivedAt":"2026-02-26T15:29:49Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 2/25/2026 8:13 AM, Jeff King wrote:\n> On Wed, Feb 25, 2026 at 06:44:51AM -0500, Derrick Stolee wrote:\n> \n>>> Looking at run-command.c:prep_childenv(), it seems that you can pass\n>>> \"VAR=VAL\" to \"export VAR=VAL\" in the child, and pass \"VAR\" to \"unset\n>>> VAR\" in the child.\n\n> But I really think you should consider keeping config-related variables\n> in place, as prepare_other_repo_env() does. Otherwise something like:\n> \n> git -c pack.threads=1 for-each-repo repack -ad\n> \n> will ignore that config in the sub-processes (whereas it currently is\n> respected).\n> \n> And for that, you do need to loop yourself.\n\nGreat point. Here's another attempt:\n\nstatic int run_command_on_repo(const char *path, int argc, const char ** argv)\n{\n\tint i = 0;\n\tstruct child_process child = CHILD_PROCESS_INIT;\n\tchar *abspath = interpolate_path(path, 0);\n\n\twhile (local_repo_env[i]) {\n\t\t/*\n\t\t * Preserve pre-builtin options:\n\t\t * - CONFIG_ENVIRONMENT, CONFIG_DATA_ENVIRONMENT, and\n\t\t * CONFIG_COUNT_ENVIRONMENT persist -c =\n\t\t * and --config-env== options.\n\t\t * - NO_REPLACE_OBJECTS_ENVIRONMENT persists the\n\t\t * --no-replace-objects option.\n\t\t *\n\t\t * Note that the following options are not in local_repo_env:\n\t\t * - EXEC_PATH_ENVIRONMENT persists --exec-path option.\n\t\t */\n\t\tif (strncmp(local_repo_env[i], \"CONFIG_\", 7) &&\n\t\t strcmp(local_repo_env[i], NO_REPLACE_OBJECTS_ENVIRONMENT))\n\t\t\tstrvec_push(&child.env, local_repo_env[i]);\n\n\t\ti++;\n\t}\n\n\tchild.git_cmd = 1;\n\tstrvec_pushl(&child.args, \"-C\", abspath, NULL);\n\n\tfor (i = 0; i < argc; i++)\n\t\tstrvec_push(&child.args, argv[i]);\n\n\tfree(abspath);\n\n\treturn run_command(&child);\n}\n\nThis comment details my findings from comparing the list in\nlocal_repo_env[] and the top-level options listed in\nDocumentation/git.adoc. That's how I was able to find that\n--exec-path sets an environment variable that's NOT in the\nlist and we want to be sure we don't set it.\n\nShould we add the comparison to EXEC_PATH_ENVIRONMENT as a\nprecaution to make sure it's not added to local_repo_env in\nthe future? Or is that too defensive?\n\nThanks,\n-Stolee\n\n"},{"id":"537203","messageId":"xmqqsean4gsc.fsf@gitster.g","threadId":"65065","inReplyTo":"08c6e203-3444-45c7-9bc9-cc2590be30c3@gmail.com","subject":"Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-26T16:21:23Z","receivedAt":"2026-02-26T16:21:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> On 2/25/2026 8:13 AM, Jeff King wrote:\n>> On Wed, Feb 25, 2026 at 06:44:51AM -0500, Derrick Stolee wrote:\n>> \n>>>> Looking at run-command.c:prep_childenv(), it seems that you can pass\n>>>> \"VAR=VAL\" to \"export VAR=VAL\" in the child, and pass \"VAR\" to \"unset\n>>>> VAR\" in the child.\n>\n>> But I really think you should consider keeping config-related variables\n>> in place, as prepare_other_repo_env() does. Otherwise something like:\n>> \n>> git -c pack.threads=1 for-each-repo repack -ad\n>> \n>> will ignore that config in the sub-processes (whereas it currently is\n>> respected).\n>> \n>> And for that, you do need to loop yourself.\n>\n> Great point. Here's another attempt:\n>\n> static int run_command_on_repo(const char *path, int argc, const char ** argv)\n> {\n> \tint i = 0;\n> \tstruct child_process child = CHILD_PROCESS_INIT;\n> \tchar *abspath = interpolate_path(path, 0);\n>\n> \twhile (local_repo_env[i]) {\n> \t\t/*\n> \t\t * Preserve pre-builtin options:\n> \t\t * - CONFIG_ENVIRONMENT, CONFIG_DATA_ENVIRONMENT, and\n> \t\t * CONFIG_COUNT_ENVIRONMENT persist -c =\n> \t\t * and --config-env== options.\n> \t\t * - NO_REPLACE_OBJECTS_ENVIRONMENT persists the\n> \t\t * --no-replace-objects option.\n> \t\t *\n> \t\t * Note that the following options are not in local_repo_env:\n> \t\t * - EXEC_PATH_ENVIRONMENT persists --exec-path option.\n> \t\t */\n> \t\tif (strncmp(local_repo_env[i], \"CONFIG_\", 7) &&\n\nMinor nit: !starts_with() lets you avoid counting bytes yourself and\nhardcoding \"7\" here.\n\n> \t\t strcmp(local_repo_env[i], NO_REPLACE_OBJECTS_ENVIRONMENT))\n> \t\t\tstrvec_push(&child.env, local_repo_env[i]);\n>\n> \t\ti++;\n> \t}\n>\n> \tchild.git_cmd = 1;\n> \tstrvec_pushl(&child.args, \"-C\", abspath, NULL);\n>\n> \tfor (i = 0; i < argc; i++)\n> \t\tstrvec_push(&child.args, argv[i]);\n\nIf argv[argc] == NULL, then here is where we want strvec_pushv().\n\n> \tfree(abspath);\n>\n> \treturn run_command(&child);\n> }\n>\n> This comment details my findings from comparing the list in\n> local_repo_env[] and the top-level options listed in\n> Documentation/git.adoc. That's how I was able to find that\n> --exec-path sets an environment variable that's NOT in the\n> list and we want to be sure we don't set it.\n\nHmph, wouldn't we want to use specified exec-path inside ...\n\n git --exec-path=~/my/git/libexec for-each-repo sh -c \"do things\"\n\n... \"do things\" script when we find Git related binaries? Or am I\nnot getting what you are describing here?\n\n> Should we add the comparison to EXEC_PATH_ENVIRONMENT as a\n> precaution to make sure it's not added to local_repo_env in\n> the future? Or is that too defensive?\n>\n> Thanks,\n> -Stolee\n"},{"id":"537215","messageId":"1ee5927a-c90d-4a4b-a468-5be3644481bc@gmail.com","threadId":"65065","inReplyTo":"xmqqsean4gsc.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-02-26T18:14:28Z","receivedAt":"2026-02-26T18:14:23Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 26/02/2026 16:21, Junio C Hamano wrote:\n> Derrick Stolee writes:\n> \n>> static int run_command_on_repo(const char *path, int argc, const char ** argv)\n>> {\n>> \tint i = 0;\n>> \tstruct child_process child = CHILD_PROCESS_INIT;\n>> \tchar *abspath = interpolate_path(path, 0);\n>>\n>> \twhile (local_repo_env[i]) {\n>> \t\t/*\n>> \t\t * Preserve pre-builtin options:\n>> \t\t * - CONFIG_ENVIRONMENT, CONFIG_DATA_ENVIRONMENT, and\n>> \t\t * CONFIG_COUNT_ENVIRONMENT persist -c =\n>> \t\t * and --config-env== options.\n>> \t\t * - NO_REPLACE_OBJECTS_ENVIRONMENT persists the\n>> \t\t * --no-replace-objects option.\n>> \t\t *\n>> \t\t * Note that the following options are not in local_repo_env:\n>> \t\t * - EXEC_PATH_ENVIRONMENT persists --exec-path option.\n>> \t\t */\n>> \t\tif (strncmp(local_repo_env[i], \"CONFIG_\", 7) &&\n> \n> Minor nit: !starts_with() lets you avoid counting bytes yourself and\n> hardcoding \"7\" here.\n\nMore seriously it should be looking for strings starting with \n\"GIT_CONFIG_\", not the name of the preprocessor definitions.\n\nThanks\n\nPhillip\n\n"},{"id":"537337","messageId":"xmqqqzq6otx7.fsf@gitster.g","threadId":"65065","inReplyTo":"1ee5927a-c90d-4a4b-a468-5be3644481bc@gmail.com","subject":"Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-27T19:41:56Z","receivedAt":"2026-02-27T19:41:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n>>> \t\t * Note that the following options are not in local_repo_env:\n>>> \t\t * - EXEC_PATH_ENVIRONMENT persists --exec-path option.\n>>> \t\t */\n>>> \t\tif (strncmp(local_repo_env[i], \"CONFIG_\", 7) &&\n>> \n>> Minor nit: !starts_with() lets you avoid counting bytes yourself and\n>> hardcoding \"7\" here.\n>\n> More seriously it should be looking for strings starting with \n> \"GIT_CONFIG_\", not the name of the preprocessor definitions.\n\nThanks. I missed that completely.\n"},{"id":"537347","messageId":"3d574b51-78e2-4850-81dc-5c55b9562c02@gmail.com","threadId":"65065","inReplyTo":"xmqqqzq6otx7.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-02-27T22:28:20Z","receivedAt":"2026-02-27T22:28:22Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 2/27/26 2:41 PM, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>>>> \t\t * Note that the following options are not in local_repo_env:\n>>>> \t\t * - EXEC_PATH_ENVIRONMENT persists --exec-path option.\n>>>> \t\t */\n>>>> \t\tif (strncmp(local_repo_env[i], \"CONFIG_\", 7) &&\n>>>\n>>> Minor nit: !starts_with() lets you avoid counting bytes yourself and\n>>> hardcoding \"7\" here.\n>>\n>> More seriously it should be looking for strings starting with\n>> \"GIT_CONFIG_\", not the name of the preprocessor definitions.\n> \n> Thanks. I missed that completely.\n\nSame! And I will try to find a way to test these things to ensure\nthese mistakes are not prevented only by careful code reviewers!\n\nThanks,\n-Stolee\n\n"},{"id":"537349","messageId":"20260227224238.GA2956443@coredump.intra.peff.net","threadId":"65065","inReplyTo":"08c6e203-3444-45c7-9bc9-cc2590be30c3@gmail.com","subject":"Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-02-27T22:42:38Z","receivedAt":"2026-02-27T22:42:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 26, 2026 at 10:29:47AM -0500, Derrick Stolee wrote:\n\n> Great point. Here's another attempt:\n> \n> static int run_command_on_repo(const char *path, int argc, const char ** argv)\n> {\n> \tint i = 0;\n> \tstruct child_process child = CHILD_PROCESS_INIT;\n> \tchar *abspath = interpolate_path(path, 0);\n> \n> \twhile (local_repo_env[i]) {\n> \t\t/*\n> \t\t * Preserve pre-builtin options:\n> \t\t * - CONFIG_ENVIRONMENT, CONFIG_DATA_ENVIRONMENT, and\n> \t\t * CONFIG_COUNT_ENVIRONMENT persist -c =\n> \t\t * and --config-env== options.\n> \t\t * - NO_REPLACE_OBJECTS_ENVIRONMENT persists the\n> \t\t * --no-replace-objects option.\n> \t\t *\n> \t\t * Note that the following options are not in local_repo_env:\n> \t\t * - EXEC_PATH_ENVIRONMENT persists --exec-path option.\n> \t\t */\n> \t\tif (strncmp(local_repo_env[i], \"CONFIG_\", 7) &&\n> \t\t strcmp(local_repo_env[i], NO_REPLACE_OBJECTS_ENVIRONMENT))\n> \t\t\tstrvec_push(&child.env, local_repo_env[i]);\n\nThis is slightly different than what prepare_other_repo_env() does:\n\n - it doesn't drop GIT_CONFIG_*, but assumes that removing\n GIT_CONFIG_COUNT is enough for GIT_CONFIG_KEY/VALUE to be ignored\n (and then also removes GIT_CONFIG_PARAMETERS, of course)\n\n - it doesn't consider NO_REPLACE_OBJECTS at all\n\nI think you could make arguments either way about what should happen\nwhen spawning a command in another repo. But I'd really prefer for us to\nhave a single spot to specify that policy, and not subtly-different\nbehavior from different commands. So I'd really like to see this using\nthat other function (or the logic from it factored out into a helper).\n\nAnd then we can consider whether to make changes to that policy.\n\nDropping GIT_CONFIG_* from the environment does make sense in general,\nbut it doesn't actually happen with the patch above (because only\nGIT_CONFIG_COUNT is in the local_repo_env list; to find the others we'd\nhave to actually enumerate the current environment).\n\nFor NO_REPLACE_OBJECTS, I think you could argue that it should not be in\nlocal_repo_env at all. It is more about the operation being performed,\nnot the repository itself. So for example in this command:\n\n git --no-replace-objects fetch\n\nI would expect that NO_REPLACE_OBJECTS to make it down to any submodule\nfetches we do. Likewise for other operation-level variables like\nGIT_LITERAL_PATHSPECS, but those are already (correctly IMHO) omitted\nfrom local_repo_env.\n\nIt looks like NO_REPLACE_OBJECTS got pulled from the connect.c code in\n48a7c1c49d (Refactor list of of repo-local env vars, 2010-02-25). And I\ncould see somebody wanting to make sure that upload-pack behaved\npredictably with respect to replace refs, but it already does: it\ndisables replace refs itself as part of its startup code.\n\n> This comment details my findings from comparing the list in\n> local_repo_env[] and the top-level options listed in\n> Documentation/git.adoc. That's how I was able to find that\n> --exec-path sets an environment variable that's NOT in the\n> list and we want to be sure we don't set it.\n> \n> Should we add the comparison to EXEC_PATH_ENVIRONMENT as a\n> precaution to make sure it's not added to local_repo_env in\n> the future? Or is that too defensive?\n\nI don't think we need to bother. Obviously adding it to local_repo_env\nwould be the wrong thing, but that is true of lots of variables. Trying\nto make a list is just going to result in a list that is out-of-date,\nbecause there's nothing pushing people to update it when they introduce\na new variable.\n\nYou can imagine a different world, where we had a single list of all\nenvironment variables, and new ones _had_ to be added to the list in\norder to function, and each entry had a bitflag for \"this is a\nlocal-repo value\", then that might force each new addition to consider\nwhether it should be added. But we don't have such a list, and I think\nstructuring things that way would introduce new complications and\nawkwardness.\n\nSo IMHO we should just rely on review to reject a patch that tries\nsomething silly like adding EXEC_PATH_ENVIRONMENT to local_repo_env.\n\n-Peff\n"},{"id":"537350","messageId":"20260227224519.GB2956443@coredump.intra.peff.net","threadId":"65065","inReplyTo":"xmqqsean4gsc.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-02-27T22:45:19Z","receivedAt":"2026-02-27T22:45:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 26, 2026 at 08:21:23AM -0800, Junio C Hamano wrote:\n\n> > This comment details my findings from comparing the list in\n> > local_repo_env[] and the top-level options listed in\n> > Documentation/git.adoc. That's how I was able to find that\n> > --exec-path sets an environment variable that's NOT in the\n> > list and we want to be sure we don't set it.\n> \n> Hmph, wouldn't we want to use specified exec-path inside ...\n> \n> git --exec-path=~/my/git/libexec for-each-repo sh -c \"do things\"\n> \n> ... \"do things\" script when we find Git related binaries? Or am I\n> not getting what you are describing here?\n\nI almost responded with the same thing, but I think the suggestion is\ngoing the other way: we (correctly) do not list EXEC_PATH_ENVIRONMENT\nvia local_repo_env, so it will never be removed from the environment.\nAnd thus we do not need to do anything here to drop it from the list of\nwhat is removed. Double negation. :)\n\nThe second paragraph:\n\n> > Should we add the comparison to EXEC_PATH_ENVIRONMENT as a\n> > precaution to make sure it's not added to local_repo_env in\n> > the future? Or is that too defensive?\n\nmakes that more clear, I think. I did have to read the whole thing\ntwice. ;)\n\n-Peff\n"},{"id":"537533","messageId":"cd9adbd9-b996-46da-b6a8-d2395be79a0f@gmail.com","threadId":"65065","inReplyTo":"20260227224238.GA2956443@coredump.intra.peff.net","subject":"Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-02T15:31:48Z","receivedAt":"2026-03-02T15:31:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 2/27/2026 5:42 PM, Jeff King wrote:\n> On Thu, Feb 26, 2026 at 10:29:47AM -0500, Derrick Stolee wrote:\n> \n>> Great point. Here's another attempt:\n>>\n>> static int run_command_on_repo(const char *path, int argc, const char ** argv)\n>> {\n>> \tint i = 0;\n>> \tstruct child_process child = CHILD_PROCESS_INIT;\n>> \tchar *abspath = interpolate_path(path, 0);\n>>\n>> \twhile (local_repo_env[i]) {\n>> \t\t/*\n>> \t\t * Preserve pre-builtin options:\n>> \t\t * - CONFIG_ENVIRONMENT, CONFIG_DATA_ENVIRONMENT, and\n>> \t\t * CONFIG_COUNT_ENVIRONMENT persist -c =\n>> \t\t * and --config-env== options.\n>> \t\t * - NO_REPLACE_OBJECTS_ENVIRONMENT persists the\n>> \t\t * --no-replace-objects option.\n>> \t\t *\n>> \t\t * Note that the following options are not in local_repo_env:\n>> \t\t * - EXEC_PATH_ENVIRONMENT persists --exec-path option.\n>> \t\t */\n>> \t\tif (strncmp(local_repo_env[i], \"CONFIG_\", 7) &&\n>> \t\t strcmp(local_repo_env[i], NO_REPLACE_OBJECTS_ENVIRONMENT))\n>> \t\t\tstrvec_push(&child.env, local_repo_env[i]);\n> \n> This is slightly different than what prepare_other_repo_env() does:\n> \n> - it doesn't drop GIT_CONFIG_*, but assumes that removing\n> GIT_CONFIG_COUNT is enough for GIT_CONFIG_KEY/VALUE to be ignored\n> (and then also removes GIT_CONFIG_PARAMETERS, of course)\n> \n> - it doesn't consider NO_REPLACE_OBJECTS at all\n> \n> I think you could make arguments either way about what should happen\n> when spawning a command in another repo. But I'd really prefer for us to\n> have a single spot to specify that policy, and not subtly-different\n> behavior from different commands. So I'd really like to see this using\n> that other function (or the logic from it factored out into a helper).\n\nI agree that it would be best to have a single place.\n\nI was looking at prepare_other_repo_env() and saw that it requires a\ncomputed gitdir, which is not easy to compute. We want the child process\nto perform that discovery based on the -C parameter.\n\nHowever, we can extract the existing environment clearing logic and use\nthat here. I'll give that a try and confirm that it passes the tests\nthat I prepared to fix the bugs in this version.\n\n> And then we can consider whether to make changes to that policy.\n> \n> Dropping GIT_CONFIG_* from the environment does make sense in general,\n> but it doesn't actually happen with the patch above (because only\n> GIT_CONFIG_COUNT is in the local_repo_env list; to find the others we'd\n> have to actually enumerate the current environment).\n\nIt has GIT_CONFIG (the local Git config file), GIT_CONFIG_COUNT, and\nGIT_CONFIG_PARAMETERS. My patch was wrong because of the string, showing\nthe value in having tests to confirm the right behavior.\n\n> For NO_REPLACE_OBJECTS, I think you could argue that it should not be in\n> local_repo_env at all. It is more about the operation being performed,\n> not the repository itself. So for example in this command:\n> \n> git --no-replace-objects fetch\n> \n> I would expect that NO_REPLACE_OBJECTS to make it down to any submodule\n> fetches we do. Likewise for other operation-level variables like\n> GIT_LITERAL_PATHSPECS, but those are already (correctly IMHO) omitted\n> from local_repo_env.\n> \n> It looks like NO_REPLACE_OBJECTS got pulled from the connect.c code in\n> 48a7c1c49d (Refactor list of of repo-local env vars, 2010-02-25). And I\n> could see somebody wanting to make sure that upload-pack behaved\n> predictably with respect to replace refs, but it already does: it\n> disables replace refs itself as part of its startup code.\n\nOK. I won't special case this myself and will let this be changed\nindependently, if that is indeed valuable.\n\n>> This comment details my findings from comparing the list in\n>> local_repo_env[] and the top-level options listed in\n>> Documentation/git.adoc. That's how I was able to find that\n>> --exec-path sets an environment variable that's NOT in the\n>> list and we want to be sure we don't set it.\n>>\n>> Should we add the comparison to EXEC_PATH_ENVIRONMENT as a\n>> precaution to make sure it's not added to local_repo_env in\n>> the future? Or is that too defensive?\n> \n> I don't think we need to bother. Obviously adding it to local_repo_env\n> would be the wrong thing, but that is true of lots of variables. Trying\n> to make a list is just going to result in a list that is out-of-date,\n> because there's nothing pushing people to update it when they introduce\n> a new variable.\n\nThis is where I was landing, too.\n\n> You can imagine a different world, where we had a single list of all\n> environment variables, and new ones _had_ to be added to the list in\n> order to function, and each entry had a bitflag for \"this is a\n> local-repo value\", then that might force each new addition to consider\n> whether it should be added. But we don't have such a list, and I think\n> structuring things that way would introduce new complications and\n> awkwardness.\n\nThis makes sense. In the meantime, having a single place that unsets\nenvironment variables for certain child processes is good enough to\ncover what we need here.\n\nThanks,\n-Stolee\n\n"},{"id":"537534","messageId":"pull.2056.v3.git.1772465805.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":"pull.2056.v2.git.1771968924.gitgitgadget@gmail.com","subject":"[PATCH v3 0/4] for-each-repo: work correctly in a worktree","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-02T15:36:41Z","receivedAt":"2026-03-02T15:36:48Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This was reported by Matthew [1] and is a quick fix.\n\n[1]\nhttps://lore.kernel.org/git/CABpCjbY=wpStuhxqRJ5TSNV3A-CmN-g-xZGJOQGSSv3GYhs2fQ@mail.gmail.com/\n\nI also took the liberty of removing the_repository as I wanted to make sure\nthat wasn't involved here.\n\n\nUpdates in v3\n=============\n\nv2 included some important changes that apparently didn't save in my cover\nletter. Sorry. The most important one was that the first patch was replaced\nwith one that tests the command outside of a Git repo so the first patch in\nv1 can be caught as a bug.\n\nBut v2 also included a bug! The filtering of GIT_CONFIG* environment\nvariables was incorrect. Further, duplicating this logic from\nprepare_other_repo_env() was a new bug waiting to happen.\n\nHere is the new setup:\n\n * Patch 1 is the same as in v2.\n * A new Patch 2 extracts clear_local_repo_env() from\n prepare_other_repo_env().\n * The old Patch 2 is now Patch 3 with a much simpler implementation that\n calls clear_local_repo_env().\n * Tests are added to check that these config-related environment variables\n are preserved. The NO_REPLACE_OBJECTS case seemed like it would take\n longer than necessary to set up for the value it provides.\n * A fourth patch is added that simplifies the use of argv. It's separate\n because it modifies the original implementation in a way that is a \"nice\n to have\" but isn't necessary for solving the bug that motivates the\n series.\n\nThanks, -Stolee\n\nDerrick Stolee (4):\n for-each-repo: test outside of repo context\n run-command: extract clear_local_repo_env helper\n for-each-repo: work correctly in a worktree\n for-each-repo: simplify passing of parameters\n\n builtin/for-each-repo.c | 12 +++----\n run-command.c | 7 ++++-\n run-command.h | 7 +++++\n t/t0068-for-each-repo.sh | 67 +++++++++++++++++++++++++++++++++-------\n 4 files changed, 74 insertions(+), 19 deletions(-)\n\n\nbase-commit: 67ad42147a7acc2af6074753ebd03d904476118f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2056%2Fderrickstolee%2Ffor-each-repo-in-gitdir-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2056/derrickstolee/for-each-repo-in-gitdir-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/2056\n\nRange-diff vs v2:\n\n 1: 6e9d4f3029 = 1: 6e9d4f3029 for-each-repo: test outside of repo context\n -: ---------- > 2: 13d783dbbd run-command: extract clear_local_repo_env helper\n 2: 4e3f4aa6cd ! 3: 2a6091095f for-each-repo: work correctly in a worktree\n @@ Commit message\n We need to be careful to unset the local Git environment variables and\n let the child process rediscover them, while also reinstating those\n variables in the parent process afterwards. Update run_command_on_repo()\n - to store, unset, then reset the non-NULL variables.\n + to use the new clear_local_repo_env() helper method to erase these\n + environment variables.\n +\n + During review of this bug fix, there were several incorrect patches\n + demonstrating different bad behaviors. Most of these are covered by\n + tests, when it is not too expensive to set it up. One case that would be\n + expensive to set up is the GIT_NO_REPLACE_OBJECTS environment variable,\n + but we trust that using clear_local_repo_env() will be sufficient to\n + capture these uncovered cases by using the common code for resetting\n + environment variables.\n \n Reported-by: Matthew Gabeler-Lee \n Signed-off-by: Derrick Stolee \n @@ builtin/for-each-repo.c: static const char * const for_each_repo_usage[] = {\n static int run_command_on_repo(const char *path, int argc, const char ** argv)\n {\n -\tint i;\n -+\tint res;\n \tstruct child_process child = CHILD_PROCESS_INIT;\n -+\tchar **envvars;\n -+\tsize_t envvar_nr = 0;\n \tchar *abspath = interpolate_path(path, 0);\n \n -+\twhile (local_repo_env[envvar_nr])\n -+\t\tenvvar_nr++;\n -+\n -+\tCALLOC_ARRAY(envvars, envvar_nr);\n -+\n -+\tfor (size_t i = 0; i < envvar_nr; i++) {\n -+\t\tenvvars[i] = getenv(local_repo_env[i]);\n -+\n -+\t\tif (envvars[i]) {\n -+\t\t\tunsetenv(local_repo_env[i]);\n -+\t\t\tenvvars[i] = xstrdup(envvars[i]);\n -+\t\t}\n -+\t}\n ++\tclear_local_repo_env(&child.env);\n +\n \tchild.git_cmd = 1;\n \tstrvec_pushl(&child.args, \"-C\", abspath, NULL);\n \n --\tfor (i = 0; i < argc; i++)\n -+\tfor (int i = 0; i < argc; i++)\n - \t\tstrvec_push(&child.args, argv[i]);\n - \n - \tfree(abspath);\n - \n --\treturn run_command(&child);\n -+\tres = run_command(&child);\n -+\n -+\tfor (size_t i = 0; i < envvar_nr; i++) {\n -+\t\tif (envvars[i]) {\n -+\t\t\tsetenv(local_repo_env[i], envvars[i], 1);\n -+\t\t\tfree(envvars[i]);\n -+\t\t}\n -+\t}\n -+\n -+\tfree(envvars);\n -+\treturn res;\n - }\n - \n - int cmd_for_each_repo(int argc,\n \n ## t/t0068-for-each-repo.sh ##\n @@ t/t0068-for-each-repo.sh: TEST_NO_CREATE_REPO=1\n + . ./test-lib.sh\n + \n test_expect_success 'run based on configured value' '\n - \tgit init one &&\n - \tgit init two &&\n +-\tgit init one &&\n +-\tgit init two &&\n -\tgit init three &&\n +-\tgit init ~/four &&\n ++\tgit init --initial-branch=one one &&\n ++\tgit init --initial-branch=two two &&\n +\tgit -C two worktree add --orphan ../three &&\n - \tgit init ~/four &&\n ++\tgit -C three checkout -b three &&\n ++\tgit init --initial-branch=four ~/four &&\n ++\n \tgit -C two commit --allow-empty -m \"DID NOT RUN\" &&\n \tgit config --global run.key \"$TRASH_DIRECTORY/one\" &&\n + \tgit config --global --add run.key \"$TRASH_DIRECTORY/three\" &&\n @@ t/t0068-for-each-repo.sh: test_expect_success 'run based on configured value' '\n \tgit -C three log -1 --pretty=format:%s >message &&\n \tgrep again message &&\n @@ t/t0068-for-each-repo.sh: test_expect_success 'run based on configured value' '\n -\tgrep again message\n +\tgrep again message &&\n +\n -+\tgit -C three for-each-repo --config=run.key -- commit --allow-empty -m \"ran from worktree\" &&\n ++\tgit -C three for-each-repo --config=run.key -- \\\n ++\t\tcommit --allow-empty -m \"ran from worktree\" &&\n +\tgit -C one log -1 --pretty=format:%s >message &&\n -+\tgrep worktree message &&\n ++\ttest_grep \"ran from worktree\" message &&\n +\tgit -C two log -1 --pretty=format:%s >message &&\n -+\t! grep worktree message &&\n ++\ttest_grep ! \"ran from worktree\" message &&\n +\tgit -C three log -1 --pretty=format:%s >message &&\n -+\tgrep worktree message &&\n ++\ttest_grep \"ran from worktree\" message &&\n +\tgit -C ~/four log -1 --pretty=format:%s >message &&\n -+\tgrep worktree message\n ++\ttest_grep \"ran from worktree\" message &&\n ++\n ++\t# Test running with config values set by environment\n ++\tcat >expect <<-EOF &&\n ++\tran from worktree (HEAD -> refs/heads/one)\n ++\tran from worktree (HEAD -> refs/heads/three)\n ++\tran from worktree (HEAD -> refs/heads/four)\n ++\tEOF\n ++\n ++\tGIT_CONFIG_PARAMETERS=\"${SQ}log.decorate=full${SQ}\" \\\n ++\t\tgit -C three for-each-repo --config=run.key -- log --format=\"%s%d\" -1 >out &&\n ++\ttest_cmp expect out &&\n ++\n ++\tcat >test-config <<-EOF &&\n ++\t[run]\n ++\t\tkey = $(pwd)/one\n ++\t\tkey = $(pwd)/three\n ++\t\tkey = $(pwd)/four\n ++\n ++\t[log]\n ++\t\tdecorate = full\n ++\tEOF\n ++\n ++\tGIT_CONFIG_GLOBAL=\"$(pwd)/test-config\" \\\n ++\t\tgit -C three for-each-repo --config=run.key -- log --format=\"%s%d\" -1 >out &&\n ++\ttest_cmp expect out\n '\n \n test_expect_success 'do nothing on empty config' '\n -: ---------- > 4: f6582e9402 for-each-repo: simplify passing of parameters\n\n-- \ngitgitgadget\n"},{"id":"537535","messageId":"6e9d4f3029daa2c0068bb16939b943e7ac924222.1772465805.git.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":"pull.2056.v3.git.1772465805.gitgitgadget@gmail.com","subject":"[PATCH v3 1/4] for-each-repo: test outside of repo context","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-02T15:36:42Z","receivedAt":"2026-03-02T15:36:50Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe 'git for-each-repo' tool is frequently run outside of a repo context\nin the real world. For example, it powers background maintenance.\nDespite this typical case, we have not been testing it without a local\nrepository.\n\nUpdate t0068 to stop creating a test repo and to use global config\neverywhere. This has some subtle changes to test across the file.\n\nThis was noticed because an earlier attempt to remove the_repository\nfrom builtin/for-each-repo.c did not catch a segmentation fault since\nthe passed 'repo' is NULL. This use of the_repository will need to stay\nuntil we have a better way to handle config queries outside of a repo\ncontext. Similar use still exists in builtin/config.c for the same\nreason.\n\nSigned-off-by: Derrick Stolee \n---\n t/t0068-for-each-repo.sh | 19 ++++++++++++-------\n 1 file changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh\nindex f2f3e50031..512af34c82 100755\n--- a/t/t0068-for-each-repo.sh\n+++ b/t/t0068-for-each-repo.sh\n@@ -2,6 +2,9 @@\n \n test_description='git for-each-repo builtin'\n \n+# We need to test running 'git for-each-repo' outside of a repo context.\n+TEST_NO_CREATE_REPO=1\n+\n . ./test-lib.sh\n \n test_expect_success 'run based on configured value' '\n@@ -10,9 +13,10 @@ test_expect_success 'run based on configured value' '\n \tgit init three &&\n \tgit init ~/four &&\n \tgit -C two commit --allow-empty -m \"DID NOT RUN\" &&\n-\tgit config run.key \"$TRASH_DIRECTORY/one\" &&\n-\tgit config --add run.key \"$TRASH_DIRECTORY/three\" &&\n-\tgit config --add run.key \"~/four\" &&\n+\tgit config --global run.key \"$TRASH_DIRECTORY/one\" &&\n+\tgit config --global --add run.key \"$TRASH_DIRECTORY/three\" &&\n+\tgit config --global --add run.key \"~/four\" &&\n+\n \tgit for-each-repo --config=run.key commit --allow-empty -m \"ran\" &&\n \tgit -C one log -1 --pretty=format:%s >message &&\n \tgrep ran message &&\n@@ -22,6 +26,7 @@ test_expect_success 'run based on configured value' '\n \tgrep ran message &&\n \tgit -C ~/four log -1 --pretty=format:%s >message &&\n \tgrep ran message &&\n+\n \tgit for-each-repo --config=run.key -- commit --allow-empty -m \"ran again\" &&\n \tgit -C one log -1 --pretty=format:%s >message &&\n \tgrep again message &&\n@@ -46,7 +51,7 @@ test_expect_success 'error on bad config keys' '\n '\n \n test_expect_success 'error on NULL value for config keys' '\n-\tcat >>.git/config <<-\\EOF &&\n+\tcat >>.gitconfig <<-\\EOF &&\n \t[empty]\n \t\tkey\n \tEOF\n@@ -59,8 +64,8 @@ test_expect_success 'error on NULL value for config keys' '\n '\n \n test_expect_success '--keep-going' '\n-\tgit config keep.going non-existing &&\n-\tgit config --add keep.going . &&\n+\tgit config --global keep.going non-existing &&\n+\tgit config --global --add keep.going one &&\n \n \ttest_must_fail git for-each-repo --config=keep.going \\\n \t\t-- branch >out 2>err &&\n@@ -70,7 +75,7 @@ test_expect_success '--keep-going' '\n \ttest_must_fail git for-each-repo --config=keep.going --keep-going \\\n \t\t-- branch >out 2>err &&\n \ttest_grep \"cannot change to .*non-existing\" err &&\n-\tgit branch >expect &&\n+\tgit -C one branch >expect &&\n \ttest_cmp expect out\n '\n \n-- \ngitgitgadget\n\n"},{"id":"537536","messageId":"13d783dbbdd77b14fed651f0508fa0e668d98c63.1772465805.git.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":"pull.2056.v3.git.1772465805.gitgitgadget@gmail.com","subject":"[PATCH v3 2/4] run-command: extract clear_local_repo_env helper","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-02T15:36:43Z","receivedAt":"2026-03-02T15:36:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe current prepare_other_repo_env() does two distinct things:\n\n 1. Strip certain known environment variables that should be set by a\n child process based on a different repository.\n\n 2. Set the GIT_DIR variable to avoid repository discovery.\n\nThe second item is valuable for child processes that operate on\nsubmodules, where the repo discovery could be mistaken for the parent\nrepository.\n\nIn the next change, we will see an important case where only the first\nitem is required as the GIT_DIR discovery should happen naturally from\nthe '-C' parameter in the child process.\n\nSigned-off-by: Derrick Stolee \n---\n run-command.c | 7 ++++++-\n run-command.h | 7 +++++++\n 2 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex e3e02475cc..7858a0ef0a 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1847,7 +1847,7 @@ int run_auto_maintenance(int quiet)\n \treturn run_command(&maint);\n }\n \n-void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)\n+void clear_local_repo_env(struct strvec *env)\n {\n \tconst char * const *var;\n \n@@ -1856,6 +1856,11 @@ void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)\n \t\t strcmp(*var, CONFIG_COUNT_ENVIRONMENT))\n \t\t\tstrvec_push(env, *var);\n \t}\n+}\n+\n+void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)\n+{\n+\tclear_local_repo_env(env);\n \tstrvec_pushf(env, \"%s=%s\", GIT_DIR_ENVIRONMENT, new_git_dir);\n }\n \ndiff --git a/run-command.h b/run-command.h\nindex 0df25e445f..76b29d4832 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -509,6 +509,13 @@ struct run_process_parallel_opts\n */\n void run_processes_parallel(const struct run_process_parallel_opts *opts);\n \n+/**\n+ * Unset all local-repo GIT_* variables in env; see local_repo_env in\n+ * environment.h. GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT are preserved\n+ * to pass -c and --config-env options from the parent process.\n+ */\n+void clear_local_repo_env(struct strvec *env);\n+\n /**\n * Convenience function which prepares env for a command to be run in a\n * new repo. This adds all GIT_* environment variables to env with the\n-- \ngitgitgadget\n\n"},{"id":"537537","messageId":"2a6091095f120426fed554a08871f2b4dcd15282.1772465805.git.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":"pull.2056.v3.git.1772465805.gitgitgadget@gmail.com","subject":"[PATCH v3 3/4] for-each-repo: work correctly in a worktree","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-02T15:36:44Z","receivedAt":"2026-03-02T15:36:53Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen run in a worktree, the GIT_DIR directory is set in a different way\nthan in a typical repository. Show this by updating t0068 to include a\nworktree and add a test that runs from that worktree. This requires\nmoving the repo.key config into a global config instead of the base test\nrepository's local config (demonstrating that it worked with\nnon-worktree Git repositories).\n\nWe need to be careful to unset the local Git environment variables and\nlet the child process rediscover them, while also reinstating those\nvariables in the parent process afterwards. Update run_command_on_repo()\nto use the new clear_local_repo_env() helper method to erase these\nenvironment variables.\n\nDuring review of this bug fix, there were several incorrect patches\ndemonstrating different bad behaviors. Most of these are covered by\ntests, when it is not too expensive to set it up. One case that would be\nexpensive to set up is the GIT_NO_REPLACE_OBJECTS environment variable,\nbut we trust that using clear_local_repo_env() will be sufficient to\ncapture these uncovered cases by using the common code for resetting\nenvironment variables.\n\nReported-by: Matthew Gabeler-Lee \nSigned-off-by: Derrick Stolee \n---\n builtin/for-each-repo.c | 4 +++-\n t/t0068-for-each-repo.sh | 48 +++++++++++++++++++++++++++++++++++-----\n 2 files changed, 46 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\nindex 325a7925f1..8bbdc33128 100644\n--- a/builtin/for-each-repo.c\n+++ b/builtin/for-each-repo.c\n@@ -2,6 +2,7 @@\n \n #include \"builtin.h\"\n #include \"config.h\"\n+#include \"environment.h\"\n #include \"gettext.h\"\n #include \"parse-options.h\"\n #include \"path.h\"\n@@ -15,10 +16,11 @@ static const char * const for_each_repo_usage[] = {\n \n static int run_command_on_repo(const char *path, int argc, const char ** argv)\n {\n-\tint i;\n \tstruct child_process child = CHILD_PROCESS_INIT;\n \tchar *abspath = interpolate_path(path, 0);\n \n+\tclear_local_repo_env(&child.env);\n+\n \tchild.git_cmd = 1;\n \tstrvec_pushl(&child.args, \"-C\", abspath, NULL);\n \ndiff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh\nindex 512af34c82..80b163ea99 100755\n--- a/t/t0068-for-each-repo.sh\n+++ b/t/t0068-for-each-repo.sh\n@@ -8,10 +8,12 @@ TEST_NO_CREATE_REPO=1\n . ./test-lib.sh\n \n test_expect_success 'run based on configured value' '\n-\tgit init one &&\n-\tgit init two &&\n-\tgit init three &&\n-\tgit init ~/four &&\n+\tgit init --initial-branch=one one &&\n+\tgit init --initial-branch=two two &&\n+\tgit -C two worktree add --orphan ../three &&\n+\tgit -C three checkout -b three &&\n+\tgit init --initial-branch=four ~/four &&\n+\n \tgit -C two commit --allow-empty -m \"DID NOT RUN\" &&\n \tgit config --global run.key \"$TRASH_DIRECTORY/one\" &&\n \tgit config --global --add run.key \"$TRASH_DIRECTORY/three\" &&\n@@ -35,7 +37,43 @@ test_expect_success 'run based on configured value' '\n \tgit -C three log -1 --pretty=format:%s >message &&\n \tgrep again message &&\n \tgit -C ~/four log -1 --pretty=format:%s >message &&\n-\tgrep again message\n+\tgrep again message &&\n+\n+\tgit -C three for-each-repo --config=run.key -- \\\n+\t\tcommit --allow-empty -m \"ran from worktree\" &&\n+\tgit -C one log -1 --pretty=format:%s >message &&\n+\ttest_grep \"ran from worktree\" message &&\n+\tgit -C two log -1 --pretty=format:%s >message &&\n+\ttest_grep ! \"ran from worktree\" message &&\n+\tgit -C three log -1 --pretty=format:%s >message &&\n+\ttest_grep \"ran from worktree\" message &&\n+\tgit -C ~/four log -1 --pretty=format:%s >message &&\n+\ttest_grep \"ran from worktree\" message &&\n+\n+\t# Test running with config values set by environment\n+\tcat >expect <<-EOF &&\n+\tran from worktree (HEAD -> refs/heads/one)\n+\tran from worktree (HEAD -> refs/heads/three)\n+\tran from worktree (HEAD -> refs/heads/four)\n+\tEOF\n+\n+\tGIT_CONFIG_PARAMETERS=\"${SQ}log.decorate=full${SQ}\" \\\n+\t\tgit -C three for-each-repo --config=run.key -- log --format=\"%s%d\" -1 >out &&\n+\ttest_cmp expect out &&\n+\n+\tcat >test-config <<-EOF &&\n+\t[run]\n+\t\tkey = $(pwd)/one\n+\t\tkey = $(pwd)/three\n+\t\tkey = $(pwd)/four\n+\n+\t[log]\n+\t\tdecorate = full\n+\tEOF\n+\n+\tGIT_CONFIG_GLOBAL=\"$(pwd)/test-config\" \\\n+\t\tgit -C three for-each-repo --config=run.key -- log --format=\"%s%d\" -1 >out &&\n+\ttest_cmp expect out\n '\n \n test_expect_success 'do nothing on empty config' '\n-- \ngitgitgadget\n\n"},{"id":"537538","messageId":"f6582e94026eb933dff6fa895775c52ebf32409a.1772465805.git.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":"pull.2056.v3.git.1772465805.gitgitgadget@gmail.com","subject":"[PATCH v3 4/4] for-each-repo: simplify passing of parameters","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-02T15:36:45Z","receivedAt":"2026-03-02T15:36:55Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThis change simplifies the code somewhat from its original\nimplementation.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/for-each-repo.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\nindex 8bbdc33128..3aafb2cfa8 100644\n--- a/builtin/for-each-repo.c\n+++ b/builtin/for-each-repo.c\n@@ -14,7 +14,7 @@ static const char * const for_each_repo_usage[] = {\n \tNULL\n };\n \n-static int run_command_on_repo(const char *path, int argc, const char ** argv)\n+static int run_command_on_repo(const char *path, const char **argv)\n {\n \tstruct child_process child = CHILD_PROCESS_INIT;\n \tchar *abspath = interpolate_path(path, 0);\n@@ -23,9 +23,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)\n \n \tchild.git_cmd = 1;\n \tstrvec_pushl(&child.args, \"-C\", abspath, NULL);\n-\n-\tfor (i = 0; i < argc; i++)\n-\t\tstrvec_push(&child.args, argv[i]);\n+\tstrvec_pushv(&child.args, argv);\n \n \tfree(abspath);\n \n@@ -65,7 +63,7 @@ int cmd_for_each_repo(int argc,\n \t\treturn 0;\n \n \tfor (size_t i = 0; i < values->nr; i++) {\n-\t\tint ret = run_command_on_repo(values->items[i].string, argc, argv);\n+\t\tint ret = run_command_on_repo(values->items[i].string, argv);\n \t\tif (ret) {\n \t\t\tif (!keep_going)\n \t\t\t\t\treturn ret;\n-- \ngitgitgadget\n"},{"id":"537561","messageId":"20260302175606.GB28275@coredump.intra.peff.net","threadId":"65065","inReplyTo":"6e9d4f3029daa2c0068bb16939b943e7ac924222.1772465805.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 1/4] for-each-repo: test outside of repo context","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-02T17:56:06Z","receivedAt":"2026-03-02T17:56:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 02, 2026 at 03:36:42PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> test_description='git for-each-repo builtin'\n> \n> +# We need to test running 'git for-each-repo' outside of a repo context.\n> +TEST_NO_CREATE_REPO=1\n> +\n> . ./test-lib.sh\n\nInteresting. I was going to point out that this won't do what you want\nby itself, because Git will keep walking out of the trash directory and\nmay find the containing repository.\n\nBut it looks like this should be enough due to 614c3d8f2e (test-lib: set\nGIT_CEILING_DIRECTORIES to protect the surrounding repository,\n2021-08-29). Supporting this case wasn't the intent of that patch, but I\ndon't see any reason why it should not work reliably.\n\n-Peff\n"},{"id":"537565","messageId":"20260302180324.GC28275@coredump.intra.peff.net","threadId":"65065","inReplyTo":"13d783dbbdd77b14fed651f0508fa0e668d98c63.1772465805.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 2/4] run-command: extract clear_local_repo_env helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-02T18:03:24Z","receivedAt":"2026-03-02T18:03:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 02, 2026 at 03:36:43PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n> \n> The current prepare_other_repo_env() does two distinct things:\n> \n> 1. Strip certain known environment variables that should be set by a\n> child process based on a different repository.\n> \n> 2. Set the GIT_DIR variable to avoid repository discovery.\n> \n> The second item is valuable for child processes that operate on\n> submodules, where the repo discovery could be mistaken for the parent\n> repository.\n> \n> In the next change, we will see an important case where only the first\n> item is required as the GIT_DIR discovery should happen naturally from\n> the '-C' parameter in the child process.\n\nYep, this is the refactoring I expected.\n\n> +/**\n> + * Unset all local-repo GIT_* variables in env; see local_repo_env in\n> + * environment.h. GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT are preserved\n> + * to pass -c and --config-env options from the parent process.\n> + */\n> +void clear_local_repo_env(struct strvec *env);\n\nI worry that the name is potentially confusing here, since it is not\njust clearing local_repo_env, but making a few exceptions. But I don't\nreally have a better name. We called this \"other_repo_env\" in the\nexisting function, which is equally opaque. I dunno, maybe the\ndocumentation you added would be sufficient.\n\nSpeaking of which, the documentation for prepare_other_repo_env() is now\nsomewhat redundant. If we ever change the behavior here, we'll have to\nremember to touch both spots.\n\nSo what about squashing in:\n\ndiff --git a/run-command.h b/run-command.h\nindex 76b29d4832..882caeccc8 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -518,11 +518,9 @@ void clear_local_repo_env(struct strvec *env);\n \n /**\n * Convenience function which prepares env for a command to be run in a\n- * new repo. This adds all GIT_* environment variables to env with the\n- * exception of GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT (which cause the\n- * corresponding environment variables to be unset in the subprocess) and adds\n- * an environment variable pointing to new_git_dir. See local_repo_env in\n- * environment.h for more information.\n+ * new repo. This removes variables pointing to the local repository (using\n+ * clear_local_repo_env() above), and adds an environment variable pointing to\n+ * new_git_dir.\n */\n void prepare_other_repo_env(struct strvec *env, const char *new_git_dir);\n \n\n-Peff\n"},{"id":"537566","messageId":"20260302180601.GD28275@coredump.intra.peff.net","threadId":"65065","inReplyTo":"2a6091095f120426fed554a08871f2b4dcd15282.1772465805.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 3/4] for-each-repo: work correctly in a worktree","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-02T18:06:01Z","receivedAt":"2026-03-02T18:06:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 02, 2026 at 03:36:44PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> @@ -15,10 +16,11 @@ static const char * const for_each_repo_usage[] = {\n> \n> static int run_command_on_repo(const char *path, int argc, const char ** argv)\n> {\n> -\tint i;\n> \tstruct child_process child = CHILD_PROCESS_INIT;\n> \tchar *abspath = interpolate_path(path, 0);\n> \n> +\tclear_local_repo_env(&child.env);\n> +\n> \tchild.git_cmd = 1;\n> \tstrvec_pushl(&child.args, \"-C\", abspath, NULL);\n\nThe second part of the hunk here is as expected, but the first one looks\nwrong. We didn't remove any references to \"i\", so either it was\nredundant to start with (and the compiler should have complained), or\nnow we've broken compilation.\n\nLooks like the latter, but we recover when we switch to using pushv in\npatch 4. So I think the declaration of \"i\" should move to that patch.\n\n-Peff\n"},{"id":"537567","messageId":"20260302180642.GE28275@coredump.intra.peff.net","threadId":"65065","inReplyTo":"f6582e94026eb933dff6fa895775c52ebf32409a.1772465805.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 4/4] for-each-repo: simplify passing of parameters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-02T18:06:42Z","receivedAt":"2026-03-02T18:06:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 02, 2026 at 03:36:45PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> This change simplifies the code somewhat from its original\n> implementation.\n\nYeah, I think this is worth doing.\n\n-Peff\n"},{"id":"537569","messageId":"20260302180904.GF28275@coredump.intra.peff.net","threadId":"65065","inReplyTo":"cd9adbd9-b996-46da-b6a8-d2395be79a0f@gmail.com","subject":"Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-02T18:09:04Z","receivedAt":"2026-03-02T18:09:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 02, 2026 at 10:31:48AM -0500, Derrick Stolee wrote:\n\n> > I think you could make arguments either way about what should happen\n> > when spawning a command in another repo. But I'd really prefer for us to\n> > have a single spot to specify that policy, and not subtly-different\n> > behavior from different commands. So I'd really like to see this using\n> > that other function (or the logic from it factored out into a helper).\n> \n> I agree that it would be best to have a single place.\n> \n> I was looking at prepare_other_repo_env() and saw that it requires a\n> computed gitdir, which is not easy to compute. We want the child process\n> to perform that discovery based on the -C parameter.\n> \n> However, we can extract the existing environment clearing logic and use\n> that here. I'll give that a try and confirm that it passes the tests\n> that I prepared to fix the bugs in this version.\n\nYeah, that was exactly the refactoring I had in mind. What you have in\nv3 looks good.\n\n> > Dropping GIT_CONFIG_* from the environment does make sense in general,\n> > but it doesn't actually happen with the patch above (because only\n> > GIT_CONFIG_COUNT is in the local_repo_env list; to find the others we'd\n> > have to actually enumerate the current environment).\n> \n> It has GIT_CONFIG (the local Git config file), GIT_CONFIG_COUNT, and\n> GIT_CONFIG_PARAMETERS. My patch was wrong because of the string, showing\n> the value in having tests to confirm the right behavior.\n\nAh, I forgot about GIT_CONFIG (though it obviously would not match\nCONFIG_, even if we correctly said GIT_CONFIG_). It's mostly a\nhistorical oddity for git-config itself and can be ignored (other\ncommands do not even look at it, and we'd never set it ourselves).\n\n-Peff\n"},{"id":"537577","messageId":"xmqqpl5m13s7.fsf@gitster.g","threadId":"65065","inReplyTo":"20260302175606.GB28275@coredump.intra.peff.net","subject":"Re: [PATCH v3 1/4] for-each-repo: test outside of repo context","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-02T18:31:52Z","receivedAt":"2026-03-02T18:31:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Mar 02, 2026 at 03:36:42PM +0000, Derrick Stolee via GitGitGadget wrote:\n>\n>> test_description='git for-each-repo builtin'\n>> \n>> +# We need to test running 'git for-each-repo' outside of a repo context.\n>> +TEST_NO_CREATE_REPO=1\n>> +\n>> . ./test-lib.sh\n>\n> Interesting. I was going to point out that this won't do what you want\n> by itself, because Git will keep walking out of the trash directory and\n> may find the containing repository.\n>\n> But it looks like this should be enough due to 614c3d8f2e (test-lib: set\n> GIT_CEILING_DIRECTORIES to protect the surrounding repository,\n> 2021-08-29). Supporting this case wasn't the intent of that patch, but I\n> don't see any reason why it should not work reliably.\n\nI am surprised that use of GIT_CEILING_DIRECTORIES was not done\nuntil 2021, actually. The reason the configuration variable was\ninvented for is exactly to avoid discovery processes going upward\nand ending up in a repository different from what we mean to work\nwith.\n\n"},{"id":"537578","messageId":"xmqqldga13mw.fsf@gitster.g","threadId":"65065","inReplyTo":"20260302180324.GC28275@coredump.intra.peff.net","subject":"Re: [PATCH v3 2/4] run-command: extract clear_local_repo_env helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-02T18:35:03Z","receivedAt":"2026-03-02T18:35:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> +/**\n>> + * Unset all local-repo GIT_* variables in env; see local_repo_env in\n>> + * environment.h. GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT are preserved\n>> + * to pass -c and --config-env options from the parent process.\n>> + */\n>> +void clear_local_repo_env(struct strvec *env);\n>\n> I worry that the name is potentially confusing here, since it is not\n> just clearing local_repo_env, but making a few exceptions. But I don't\n> really have a better name. We called this \"other_repo_env\" in the\n> existing function, which is equally opaque. I dunno, maybe the\n> documentation you added would be sufficient.\n\nperhaps \"clear_local\" -> \"sanitize\" or something, with \"env\" ->\n\"other_env\" to clarify that we are not emptying ours, but the one\nthat will be used by somebody else?\n\n> Speaking of which, the documentation for prepare_other_repo_env() is now\n> somewhat redundant. If we ever change the behavior here, we'll have to\n> remember to touch both spots.\n>\n> So what about squashing in:\n\n> diff --git a/run-command.h b/run-command.h\n> index 76b29d4832..882caeccc8 100644\n> --- a/run-command.h\n> +++ b/run-command.h\n> @@ -518,11 +518,9 @@ void clear_local_repo_env(struct strvec *env);\n> \n> /**\n> * Convenience function which prepares env for a command to be run in a\n> - * new repo. This adds all GIT_* environment variables to env with the\n> - * exception of GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT (which cause the\n> - * corresponding environment variables to be unset in the subprocess) and adds\n> - * an environment variable pointing to new_git_dir. See local_repo_env in\n> - * environment.h for more information.\n> + * new repo. This removes variables pointing to the local repository (using\n> + * clear_local_repo_env() above), and adds an environment variable pointing to\n> + * new_git_dir.\n> */\n> void prepare_other_repo_env(struct strvec *env, const char *new_git_dir);\n\n\nThat reads very well.\n\n"},{"id":"537579","messageId":"c747c645-7773-44e8-9d1a-74f5eb89e318@gmail.com","threadId":"65065","inReplyTo":"xmqqpl5m13s7.fsf@gitster.g","subject":"Re: [PATCH v3 1/4] for-each-repo: test outside of repo context","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-02T18:36:26Z","receivedAt":"2026-03-02T18:36:28Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/2/2026 1:31 PM, Junio C Hamano wrote:\n> Jeff King writes:\n> \n>> On Mon, Mar 02, 2026 at 03:36:42PM +0000, Derrick Stolee via GitGitGadget wrote:\n>>\n>>> test_description='git for-each-repo builtin'\n>>> \n>>> +# We need to test running 'git for-each-repo' outside of a repo context.\n>>> +TEST_NO_CREATE_REPO=1\n>>> +\n>>> . ./test-lib.sh\n>>\n>> Interesting. I was going to point out that this won't do what you want\n>> by itself, because Git will keep walking out of the trash directory and\n>> may find the containing repository.\n>>\n>> But it looks like this should be enough due to 614c3d8f2e (test-lib: set\n>> GIT_CEILING_DIRECTORIES to protect the surrounding repository,\n>> 2021-08-29). Supporting this case wasn't the intent of that patch, but I\n>> don't see any reason why it should not work reliably.\n> \n> I am surprised that use of GIT_CEILING_DIRECTORIES was not done\n> until 2021, actually. The reason the configuration variable was\n> invented for is exactly to avoid discovery processes going upward\n> and ending up in a repository different from what we mean to work\n> with.\n \nI didn't know about these historical details. All I know is that I\nwrote these changes on top of the buggy patch in [1] and confirmed that\nit failed with a segfault. Thanks for confirming the reason that this\nworks!\n\n[1] https://lore.kernel.org/git/86cd83f65b30aab3233e27b3e5c4f03041e68766.1771903950.git.gitgitgadget@gmail.com/\n\nThanks,\n-Stolee\n\n"},{"id":"537580","messageId":"17dea0d7-b67c-460a-a08a-1f3a2986c524@gmail.com","threadId":"65065","inReplyTo":"20260302180324.GC28275@coredump.intra.peff.net","subject":"Re: [PATCH v3 2/4] run-command: extract clear_local_repo_env helper","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-02T18:37:32Z","receivedAt":"2026-03-02T18:37:34Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/2/2026 1:03 PM, Jeff King wrote:\n> So what about squashing in:\n> \n> diff --git a/run-command.h b/run-command.h\n> index 76b29d4832..882caeccc8 100644\n> --- a/run-command.h\n> +++ b/run-command.h\n> @@ -518,11 +518,9 @@ void clear_local_repo_env(struct strvec *env);\n> \n> /**\n> * Convenience function which prepares env for a command to be run in a\n> - * new repo. This adds all GIT_* environment variables to env with the\n> - * exception of GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT (which cause the\n> - * corresponding environment variables to be unset in the subprocess) and adds\n> - * an environment variable pointing to new_git_dir. See local_repo_env in\n> - * environment.h for more information.\n> + * new repo. This removes variables pointing to the local repository (using\n> + * clear_local_repo_env() above), and adds an environment variable pointing to\n> + * new_git_dir.\n> */\n> void prepare_other_repo_env(struct strvec *env, const char *new_git_dir);\n\nI'm happy to squash this in.\n\nPerhaps Junio can do it if we don't need other changes to v3. (I haven't\nread the rest of the feedback.)\n\nThanks,\n-Stolee\n"},{"id":"537581","messageId":"15eb8691-a55d-4edc-94fe-ac8a4b37b90c@gmail.com","threadId":"65065","inReplyTo":"20260302180601.GD28275@coredump.intra.peff.net","subject":"Re: [PATCH v3 3/4] for-each-repo: work correctly in a worktree","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-02T18:39:16Z","receivedAt":"2026-03-02T18:39:18Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/2/2026 1:06 PM, Jeff King wrote:\n> On Mon, Mar 02, 2026 at 03:36:44PM +0000, Derrick Stolee via GitGitGadget wrote:\n> \n>> @@ -15,10 +16,11 @@ static const char * const for_each_repo_usage[] = {\n>> \n>> static int run_command_on_repo(const char *path, int argc, const char ** argv)\n>> {\n>> -\tint i;\n>> \tstruct child_process child = CHILD_PROCESS_INIT;\n>> \tchar *abspath = interpolate_path(path, 0);\n>> \n>> +\tclear_local_repo_env(&child.env);\n>> +\n>> \tchild.git_cmd = 1;\n>> \tstrvec_pushl(&child.args, \"-C\", abspath, NULL);\n> \n> The second part of the hunk here is as expected, but the first one looks\n> wrong. We didn't remove any references to \"i\", so either it was\n> redundant to start with (and the compiler should have complained), or\n> now we've broken compilation.\n\nYou are correct. I did a --fixup here and it messed up the diff. I should\nhave double-checked the commit-by-commit compilation and testing post-\nrebase.\n\n> Looks like the latter, but we recover when we switch to using pushv in\n> patch 4. So I think the declaration of \"i\" should move to that patch.\n\nCan do. Looks like a small v4 update _is_ required.\n\nThanks,\n-Stolee\n\n"},{"id":"537603","messageId":"xmqqcy1lzzmb.fsf@gitster.g","threadId":"65065","inReplyTo":"15eb8691-a55d-4edc-94fe-ac8a4b37b90c@gmail.com","subject":"Re: [PATCH v3 3/4] for-each-repo: work correctly in a worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-02T21:32:28Z","receivedAt":"2026-03-02T21:32:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> On 3/2/2026 1:06 PM, Jeff King wrote:\n>> On Mon, Mar 02, 2026 at 03:36:44PM +0000, Derrick Stolee via GitGitGadget wrote:\n>> \n>>> @@ -15,10 +16,11 @@ static const char * const for_each_repo_usage[] = {\n>>> \n>>> static int run_command_on_repo(const char *path, int argc, const char ** argv)\n>>> {\n>>> -\tint i;\n>>> \tstruct child_process child = CHILD_PROCESS_INIT;\n>>> \tchar *abspath = interpolate_path(path, 0);\n>>> \n>>> +\tclear_local_repo_env(&child.env);\n>>> +\n>>> \tchild.git_cmd = 1;\n>>> \tstrvec_pushl(&child.args, \"-C\", abspath, NULL);\n>> \n>> The second part of the hunk here is as expected, but the first one looks\n>> wrong. We didn't remove any references to \"i\", so either it was\n>> redundant to start with (and the compiler should have complained), or\n>> now we've broken compilation.\n>\n> You are correct. I did a --fixup here and it messed up the diff. I should\n> have double-checked the commit-by-commit compilation and testing post-\n> rebase.\n>\n>> Looks like the latter, but we recover when we switch to using pushv in\n>> patch 4. So I think the declaration of \"i\" should move to that patch.\n>\n> Can do. Looks like a small v4 update _is_ required.\n\nI could do this too ;-) but I probably won't get to queuing this\ntopic before you update on your own, I suspect, so ...\n"},{"id":"537699","messageId":"pull.2056.v4.git.1772559114.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":"pull.2056.v3.git.1772465805.gitgitgadget@gmail.com","subject":"[PATCH v4 0/4] for-each-repo: work correctly in a worktree","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-03T17:31:50Z","receivedAt":"2026-03-03T17:31:58Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This was reported by Matthew [1] and I thought it would be a quick fix. It\nturns out to have had a lot of implications to get exactly right.\n\n[1]\nhttps://lore.kernel.org/git/CABpCjbY=wpStuhxqRJ5TSNV3A-CmN-g-xZGJOQGSSv3GYhs2fQ@mail.gmail.com/\n\n\nUpdates in v3\n=============\n\nv2 included some important changes that apparently didn't save in my cover\nletter. Sorry. The most important one was that the first patch was replaced\nwith one that tests the command outside of a Git repo so the first patch in\nv1 can be caught as a bug.\n\nBut v2 also included a bug! The filtering of GIT_CONFIG* environment\nvariables was incorrect. Further, duplicating this logic from\nprepare_other_repo_env() was a new bug waiting to happen.\n\nHere is the new setup:\n\n * Patch 1 is the same as in v2.\n * A new Patch 2 extracts clear_local_repo_env() from\n prepare_other_repo_env().\n * The old Patch 2 is now Patch 3 with a much simpler implementation that\n calls clear_local_repo_env().\n * Tests are added to check that these config-related environment variables\n are preserved. The NO_REPLACE_OBJECTS case seemed like it would take\n longer than necessary to set up for the value it provides.\n * A fourth patch is added that simplifies the use of argv. It's separate\n because it modifies the original implementation in a way that is a \"nice\n to have\" but isn't necessary for solving the bug that motivates the\n series.\n\n\nUpdates in V4\n=============\n\nMinor updates from Peff's review:\n\n 1. Update the comment of prepare_other_repo_env() to avoid duplication.\n 2. Rename the new method to sanitize_repo_env().\n 3. Move incorrect removal of 'int i;' to correct patch.\n\nThanks, -Stolee\n\nDerrick Stolee (4):\n for-each-repo: test outside of repo context\n run-command: extract sanitize_repo_env helper\n for-each-repo: work correctly in a worktree\n for-each-repo: simplify passing of parameters\n\n builtin/for-each-repo.c | 12 +++----\n run-command.c | 7 ++++-\n run-command.h | 15 ++++++---\n t/t0068-for-each-repo.sh | 67 +++++++++++++++++++++++++++++++++-------\n 4 files changed, 77 insertions(+), 24 deletions(-)\n\n\nbase-commit: 67ad42147a7acc2af6074753ebd03d904476118f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2056%2Fderrickstolee%2Ffor-each-repo-in-gitdir-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2056/derrickstolee/for-each-repo-in-gitdir-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/2056\n\nRange-diff vs v3:\n\n 1: 6e9d4f3029 = 1: 6e9d4f3029 for-each-repo: test outside of repo context\n 2: 13d783dbbd ! 2: 24398664c2 run-command: extract clear_local_repo_env helper\n @@ Metadata\n Author: Derrick Stolee \n \n ## Commit message ##\n - run-command: extract clear_local_repo_env helper\n + run-command: extract sanitize_repo_env helper\n \n The current prepare_other_repo_env() does two distinct things:\n \n @@ Commit message\n item is required as the GIT_DIR discovery should happen naturally from\n the '-C' parameter in the child process.\n \n + Helped-by: Jeff King \n Signed-off-by: Derrick Stolee \n \n ## run-command.c ##\n @@ run-command.c: int run_auto_maintenance(int quiet)\n }\n \n -void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)\n -+void clear_local_repo_env(struct strvec *env)\n ++void sanitize_repo_env(struct strvec *env)\n {\n \tconst char * const *var;\n \n @@ run-command.c: void prepare_other_repo_env(struct strvec *env, const char *new_g\n +\n +void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)\n +{\n -+\tclear_local_repo_env(env);\n ++\tsanitize_repo_env(env);\n \tstrvec_pushf(env, \"%s=%s\", GIT_DIR_ENVIRONMENT, new_git_dir);\n }\n \n @@ run-command.h: struct run_process_parallel_opts\n + * environment.h. GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT are preserved\n + * to pass -c and --config-env options from the parent process.\n + */\n -+void clear_local_repo_env(struct strvec *env);\n ++void sanitize_repo_env(struct strvec *env);\n +\n /**\n * Convenience function which prepares env for a command to be run in a\n - * new repo. This adds all GIT_* environment variables to env with the\n +- * new repo. This adds all GIT_* environment variables to env with the\n +- * exception of GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT (which cause the\n +- * corresponding environment variables to be unset in the subprocess) and adds\n +- * an environment variable pointing to new_git_dir. See local_repo_env in\n +- * environment.h for more information.\n ++ * new repo. This removes variables pointing to the local repository (using\n ++ * sanitize_repo_env() above), and adds an environment variable pointing to\n ++ * new_git_dir.\n + */\n + void prepare_other_repo_env(struct strvec *env, const char *new_git_dir);\n + \n 3: 2a6091095f ! 3: e759b35069 for-each-repo: work correctly in a worktree\n @@ Commit message\n We need to be careful to unset the local Git environment variables and\n let the child process rediscover them, while also reinstating those\n variables in the parent process afterwards. Update run_command_on_repo()\n - to use the new clear_local_repo_env() helper method to erase these\n + to use the new sanitize_repo_env() helper method to erase these\n environment variables.\n \n During review of this bug fix, there were several incorrect patches\n demonstrating different bad behaviors. Most of these are covered by\n tests, when it is not too expensive to set it up. One case that would be\n expensive to set up is the GIT_NO_REPLACE_OBJECTS environment variable,\n - but we trust that using clear_local_repo_env() will be sufficient to\n + but we trust that using sanitize_repo_env() will be sufficient to\n capture these uncovered cases by using the common code for resetting\n environment variables.\n \n @@ builtin/for-each-repo.c\n #include \"gettext.h\"\n #include \"parse-options.h\"\n #include \"path.h\"\n -@@ builtin/for-each-repo.c: static const char * const for_each_repo_usage[] = {\n - \n - static int run_command_on_repo(const char *path, int argc, const char ** argv)\n - {\n --\tint i;\n +@@ builtin/for-each-repo.c: static int run_command_on_repo(const char *path, int argc, const char ** argv)\n \tstruct child_process child = CHILD_PROCESS_INIT;\n \tchar *abspath = interpolate_path(path, 0);\n \n -+\tclear_local_repo_env(&child.env);\n ++\tsanitize_repo_env(&child.env);\n +\n \tchild.git_cmd = 1;\n \tstrvec_pushl(&child.args, \"-C\", abspath, NULL);\n 4: f6582e9402 ! 4: 8b1f083da1 for-each-repo: simplify passing of parameters\n @@ builtin/for-each-repo.c: static const char * const for_each_repo_usage[] = {\n -static int run_command_on_repo(const char *path, int argc, const char ** argv)\n +static int run_command_on_repo(const char *path, const char **argv)\n {\n +-\tint i;\n \tstruct child_process child = CHILD_PROCESS_INIT;\n \tchar *abspath = interpolate_path(path, 0);\n + \n @@ builtin/for-each-repo.c: static int run_command_on_repo(const char *path, int argc, const char ** argv)\n \n \tchild.git_cmd = 1;\n\n-- \ngitgitgadget\n"},{"id":"537700","messageId":"6e9d4f3029daa2c0068bb16939b943e7ac924222.1772559114.git.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":"pull.2056.v4.git.1772559114.gitgitgadget@gmail.com","subject":"[PATCH v4 1/4] for-each-repo: test outside of repo context","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-03T17:31:51Z","receivedAt":"2026-03-03T17:31:59Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe 'git for-each-repo' tool is frequently run outside of a repo context\nin the real world. For example, it powers background maintenance.\nDespite this typical case, we have not been testing it without a local\nrepository.\n\nUpdate t0068 to stop creating a test repo and to use global config\neverywhere. This has some subtle changes to test across the file.\n\nThis was noticed because an earlier attempt to remove the_repository\nfrom builtin/for-each-repo.c did not catch a segmentation fault since\nthe passed 'repo' is NULL. This use of the_repository will need to stay\nuntil we have a better way to handle config queries outside of a repo\ncontext. Similar use still exists in builtin/config.c for the same\nreason.\n\nSigned-off-by: Derrick Stolee \n---\n t/t0068-for-each-repo.sh | 19 ++++++++++++-------\n 1 file changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh\nindex f2f3e50031..512af34c82 100755\n--- a/t/t0068-for-each-repo.sh\n+++ b/t/t0068-for-each-repo.sh\n@@ -2,6 +2,9 @@\n \n test_description='git for-each-repo builtin'\n \n+# We need to test running 'git for-each-repo' outside of a repo context.\n+TEST_NO_CREATE_REPO=1\n+\n . ./test-lib.sh\n \n test_expect_success 'run based on configured value' '\n@@ -10,9 +13,10 @@ test_expect_success 'run based on configured value' '\n \tgit init three &&\n \tgit init ~/four &&\n \tgit -C two commit --allow-empty -m \"DID NOT RUN\" &&\n-\tgit config run.key \"$TRASH_DIRECTORY/one\" &&\n-\tgit config --add run.key \"$TRASH_DIRECTORY/three\" &&\n-\tgit config --add run.key \"~/four\" &&\n+\tgit config --global run.key \"$TRASH_DIRECTORY/one\" &&\n+\tgit config --global --add run.key \"$TRASH_DIRECTORY/three\" &&\n+\tgit config --global --add run.key \"~/four\" &&\n+\n \tgit for-each-repo --config=run.key commit --allow-empty -m \"ran\" &&\n \tgit -C one log -1 --pretty=format:%s >message &&\n \tgrep ran message &&\n@@ -22,6 +26,7 @@ test_expect_success 'run based on configured value' '\n \tgrep ran message &&\n \tgit -C ~/four log -1 --pretty=format:%s >message &&\n \tgrep ran message &&\n+\n \tgit for-each-repo --config=run.key -- commit --allow-empty -m \"ran again\" &&\n \tgit -C one log -1 --pretty=format:%s >message &&\n \tgrep again message &&\n@@ -46,7 +51,7 @@ test_expect_success 'error on bad config keys' '\n '\n \n test_expect_success 'error on NULL value for config keys' '\n-\tcat >>.git/config <<-\\EOF &&\n+\tcat >>.gitconfig <<-\\EOF &&\n \t[empty]\n \t\tkey\n \tEOF\n@@ -59,8 +64,8 @@ test_expect_success 'error on NULL value for config keys' '\n '\n \n test_expect_success '--keep-going' '\n-\tgit config keep.going non-existing &&\n-\tgit config --add keep.going . &&\n+\tgit config --global keep.going non-existing &&\n+\tgit config --global --add keep.going one &&\n \n \ttest_must_fail git for-each-repo --config=keep.going \\\n \t\t-- branch >out 2>err &&\n@@ -70,7 +75,7 @@ test_expect_success '--keep-going' '\n \ttest_must_fail git for-each-repo --config=keep.going --keep-going \\\n \t\t-- branch >out 2>err &&\n \ttest_grep \"cannot change to .*non-existing\" err &&\n-\tgit branch >expect &&\n+\tgit -C one branch >expect &&\n \ttest_cmp expect out\n '\n \n-- \ngitgitgadget\n\n"},{"id":"537701","messageId":"24398664c2009cc1fe94f8cebec145d062d96abd.1772559114.git.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":"pull.2056.v4.git.1772559114.gitgitgadget@gmail.com","subject":"[PATCH v4 2/4] run-command: extract sanitize_repo_env helper","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-03T17:31:52Z","receivedAt":"2026-03-03T17:32:00Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe current prepare_other_repo_env() does two distinct things:\n\n 1. Strip certain known environment variables that should be set by a\n child process based on a different repository.\n\n 2. Set the GIT_DIR variable to avoid repository discovery.\n\nThe second item is valuable for child processes that operate on\nsubmodules, where the repo discovery could be mistaken for the parent\nrepository.\n\nIn the next change, we will see an important case where only the first\nitem is required as the GIT_DIR discovery should happen naturally from\nthe '-C' parameter in the child process.\n\nHelped-by: Jeff King \nSigned-off-by: Derrick Stolee \n---\n run-command.c | 7 ++++++-\n run-command.h | 15 ++++++++++-----\n 2 files changed, 16 insertions(+), 6 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex e3e02475cc..89dbe62ab8 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1847,7 +1847,7 @@ int run_auto_maintenance(int quiet)\n \treturn run_command(&maint);\n }\n \n-void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)\n+void sanitize_repo_env(struct strvec *env)\n {\n \tconst char * const *var;\n \n@@ -1856,6 +1856,11 @@ void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)\n \t\t strcmp(*var, CONFIG_COUNT_ENVIRONMENT))\n \t\t\tstrvec_push(env, *var);\n \t}\n+}\n+\n+void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)\n+{\n+\tsanitize_repo_env(env);\n \tstrvec_pushf(env, \"%s=%s\", GIT_DIR_ENVIRONMENT, new_git_dir);\n }\n \ndiff --git a/run-command.h b/run-command.h\nindex 0df25e445f..7e5a263ee6 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -509,13 +509,18 @@ struct run_process_parallel_opts\n */\n void run_processes_parallel(const struct run_process_parallel_opts *opts);\n \n+/**\n+ * Unset all local-repo GIT_* variables in env; see local_repo_env in\n+ * environment.h. GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT are preserved\n+ * to pass -c and --config-env options from the parent process.\n+ */\n+void sanitize_repo_env(struct strvec *env);\n+\n /**\n * Convenience function which prepares env for a command to be run in a\n- * new repo. This adds all GIT_* environment variables to env with the\n- * exception of GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT (which cause the\n- * corresponding environment variables to be unset in the subprocess) and adds\n- * an environment variable pointing to new_git_dir. See local_repo_env in\n- * environment.h for more information.\n+ * new repo. This removes variables pointing to the local repository (using\n+ * sanitize_repo_env() above), and adds an environment variable pointing to\n+ * new_git_dir.\n */\n void prepare_other_repo_env(struct strvec *env, const char *new_git_dir);\n \n-- \ngitgitgadget\n\n"},{"id":"537702","messageId":"e759b350692360e968a61e2f9744138104e77ba6.1772559114.git.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":"pull.2056.v4.git.1772559114.gitgitgadget@gmail.com","subject":"[PATCH v4 3/4] for-each-repo: work correctly in a worktree","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-03T17:31:53Z","receivedAt":"2026-03-03T17:32:01Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen run in a worktree, the GIT_DIR directory is set in a different way\nthan in a typical repository. Show this by updating t0068 to include a\nworktree and add a test that runs from that worktree. This requires\nmoving the repo.key config into a global config instead of the base test\nrepository's local config (demonstrating that it worked with\nnon-worktree Git repositories).\n\nWe need to be careful to unset the local Git environment variables and\nlet the child process rediscover them, while also reinstating those\nvariables in the parent process afterwards. Update run_command_on_repo()\nto use the new sanitize_repo_env() helper method to erase these\nenvironment variables.\n\nDuring review of this bug fix, there were several incorrect patches\ndemonstrating different bad behaviors. Most of these are covered by\ntests, when it is not too expensive to set it up. One case that would be\nexpensive to set up is the GIT_NO_REPLACE_OBJECTS environment variable,\nbut we trust that using sanitize_repo_env() will be sufficient to\ncapture these uncovered cases by using the common code for resetting\nenvironment variables.\n\nReported-by: Matthew Gabeler-Lee \nSigned-off-by: Derrick Stolee \n---\n builtin/for-each-repo.c | 3 +++\n t/t0068-for-each-repo.sh | 48 +++++++++++++++++++++++++++++++++++-----\n 2 files changed, 46 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\nindex 325a7925f1..82727c4aa2 100644\n--- a/builtin/for-each-repo.c\n+++ b/builtin/for-each-repo.c\n@@ -2,6 +2,7 @@\n \n #include \"builtin.h\"\n #include \"config.h\"\n+#include \"environment.h\"\n #include \"gettext.h\"\n #include \"parse-options.h\"\n #include \"path.h\"\n@@ -19,6 +20,8 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)\n \tstruct child_process child = CHILD_PROCESS_INIT;\n \tchar *abspath = interpolate_path(path, 0);\n \n+\tsanitize_repo_env(&child.env);\n+\n \tchild.git_cmd = 1;\n \tstrvec_pushl(&child.args, \"-C\", abspath, NULL);\n \ndiff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh\nindex 512af34c82..80b163ea99 100755\n--- a/t/t0068-for-each-repo.sh\n+++ b/t/t0068-for-each-repo.sh\n@@ -8,10 +8,12 @@ TEST_NO_CREATE_REPO=1\n . ./test-lib.sh\n \n test_expect_success 'run based on configured value' '\n-\tgit init one &&\n-\tgit init two &&\n-\tgit init three &&\n-\tgit init ~/four &&\n+\tgit init --initial-branch=one one &&\n+\tgit init --initial-branch=two two &&\n+\tgit -C two worktree add --orphan ../three &&\n+\tgit -C three checkout -b three &&\n+\tgit init --initial-branch=four ~/four &&\n+\n \tgit -C two commit --allow-empty -m \"DID NOT RUN\" &&\n \tgit config --global run.key \"$TRASH_DIRECTORY/one\" &&\n \tgit config --global --add run.key \"$TRASH_DIRECTORY/three\" &&\n@@ -35,7 +37,43 @@ test_expect_success 'run based on configured value' '\n \tgit -C three log -1 --pretty=format:%s >message &&\n \tgrep again message &&\n \tgit -C ~/four log -1 --pretty=format:%s >message &&\n-\tgrep again message\n+\tgrep again message &&\n+\n+\tgit -C three for-each-repo --config=run.key -- \\\n+\t\tcommit --allow-empty -m \"ran from worktree\" &&\n+\tgit -C one log -1 --pretty=format:%s >message &&\n+\ttest_grep \"ran from worktree\" message &&\n+\tgit -C two log -1 --pretty=format:%s >message &&\n+\ttest_grep ! \"ran from worktree\" message &&\n+\tgit -C three log -1 --pretty=format:%s >message &&\n+\ttest_grep \"ran from worktree\" message &&\n+\tgit -C ~/four log -1 --pretty=format:%s >message &&\n+\ttest_grep \"ran from worktree\" message &&\n+\n+\t# Test running with config values set by environment\n+\tcat >expect <<-EOF &&\n+\tran from worktree (HEAD -> refs/heads/one)\n+\tran from worktree (HEAD -> refs/heads/three)\n+\tran from worktree (HEAD -> refs/heads/four)\n+\tEOF\n+\n+\tGIT_CONFIG_PARAMETERS=\"${SQ}log.decorate=full${SQ}\" \\\n+\t\tgit -C three for-each-repo --config=run.key -- log --format=\"%s%d\" -1 >out &&\n+\ttest_cmp expect out &&\n+\n+\tcat >test-config <<-EOF &&\n+\t[run]\n+\t\tkey = $(pwd)/one\n+\t\tkey = $(pwd)/three\n+\t\tkey = $(pwd)/four\n+\n+\t[log]\n+\t\tdecorate = full\n+\tEOF\n+\n+\tGIT_CONFIG_GLOBAL=\"$(pwd)/test-config\" \\\n+\t\tgit -C three for-each-repo --config=run.key -- log --format=\"%s%d\" -1 >out &&\n+\ttest_cmp expect out\n '\n \n test_expect_success 'do nothing on empty config' '\n-- \ngitgitgadget\n\n"},{"id":"537703","messageId":"8b1f083da1c91c3b3dff8a3af4fc0d57c1cb5ab9.1772559114.git.gitgitgadget@gmail.com","threadId":"65065","inReplyTo":"pull.2056.v4.git.1772559114.gitgitgadget@gmail.com","subject":"[PATCH v4 4/4] for-each-repo: simplify passing of parameters","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-03T17:31:54Z","receivedAt":"2026-03-03T17:32:03Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThis change simplifies the code somewhat from its original\nimplementation.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/for-each-repo.c | 9 +++------\n 1 file changed, 3 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c\nindex 82727c4aa2..927d3d92da 100644\n--- a/builtin/for-each-repo.c\n+++ b/builtin/for-each-repo.c\n@@ -14,9 +14,8 @@ static const char * const for_each_repo_usage[] = {\n \tNULL\n };\n \n-static int run_command_on_repo(const char *path, int argc, const char ** argv)\n+static int run_command_on_repo(const char *path, const char **argv)\n {\n-\tint i;\n \tstruct child_process child = CHILD_PROCESS_INIT;\n \tchar *abspath = interpolate_path(path, 0);\n \n@@ -24,9 +23,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)\n \n \tchild.git_cmd = 1;\n \tstrvec_pushl(&child.args, \"-C\", abspath, NULL);\n-\n-\tfor (i = 0; i < argc; i++)\n-\t\tstrvec_push(&child.args, argv[i]);\n+\tstrvec_pushv(&child.args, argv);\n \n \tfree(abspath);\n \n@@ -66,7 +63,7 @@ int cmd_for_each_repo(int argc,\n \t\treturn 0;\n \n \tfor (size_t i = 0; i < values->nr; i++) {\n-\t\tint ret = run_command_on_repo(values->items[i].string, argc, argv);\n+\t\tint ret = run_command_on_repo(values->items[i].string, argv);\n \t\tif (ret) {\n \t\t\tif (!keep_going)\n \t\t\t\t\treturn ret;\n-- \ngitgitgadget\n"},{"id":"537885","messageId":"20260305012035.GA53966@coredump.intra.peff.net","threadId":"65065","inReplyTo":"pull.2056.v4.git.1772559114.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 0/4] for-each-repo: work correctly in a worktree","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-05T01:20:35Z","receivedAt":"2026-03-05T01:20:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 03, 2026 at 05:31:50PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> Updates in V4\n> =============\n> \n> Minor updates from Peff's review:\n> \n> 1. Update the comment of prepare_other_repo_env() to avoid duplication.\n> 2. Rename the new method to sanitize_repo_env().\n> 3. Move incorrect removal of 'int i;' to correct patch.\n\nThis looks good to me. Thanks for accommodating my somewhat-bikeshedding\nreview.\n\n-Peff\n"},{"id":"537888","messageId":"aakfT3oio1XQSl4R@pks.im","threadId":"65065","inReplyTo":"20260305012035.GA53966@coredump.intra.peff.net","subject":"Re: [PATCH v4 0/4] for-each-repo: work correctly in a worktree","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-05T06:14:39Z","receivedAt":"2026-03-05T06:14:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Mar 04, 2026 at 08:20:35PM -0500, Jeff King wrote:\n> On Tue, Mar 03, 2026 at 05:31:50PM +0000, Derrick Stolee via GitGitGadget wrote:\n> \n> > Updates in V4\n> > =============\n> > \n> > Minor updates from Peff's review:\n> > \n> > 1. Update the comment of prepare_other_repo_env() to avoid duplication.\n> > 2. Rename the new method to sanitize_repo_env().\n> > 3. Move incorrect removal of 'int i;' to correct patch.\n> \n> This looks good to me. Thanks for accommodating my somewhat-bikeshedding\n> review.\n\nLikewise, this patch series looks good to me. Thanks!\n\nPatrick\n"},{"id":"537986","messageId":"fee9576a-787c-44f6-8630-cdec29df2b3b@gmail.com","threadId":"65065","inReplyTo":"aakfT3oio1XQSl4R@pks.im","subject":"Re: [PATCH v4 0/4] for-each-repo: work correctly in a worktree","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-05T17:23:45Z","receivedAt":"2026-03-05T17:23:47Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/5/2026 1:14 AM, Patrick Steinhardt wrote:\n> On Wed, Mar 04, 2026 at 08:20:35PM -0500, Jeff King wrote:\n>> On Tue, Mar 03, 2026 at 05:31:50PM +0000, Derrick Stolee via GitGitGadget wrote:\n>>\n>>> Updates in V4\n>>> =============\n>>>\n>>> Minor updates from Peff's review:\n>>>\n>>> 1. Update the comment of prepare_other_repo_env() to avoid duplication.\n>>> 2. Rename the new method to sanitize_repo_env().\n>>> 3. Move incorrect removal of 'int i;' to correct patch.\n>>\n>> This looks good to me. Thanks for accommodating my somewhat-bikeshedding\n>> review.\n> \n> Likewise, this patch series looks good to me. Thanks!\n\nThanks, all. This was far less trivial than I thought going\ninto it, so the careful review was essential.\n\nThanks,\n-Stolee\n\n"}]}