From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 02 Mar 2026 18:35:03 GMT
Subject: Re: [PATCH v3 2/4] run-command: extract clear_local_repo_env helper
Message-ID: <xmqqldga13mw.fsf@gitster.g>
In-Reply-To: <20260302180324.GC28275@coredump.intra.peff.net>

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.