Re: [PATCH v2 3/4] run-command: move envvar-resetting function
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 8, 2021, 04:14 UTC
- Message-ID
- <xmqqo8chq9g8.fsf@gitster.g>
- In-Reply-To
- <9cbdf60981e66e938b6c57c041c15e85245c734d.1623111879.git.jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
Show 30 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 | 9 +++++++++
> submodule.c | 14 ++------------
> 3 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/run-command.c b/run-command.c
> index be6bc128cd..a6c458119c 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -1892,3 +1892,13 @@ int run_auto_maintenance(int quiet)
>
> return run_command(&maint);
> }
> +
> +void prepare_other_repo_env(struct strvec *env_array)
> +{
> + const char * const *var;
> +
> + for (var = local_repo_env; *var; var++) {
> + if (strcmp(*var, CONFIG_DATA_ENVIRONMENT))
> + strvec_push(env_array, *var);
> + }
> +}It does make sense to move this to run-command.c from submodule.c and the function name is already suitable for being global. I however cannot help wondering if we should also pay attention to the GIT_CONFIG_KEY_$n and GIT_CONFIG_VALUE_$n pairs (which is not a new problem in this patch).
This helper may sit better next to prep_childenv(), instead of just saying "the location does not matter, just append randomly at the end", though.
Otherwise looking good.
Thanks.
Show 50 quoted lines
> diff --git a/run-command.h b/run-command.h
> index d08414a92e..a1d9107f5b 100644
> --- a/run-command.h
> +++ b/run-command.h
> @@ -483,4 +483,13 @@ 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 which adds all GIT_* environment variables to env_array
> + * with the exception of GIT_CONFIG_PARAMETERS. When used as the env_array of a
> + * subprocess, these entries cause the corresponding environment variables to
> + * be unset in the subprocess. See local_repo_env in cache.h for more
> + * information.
> + */
> +void prepare_other_repo_env(struct strvec *env_array);
> +
> #endif
> diff --git a/submodule.c b/submodule.c
> index 0b1d9c1dde..a30216db52 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -484,26 +484,16 @@ static void print_submodule_diff_summary(struct repository *r, struct rev_info *
> strbuf_release(&sb);
> }
>
> -static void prepare_submodule_repo_env_no_git_dir(struct strvec *out)
> -{
> - const char * const *var;
> -
> - for (var = local_repo_env; *var; var++) {
> - if (strcmp(*var, CONFIG_DATA_ENVIRONMENT))
> - strvec_push(out, *var);
> - }
> -}
> -
> 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);
> }