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

From: Jeff King <peff@peff.net>
Date: Mar 2, 2026, 18:03 UTC
Message-ID: <20260302180324.GC28275@coredump.intra.peff.net>
In-Reply-To: <13d783dbbdd77b14fed651f0508fa0e668d98c63.1772465805.git.gitgitgadget@gmail.com>

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

Show 16 quoted lines

> 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.

Show 6 quoted lines

> +/**
> + * 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