Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree
- From
Jeff King <peff@peff.net>
- Date
- Mar 2, 2026, 18:09 UTC
- Message-ID
- <20260302180904.GF28275@coredump.intra.peff.net>
- In-Reply-To
- <cd9adbd9-b996-46da-b6a8-d2395be79a0f@gmail.com>
On Mon, Mar 02, 2026 at 10:31:48AM -0500, Derrick Stolee wrote:
Show 15 quoted lines
> > 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.
Show 8 quoted lines
> > 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