[PATCH 0/2] for-each-repo: work correctly in a worktree

[PATCH 0/2] for-each-repo: work correctly in a worktree

[Derrick Stolee via GitGitGadget]

This was reported by Matthew [1] and is a quick fix.

[1]
https://lore.kernel.org/git/CABpCjbY=wpStuhxqRJ5TSNV3A-CmN-g-xZGJOQGSSv3GYhs2fQ@mail.gmail.com/

I also took the liberty of removing the_repository as I wanted to make sure
that wasn't involved here.

Thanks, -Stolee

Derrick Stolee (2):
  for-each-repo: stop using the_repository
  for-each-repo: work correctly in a worktree

 builtin/for-each-repo.c  | 10 ++++++----
 t/t0068-for-each-repo.sh | 22 +++++++++++++++++-----
 2 files changed, 23 insertions(+), 9 deletions(-)


base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2056%2Fderrickstolee%2Ffor-each-repo-in-gitdir-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2056/derrickstolee/for-each-repo-in-gitdir-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2056
-- 
gitgitgadget

[PATCH 1/2] for-each-repo: stop using the_repository

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

This is a simple refactor before digging into a bug fix.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/for-each-repo.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
index 325a7925f1..478ccf1287 100644
--- a/builtin/for-each-repo.c
+++ b/builtin/for-each-repo.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
 #include "builtin.h"
 #include "config.h"
 #include "gettext.h"
@@ -33,7 +31,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)
 int cmd_for_each_repo(int argc,
 		      const char **argv,
 		      const char *prefix,
-		      struct repository *repo UNUSED)
+		      struct repository *repo)
 {
 	static const char *config_key = NULL;
 	int keep_going = 0;
@@ -55,7 +53,7 @@ int cmd_for_each_repo(int argc,
 	if (!config_key)
 		die(_("missing --config=<config>"));
 
-	err = repo_config_get_string_multi(the_repository, config_key, &values);
+	err = repo_config_get_string_multi(repo, config_key, &values);
 	if (err < 0)
 		usage_msg_optf(_("got bad config --config=%s"),
 			       for_each_repo_usage, options, config_key);
-- 
gitgitgadget

Re: [PATCH 1/2] for-each-repo: stop using the_repository

[Patrick Steinhardt]

On Tue, Feb 24, 2026 at 03:32:29AM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
> index 325a7925f1..478ccf1287 100644
> --- a/builtin/for-each-repo.c
> +++ b/builtin/for-each-repo.c
> @@ -1,5 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
> -
>  #include "builtin.h"
>  #include "config.h"
>  #include "gettext.h"
> @@ -33,7 +31,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)
>  int cmd_for_each_repo(int argc,
>  		      const char **argv,
>  		      const char *prefix,
> -		      struct repository *repo UNUSED)
> +		      struct repository *repo)
>  {
>  	static const char *config_key = NULL;
>  	int keep_going = 0;
> @@ -55,7 +53,7 @@ int cmd_for_each_repo(int argc,
>  	if (!config_key)
>  		die(_("missing --config=<config>"));
>  
> -	err = repo_config_get_string_multi(the_repository, config_key, &values);
> +	err = repo_config_get_string_multi(repo, config_key, &values);
>  	if (err < 0)
>  		usage_msg_optf(_("got bad config --config=%s"),
>  			       for_each_repo_usage, options, config_key);

The command is marked as `RUN_SETUP_GENTLY`, so it may run in a context
where there is no repository. In such cases, `repo` would be `NULL`, and
that would cause the command to segfault here, wouldn't it?

Patrick

Re: [PATCH 1/2] for-each-repo: stop using the_repository

[Derrick Stolee]

On 2/24/26 4:18 AM, Patrick Steinhardt wrote:
> On Tue, Feb 24, 2026 at 03:32:29AM +0000, Derrick Stolee via GitGitGadget wrote:
>> diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
>> index 325a7925f1..478ccf1287 100644
>> --- a/builtin/for-each-repo.c
>> +++ b/builtin/for-each-repo.c
>> @@ -1,5 +1,3 @@
>> -#define USE_THE_REPOSITORY_VARIABLE
>> -
>>   #include "builtin.h"
>>   #include "config.h"
>>   #include "gettext.h"
>> @@ -33,7 +31,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)
>>   int cmd_for_each_repo(int argc,
>>   		      const char **argv,
>>   		      const char *prefix,
>> -		      struct repository *repo UNUSED)
>> +		      struct repository *repo)
>>   {
>>   	static const char *config_key = NULL;
>>   	int keep_going = 0;
>> @@ -55,7 +53,7 @@ int cmd_for_each_repo(int argc,
>>   	if (!config_key)
>>   		die(_("missing --config=<config>"));
>>   
>> -	err = repo_config_get_string_multi(the_repository, config_key, &values);
>> +	err = repo_config_get_string_multi(repo, config_key, &values);
>>   	if (err < 0)
>>   		usage_msg_optf(_("got bad config --config=%s"),
>>   			       for_each_repo_usage, options, config_key);
> 
> The command is marked as `RUN_SETUP_GENTLY`, so it may run in a context
> where there is no repository. In such cases, `repo` would be `NULL`, and
> that would cause the command to segfault here, wouldn't it?

Ah. That's an interesting subtlety of the setup that I did not know.

I'll make sure this is covered in tests, because the current tests run in
the default test repo but our expected use case should be outside a repo.

Thanks,
-Stolee

[PATCH 2/2] for-each-repo: work correctly in a worktree

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

When run in a worktree, the GIT_DIR directory is set in a different way
than in a typical repository. Show this by updating t0068 to include a
worktree and add a test that runs from that worktree. This requires
moving the repo.key config into a global config instead of the base test
repository's local config (demonstrating that it worked with
non-worktree Git repositories).

The fix is simple: unset the environment variable before looping over
the repos.

Reported-by: Matthew Gabeler-Lee <fastcat@gmail.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/for-each-repo.c  |  4 ++++
 t/t0068-for-each-repo.sh | 22 +++++++++++++++++-----
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
index 478ccf1287..f39b085d7d 100644
--- a/builtin/for-each-repo.c
+++ b/builtin/for-each-repo.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "config.h"
+#include "environment.h"
 #include "gettext.h"
 #include "parse-options.h"
 #include "path.h"
@@ -60,6 +61,9 @@ int cmd_for_each_repo(int argc,
 	else if (err)
 		return 0;
 
+	/* Be sure to not pass GIT_DIR to children. */
+	unsetenv(GIT_DIR_ENVIRONMENT);
+
 	for (size_t i = 0; i < values->nr; i++) {
 		int ret = run_command_on_repo(values->items[i].string, argc, argv);
 		if (ret) {
diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh
index f2f3e50031..00b72dcac1 100755
--- a/t/t0068-for-each-repo.sh
+++ b/t/t0068-for-each-repo.sh
@@ -7,12 +7,13 @@ test_description='git for-each-repo builtin'
 test_expect_success 'run based on configured value' '
 	git init one &&
 	git init two &&
-	git init three &&
+	git -C two worktree add --orphan ../three &&
 	git init ~/four &&
 	git -C two commit --allow-empty -m "DID NOT RUN" &&
-	git config run.key "$TRASH_DIRECTORY/one" &&
-	git config --add run.key "$TRASH_DIRECTORY/three" &&
-	git config --add run.key "~/four" &&
+	git config --global run.key "$TRASH_DIRECTORY/one" &&
+	git config --global --add run.key "$TRASH_DIRECTORY/three" &&
+	git config --global --add run.key "~/four" &&
+
 	git for-each-repo --config=run.key commit --allow-empty -m "ran" &&
 	git -C one log -1 --pretty=format:%s >message &&
 	grep ran message &&
@@ -22,6 +23,7 @@ test_expect_success 'run based on configured value' '
 	grep ran message &&
 	git -C ~/four log -1 --pretty=format:%s >message &&
 	grep ran message &&
+
 	git for-each-repo --config=run.key -- commit --allow-empty -m "ran again" &&
 	git -C one log -1 --pretty=format:%s >message &&
 	grep again message &&
@@ -30,7 +32,17 @@ test_expect_success 'run based on configured value' '
 	git -C three log -1 --pretty=format:%s >message &&
 	grep again message &&
 	git -C ~/four log -1 --pretty=format:%s >message &&
-	grep again message
+	grep again message &&
+
+	git -C three for-each-repo --config=run.key -- commit --allow-empty -m "ran from worktree" &&
+	git -C one log -1 --pretty=format:%s >message &&
+	grep worktree message &&
+	git -C two log -1 --pretty=format:%s >message &&
+	! grep worktree message &&
+	git -C three log -1 --pretty=format:%s >message &&
+	grep worktree message &&
+	git -C ~/four log -1 --pretty=format:%s >message &&
+	grep worktree message
 '
 
 test_expect_success 'do nothing on empty config' '
-- 
gitgitgadget

Re: [PATCH 2/2] for-each-repo: work correctly in a worktree

[Eric Sunshine]

[Cc:+peff]

On Mon, Feb 23, 2026 at 10:32 PM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> When run in a worktree, the GIT_DIR directory is set in a different way
> than in a typical repository. Show this by updating t0068 to include a
> worktree and add a test that runs from that worktree. This requires
> moving the repo.key config into a global config instead of the base test
> repository's local config (demonstrating that it worked with
> non-worktree Git repositories).
>
> The fix is simple: unset the environment variable before looping over
> the repos.
>
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
> diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
> @@ -60,6 +61,9 @@ int cmd_for_each_repo(int argc,
> +       /* Be sure to not pass GIT_DIR to children. */
> +       unsetenv(GIT_DIR_ENVIRONMENT);

This only unsets GIT_DIR. Is that sufficient in the general case?
Elsewhere, we recommend[*] unsetting all of Git's local environment
variables.

[*]: From the "githooks" man page: "Environment variables, such as
GIT_DIR, GIT_WORK_TREE, etc., are exported so that Git commands run by
the hook can correctly locate the repository. If your hook needs to
invoke Git commands in a foreign repository or in a different working
tree of the same repository, then it should clear these environment
variables so they do not interfere with Git operations at the foreign
location. For example: `unset $(git rev-parse --local-env-vars)`"

Re: [PATCH 2/2] for-each-repo: work correctly in a worktree

[Jeff King]

On Mon, Feb 23, 2026 at 10:34:30PM -0500, Eric Sunshine wrote:

> > diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
> > @@ -60,6 +61,9 @@ int cmd_for_each_repo(int argc,
> > +       /* Be sure to not pass GIT_DIR to children. */
> > +       unsetenv(GIT_DIR_ENVIRONMENT);
> 
> This only unsets GIT_DIR. Is that sufficient in the general case?
> Elsewhere, we recommend[*] unsetting all of Git's local environment
> variables.
> 
> [*]: From the "githooks" man page: "Environment variables, such as
> GIT_DIR, GIT_WORK_TREE, etc., are exported so that Git commands run by
> the hook can correctly locate the repository. If your hook needs to
> invoke Git commands in a foreign repository or in a different working
> tree of the same repository, then it should clear these environment
> variables so they do not interfere with Git operations at the foreign
> location. For example: `unset $(git rev-parse --local-env-vars)`"

Yeah, agreed. There's another subtle issue, which is that this is
unsetting GIT_DIR in the parent process. So any other code we call that
is meant to run in the original repo might get confused. I can well
believe there isn't any such code for a command like for-each-repo, but
as a general principle, the change should be made in the sub-process.

You can stick the elements of local_repo_env into the "env" list of the
child_process struct. If you grep around, you can find some instances of
this.

There's an open question there of how to handle config in the
environment, though. Depending on the sub-process, you may or may not
want such config to pass down to it. For for-each-repo, I'd guess that
you'd want:

  git -c foo.bar=baz for-each-repo ...

to pass that foo.bar value. We do have a helper to handle that in
run-command.h:

  /**
   * Convenience function which prepares env for a command to be run in a
   * new repo. This adds all GIT_* environment variables to env with the
   * exception of GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT (which cause the
   * corresponding environment variables to be unset in the subprocess) and adds
   * an environment variable pointing to new_git_dir. See local_repo_env in
   * environment.h for more information.
   */
  void prepare_other_repo_env(struct strvec *env, const char *new_git_dir);

Do be careful using it here, though. It expects to set GIT_DIR itself to
point to the new repo (which is passed in). But I'm not sure that's 100%
compatible with how for-each-repo works, which is using "git -C $repo"
under the hood, and letting the usual discovery happen.

So for a bare repo, you'd want to pass the repo directory. But for a
non-bare one, you'd want $repo/.git. And there are even more weird
corner cases, like the fact that using "/my/repo/but/inside/a/subdir"
with for-each-repo will find "/my/repo".

So you might need to refactor prepare_other_repo_env() to split out the
"everything but the config" logic versus the "set GIT_DIR" logic. Or
just inline the former in run_command_on_repo(), though it probably is
better to keep the logic in one place (it's not many lines, but it has
to know about all of the env variables that affect config).

Alternatively, for-each-repo could do repo discovery itself on the paths
it is passed, before calling sub-programs. That's a bigger change, but
possibly it could or should be flagging an error for some cases? I
dunno.

-Peff

Re: [PATCH 2/2] for-each-repo: work correctly in a worktree

[Derrick Stolee]

On 2/24/26 4:18 AM, Jeff King wrote:
> On Mon, Feb 23, 2026 at 10:34:30PM -0500, Eric Sunshine wrote:
> 
>>> diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
>>> @@ -60,6 +61,9 @@ int cmd_for_each_repo(int argc,
>>> +       /* Be sure to not pass GIT_DIR to children. */
>>> +       unsetenv(GIT_DIR_ENVIRONMENT);
>>
>> This only unsets GIT_DIR. Is that sufficient in the general case?
>> Elsewhere, we recommend[*] unsetting all of Git's local environment
>> variables.
>>
>> [*]: From the "githooks" man page: "Environment variables, such as
>> GIT_DIR, GIT_WORK_TREE, etc., are exported so that Git commands run by
>> the hook can correctly locate the repository. If your hook needs to
>> invoke Git commands in a foreign repository or in a different working
>> tree of the same repository, then it should clear these environment
>> variables so they do not interfere with Git operations at the foreign
>> location. For example: `unset $(git rev-parse --local-env-vars)`"
> 
> Yeah, agreed. There's another subtle issue, which is that this is
> unsetting GIT_DIR in the parent process. So any other code we call that
> is meant to run in the original repo might get confused. I can well
> believe there isn't any such code for a command like for-each-repo, but
> as a general principle, the change should be made in the sub-process.
> 
> You can stick the elements of local_repo_env into the "env" list of the
> child_process struct. If you grep around, you can find some instances of
> this.
> 
> There's an open question there of how to handle config in the
> environment, though. Depending on the sub-process, you may or may not
> want such config to pass down to it. For for-each-repo, I'd guess that
> you'd want:
> 
>    git -c foo.bar=baz for-each-repo ...
> 
> to pass that foo.bar value. We do have a helper to handle that in
> run-command.h:
> 
>    /**
>     * Convenience function which prepares env for a command to be run in a
>     * new repo. This adds all GIT_* environment variables to env with the
>     * exception of GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT (which cause the
>     * corresponding environment variables to be unset in the subprocess) and adds
>     * an environment variable pointing to new_git_dir. See local_repo_env in
>     * environment.h for more information.
>     */
>    void prepare_other_repo_env(struct strvec *env, const char *new_git_dir);
> 
> Do be careful using it here, though. It expects to set GIT_DIR itself to
> point to the new repo (which is passed in). But I'm not sure that's 100%
> compatible with how for-each-repo works, which is using "git -C $repo"
> under the hood, and letting the usual discovery happen.
> 
> So for a bare repo, you'd want to pass the repo directory. But for a
> non-bare one, you'd want $repo/.git. And there are even more weird
> corner cases, like the fact that using "/my/repo/but/inside/a/subdir"
> with for-each-repo will find "/my/repo".
> 
> So you might need to refactor prepare_other_repo_env() to split out the
> "everything but the config" logic versus the "set GIT_DIR" logic. Or
> just inline the former in run_command_on_repo(), though it probably is
> better to keep the logic in one place (it's not many lines, but it has
> to know about all of the env variables that affect config).

Thanks for the recommendations. I'll come back with a more sophisticated
v2 that handles these issues.

> Alternatively, for-each-repo could do repo discovery itself on the paths
> it is passed, before calling sub-programs. That's a bigger change, but
> possibly it could or should be flagging an error for some cases? I
> dunno.
I'm surprised that passing '-C <repo>' doesn't already overwrite these
variables but I suppose environment variables override arguments in this
case. (This is the root of the bug.)

Thanks,
-Stolee

Re: [PATCH 2/2] for-each-repo: work correctly in a worktree

[Jeff King]

On Tue, Feb 24, 2026 at 07:11:13AM -0500, Derrick Stolee wrote:

> > it is passed, before calling sub-programs. That's a bigger change, but
> > possibly it could or should be flagging an error for some cases? I
> > dunno.
> I'm surprised that passing '-C <repo>' doesn't already overwrite these
> variables but I suppose environment variables override arguments in this
> case. (This is the root of the bug.)

I can see why you'd be surprised if you think of "-C" as "change to this
git repo". But it really is "change to this directory". It is perfectly
OK to "git -C" into a non-toplevel directory of a repo (and continue
respecting any repo discovery that happened already and is in the
environment), or even weird stuff like:

  GIT_DIR=/some/repo.git git -C /some/worktree add foo

What you almost kind-of want is "--git-dir", except it puts the onus on
the caller to find the actual repo directory (so detecting bare vs
discovering the .git). Part of the point of introducing -C long ago was
because --git-dir was so annoying to use.

Probably there is room for some middle-ground option, which is "do repo
detection starting in this directory and use that as the --git-dir" (and
I guess also do worktree discovery in the same way). But I don't think
we would ever switch -C to that. It would almost certainly break lots of
people if we changed it now.

-Peff

Re: [PATCH 2/2] for-each-repo: work correctly in a worktree

[Patrick Steinhardt]

On Mon, Feb 23, 2026 at 10:34:30PM -0500, Eric Sunshine wrote:
> [Cc:+peff]
> 
> On Mon, Feb 23, 2026 at 10:32 PM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > When run in a worktree, the GIT_DIR directory is set in a different way
> > than in a typical repository. Show this by updating t0068 to include a
> > worktree and add a test that runs from that worktree. This requires
> > moving the repo.key config into a global config instead of the base test
> > repository's local config (demonstrating that it worked with
> > non-worktree Git repositories).
> >
> > The fix is simple: unset the environment variable before looping over
> > the repos.
> >
> > Signed-off-by: Derrick Stolee <stolee@gmail.com>
> > ---
> > diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
> > @@ -60,6 +61,9 @@ int cmd_for_each_repo(int argc,
> > +       /* Be sure to not pass GIT_DIR to children. */
> > +       unsetenv(GIT_DIR_ENVIRONMENT);
> 
> This only unsets GIT_DIR. Is that sufficient in the general case?
> Elsewhere, we recommend[*] unsetting all of Git's local environment
> variables.

Good question indeed. We have the `local_repo_env` array that contains
all the environment variables that may influence repository discovery.

Patrick

[PATCH v2 0/2] for-each-repo: work correctly in a worktree

[Derrick Stolee via GitGitGadget]

This was reported by Matthew [1] and is a quick fix.

[1]
https://lore.kernel.org/git/CABpCjbY=wpStuhxqRJ5TSNV3A-CmN-g-xZGJOQGSSv3GYhs2fQ@mail.gmail.com/

I also took the liberty of removing the_repository as I wanted to make sure
that wasn't involved here.

Thanks, -Stolee

Derrick Stolee (2):
  for-each-repo: test outside of repo context
  for-each-repo: work correctly in a worktree

 builtin/for-each-repo.c  | 33 ++++++++++++++++++++++++++++++---
 t/t0068-for-each-repo.sh | 33 ++++++++++++++++++++++++---------
 2 files changed, 54 insertions(+), 12 deletions(-)


base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2056%2Fderrickstolee%2Ffor-each-repo-in-gitdir-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2056/derrickstolee/for-each-repo-in-gitdir-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2056

Range-diff vs v1:

 1:  86cd83f65b < -:  ---------- for-each-repo: stop using the_repository
 -:  ---------- > 1:  6e9d4f3029 for-each-repo: test outside of repo context
 2:  a47f9e9386 ! 2:  4e3f4aa6cd for-each-repo: work correctly in a worktree
     @@ Commit message
          repository's local config (demonstrating that it worked with
          non-worktree Git repositories).
      
     -    The fix is simple: unset the environment variable before looping over
     -    the repos.
     +    We need to be careful to unset the local Git environment variables and
     +    let the child process rediscover them, while also reinstating those
     +    variables in the parent process afterwards. Update run_command_on_repo()
     +    to store, unset, then reset the non-NULL variables.
      
          Reported-by: Matthew Gabeler-Lee <fastcat@gmail.com>
          Signed-off-by: Derrick Stolee <stolee@gmail.com>
      
       ## builtin/for-each-repo.c ##
      @@
     + 
       #include "builtin.h"
       #include "config.h"
      +#include "environment.h"
       #include "gettext.h"
       #include "parse-options.h"
       #include "path.h"
     -@@ builtin/for-each-repo.c: int cmd_for_each_repo(int argc,
     - 	else if (err)
     - 		return 0;
     +@@ builtin/for-each-repo.c: static const char * const for_each_repo_usage[] = {
     + 
     + static int run_command_on_repo(const char *path, int argc, const char ** argv)
     + {
     +-	int i;
     ++	int res;
     + 	struct child_process child = CHILD_PROCESS_INIT;
     ++	char **envvars;
     ++	size_t envvar_nr = 0;
     + 	char *abspath = interpolate_path(path, 0);
     + 
     ++	while (local_repo_env[envvar_nr])
     ++		envvar_nr++;
     ++
     ++	CALLOC_ARRAY(envvars, envvar_nr);
     ++
     ++	for (size_t i = 0; i < envvar_nr; i++) {
     ++		envvars[i] = getenv(local_repo_env[i]);
     ++
     ++		if (envvars[i]) {
     ++			unsetenv(local_repo_env[i]);
     ++			envvars[i] = xstrdup(envvars[i]);
     ++		}
     ++	}
     ++
     + 	child.git_cmd = 1;
     + 	strvec_pushl(&child.args, "-C", abspath, NULL);
     + 
     +-	for (i = 0; i < argc; i++)
     ++	for (int i = 0; i < argc; i++)
     + 		strvec_push(&child.args, argv[i]);
       
     -+	/* Be sure to not pass GIT_DIR to children. */
     -+	unsetenv(GIT_DIR_ENVIRONMENT);
     + 	free(abspath);
     + 
     +-	return run_command(&child);
     ++	res = run_command(&child);
     ++
     ++	for (size_t i = 0; i < envvar_nr; i++) {
     ++		if (envvars[i]) {
     ++			setenv(local_repo_env[i], envvars[i], 1);
     ++			free(envvars[i]);
     ++		}
     ++	}
      +
     - 	for (size_t i = 0; i < values->nr; i++) {
     - 		int ret = run_command_on_repo(values->items[i].string, argc, argv);
     - 		if (ret) {
     ++	free(envvars);
     ++	return res;
     + }
     + 
     + int cmd_for_each_repo(int argc,
      
       ## t/t0068-for-each-repo.sh ##
     -@@ t/t0068-for-each-repo.sh: test_description='git for-each-repo builtin'
     +@@ t/t0068-for-each-repo.sh: TEST_NO_CREATE_REPO=1
       test_expect_success 'run based on configured value' '
       	git init one &&
       	git init two &&
     @@ t/t0068-for-each-repo.sh: test_description='git for-each-repo builtin'
      +	git -C two worktree add --orphan ../three &&
       	git init ~/four &&
       	git -C two commit --allow-empty -m "DID NOT RUN" &&
     --	git config run.key "$TRASH_DIRECTORY/one" &&
     --	git config --add run.key "$TRASH_DIRECTORY/three" &&
     --	git config --add run.key "~/four" &&
     -+	git config --global run.key "$TRASH_DIRECTORY/one" &&
     -+	git config --global --add run.key "$TRASH_DIRECTORY/three" &&
     -+	git config --global --add run.key "~/four" &&
     -+
     - 	git for-each-repo --config=run.key commit --allow-empty -m "ran" &&
     - 	git -C one log -1 --pretty=format:%s >message &&
     - 	grep ran message &&
     -@@ t/t0068-for-each-repo.sh: test_expect_success 'run based on configured value' '
     - 	grep ran message &&
     - 	git -C ~/four log -1 --pretty=format:%s >message &&
     - 	grep ran message &&
     -+
     - 	git for-each-repo --config=run.key -- commit --allow-empty -m "ran again" &&
     - 	git -C one log -1 --pretty=format:%s >message &&
     - 	grep again message &&
     + 	git config --global run.key "$TRASH_DIRECTORY/one" &&
      @@ t/t0068-for-each-repo.sh: test_expect_success 'run based on configured value' '
       	git -C three log -1 --pretty=format:%s >message &&
       	grep again message &&

-- 
gitgitgadget

[PATCH v2 1/2] for-each-repo: test outside of repo context

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

The 'git for-each-repo' tool is frequently run outside of a repo context
in the real world. For example, it powers background maintenance.
Despite this typical case, we have not been testing it without a local
repository.

Update t0068 to stop creating a test repo and to use global config
everywhere. This has some subtle changes to test across the file.

This was noticed because an earlier attempt to remove the_repository
from builtin/for-each-repo.c did not catch a segmentation fault since
the passed 'repo' is NULL. This use of the_repository will need to stay
until we have a better way to handle config queries outside of a repo
context. Similar use still exists in builtin/config.c for the same
reason.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 t/t0068-for-each-repo.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh
index f2f3e50031..512af34c82 100755
--- a/t/t0068-for-each-repo.sh
+++ b/t/t0068-for-each-repo.sh
@@ -2,6 +2,9 @@
 
 test_description='git for-each-repo builtin'
 
+# We need to test running 'git for-each-repo' outside of a repo context.
+TEST_NO_CREATE_REPO=1
+
 . ./test-lib.sh
 
 test_expect_success 'run based on configured value' '
@@ -10,9 +13,10 @@ test_expect_success 'run based on configured value' '
 	git init three &&
 	git init ~/four &&
 	git -C two commit --allow-empty -m "DID NOT RUN" &&
-	git config run.key "$TRASH_DIRECTORY/one" &&
-	git config --add run.key "$TRASH_DIRECTORY/three" &&
-	git config --add run.key "~/four" &&
+	git config --global run.key "$TRASH_DIRECTORY/one" &&
+	git config --global --add run.key "$TRASH_DIRECTORY/three" &&
+	git config --global --add run.key "~/four" &&
+
 	git for-each-repo --config=run.key commit --allow-empty -m "ran" &&
 	git -C one log -1 --pretty=format:%s >message &&
 	grep ran message &&
@@ -22,6 +26,7 @@ test_expect_success 'run based on configured value' '
 	grep ran message &&
 	git -C ~/four log -1 --pretty=format:%s >message &&
 	grep ran message &&
+
 	git for-each-repo --config=run.key -- commit --allow-empty -m "ran again" &&
 	git -C one log -1 --pretty=format:%s >message &&
 	grep again message &&
@@ -46,7 +51,7 @@ test_expect_success 'error on bad config keys' '
 '
 
 test_expect_success 'error on NULL value for config keys' '
-	cat >>.git/config <<-\EOF &&
+	cat >>.gitconfig <<-\EOF &&
 	[empty]
 		key
 	EOF
@@ -59,8 +64,8 @@ test_expect_success 'error on NULL value for config keys' '
 '
 
 test_expect_success '--keep-going' '
-	git config keep.going non-existing &&
-	git config --add keep.going . &&
+	git config --global keep.going non-existing &&
+	git config --global --add keep.going one &&
 
 	test_must_fail git for-each-repo --config=keep.going \
 		-- branch >out 2>err &&
@@ -70,7 +75,7 @@ test_expect_success '--keep-going' '
 	test_must_fail git for-each-repo --config=keep.going --keep-going \
 		-- branch >out 2>err &&
 	test_grep "cannot change to .*non-existing" err &&
-	git branch >expect &&
+	git -C one branch >expect &&
 	test_cmp expect out
 '
 
-- 
gitgitgadget

[PATCH v2 2/2] for-each-repo: work correctly in a worktree

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

When run in a worktree, the GIT_DIR directory is set in a different way
than in a typical repository. Show this by updating t0068 to include a
worktree and add a test that runs from that worktree. This requires
moving the repo.key config into a global config instead of the base test
repository's local config (demonstrating that it worked with
non-worktree Git repositories).

We need to be careful to unset the local Git environment variables and
let the child process rediscover them, while also reinstating those
variables in the parent process afterwards. Update run_command_on_repo()
to store, unset, then reset the non-NULL variables.

Reported-by: Matthew Gabeler-Lee <fastcat@gmail.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/for-each-repo.c  | 33 ++++++++++++++++++++++++++++++---
 t/t0068-for-each-repo.sh | 14 ++++++++++++--
 2 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
index 325a7925f1..3f3e71979c 100644
--- a/builtin/for-each-repo.c
+++ b/builtin/for-each-repo.c
@@ -2,6 +2,7 @@
 
 #include "builtin.h"
 #include "config.h"
+#include "environment.h"
 #include "gettext.h"
 #include "parse-options.h"
 #include "path.h"
@@ -15,19 +16,45 @@ static const char * const for_each_repo_usage[] = {
 
 static int run_command_on_repo(const char *path, int argc, const char ** argv)
 {
-	int i;
+	int res;
 	struct child_process child = CHILD_PROCESS_INIT;
+	char **envvars;
+	size_t envvar_nr = 0;
 	char *abspath = interpolate_path(path, 0);
 
+	while (local_repo_env[envvar_nr])
+		envvar_nr++;
+
+	CALLOC_ARRAY(envvars, envvar_nr);
+
+	for (size_t i = 0; i < envvar_nr; i++) {
+		envvars[i] = getenv(local_repo_env[i]);
+
+		if (envvars[i]) {
+			unsetenv(local_repo_env[i]);
+			envvars[i] = xstrdup(envvars[i]);
+		}
+	}
+
 	child.git_cmd = 1;
 	strvec_pushl(&child.args, "-C", abspath, NULL);
 
-	for (i = 0; i < argc; i++)
+	for (int i = 0; i < argc; i++)
 		strvec_push(&child.args, argv[i]);
 
 	free(abspath);
 
-	return run_command(&child);
+	res = run_command(&child);
+
+	for (size_t i = 0; i < envvar_nr; i++) {
+		if (envvars[i]) {
+			setenv(local_repo_env[i], envvars[i], 1);
+			free(envvars[i]);
+		}
+	}
+
+	free(envvars);
+	return res;
 }
 
 int cmd_for_each_repo(int argc,
diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh
index 512af34c82..d55557a934 100755
--- a/t/t0068-for-each-repo.sh
+++ b/t/t0068-for-each-repo.sh
@@ -10,7 +10,7 @@ TEST_NO_CREATE_REPO=1
 test_expect_success 'run based on configured value' '
 	git init one &&
 	git init two &&
-	git init three &&
+	git -C two worktree add --orphan ../three &&
 	git init ~/four &&
 	git -C two commit --allow-empty -m "DID NOT RUN" &&
 	git config --global run.key "$TRASH_DIRECTORY/one" &&
@@ -35,7 +35,17 @@ test_expect_success 'run based on configured value' '
 	git -C three log -1 --pretty=format:%s >message &&
 	grep again message &&
 	git -C ~/four log -1 --pretty=format:%s >message &&
-	grep again message
+	grep again message &&
+
+	git -C three for-each-repo --config=run.key -- commit --allow-empty -m "ran from worktree" &&
+	git -C one log -1 --pretty=format:%s >message &&
+	grep worktree message &&
+	git -C two log -1 --pretty=format:%s >message &&
+	! grep worktree message &&
+	git -C three log -1 --pretty=format:%s >message &&
+	grep worktree message &&
+	git -C ~/four log -1 --pretty=format:%s >message &&
+	grep worktree message
 '
 
 test_expect_success 'do nothing on empty config' '
-- 
gitgitgadget

Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  static int run_command_on_repo(const char *path, int argc, const char ** argv)
>  {
> -	int i;
> +	int res;
>  	struct child_process child = CHILD_PROCESS_INIT;
> +	char **envvars;
> +	size_t envvar_nr = 0;
>  	char *abspath = interpolate_path(path, 0);
>  
> +	while (local_repo_env[envvar_nr])
> +		envvar_nr++;
> +
> +	CALLOC_ARRAY(envvars, envvar_nr);
> +
> +	for (size_t i = 0; i < envvar_nr; i++) {
> +		envvars[i] = getenv(local_repo_env[i]);
> +
> +		if (envvars[i]) {
> +			unsetenv(local_repo_env[i]);
> +			envvars[i] = xstrdup(envvars[i]);
> +		}
> +	}
>
>  	child.git_cmd = 1;
>  	strvec_pushl(&child.args, "-C", abspath, NULL);
>  
> -	for (i = 0; i < argc; i++)
> +	for (int i = 0; i < argc; i++)
>  		strvec_push(&child.args, argv[i]);
>  
>  	free(abspath);
>  
> -	return run_command(&child);
> +	res = run_command(&child);
> +
> +	for (size_t i = 0; i < envvar_nr; i++) {
> +		if (envvars[i]) {
> +			setenv(local_repo_env[i], envvars[i], 1);
> +			free(envvars[i]);
> +		}
> +	}
> +
> +	free(envvars);
> +	return res;
>  }
  

Doesn't run_command() let you unsetenv in the child without
affecting the parent process?

Looking at run-command.c:prep_childenv(), it seems that you can pass
"VAR=VAL" to "export VAR=VAL" in the child, and pass "VAR" to "unset
VAR" in the child.

Or is it essential to unset in both parent and child while the child
is working and that is why we unset in the parent and then restore
later?  I find this highly confusing.


>  int cmd_for_each_repo(int argc,
> diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh
> index 512af34c82..d55557a934 100755
> --- a/t/t0068-for-each-repo.sh
> +++ b/t/t0068-for-each-repo.sh
> @@ -10,7 +10,7 @@ TEST_NO_CREATE_REPO=1
>  test_expect_success 'run based on configured value' '
>  	git init one &&
>  	git init two &&
> -	git init three &&
> +	git -C two worktree add --orphan ../three &&
>  	git init ~/four &&
>  	git -C two commit --allow-empty -m "DID NOT RUN" &&
>  	git config --global run.key "$TRASH_DIRECTORY/one" &&
> @@ -35,7 +35,17 @@ test_expect_success 'run based on configured value' '
>  	git -C three log -1 --pretty=format:%s >message &&
>  	grep again message &&
>  	git -C ~/four log -1 --pretty=format:%s >message &&
> -	grep again message
> +	grep again message &&
> +
> +	git -C three for-each-repo --config=run.key -- commit --allow-empty -m "ran from worktree" &&
> +	git -C one log -1 --pretty=format:%s >message &&
> +	grep worktree message &&
> +	git -C two log -1 --pretty=format:%s >message &&
> +	! grep worktree message &&
> +	git -C three log -1 --pretty=format:%s >message &&
> +	grep worktree message &&
> +	git -C ~/four log -1 --pretty=format:%s >message &&
> +	grep worktree message
>  '
>  
>  test_expect_success 'do nothing on empty config' '

Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree

[Derrick Stolee]

On 2/24/26 4:47 PM, Junio C Hamano wrote:

> Doesn't run_command() let you unsetenv in the child without
> affecting the parent process?
> 
> Looking at run-command.c:prep_childenv(), it seems that you can pass
> "VAR=VAL" to "export VAR=VAL" in the child, and pass "VAR" to "unset
> VAR" in the child.

You're right. Here's a much simpler implementation:

static int run_command_on_repo(const char *path, int argc, const char ** argv)
{
	int i = 0;
	struct child_process child = CHILD_PROCESS_INIT;
	char *abspath = interpolate_path(path, 0);

	while (local_repo_env[i]) {
		strvec_push(&child.env, local_repo_env[i]);
		i++;
	}

	child.git_cmd = 1;
	strvec_pushl(&child.args, "-C", abspath, NULL);

	for (i = 0; i < argc; i++)
		strvec_push(&child.args, argv[i]);

	free(abspath);

	return run_command(&child);
}

> Or is it essential to unset in both parent and child while the child
> is working and that is why we unset in the parent and then restore
> later?  I find this highly confusing.

Nope, not necessary to adjust it in the parent. The simpler version
above works in my test case. I'll apply it to an upcoming v3, but
will wait a couple of days to see if there is any more feedback on
this v2.5 before doing so.

Thanks,
-Stolee

Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree

[Jeff King]

On Wed, Feb 25, 2026 at 06:44:51AM -0500, Derrick Stolee wrote:

> > Looking at run-command.c:prep_childenv(), it seems that you can pass
> > "VAR=VAL" to "export VAR=VAL" in the child, and pass "VAR" to "unset
> > VAR" in the child.
> 
> You're right. Here's a much simpler implementation:
> 
> static int run_command_on_repo(const char *path, int argc, const char ** argv)
> {
> 	int i = 0;
> 	struct child_process child = CHILD_PROCESS_INIT;
> 	char *abspath = interpolate_path(path, 0);
> 
> 	while (local_repo_env[i]) {
> 		strvec_push(&child.env, local_repo_env[i]);
> 		i++;
> 	}

You can actually just use strvec_pushv() to do this as a one-liner
(though annoyingly you need a cast because of how const works; you can
easily find an example with grep).

But I really think you should consider keeping config-related variables
in place, as prepare_other_repo_env() does. Otherwise something like:

  git -c pack.threads=1 for-each-repo repack -ad

will ignore that config in the sub-processes (whereas it currently is
respected).

And for that, you do need to loop yourself.

-Peff

Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree

[Derrick Stolee]

On 2/25/2026 8:13 AM, Jeff King wrote:
> On Wed, Feb 25, 2026 at 06:44:51AM -0500, Derrick Stolee wrote:
> 
>>> Looking at run-command.c:prep_childenv(), it seems that you can pass
>>> "VAR=VAL" to "export VAR=VAL" in the child, and pass "VAR" to "unset
>>> VAR" in the child.

> But I really think you should consider keeping config-related variables
> in place, as prepare_other_repo_env() does. Otherwise something like:
> 
>   git -c pack.threads=1 for-each-repo repack -ad
> 
> will ignore that config in the sub-processes (whereas it currently is
> respected).
> 
> And for that, you do need to loop yourself.

Great point. Here's another attempt:

static int run_command_on_repo(const char *path, int argc, const char ** argv)
{
	int i = 0;
	struct child_process child = CHILD_PROCESS_INIT;
	char *abspath = interpolate_path(path, 0);

	while (local_repo_env[i]) {
		/*
		 * Preserve pre-builtin options:
		 * - CONFIG_ENVIRONMENT, CONFIG_DATA_ENVIRONMENT, and
		 *   CONFIG_COUNT_ENVIRONMENT persist -c <name>=<value>
		 *   and --config-env=<name>=<envvar> options.
		 * - NO_REPLACE_OBJECTS_ENVIRONMENT persists the
		 *   --no-replace-objects option.
		 *
		 * Note that the following options are not in local_repo_env:
		 * - EXEC_PATH_ENVIRONMENT persists --exec-path option.
		 */
		if (strncmp(local_repo_env[i], "CONFIG_", 7) &&
		    strcmp(local_repo_env[i], NO_REPLACE_OBJECTS_ENVIRONMENT))
			strvec_push(&child.env, local_repo_env[i]);

		i++;
	}

	child.git_cmd = 1;
	strvec_pushl(&child.args, "-C", abspath, NULL);

	for (i = 0; i < argc; i++)
		strvec_push(&child.args, argv[i]);

	free(abspath);

	return run_command(&child);
}

This comment details my findings from comparing the list in
local_repo_env[] and the top-level options listed in
Documentation/git.adoc. That's how I was able to find that
--exec-path sets an environment variable that's NOT in the
list and we want to be sure we don't set it.

Should we add the comparison to EXEC_PATH_ENVIRONMENT as a
precaution to make sure it's not added to local_repo_env in
the future? Or is that too defensive?

Thanks,
-Stolee

Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree

[Junio C Hamano]

Derrick Stolee <stolee@gmail.com> writes:

> On 2/25/2026 8:13 AM, Jeff King wrote:
>> On Wed, Feb 25, 2026 at 06:44:51AM -0500, Derrick Stolee wrote:
>> 
>>>> Looking at run-command.c:prep_childenv(), it seems that you can pass
>>>> "VAR=VAL" to "export VAR=VAL" in the child, and pass "VAR" to "unset
>>>> VAR" in the child.
>
>> But I really think you should consider keeping config-related variables
>> in place, as prepare_other_repo_env() does. Otherwise something like:
>> 
>>   git -c pack.threads=1 for-each-repo repack -ad
>> 
>> will ignore that config in the sub-processes (whereas it currently is
>> respected).
>> 
>> And for that, you do need to loop yourself.
>
> Great point. Here's another attempt:
>
> static int run_command_on_repo(const char *path, int argc, const char ** argv)
> {
> 	int i = 0;
> 	struct child_process child = CHILD_PROCESS_INIT;
> 	char *abspath = interpolate_path(path, 0);
>
> 	while (local_repo_env[i]) {
> 		/*
> 		 * Preserve pre-builtin options:
> 		 * - CONFIG_ENVIRONMENT, CONFIG_DATA_ENVIRONMENT, and
> 		 *   CONFIG_COUNT_ENVIRONMENT persist -c <name>=<value>
> 		 *   and --config-env=<name>=<envvar> options.
> 		 * - NO_REPLACE_OBJECTS_ENVIRONMENT persists the
> 		 *   --no-replace-objects option.
> 		 *
> 		 * Note that the following options are not in local_repo_env:
> 		 * - EXEC_PATH_ENVIRONMENT persists --exec-path option.
> 		 */
> 		if (strncmp(local_repo_env[i], "CONFIG_", 7) &&

Minor nit: !starts_with() lets you avoid counting bytes yourself and
hardcoding "7" here.

> 		    strcmp(local_repo_env[i], NO_REPLACE_OBJECTS_ENVIRONMENT))
> 			strvec_push(&child.env, local_repo_env[i]);
>
> 		i++;
> 	}
>
> 	child.git_cmd = 1;
> 	strvec_pushl(&child.args, "-C", abspath, NULL);
>
> 	for (i = 0; i < argc; i++)
> 		strvec_push(&child.args, argv[i]);

If argv[argc] == NULL, then here is where we want strvec_pushv().

> 	free(abspath);
>
> 	return run_command(&child);
> }
>
> This comment details my findings from comparing the list in
> local_repo_env[] and the top-level options listed in
> Documentation/git.adoc. That's how I was able to find that
> --exec-path sets an environment variable that's NOT in the
> list and we want to be sure we don't set it.

Hmph, wouldn't we want to use specified exec-path inside ...

    git --exec-path=~/my/git/libexec for-each-repo sh -c "do things"

... "do things" script when we find Git related binaries?  Or am I
not getting what you are describing here?

> Should we add the comparison to EXEC_PATH_ENVIRONMENT as a
> precaution to make sure it's not added to local_repo_env in
> the future? Or is that too defensive?
>
> Thanks,
> -Stolee

Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree

[Phillip Wood]

On 26/02/2026 16:21, Junio C Hamano wrote:
> Derrick Stolee <stolee@gmail.com> writes:
> 
>> static int run_command_on_repo(const char *path, int argc, const char ** argv)
>> {
>> 	int i = 0;
>> 	struct child_process child = CHILD_PROCESS_INIT;
>> 	char *abspath = interpolate_path(path, 0);
>>
>> 	while (local_repo_env[i]) {
>> 		/*
>> 		 * Preserve pre-builtin options:
>> 		 * - CONFIG_ENVIRONMENT, CONFIG_DATA_ENVIRONMENT, and
>> 		 *   CONFIG_COUNT_ENVIRONMENT persist -c <name>=<value>
>> 		 *   and --config-env=<name>=<envvar> options.
>> 		 * - NO_REPLACE_OBJECTS_ENVIRONMENT persists the
>> 		 *   --no-replace-objects option.
>> 		 *
>> 		 * Note that the following options are not in local_repo_env:
>> 		 * - EXEC_PATH_ENVIRONMENT persists --exec-path option.
>> 		 */
>> 		if (strncmp(local_repo_env[i], "CONFIG_", 7) &&
> 
> Minor nit: !starts_with() lets you avoid counting bytes yourself and
> hardcoding "7" here.

More seriously it should be looking for strings starting with 
"GIT_CONFIG_", not the name of the preprocessor definitions.

Thanks

Phillip

Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

>>> 		 * Note that the following options are not in local_repo_env:
>>> 		 * - EXEC_PATH_ENVIRONMENT persists --exec-path option.
>>> 		 */
>>> 		if (strncmp(local_repo_env[i], "CONFIG_", 7) &&
>> 
>> Minor nit: !starts_with() lets you avoid counting bytes yourself and
>> hardcoding "7" here.
>
> More seriously it should be looking for strings starting with 
> "GIT_CONFIG_", not the name of the preprocessor definitions.

Thanks.  I missed that completely.

Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree

[Derrick Stolee]

On 2/27/26 2:41 PM, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>>>> 		 * Note that the following options are not in local_repo_env:
>>>> 		 * - EXEC_PATH_ENVIRONMENT persists --exec-path option.
>>>> 		 */
>>>> 		if (strncmp(local_repo_env[i], "CONFIG_", 7) &&
>>>
>>> Minor nit: !starts_with() lets you avoid counting bytes yourself and
>>> hardcoding "7" here.
>>
>> More seriously it should be looking for strings starting with
>> "GIT_CONFIG_", not the name of the preprocessor definitions.
> 
> Thanks.  I missed that completely.

Same! And I will try to find a way to test these things to ensure
these mistakes are not prevented only by careful code reviewers!

Thanks,
-Stolee

Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree

[Jeff King]

On Thu, Feb 26, 2026 at 08:21:23AM -0800, Junio C Hamano wrote:

> > This comment details my findings from comparing the list in
> > local_repo_env[] and the top-level options listed in
> > Documentation/git.adoc. That's how I was able to find that
> > --exec-path sets an environment variable that's NOT in the
> > list and we want to be sure we don't set it.
> 
> Hmph, wouldn't we want to use specified exec-path inside ...
> 
>     git --exec-path=~/my/git/libexec for-each-repo sh -c "do things"
> 
> ... "do things" script when we find Git related binaries?  Or am I
> not getting what you are describing here?

I almost responded with the same thing, but I think the suggestion is
going the other way: we (correctly) do not list EXEC_PATH_ENVIRONMENT
via local_repo_env, so it will never be removed from the environment.
And thus we do not need to do anything here to drop it from the list of
what is removed. Double negation. :)

The second paragraph:

> > Should we add the comparison to EXEC_PATH_ENVIRONMENT as a
> > precaution to make sure it's not added to local_repo_env in
> > the future? Or is that too defensive?

makes that more clear, I think. I did have to read the whole thing
twice. ;)

-Peff

Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree

[Jeff King]

On Thu, Feb 26, 2026 at 10:29:47AM -0500, Derrick Stolee wrote:

> Great point. Here's another attempt:
> 
> static int run_command_on_repo(const char *path, int argc, const char ** argv)
> {
> 	int i = 0;
> 	struct child_process child = CHILD_PROCESS_INIT;
> 	char *abspath = interpolate_path(path, 0);
> 
> 	while (local_repo_env[i]) {
> 		/*
> 		 * Preserve pre-builtin options:
> 		 * - CONFIG_ENVIRONMENT, CONFIG_DATA_ENVIRONMENT, and
> 		 *   CONFIG_COUNT_ENVIRONMENT persist -c <name>=<value>
> 		 *   and --config-env=<name>=<envvar> options.
> 		 * - NO_REPLACE_OBJECTS_ENVIRONMENT persists the
> 		 *   --no-replace-objects option.
> 		 *
> 		 * Note that the following options are not in local_repo_env:
> 		 * - EXEC_PATH_ENVIRONMENT persists --exec-path option.
> 		 */
> 		if (strncmp(local_repo_env[i], "CONFIG_", 7) &&
> 		    strcmp(local_repo_env[i], NO_REPLACE_OBJECTS_ENVIRONMENT))
> 			strvec_push(&child.env, local_repo_env[i]);

This is slightly different than what prepare_other_repo_env() does:

  - it doesn't drop GIT_CONFIG_*, but assumes that removing
    GIT_CONFIG_COUNT is enough for GIT_CONFIG_KEY/VALUE to be ignored
    (and then also removes GIT_CONFIG_PARAMETERS, of course)

  - it doesn't consider NO_REPLACE_OBJECTS at all

I think you could make arguments either way about what should happen
when spawning a command in another repo. But I'd really prefer for us to
have a single spot to specify that policy, and not subtly-different
behavior from different commands. So I'd really like to see this using
that other function (or the logic from it factored out into a helper).

And then we can consider whether to make changes to that policy.

Dropping GIT_CONFIG_* from the environment does make sense in general,
but it doesn't actually happen with the patch above (because only
GIT_CONFIG_COUNT is in the local_repo_env list; to find the others we'd
have to actually enumerate the current environment).

For NO_REPLACE_OBJECTS, I think you could argue that it should not be in
local_repo_env at all. It is more about the operation being performed,
not the repository itself. So for example in this command:

  git --no-replace-objects fetch

I would expect that NO_REPLACE_OBJECTS to make it down to any submodule
fetches we do. Likewise for other operation-level variables like
GIT_LITERAL_PATHSPECS, but those are already (correctly IMHO) omitted
from local_repo_env.

It looks like NO_REPLACE_OBJECTS got pulled from the connect.c code in
48a7c1c49d (Refactor list of of repo-local env vars, 2010-02-25). And I
could see somebody wanting to make sure that upload-pack behaved
predictably with respect to replace refs, but it already does: it
disables replace refs itself as part of its startup code.

> This comment details my findings from comparing the list in
> local_repo_env[] and the top-level options listed in
> Documentation/git.adoc. That's how I was able to find that
> --exec-path sets an environment variable that's NOT in the
> list and we want to be sure we don't set it.
> 
> Should we add the comparison to EXEC_PATH_ENVIRONMENT as a
> precaution to make sure it's not added to local_repo_env in
> the future? Or is that too defensive?

I don't think we need to bother. Obviously adding it to local_repo_env
would be the wrong thing, but that is true of lots of variables. Trying
to make a list is just going to result in a list that is out-of-date,
because there's nothing pushing people to update it when they introduce
a new variable.

You can imagine a different world, where we had a single list of all
environment variables, and new ones _had_ to be added to the list in
order to function, and each entry had a bitflag for "this is a
local-repo value", then that might force each new addition to consider
whether it should be added. But we don't have such a list, and I think
structuring things that way would introduce new complications and
awkwardness.

So IMHO we should just rely on review to reject a patch that tries
something silly like adding EXEC_PATH_ENVIRONMENT to local_repo_env.

-Peff

Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree

[Derrick Stolee]

On 2/27/2026 5:42 PM, Jeff King wrote:
> On Thu, Feb 26, 2026 at 10:29:47AM -0500, Derrick Stolee wrote:
> 
>> Great point. Here's another attempt:
>>
>> static int run_command_on_repo(const char *path, int argc, const char ** argv)
>> {
>> 	int i = 0;
>> 	struct child_process child = CHILD_PROCESS_INIT;
>> 	char *abspath = interpolate_path(path, 0);
>>
>> 	while (local_repo_env[i]) {
>> 		/*
>> 		 * Preserve pre-builtin options:
>> 		 * - CONFIG_ENVIRONMENT, CONFIG_DATA_ENVIRONMENT, and
>> 		 *   CONFIG_COUNT_ENVIRONMENT persist -c <name>=<value>
>> 		 *   and --config-env=<name>=<envvar> options.
>> 		 * - NO_REPLACE_OBJECTS_ENVIRONMENT persists the
>> 		 *   --no-replace-objects option.
>> 		 *
>> 		 * Note that the following options are not in local_repo_env:
>> 		 * - EXEC_PATH_ENVIRONMENT persists --exec-path option.
>> 		 */
>> 		if (strncmp(local_repo_env[i], "CONFIG_", 7) &&
>> 		    strcmp(local_repo_env[i], NO_REPLACE_OBJECTS_ENVIRONMENT))
>> 			strvec_push(&child.env, local_repo_env[i]);
> 
> This is slightly different than what prepare_other_repo_env() does:
> 
>   - it doesn't drop GIT_CONFIG_*, but assumes that removing
>     GIT_CONFIG_COUNT is enough for GIT_CONFIG_KEY/VALUE to be ignored
>     (and then also removes GIT_CONFIG_PARAMETERS, of course)
> 
>   - it doesn't consider NO_REPLACE_OBJECTS at all
> 
> I think you could make arguments either way about what should happen
> when spawning a command in another repo. But I'd really prefer for us to
> have a single spot to specify that policy, and not subtly-different
> behavior from different commands. So I'd really like to see this using
> that other function (or the logic from it factored out into a helper).

I agree that it would be best to have a single place.

I was looking at prepare_other_repo_env() and saw that it requires a
computed gitdir, which is not easy to compute. We want the child process
to perform that discovery based on the -C parameter.

However, we can extract the existing environment clearing logic and use
that here. I'll give that a try and confirm that it passes the tests
that I prepared to fix the bugs in this version.

> And then we can consider whether to make changes to that policy.
> 
> Dropping GIT_CONFIG_* from the environment does make sense in general,
> but it doesn't actually happen with the patch above (because only
> GIT_CONFIG_COUNT is in the local_repo_env list; to find the others we'd
> have to actually enumerate the current environment).

It has GIT_CONFIG (the local Git config file), GIT_CONFIG_COUNT, and
GIT_CONFIG_PARAMETERS. My patch was wrong because of the string, showing
the value in having tests to confirm the right behavior.

> For NO_REPLACE_OBJECTS, I think you could argue that it should not be in
> local_repo_env at all. It is more about the operation being performed,
> not the repository itself. So for example in this command:
> 
>   git --no-replace-objects fetch
> 
> I would expect that NO_REPLACE_OBJECTS to make it down to any submodule
> fetches we do. Likewise for other operation-level variables like
> GIT_LITERAL_PATHSPECS, but those are already (correctly IMHO) omitted
> from local_repo_env.
> 
> It looks like NO_REPLACE_OBJECTS got pulled from the connect.c code in
> 48a7c1c49d (Refactor list of of repo-local env vars, 2010-02-25). And I
> could see somebody wanting to make sure that upload-pack behaved
> predictably with respect to replace refs, but it already does: it
> disables replace refs itself as part of its startup code.

OK. I won't special case this myself and will let this be changed
independently, if that is indeed valuable.

>> This comment details my findings from comparing the list in
>> local_repo_env[] and the top-level options listed in
>> Documentation/git.adoc. That's how I was able to find that
>> --exec-path sets an environment variable that's NOT in the
>> list and we want to be sure we don't set it.
>>
>> Should we add the comparison to EXEC_PATH_ENVIRONMENT as a
>> precaution to make sure it's not added to local_repo_env in
>> the future? Or is that too defensive?
> 
> I don't think we need to bother. Obviously adding it to local_repo_env
> would be the wrong thing, but that is true of lots of variables. Trying
> to make a list is just going to result in a list that is out-of-date,
> because there's nothing pushing people to update it when they introduce
> a new variable.

This is where I was landing, too.

> You can imagine a different world, where we had a single list of all
> environment variables, and new ones _had_ to be added to the list in
> order to function, and each entry had a bitflag for "this is a
> local-repo value", then that might force each new addition to consider
> whether it should be added. But we don't have such a list, and I think
> structuring things that way would introduce new complications and
> awkwardness.

This makes sense. In the meantime, having a single place that unsets
environment variables for certain child processes is good enough to
cover what we need here.

Thanks,
-Stolee

Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree

[Jeff King]

On Mon, Mar 02, 2026 at 10:31:48AM -0500, Derrick Stolee wrote:

> > I think you could make arguments either way about what should happen
> > when spawning a command in another repo. But I'd really prefer for us to
> > have a single spot to specify that policy, and not subtly-different
> > behavior from different commands. So I'd really like to see this using
> > that other function (or the logic from it factored out into a helper).
> 
> I agree that it would be best to have a single place.
> 
> I was looking at prepare_other_repo_env() and saw that it requires a
> computed gitdir, which is not easy to compute. We want the child process
> to perform that discovery based on the -C parameter.
> 
> However, we can extract the existing environment clearing logic and use
> that here. I'll give that a try and confirm that it passes the tests
> that I prepared to fix the bugs in this version.

Yeah, that was exactly the refactoring I had in mind. What you have in
v3 looks good.

> > Dropping GIT_CONFIG_* from the environment does make sense in general,
> > but it doesn't actually happen with the patch above (because only
> > GIT_CONFIG_COUNT is in the local_repo_env list; to find the others we'd
> > have to actually enumerate the current environment).
> 
> It has GIT_CONFIG (the local Git config file), GIT_CONFIG_COUNT, and
> GIT_CONFIG_PARAMETERS. My patch was wrong because of the string, showing
> the value in having tests to confirm the right behavior.

Ah, I forgot about GIT_CONFIG (though it obviously would not match
CONFIG_, even if we correctly said GIT_CONFIG_). It's mostly a
historical oddity for git-config itself and can be ignored (other
commands do not even look at it, and we'd never set it ourselves).

-Peff

[PATCH v3 0/4] for-each-repo: work correctly in a worktree

[Derrick Stolee via GitGitGadget]

This was reported by Matthew [1] and is a quick fix.

[1]
https://lore.kernel.org/git/CABpCjbY=wpStuhxqRJ5TSNV3A-CmN-g-xZGJOQGSSv3GYhs2fQ@mail.gmail.com/

I also took the liberty of removing the_repository as I wanted to make sure
that wasn't involved here.


Updates in v3
=============

v2 included some important changes that apparently didn't save in my cover
letter. Sorry. The most important one was that the first patch was replaced
with one that tests the command outside of a Git repo so the first patch in
v1 can be caught as a bug.

But v2 also included a bug! The filtering of GIT_CONFIG* environment
variables was incorrect. Further, duplicating this logic from
prepare_other_repo_env() was a new bug waiting to happen.

Here is the new setup:

 * Patch 1 is the same as in v2.
 * A new Patch 2 extracts clear_local_repo_env() from
   prepare_other_repo_env().
 * The old Patch 2 is now Patch 3 with a much simpler implementation that
   calls clear_local_repo_env().
 * Tests are added to check that these config-related environment variables
   are preserved. The NO_REPLACE_OBJECTS case seemed like it would take
   longer than necessary to set up for the value it provides.
 * A fourth patch is added that simplifies the use of argv. It's separate
   because it modifies the original implementation in a way that is a "nice
   to have" but isn't necessary for solving the bug that motivates the
   series.

Thanks, -Stolee

Derrick Stolee (4):
  for-each-repo: test outside of repo context
  run-command: extract clear_local_repo_env helper
  for-each-repo: work correctly in a worktree
  for-each-repo: simplify passing of parameters

 builtin/for-each-repo.c  | 12 +++----
 run-command.c            |  7 ++++-
 run-command.h            |  7 +++++
 t/t0068-for-each-repo.sh | 67 +++++++++++++++++++++++++++++++++-------
 4 files changed, 74 insertions(+), 19 deletions(-)


base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2056%2Fderrickstolee%2Ffor-each-repo-in-gitdir-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2056/derrickstolee/for-each-repo-in-gitdir-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2056

Range-diff vs v2:

 1:  6e9d4f3029 = 1:  6e9d4f3029 for-each-repo: test outside of repo context
 -:  ---------- > 2:  13d783dbbd run-command: extract clear_local_repo_env helper
 2:  4e3f4aa6cd ! 3:  2a6091095f for-each-repo: work correctly in a worktree
     @@ Commit message
          We need to be careful to unset the local Git environment variables and
          let the child process rediscover them, while also reinstating those
          variables in the parent process afterwards. Update run_command_on_repo()
     -    to store, unset, then reset the non-NULL variables.
     +    to use the new clear_local_repo_env() helper method to erase these
     +    environment variables.
     +
     +    During review of this bug fix, there were several incorrect patches
     +    demonstrating different bad behaviors. Most of these are covered by
     +    tests, when it is not too expensive to set it up. One case that would be
     +    expensive to set up is the GIT_NO_REPLACE_OBJECTS environment variable,
     +    but we trust that using clear_local_repo_env() will be sufficient to
     +    capture these uncovered cases by using the common code for resetting
     +    environment variables.
      
          Reported-by: Matthew Gabeler-Lee <fastcat@gmail.com>
          Signed-off-by: Derrick Stolee <stolee@gmail.com>
     @@ builtin/for-each-repo.c: static const char * const for_each_repo_usage[] = {
       static int run_command_on_repo(const char *path, int argc, const char ** argv)
       {
      -	int i;
     -+	int res;
       	struct child_process child = CHILD_PROCESS_INIT;
     -+	char **envvars;
     -+	size_t envvar_nr = 0;
       	char *abspath = interpolate_path(path, 0);
       
     -+	while (local_repo_env[envvar_nr])
     -+		envvar_nr++;
     -+
     -+	CALLOC_ARRAY(envvars, envvar_nr);
     -+
     -+	for (size_t i = 0; i < envvar_nr; i++) {
     -+		envvars[i] = getenv(local_repo_env[i]);
     -+
     -+		if (envvars[i]) {
     -+			unsetenv(local_repo_env[i]);
     -+			envvars[i] = xstrdup(envvars[i]);
     -+		}
     -+	}
     ++	clear_local_repo_env(&child.env);
      +
       	child.git_cmd = 1;
       	strvec_pushl(&child.args, "-C", abspath, NULL);
       
     --	for (i = 0; i < argc; i++)
     -+	for (int i = 0; i < argc; i++)
     - 		strvec_push(&child.args, argv[i]);
     - 
     - 	free(abspath);
     - 
     --	return run_command(&child);
     -+	res = run_command(&child);
     -+
     -+	for (size_t i = 0; i < envvar_nr; i++) {
     -+		if (envvars[i]) {
     -+			setenv(local_repo_env[i], envvars[i], 1);
     -+			free(envvars[i]);
     -+		}
     -+	}
     -+
     -+	free(envvars);
     -+	return res;
     - }
     - 
     - int cmd_for_each_repo(int argc,
      
       ## t/t0068-for-each-repo.sh ##
      @@ t/t0068-for-each-repo.sh: TEST_NO_CREATE_REPO=1
     + . ./test-lib.sh
     + 
       test_expect_success 'run based on configured value' '
     - 	git init one &&
     - 	git init two &&
     +-	git init one &&
     +-	git init two &&
      -	git init three &&
     +-	git init ~/four &&
     ++	git init --initial-branch=one one &&
     ++	git init --initial-branch=two two &&
      +	git -C two worktree add --orphan ../three &&
     - 	git init ~/four &&
     ++	git -C three checkout -b three &&
     ++	git init --initial-branch=four ~/four &&
     ++
       	git -C two commit --allow-empty -m "DID NOT RUN" &&
       	git config --global run.key "$TRASH_DIRECTORY/one" &&
     + 	git config --global --add run.key "$TRASH_DIRECTORY/three" &&
      @@ t/t0068-for-each-repo.sh: test_expect_success 'run based on configured value' '
       	git -C three log -1 --pretty=format:%s >message &&
       	grep again message &&
     @@ t/t0068-for-each-repo.sh: test_expect_success 'run based on configured value' '
      -	grep again message
      +	grep again message &&
      +
     -+	git -C three for-each-repo --config=run.key -- commit --allow-empty -m "ran from worktree" &&
     ++	git -C three for-each-repo --config=run.key -- \
     ++		commit --allow-empty -m "ran from worktree" &&
      +	git -C one log -1 --pretty=format:%s >message &&
     -+	grep worktree message &&
     ++	test_grep "ran from worktree" message &&
      +	git -C two log -1 --pretty=format:%s >message &&
     -+	! grep worktree message &&
     ++	test_grep ! "ran from worktree" message &&
      +	git -C three log -1 --pretty=format:%s >message &&
     -+	grep worktree message &&
     ++	test_grep "ran from worktree" message &&
      +	git -C ~/four log -1 --pretty=format:%s >message &&
     -+	grep worktree message
     ++	test_grep "ran from worktree" message &&
     ++
     ++	# Test running with config values set by environment
     ++	cat >expect <<-EOF &&
     ++	ran from worktree (HEAD -> refs/heads/one)
     ++	ran from worktree (HEAD -> refs/heads/three)
     ++	ran from worktree (HEAD -> refs/heads/four)
     ++	EOF
     ++
     ++	GIT_CONFIG_PARAMETERS="${SQ}log.decorate=full${SQ}" \
     ++		git -C three for-each-repo --config=run.key -- log --format="%s%d" -1 >out &&
     ++	test_cmp expect out &&
     ++
     ++	cat >test-config <<-EOF &&
     ++	[run]
     ++		key = $(pwd)/one
     ++		key = $(pwd)/three
     ++		key = $(pwd)/four
     ++
     ++	[log]
     ++		decorate = full
     ++	EOF
     ++
     ++	GIT_CONFIG_GLOBAL="$(pwd)/test-config" \
     ++		git -C three for-each-repo --config=run.key -- log --format="%s%d" -1 >out &&
     ++	test_cmp expect out
       '
       
       test_expect_success 'do nothing on empty config' '
 -:  ---------- > 4:  f6582e9402 for-each-repo: simplify passing of parameters

-- 
gitgitgadget

[PATCH v3 1/4] for-each-repo: test outside of repo context

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

The 'git for-each-repo' tool is frequently run outside of a repo context
in the real world. For example, it powers background maintenance.
Despite this typical case, we have not been testing it without a local
repository.

Update t0068 to stop creating a test repo and to use global config
everywhere. This has some subtle changes to test across the file.

This was noticed because an earlier attempt to remove the_repository
from builtin/for-each-repo.c did not catch a segmentation fault since
the passed 'repo' is NULL. This use of the_repository will need to stay
until we have a better way to handle config queries outside of a repo
context. Similar use still exists in builtin/config.c for the same
reason.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 t/t0068-for-each-repo.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh
index f2f3e50031..512af34c82 100755
--- a/t/t0068-for-each-repo.sh
+++ b/t/t0068-for-each-repo.sh
@@ -2,6 +2,9 @@
 
 test_description='git for-each-repo builtin'
 
+# We need to test running 'git for-each-repo' outside of a repo context.
+TEST_NO_CREATE_REPO=1
+
 . ./test-lib.sh
 
 test_expect_success 'run based on configured value' '
@@ -10,9 +13,10 @@ test_expect_success 'run based on configured value' '
 	git init three &&
 	git init ~/four &&
 	git -C two commit --allow-empty -m "DID NOT RUN" &&
-	git config run.key "$TRASH_DIRECTORY/one" &&
-	git config --add run.key "$TRASH_DIRECTORY/three" &&
-	git config --add run.key "~/four" &&
+	git config --global run.key "$TRASH_DIRECTORY/one" &&
+	git config --global --add run.key "$TRASH_DIRECTORY/three" &&
+	git config --global --add run.key "~/four" &&
+
 	git for-each-repo --config=run.key commit --allow-empty -m "ran" &&
 	git -C one log -1 --pretty=format:%s >message &&
 	grep ran message &&
@@ -22,6 +26,7 @@ test_expect_success 'run based on configured value' '
 	grep ran message &&
 	git -C ~/four log -1 --pretty=format:%s >message &&
 	grep ran message &&
+
 	git for-each-repo --config=run.key -- commit --allow-empty -m "ran again" &&
 	git -C one log -1 --pretty=format:%s >message &&
 	grep again message &&
@@ -46,7 +51,7 @@ test_expect_success 'error on bad config keys' '
 '
 
 test_expect_success 'error on NULL value for config keys' '
-	cat >>.git/config <<-\EOF &&
+	cat >>.gitconfig <<-\EOF &&
 	[empty]
 		key
 	EOF
@@ -59,8 +64,8 @@ test_expect_success 'error on NULL value for config keys' '
 '
 
 test_expect_success '--keep-going' '
-	git config keep.going non-existing &&
-	git config --add keep.going . &&
+	git config --global keep.going non-existing &&
+	git config --global --add keep.going one &&
 
 	test_must_fail git for-each-repo --config=keep.going \
 		-- branch >out 2>err &&
@@ -70,7 +75,7 @@ test_expect_success '--keep-going' '
 	test_must_fail git for-each-repo --config=keep.going --keep-going \
 		-- branch >out 2>err &&
 	test_grep "cannot change to .*non-existing" err &&
-	git branch >expect &&
+	git -C one branch >expect &&
 	test_cmp expect out
 '
 
-- 
gitgitgadget

Re: [PATCH v3 1/4] for-each-repo: test outside of repo context

[Jeff King]

On Mon, Mar 02, 2026 at 03:36:42PM +0000, Derrick Stolee via GitGitGadget wrote:

>  test_description='git for-each-repo builtin'
>  
> +# We need to test running 'git for-each-repo' outside of a repo context.
> +TEST_NO_CREATE_REPO=1
> +
>  . ./test-lib.sh

Interesting. I was going to point out that this won't do what you want
by itself, because Git will keep walking out of the trash directory and
may find the containing repository.

But it looks like this should be enough due to 614c3d8f2e (test-lib: set
GIT_CEILING_DIRECTORIES to protect the surrounding repository,
2021-08-29). Supporting this case wasn't the intent of that patch, but I
don't see any reason why it should not work reliably.

-Peff

Re: [PATCH v3 1/4] for-each-repo: test outside of repo context

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Mar 02, 2026 at 03:36:42PM +0000, Derrick Stolee via GitGitGadget wrote:
>
>>  test_description='git for-each-repo builtin'
>>  
>> +# We need to test running 'git for-each-repo' outside of a repo context.
>> +TEST_NO_CREATE_REPO=1
>> +
>>  . ./test-lib.sh
>
> Interesting. I was going to point out that this won't do what you want
> by itself, because Git will keep walking out of the trash directory and
> may find the containing repository.
>
> But it looks like this should be enough due to 614c3d8f2e (test-lib: set
> GIT_CEILING_DIRECTORIES to protect the surrounding repository,
> 2021-08-29). Supporting this case wasn't the intent of that patch, but I
> don't see any reason why it should not work reliably.

I am surprised that use of GIT_CEILING_DIRECTORIES was not done
until 2021, actually.  The reason the configuration variable was
invented for is exactly to avoid discovery processes going upward
and ending up in a repository different from what we mean to work
with.

Re: [PATCH v3 1/4] for-each-repo: test outside of repo context

[Derrick Stolee]

On 3/2/2026 1:31 PM, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
> 
>> On Mon, Mar 02, 2026 at 03:36:42PM +0000, Derrick Stolee via GitGitGadget wrote:
>>
>>>  test_description='git for-each-repo builtin'
>>>  
>>> +# We need to test running 'git for-each-repo' outside of a repo context.
>>> +TEST_NO_CREATE_REPO=1
>>> +
>>>  . ./test-lib.sh
>>
>> Interesting. I was going to point out that this won't do what you want
>> by itself, because Git will keep walking out of the trash directory and
>> may find the containing repository.
>>
>> But it looks like this should be enough due to 614c3d8f2e (test-lib: set
>> GIT_CEILING_DIRECTORIES to protect the surrounding repository,
>> 2021-08-29). Supporting this case wasn't the intent of that patch, but I
>> don't see any reason why it should not work reliably.
> 
> I am surprised that use of GIT_CEILING_DIRECTORIES was not done
> until 2021, actually.  The reason the configuration variable was
> invented for is exactly to avoid discovery processes going upward
> and ending up in a repository different from what we mean to work
> with.
 
I didn't know about these historical details. All I know is that I
wrote these changes on top of the buggy patch in [1] and confirmed that
it failed with a segfault. Thanks for confirming the reason that this
works!

[1] https://lore.kernel.org/git/86cd83f65b30aab3233e27b3e5c4f03041e68766.1771903950.git.gitgitgadget@gmail.com/

Thanks,
-Stolee

[PATCH v3 2/4] run-command: extract clear_local_repo_env helper

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

The current prepare_other_repo_env() does two distinct things:

 1. Strip certain known environment variables that should be set by a
    child process based on a different repository.

 2. Set the GIT_DIR variable to avoid repository discovery.

The second item is valuable for child processes that operate on
submodules, where the repo discovery could be mistaken for the parent
repository.

In the next change, we will see an important case where only the first
item is required as the GIT_DIR discovery should happen naturally from
the '-C' parameter in the child process.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 run-command.c | 7 ++++++-
 run-command.h | 7 +++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index e3e02475cc..7858a0ef0a 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1847,7 +1847,7 @@ int run_auto_maintenance(int quiet)
 	return run_command(&maint);
 }
 
-void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)
+void clear_local_repo_env(struct strvec *env)
 {
 	const char * const *var;
 
@@ -1856,6 +1856,11 @@ void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)
 		    strcmp(*var, CONFIG_COUNT_ENVIRONMENT))
 			strvec_push(env, *var);
 	}
+}
+
+void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)
+{
+	clear_local_repo_env(env);
 	strvec_pushf(env, "%s=%s", GIT_DIR_ENVIRONMENT, new_git_dir);
 }
 
diff --git a/run-command.h b/run-command.h
index 0df25e445f..76b29d4832 100644
--- a/run-command.h
+++ b/run-command.h
@@ -509,6 +509,13 @@ struct run_process_parallel_opts
  */
 void run_processes_parallel(const struct run_process_parallel_opts *opts);
 
+/**
+ * Unset all local-repo GIT_* variables in env; see local_repo_env in
+ * environment.h. GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT are preserved
+ * to pass -c and --config-env options from the parent process.
+ */
+void clear_local_repo_env(struct strvec *env);
+
 /**
  * Convenience function which prepares env for a command to be run in a
  * new repo. This adds all GIT_* environment variables to env with the
-- 
gitgitgadget

Re: [PATCH v3 2/4] run-command: extract clear_local_repo_env helper

[Jeff King]

On Mon, Mar 02, 2026 at 03:36:43PM +0000, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <stolee@gmail.com>
> 
> The current prepare_other_repo_env() does two distinct things:
> 
>  1. Strip certain known environment variables that should be set by a
>     child process based on a different repository.
> 
>  2. Set the GIT_DIR variable to avoid repository discovery.
> 
> The second item is valuable for child processes that operate on
> submodules, where the repo discovery could be mistaken for the parent
> repository.
> 
> In the next change, we will see an important case where only the first
> item is required as the GIT_DIR discovery should happen naturally from
> the '-C' parameter in the child process.

Yep, this is the refactoring I expected.

> +/**
> + * Unset all local-repo GIT_* variables in env; see local_repo_env in
> + * environment.h. GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT are preserved
> + * to pass -c and --config-env options from the parent process.
> + */
> +void clear_local_repo_env(struct strvec *env);

I worry that the name is potentially confusing here, since it is not
just clearing local_repo_env, but making a few exceptions. But I don't
really have a better name. We called this "other_repo_env" in the
existing function, which is equally opaque. I dunno, maybe the
documentation you added would be sufficient.

Speaking of which, the documentation for prepare_other_repo_env() is now
somewhat redundant. If we ever change the behavior here, we'll have to
remember to touch both spots.

So what about squashing in:

diff --git a/run-command.h b/run-command.h
index 76b29d4832..882caeccc8 100644
--- a/run-command.h
+++ b/run-command.h
@@ -518,11 +518,9 @@ void clear_local_repo_env(struct strvec *env);
 
 /**
  * Convenience function which prepares env for a command to be run in a
- * new repo. This adds all GIT_* environment variables to env with the
- * exception of GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT (which cause the
- * corresponding environment variables to be unset in the subprocess) and adds
- * an environment variable pointing to new_git_dir. See local_repo_env in
- * environment.h for more information.
+ * new repo. This removes variables pointing to the local repository (using
+ * clear_local_repo_env() above), and adds an environment variable pointing to
+ * new_git_dir.
  */
 void prepare_other_repo_env(struct strvec *env, const char *new_git_dir);
 

-Peff

Re: [PATCH v3 2/4] run-command: extract clear_local_repo_env helper

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> +/**
>> + * Unset all local-repo GIT_* variables in env; see local_repo_env in
>> + * environment.h. GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT are preserved
>> + * to pass -c and --config-env options from the parent process.
>> + */
>> +void clear_local_repo_env(struct strvec *env);
>
> I worry that the name is potentially confusing here, since it is not
> just clearing local_repo_env, but making a few exceptions. But I don't
> really have a better name. We called this "other_repo_env" in the
> existing function, which is equally opaque. I dunno, maybe the
> documentation you added would be sufficient.

perhaps "clear_local" -> "sanitize" or something, with "env" ->
"other_env" to clarify that we are not emptying ours, but the one
that will be used by somebody else?

> Speaking of which, the documentation for prepare_other_repo_env() is now
> somewhat redundant. If we ever change the behavior here, we'll have to
> remember to touch both spots.
>
> So what about squashing in:

> diff --git a/run-command.h b/run-command.h
> index 76b29d4832..882caeccc8 100644
> --- a/run-command.h
> +++ b/run-command.h
> @@ -518,11 +518,9 @@ void clear_local_repo_env(struct strvec *env);
>  
>  /**
>   * Convenience function which prepares env for a command to be run in a
> - * new repo. This adds all GIT_* environment variables to env with the
> - * exception of GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT (which cause the
> - * corresponding environment variables to be unset in the subprocess) and adds
> - * an environment variable pointing to new_git_dir. See local_repo_env in
> - * environment.h for more information.
> + * new repo. This removes variables pointing to the local repository (using
> + * clear_local_repo_env() above), and adds an environment variable pointing to
> + * new_git_dir.
>   */
>  void prepare_other_repo_env(struct strvec *env, const char *new_git_dir);


That reads very well.

Re: [PATCH v3 2/4] run-command: extract clear_local_repo_env helper

[Derrick Stolee]

On 3/2/2026 1:03 PM, Jeff King wrote:
> So what about squashing in:
> 
> diff --git a/run-command.h b/run-command.h
> index 76b29d4832..882caeccc8 100644
> --- a/run-command.h
> +++ b/run-command.h
> @@ -518,11 +518,9 @@ void clear_local_repo_env(struct strvec *env);
>  
>  /**
>   * Convenience function which prepares env for a command to be run in a
> - * new repo. This adds all GIT_* environment variables to env with the
> - * exception of GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT (which cause the
> - * corresponding environment variables to be unset in the subprocess) and adds
> - * an environment variable pointing to new_git_dir. See local_repo_env in
> - * environment.h for more information.
> + * new repo. This removes variables pointing to the local repository (using
> + * clear_local_repo_env() above), and adds an environment variable pointing to
> + * new_git_dir.
>   */
>  void prepare_other_repo_env(struct strvec *env, const char *new_git_dir);

I'm happy to squash this in.

Perhaps Junio can do it if we don't need other changes to v3. (I haven't
read the rest of the feedback.)

Thanks,
-Stolee

[PATCH v3 3/4] for-each-repo: work correctly in a worktree

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

When run in a worktree, the GIT_DIR directory is set in a different way
than in a typical repository. Show this by updating t0068 to include a
worktree and add a test that runs from that worktree. This requires
moving the repo.key config into a global config instead of the base test
repository's local config (demonstrating that it worked with
non-worktree Git repositories).

We need to be careful to unset the local Git environment variables and
let the child process rediscover them, while also reinstating those
variables in the parent process afterwards. Update run_command_on_repo()
to use the new clear_local_repo_env() helper method to erase these
environment variables.

During review of this bug fix, there were several incorrect patches
demonstrating different bad behaviors. Most of these are covered by
tests, when it is not too expensive to set it up. One case that would be
expensive to set up is the GIT_NO_REPLACE_OBJECTS environment variable,
but we trust that using clear_local_repo_env() will be sufficient to
capture these uncovered cases by using the common code for resetting
environment variables.

Reported-by: Matthew Gabeler-Lee <fastcat@gmail.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/for-each-repo.c  |  4 +++-
 t/t0068-for-each-repo.sh | 48 +++++++++++++++++++++++++++++++++++-----
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
index 325a7925f1..8bbdc33128 100644
--- a/builtin/for-each-repo.c
+++ b/builtin/for-each-repo.c
@@ -2,6 +2,7 @@
 
 #include "builtin.h"
 #include "config.h"
+#include "environment.h"
 #include "gettext.h"
 #include "parse-options.h"
 #include "path.h"
@@ -15,10 +16,11 @@ static const char * const for_each_repo_usage[] = {
 
 static int run_command_on_repo(const char *path, int argc, const char ** argv)
 {
-	int i;
 	struct child_process child = CHILD_PROCESS_INIT;
 	char *abspath = interpolate_path(path, 0);
 
+	clear_local_repo_env(&child.env);
+
 	child.git_cmd = 1;
 	strvec_pushl(&child.args, "-C", abspath, NULL);
 
diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh
index 512af34c82..80b163ea99 100755
--- a/t/t0068-for-each-repo.sh
+++ b/t/t0068-for-each-repo.sh
@@ -8,10 +8,12 @@ TEST_NO_CREATE_REPO=1
 . ./test-lib.sh
 
 test_expect_success 'run based on configured value' '
-	git init one &&
-	git init two &&
-	git init three &&
-	git init ~/four &&
+	git init --initial-branch=one one &&
+	git init --initial-branch=two two &&
+	git -C two worktree add --orphan ../three &&
+	git -C three checkout -b three &&
+	git init --initial-branch=four ~/four &&
+
 	git -C two commit --allow-empty -m "DID NOT RUN" &&
 	git config --global run.key "$TRASH_DIRECTORY/one" &&
 	git config --global --add run.key "$TRASH_DIRECTORY/three" &&
@@ -35,7 +37,43 @@ test_expect_success 'run based on configured value' '
 	git -C three log -1 --pretty=format:%s >message &&
 	grep again message &&
 	git -C ~/four log -1 --pretty=format:%s >message &&
-	grep again message
+	grep again message &&
+
+	git -C three for-each-repo --config=run.key -- \
+		commit --allow-empty -m "ran from worktree" &&
+	git -C one log -1 --pretty=format:%s >message &&
+	test_grep "ran from worktree" message &&
+	git -C two log -1 --pretty=format:%s >message &&
+	test_grep ! "ran from worktree" message &&
+	git -C three log -1 --pretty=format:%s >message &&
+	test_grep "ran from worktree" message &&
+	git -C ~/four log -1 --pretty=format:%s >message &&
+	test_grep "ran from worktree" message &&
+
+	# Test running with config values set by environment
+	cat >expect <<-EOF &&
+	ran from worktree (HEAD -> refs/heads/one)
+	ran from worktree (HEAD -> refs/heads/three)
+	ran from worktree (HEAD -> refs/heads/four)
+	EOF
+
+	GIT_CONFIG_PARAMETERS="${SQ}log.decorate=full${SQ}" \
+		git -C three for-each-repo --config=run.key -- log --format="%s%d" -1 >out &&
+	test_cmp expect out &&
+
+	cat >test-config <<-EOF &&
+	[run]
+		key = $(pwd)/one
+		key = $(pwd)/three
+		key = $(pwd)/four
+
+	[log]
+		decorate = full
+	EOF
+
+	GIT_CONFIG_GLOBAL="$(pwd)/test-config" \
+		git -C three for-each-repo --config=run.key -- log --format="%s%d" -1 >out &&
+	test_cmp expect out
 '
 
 test_expect_success 'do nothing on empty config' '
-- 
gitgitgadget

Re: [PATCH v3 3/4] for-each-repo: work correctly in a worktree

[Jeff King]

On Mon, Mar 02, 2026 at 03:36:44PM +0000, Derrick Stolee via GitGitGadget wrote:

> @@ -15,10 +16,11 @@ static const char * const for_each_repo_usage[] = {
>  
>  static int run_command_on_repo(const char *path, int argc, const char ** argv)
>  {
> -	int i;
>  	struct child_process child = CHILD_PROCESS_INIT;
>  	char *abspath = interpolate_path(path, 0);
>  
> +	clear_local_repo_env(&child.env);
> +
>  	child.git_cmd = 1;
>  	strvec_pushl(&child.args, "-C", abspath, NULL);

The second part of the hunk here is as expected, but the first one looks
wrong. We didn't remove any references to "i", so either it was
redundant to start with (and the compiler should have complained), or
now we've broken compilation.

Looks like the latter, but we recover when we switch to using pushv in
patch 4. So I think the declaration of "i" should move to that patch.

-Peff

Re: [PATCH v3 3/4] for-each-repo: work correctly in a worktree

[Derrick Stolee]

On 3/2/2026 1:06 PM, Jeff King wrote:
> On Mon, Mar 02, 2026 at 03:36:44PM +0000, Derrick Stolee via GitGitGadget wrote:
> 
>> @@ -15,10 +16,11 @@ static const char * const for_each_repo_usage[] = {
>>  
>>  static int run_command_on_repo(const char *path, int argc, const char ** argv)
>>  {
>> -	int i;
>>  	struct child_process child = CHILD_PROCESS_INIT;
>>  	char *abspath = interpolate_path(path, 0);
>>  
>> +	clear_local_repo_env(&child.env);
>> +
>>  	child.git_cmd = 1;
>>  	strvec_pushl(&child.args, "-C", abspath, NULL);
> 
> The second part of the hunk here is as expected, but the first one looks
> wrong. We didn't remove any references to "i", so either it was
> redundant to start with (and the compiler should have complained), or
> now we've broken compilation.

You are correct. I did a --fixup here and it messed up the diff. I should
have double-checked the commit-by-commit compilation and testing post-
rebase.

> Looks like the latter, but we recover when we switch to using pushv in
> patch 4. So I think the declaration of "i" should move to that patch.

Can do. Looks like a small v4 update _is_ required.

Thanks,
-Stolee

Re: [PATCH v3 3/4] for-each-repo: work correctly in a worktree

[Junio C Hamano]

Derrick Stolee <stolee@gmail.com> writes:

> On 3/2/2026 1:06 PM, Jeff King wrote:
>> On Mon, Mar 02, 2026 at 03:36:44PM +0000, Derrick Stolee via GitGitGadget wrote:
>> 
>>> @@ -15,10 +16,11 @@ static const char * const for_each_repo_usage[] = {
>>>  
>>>  static int run_command_on_repo(const char *path, int argc, const char ** argv)
>>>  {
>>> -	int i;
>>>  	struct child_process child = CHILD_PROCESS_INIT;
>>>  	char *abspath = interpolate_path(path, 0);
>>>  
>>> +	clear_local_repo_env(&child.env);
>>> +
>>>  	child.git_cmd = 1;
>>>  	strvec_pushl(&child.args, "-C", abspath, NULL);
>> 
>> The second part of the hunk here is as expected, but the first one looks
>> wrong. We didn't remove any references to "i", so either it was
>> redundant to start with (and the compiler should have complained), or
>> now we've broken compilation.
>
> You are correct. I did a --fixup here and it messed up the diff. I should
> have double-checked the commit-by-commit compilation and testing post-
> rebase.
>
>> Looks like the latter, but we recover when we switch to using pushv in
>> patch 4. So I think the declaration of "i" should move to that patch.
>
> Can do. Looks like a small v4 update _is_ required.

I could do this too ;-) but I probably won't get to queuing this
topic before you update on your own, I suspect, so ...

[PATCH v3 4/4] for-each-repo: simplify passing of parameters

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

This change simplifies the code somewhat from its original
implementation.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/for-each-repo.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
index 8bbdc33128..3aafb2cfa8 100644
--- a/builtin/for-each-repo.c
+++ b/builtin/for-each-repo.c
@@ -14,7 +14,7 @@ static const char * const for_each_repo_usage[] = {
 	NULL
 };
 
-static int run_command_on_repo(const char *path, int argc, const char ** argv)
+static int run_command_on_repo(const char *path, const char **argv)
 {
 	struct child_process child = CHILD_PROCESS_INIT;
 	char *abspath = interpolate_path(path, 0);
@@ -23,9 +23,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)
 
 	child.git_cmd = 1;
 	strvec_pushl(&child.args, "-C", abspath, NULL);
-
-	for (i = 0; i < argc; i++)
-		strvec_push(&child.args, argv[i]);
+	strvec_pushv(&child.args, argv);
 
 	free(abspath);
 
@@ -65,7 +63,7 @@ int cmd_for_each_repo(int argc,
 		return 0;
 
 	for (size_t i = 0; i < values->nr; i++) {
-		int ret = run_command_on_repo(values->items[i].string, argc, argv);
+		int ret = run_command_on_repo(values->items[i].string, argv);
 		if (ret) {
 			if (!keep_going)
 					return ret;
-- 
gitgitgadget

Re: [PATCH v3 4/4] for-each-repo: simplify passing of parameters

[Jeff King]

On Mon, Mar 02, 2026 at 03:36:45PM +0000, Derrick Stolee via GitGitGadget wrote:

> This change simplifies the code somewhat from its original
> implementation.

Yeah, I think this is worth doing.

-Peff

[PATCH v4 0/4] for-each-repo: work correctly in a worktree

[Derrick Stolee via GitGitGadget]

This was reported by Matthew [1] and I thought it would be a quick fix. It
turns out to have had a lot of implications to get exactly right.

[1]
https://lore.kernel.org/git/CABpCjbY=wpStuhxqRJ5TSNV3A-CmN-g-xZGJOQGSSv3GYhs2fQ@mail.gmail.com/


Updates in v3
=============

v2 included some important changes that apparently didn't save in my cover
letter. Sorry. The most important one was that the first patch was replaced
with one that tests the command outside of a Git repo so the first patch in
v1 can be caught as a bug.

But v2 also included a bug! The filtering of GIT_CONFIG* environment
variables was incorrect. Further, duplicating this logic from
prepare_other_repo_env() was a new bug waiting to happen.

Here is the new setup:

 * Patch 1 is the same as in v2.
 * A new Patch 2 extracts clear_local_repo_env() from
   prepare_other_repo_env().
 * The old Patch 2 is now Patch 3 with a much simpler implementation that
   calls clear_local_repo_env().
 * Tests are added to check that these config-related environment variables
   are preserved. The NO_REPLACE_OBJECTS case seemed like it would take
   longer than necessary to set up for the value it provides.
 * A fourth patch is added that simplifies the use of argv. It's separate
   because it modifies the original implementation in a way that is a "nice
   to have" but isn't necessary for solving the bug that motivates the
   series.


Updates in V4
=============

Minor updates from Peff's review:

 1. Update the comment of prepare_other_repo_env() to avoid duplication.
 2. Rename the new method to sanitize_repo_env().
 3. Move incorrect removal of 'int i;' to correct patch.

Thanks, -Stolee

Derrick Stolee (4):
  for-each-repo: test outside of repo context
  run-command: extract sanitize_repo_env helper
  for-each-repo: work correctly in a worktree
  for-each-repo: simplify passing of parameters

 builtin/for-each-repo.c  | 12 +++----
 run-command.c            |  7 ++++-
 run-command.h            | 15 ++++++---
 t/t0068-for-each-repo.sh | 67 +++++++++++++++++++++++++++++++++-------
 4 files changed, 77 insertions(+), 24 deletions(-)


base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2056%2Fderrickstolee%2Ffor-each-repo-in-gitdir-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2056/derrickstolee/for-each-repo-in-gitdir-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/2056

Range-diff vs v3:

 1:  6e9d4f3029 = 1:  6e9d4f3029 for-each-repo: test outside of repo context
 2:  13d783dbbd ! 2:  24398664c2 run-command: extract clear_local_repo_env helper
     @@ Metadata
      Author: Derrick Stolee <stolee@gmail.com>
      
       ## Commit message ##
     -    run-command: extract clear_local_repo_env helper
     +    run-command: extract sanitize_repo_env helper
      
          The current prepare_other_repo_env() does two distinct things:
      
     @@ Commit message
          item is required as the GIT_DIR discovery should happen naturally from
          the '-C' parameter in the child process.
      
     +    Helped-by: Jeff King <peff@peff.net>
          Signed-off-by: Derrick Stolee <stolee@gmail.com>
      
       ## run-command.c ##
     @@ run-command.c: int run_auto_maintenance(int quiet)
       }
       
      -void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)
     -+void clear_local_repo_env(struct strvec *env)
     ++void sanitize_repo_env(struct strvec *env)
       {
       	const char * const *var;
       
     @@ run-command.c: void prepare_other_repo_env(struct strvec *env, const char *new_g
      +
      +void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)
      +{
     -+	clear_local_repo_env(env);
     ++	sanitize_repo_env(env);
       	strvec_pushf(env, "%s=%s", GIT_DIR_ENVIRONMENT, new_git_dir);
       }
       
     @@ run-command.h: struct run_process_parallel_opts
      + * environment.h. GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT are preserved
      + * to pass -c and --config-env options from the parent process.
      + */
     -+void clear_local_repo_env(struct strvec *env);
     ++void sanitize_repo_env(struct strvec *env);
      +
       /**
        * Convenience function which prepares env for a command to be run in a
     -  * new repo. This adds all GIT_* environment variables to env with the
     +- * new repo. This adds all GIT_* environment variables to env with the
     +- * exception of GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT (which cause the
     +- * corresponding environment variables to be unset in the subprocess) and adds
     +- * an environment variable pointing to new_git_dir. See local_repo_env in
     +- * environment.h for more information.
     ++ * new repo. This removes variables pointing to the local repository (using
     ++ * sanitize_repo_env() above), and adds an environment variable pointing to
     ++ * new_git_dir.
     +  */
     + void prepare_other_repo_env(struct strvec *env, const char *new_git_dir);
     + 
 3:  2a6091095f ! 3:  e759b35069 for-each-repo: work correctly in a worktree
     @@ Commit message
          We need to be careful to unset the local Git environment variables and
          let the child process rediscover them, while also reinstating those
          variables in the parent process afterwards. Update run_command_on_repo()
     -    to use the new clear_local_repo_env() helper method to erase these
     +    to use the new sanitize_repo_env() helper method to erase these
          environment variables.
      
          During review of this bug fix, there were several incorrect patches
          demonstrating different bad behaviors. Most of these are covered by
          tests, when it is not too expensive to set it up. One case that would be
          expensive to set up is the GIT_NO_REPLACE_OBJECTS environment variable,
     -    but we trust that using clear_local_repo_env() will be sufficient to
     +    but we trust that using sanitize_repo_env() will be sufficient to
          capture these uncovered cases by using the common code for resetting
          environment variables.
      
     @@ builtin/for-each-repo.c
       #include "gettext.h"
       #include "parse-options.h"
       #include "path.h"
     -@@ builtin/for-each-repo.c: static const char * const for_each_repo_usage[] = {
     - 
     - static int run_command_on_repo(const char *path, int argc, const char ** argv)
     - {
     --	int i;
     +@@ builtin/for-each-repo.c: static int run_command_on_repo(const char *path, int argc, const char ** argv)
       	struct child_process child = CHILD_PROCESS_INIT;
       	char *abspath = interpolate_path(path, 0);
       
     -+	clear_local_repo_env(&child.env);
     ++	sanitize_repo_env(&child.env);
      +
       	child.git_cmd = 1;
       	strvec_pushl(&child.args, "-C", abspath, NULL);
 4:  f6582e9402 ! 4:  8b1f083da1 for-each-repo: simplify passing of parameters
     @@ builtin/for-each-repo.c: static const char * const for_each_repo_usage[] = {
      -static int run_command_on_repo(const char *path, int argc, const char ** argv)
      +static int run_command_on_repo(const char *path, const char **argv)
       {
     +-	int i;
       	struct child_process child = CHILD_PROCESS_INIT;
       	char *abspath = interpolate_path(path, 0);
     + 
      @@ builtin/for-each-repo.c: static int run_command_on_repo(const char *path, int argc, const char ** argv)
       
       	child.git_cmd = 1;

-- 
gitgitgadget

[PATCH v4 1/4] for-each-repo: test outside of repo context

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

The 'git for-each-repo' tool is frequently run outside of a repo context
in the real world. For example, it powers background maintenance.
Despite this typical case, we have not been testing it without a local
repository.

Update t0068 to stop creating a test repo and to use global config
everywhere. This has some subtle changes to test across the file.

This was noticed because an earlier attempt to remove the_repository
from builtin/for-each-repo.c did not catch a segmentation fault since
the passed 'repo' is NULL. This use of the_repository will need to stay
until we have a better way to handle config queries outside of a repo
context. Similar use still exists in builtin/config.c for the same
reason.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 t/t0068-for-each-repo.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh
index f2f3e50031..512af34c82 100755
--- a/t/t0068-for-each-repo.sh
+++ b/t/t0068-for-each-repo.sh
@@ -2,6 +2,9 @@
 
 test_description='git for-each-repo builtin'
 
+# We need to test running 'git for-each-repo' outside of a repo context.
+TEST_NO_CREATE_REPO=1
+
 . ./test-lib.sh
 
 test_expect_success 'run based on configured value' '
@@ -10,9 +13,10 @@ test_expect_success 'run based on configured value' '
 	git init three &&
 	git init ~/four &&
 	git -C two commit --allow-empty -m "DID NOT RUN" &&
-	git config run.key "$TRASH_DIRECTORY/one" &&
-	git config --add run.key "$TRASH_DIRECTORY/three" &&
-	git config --add run.key "~/four" &&
+	git config --global run.key "$TRASH_DIRECTORY/one" &&
+	git config --global --add run.key "$TRASH_DIRECTORY/three" &&
+	git config --global --add run.key "~/four" &&
+
 	git for-each-repo --config=run.key commit --allow-empty -m "ran" &&
 	git -C one log -1 --pretty=format:%s >message &&
 	grep ran message &&
@@ -22,6 +26,7 @@ test_expect_success 'run based on configured value' '
 	grep ran message &&
 	git -C ~/four log -1 --pretty=format:%s >message &&
 	grep ran message &&
+
 	git for-each-repo --config=run.key -- commit --allow-empty -m "ran again" &&
 	git -C one log -1 --pretty=format:%s >message &&
 	grep again message &&
@@ -46,7 +51,7 @@ test_expect_success 'error on bad config keys' '
 '
 
 test_expect_success 'error on NULL value for config keys' '
-	cat >>.git/config <<-\EOF &&
+	cat >>.gitconfig <<-\EOF &&
 	[empty]
 		key
 	EOF
@@ -59,8 +64,8 @@ test_expect_success 'error on NULL value for config keys' '
 '
 
 test_expect_success '--keep-going' '
-	git config keep.going non-existing &&
-	git config --add keep.going . &&
+	git config --global keep.going non-existing &&
+	git config --global --add keep.going one &&
 
 	test_must_fail git for-each-repo --config=keep.going \
 		-- branch >out 2>err &&
@@ -70,7 +75,7 @@ test_expect_success '--keep-going' '
 	test_must_fail git for-each-repo --config=keep.going --keep-going \
 		-- branch >out 2>err &&
 	test_grep "cannot change to .*non-existing" err &&
-	git branch >expect &&
+	git -C one branch >expect &&
 	test_cmp expect out
 '
 
-- 
gitgitgadget

[PATCH v4 2/4] run-command: extract sanitize_repo_env helper

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

The current prepare_other_repo_env() does two distinct things:

 1. Strip certain known environment variables that should be set by a
    child process based on a different repository.

 2. Set the GIT_DIR variable to avoid repository discovery.

The second item is valuable for child processes that operate on
submodules, where the repo discovery could be mistaken for the parent
repository.

In the next change, we will see an important case where only the first
item is required as the GIT_DIR discovery should happen naturally from
the '-C' parameter in the child process.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 run-command.c |  7 ++++++-
 run-command.h | 15 ++++++++++-----
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/run-command.c b/run-command.c
index e3e02475cc..89dbe62ab8 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1847,7 +1847,7 @@ int run_auto_maintenance(int quiet)
 	return run_command(&maint);
 }
 
-void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)
+void sanitize_repo_env(struct strvec *env)
 {
 	const char * const *var;
 
@@ -1856,6 +1856,11 @@ void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)
 		    strcmp(*var, CONFIG_COUNT_ENVIRONMENT))
 			strvec_push(env, *var);
 	}
+}
+
+void prepare_other_repo_env(struct strvec *env, const char *new_git_dir)
+{
+	sanitize_repo_env(env);
 	strvec_pushf(env, "%s=%s", GIT_DIR_ENVIRONMENT, new_git_dir);
 }
 
diff --git a/run-command.h b/run-command.h
index 0df25e445f..7e5a263ee6 100644
--- a/run-command.h
+++ b/run-command.h
@@ -509,13 +509,18 @@ struct run_process_parallel_opts
  */
 void run_processes_parallel(const struct run_process_parallel_opts *opts);
 
+/**
+ * Unset all local-repo GIT_* variables in env; see local_repo_env in
+ * environment.h. GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT are preserved
+ * to pass -c and --config-env options from the parent process.
+ */
+void sanitize_repo_env(struct strvec *env);
+
 /**
  * Convenience function which prepares env for a command to be run in a
- * new repo. This adds all GIT_* environment variables to env with the
- * exception of GIT_CONFIG_PARAMETERS and GIT_CONFIG_COUNT (which cause the
- * corresponding environment variables to be unset in the subprocess) and adds
- * an environment variable pointing to new_git_dir. See local_repo_env in
- * environment.h for more information.
+ * new repo. This removes variables pointing to the local repository (using
+ * sanitize_repo_env() above), and adds an environment variable pointing to
+ * new_git_dir.
  */
 void prepare_other_repo_env(struct strvec *env, const char *new_git_dir);
 
-- 
gitgitgadget

[PATCH v4 3/4] for-each-repo: work correctly in a worktree

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

When run in a worktree, the GIT_DIR directory is set in a different way
than in a typical repository. Show this by updating t0068 to include a
worktree and add a test that runs from that worktree. This requires
moving the repo.key config into a global config instead of the base test
repository's local config (demonstrating that it worked with
non-worktree Git repositories).

We need to be careful to unset the local Git environment variables and
let the child process rediscover them, while also reinstating those
variables in the parent process afterwards. Update run_command_on_repo()
to use the new sanitize_repo_env() helper method to erase these
environment variables.

During review of this bug fix, there were several incorrect patches
demonstrating different bad behaviors. Most of these are covered by
tests, when it is not too expensive to set it up. One case that would be
expensive to set up is the GIT_NO_REPLACE_OBJECTS environment variable,
but we trust that using sanitize_repo_env() will be sufficient to
capture these uncovered cases by using the common code for resetting
environment variables.

Reported-by: Matthew Gabeler-Lee <fastcat@gmail.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/for-each-repo.c  |  3 +++
 t/t0068-for-each-repo.sh | 48 +++++++++++++++++++++++++++++++++++-----
 2 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
index 325a7925f1..82727c4aa2 100644
--- a/builtin/for-each-repo.c
+++ b/builtin/for-each-repo.c
@@ -2,6 +2,7 @@
 
 #include "builtin.h"
 #include "config.h"
+#include "environment.h"
 #include "gettext.h"
 #include "parse-options.h"
 #include "path.h"
@@ -19,6 +20,8 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)
 	struct child_process child = CHILD_PROCESS_INIT;
 	char *abspath = interpolate_path(path, 0);
 
+	sanitize_repo_env(&child.env);
+
 	child.git_cmd = 1;
 	strvec_pushl(&child.args, "-C", abspath, NULL);
 
diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh
index 512af34c82..80b163ea99 100755
--- a/t/t0068-for-each-repo.sh
+++ b/t/t0068-for-each-repo.sh
@@ -8,10 +8,12 @@ TEST_NO_CREATE_REPO=1
 . ./test-lib.sh
 
 test_expect_success 'run based on configured value' '
-	git init one &&
-	git init two &&
-	git init three &&
-	git init ~/four &&
+	git init --initial-branch=one one &&
+	git init --initial-branch=two two &&
+	git -C two worktree add --orphan ../three &&
+	git -C three checkout -b three &&
+	git init --initial-branch=four ~/four &&
+
 	git -C two commit --allow-empty -m "DID NOT RUN" &&
 	git config --global run.key "$TRASH_DIRECTORY/one" &&
 	git config --global --add run.key "$TRASH_DIRECTORY/three" &&
@@ -35,7 +37,43 @@ test_expect_success 'run based on configured value' '
 	git -C three log -1 --pretty=format:%s >message &&
 	grep again message &&
 	git -C ~/four log -1 --pretty=format:%s >message &&
-	grep again message
+	grep again message &&
+
+	git -C three for-each-repo --config=run.key -- \
+		commit --allow-empty -m "ran from worktree" &&
+	git -C one log -1 --pretty=format:%s >message &&
+	test_grep "ran from worktree" message &&
+	git -C two log -1 --pretty=format:%s >message &&
+	test_grep ! "ran from worktree" message &&
+	git -C three log -1 --pretty=format:%s >message &&
+	test_grep "ran from worktree" message &&
+	git -C ~/four log -1 --pretty=format:%s >message &&
+	test_grep "ran from worktree" message &&
+
+	# Test running with config values set by environment
+	cat >expect <<-EOF &&
+	ran from worktree (HEAD -> refs/heads/one)
+	ran from worktree (HEAD -> refs/heads/three)
+	ran from worktree (HEAD -> refs/heads/four)
+	EOF
+
+	GIT_CONFIG_PARAMETERS="${SQ}log.decorate=full${SQ}" \
+		git -C three for-each-repo --config=run.key -- log --format="%s%d" -1 >out &&
+	test_cmp expect out &&
+
+	cat >test-config <<-EOF &&
+	[run]
+		key = $(pwd)/one
+		key = $(pwd)/three
+		key = $(pwd)/four
+
+	[log]
+		decorate = full
+	EOF
+
+	GIT_CONFIG_GLOBAL="$(pwd)/test-config" \
+		git -C three for-each-repo --config=run.key -- log --format="%s%d" -1 >out &&
+	test_cmp expect out
 '
 
 test_expect_success 'do nothing on empty config' '
-- 
gitgitgadget

[PATCH v4 4/4] for-each-repo: simplify passing of parameters

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

This change simplifies the code somewhat from its original
implementation.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/for-each-repo.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
index 82727c4aa2..927d3d92da 100644
--- a/builtin/for-each-repo.c
+++ b/builtin/for-each-repo.c
@@ -14,9 +14,8 @@ static const char * const for_each_repo_usage[] = {
 	NULL
 };
 
-static int run_command_on_repo(const char *path, int argc, const char ** argv)
+static int run_command_on_repo(const char *path, const char **argv)
 {
-	int i;
 	struct child_process child = CHILD_PROCESS_INIT;
 	char *abspath = interpolate_path(path, 0);
 
@@ -24,9 +23,7 @@ static int run_command_on_repo(const char *path, int argc, const char ** argv)
 
 	child.git_cmd = 1;
 	strvec_pushl(&child.args, "-C", abspath, NULL);
-
-	for (i = 0; i < argc; i++)
-		strvec_push(&child.args, argv[i]);
+	strvec_pushv(&child.args, argv);
 
 	free(abspath);
 
@@ -66,7 +63,7 @@ int cmd_for_each_repo(int argc,
 		return 0;
 
 	for (size_t i = 0; i < values->nr; i++) {
-		int ret = run_command_on_repo(values->items[i].string, argc, argv);
+		int ret = run_command_on_repo(values->items[i].string, argv);
 		if (ret) {
 			if (!keep_going)
 					return ret;
-- 
gitgitgadget

Re: [PATCH v4 0/4] for-each-repo: work correctly in a worktree

[Jeff King]

On Tue, Mar 03, 2026 at 05:31:50PM +0000, Derrick Stolee via GitGitGadget wrote:

> Updates in V4
> =============
> 
> Minor updates from Peff's review:
> 
>  1. Update the comment of prepare_other_repo_env() to avoid duplication.
>  2. Rename the new method to sanitize_repo_env().
>  3. Move incorrect removal of 'int i;' to correct patch.

This looks good to me. Thanks for accommodating my somewhat-bikeshedding
review.

-Peff

Re: [PATCH v4 0/4] for-each-repo: work correctly in a worktree

[Patrick Steinhardt]

On Wed, Mar 04, 2026 at 08:20:35PM -0500, Jeff King wrote:
> On Tue, Mar 03, 2026 at 05:31:50PM +0000, Derrick Stolee via GitGitGadget wrote:
> 
> > Updates in V4
> > =============
> > 
> > Minor updates from Peff's review:
> > 
> >  1. Update the comment of prepare_other_repo_env() to avoid duplication.
> >  2. Rename the new method to sanitize_repo_env().
> >  3. Move incorrect removal of 'int i;' to correct patch.
> 
> This looks good to me. Thanks for accommodating my somewhat-bikeshedding
> review.

Likewise, this patch series looks good to me. Thanks!

Patrick

Re: [PATCH v4 0/4] for-each-repo: work correctly in a worktree

[Derrick Stolee]

On 3/5/2026 1:14 AM, Patrick Steinhardt wrote:
> On Wed, Mar 04, 2026 at 08:20:35PM -0500, Jeff King wrote:
>> On Tue, Mar 03, 2026 at 05:31:50PM +0000, Derrick Stolee via GitGitGadget wrote:
>>
>>> Updates in V4
>>> =============
>>>
>>> Minor updates from Peff's review:
>>>
>>>  1. Update the comment of prepare_other_repo_env() to avoid duplication.
>>>  2. Rename the new method to sanitize_repo_env().
>>>  3. Move incorrect removal of 'int i;' to correct patch.
>>
>> This looks good to me. Thanks for accommodating my somewhat-bikeshedding
>> review.
> 
> Likewise, this patch series looks good to me. Thanks!

Thanks, all. This was far less trivial than I thought going
into it, so the careful review was essential.

Thanks,
-Stolee