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

[Jeff King <peff@peff.net>]

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