From: Jeff King Date: Mon, 02 Mar 2026 18:09:04 GMT Subject: Re: [PATCH v2 2/2] for-each-repo: work correctly in a worktree Message-ID: <20260302180904.GF28275@coredump.intra.peff.net> In-Reply-To: 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