Re: [PATCH 3/4] run-command: move envvar-resetting function
- From
Emily Shaffer <emilyshaffer@google.com>
- Date
- Jun 8, 2021, 00:54 UTC
- Message-ID
- <YL6/2eFRtdG42JdG@google.com>
- In-Reply-To
- <c5307a9f02f3f0f9b30c87bb3e9f8fa3163f839d.1622580781.git.jonathantanmy@google.com>
On Tue, Jun 01, 2021 at 02:34:18PM -0700, Jonathan Tan wrote:
Show 26 quoted lines
> > There is a function that resets environment variables, used when > invoking a sub-process in a submodule. The lazy-fetching code (used in > partial clones) will need this function in a subsequent commit, so move > it to a more central location. > > Signed-off-by: Jonathan Tan <jonathantanmy@google.com> > --- > run-command.c | 10 ++++++++++ > run-command.h | 7 +++++++ > submodule.c | 14 ++------------ > 3 files changed, 19 insertions(+), 12 deletions(-) > diff --git a/run-command.h b/run-command.h > index d08414a92e..6f61ec7703 100644 > --- a/run-command.h > +++ b/run-command.h > @@ -483,4 +483,11 @@ int run_processes_parallel_tr2(int n, get_next_task_fn, start_failure_fn, > task_finished_fn, void *pp_cb, > const char *tr2_category, const char *tr2_label); > > +/** > + * Convenience function that adds entries to env_array that resets all > + * repo-specific environment variables except for CONFIG_DATA_ENVIRONMENT. See > + * local_repo_env in cache.h for more information. > + */ > +void prepare_other_repo_env(struct strvec *env_array);
I like the name change on the function as well.
Show 14 quoted lines
> void prepare_submodule_repo_env(struct strvec *out)
> {
> - prepare_submodule_repo_env_no_git_dir(out);
> + prepare_other_repo_env(out);
> strvec_pushf(out, "%s=%s", GIT_DIR_ENVIRONMENT,
> DEFAULT_GIT_DIR_ENVIRONMENT);
> }
>
> static void prepare_submodule_repo_env_in_gitdir(struct strvec *out)
> {
> - prepare_submodule_repo_env_no_git_dir(out);
> + prepare_other_repo_env(out);
> strvec_pushf(out, "%s=.", GIT_DIR_ENVIRONMENT);
> }This call was used in less places than I thought (I guess that's part of why you're making it more public/central), so my worry about having some large scale change was for nothing.
As for Taylor's comment about the CONFIG_DATA_ENVIRONMENT variable, it was named like that before you got here, so I am not too worried whether or not you change it.
Reviewed-by: Emily Shaffer <emilyshaffer@google.com>