Re: [PATCH v3 4/5] run-command: refactor subprocess env preparation

[Elijah Newren <newren@gmail.com>]

On Thu, Jun 10, 2021 at 10:35 AM Jonathan Tan <jonathantanmy@google.com> wrote:
>
> submodule.c has functionality that prepares the environment for running
> a subprocess in a new repo. The lazy-fetching code (used in partial
> clones) will need this in a subsequent commit, so move it to a more
> central location.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  run-command.c | 12 ++++++++++++
>  run-command.h | 10 ++++++++++
>  submodule.c   | 18 ++----------------
>  3 files changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/run-command.c b/run-command.c
> index be6bc128cd..549a94a6a4 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -1892,3 +1892,15 @@ int run_auto_maintenance(int quiet)
>
>         return run_command(&maint);
>  }
> +
> +void prepare_other_repo_env(struct strvec *env_array, const char *new_git_dir)
> +{
> +       const char * const *var;
> +
> +       for (var = local_repo_env; *var; var++) {
> +               if (strcmp(*var, CONFIG_DATA_ENVIRONMENT) &&
> +                   strcmp(*var, CONFIG_COUNT_ENVIRONMENT))
> +                       strvec_push(env_array, *var);
> +       }
> +       strvec_pushf(env_array, "%s=%s", GIT_DIR_ENVIRONMENT, new_git_dir);
> +}
> diff --git a/run-command.h b/run-command.h
> index d08414a92e..92f1c00b11 100644
> --- a/run-command.h
> +++ b/run-command.h
> @@ -483,4 +483,14 @@ 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 prepares env_array for a command to be run in a
> + * new repo. This adds all GIT_* environment variables to env_array with the
> + * exception of GIT_CONFIG_PARAMETERS (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 cache.h for more
> + * information.

This comment is out-of-date as of your previous patch.  There's (at
least) one more variable that is also excluded.

> + */
> +void prepare_other_repo_env(struct strvec *env_array, const char *new_git_dir);
> +
>  #endif
> diff --git a/submodule.c b/submodule.c
> index f09031e397..8e611fe1db 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -484,28 +484,14 @@ 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) &&
> -                   strcmp(*var, CONFIG_COUNT_ENVIRONMENT))
> -                       strvec_push(out, *var);
> -       }
> -}
> -
>  void prepare_submodule_repo_env(struct strvec *out)
>  {
> -       prepare_submodule_repo_env_no_git_dir(out);
> -       strvec_pushf(out, "%s=%s", GIT_DIR_ENVIRONMENT,
> -                    DEFAULT_GIT_DIR_ENVIRONMENT);
> +       prepare_other_repo_env(out, DEFAULT_GIT_DIR_ENVIRONMENT);
>  }
>
>  static void prepare_submodule_repo_env_in_gitdir(struct strvec *out)
>  {
> -       prepare_submodule_repo_env_no_git_dir(out);
> -       strvec_pushf(out, "%s=.", GIT_DIR_ENVIRONMENT);
> +       prepare_other_repo_env(out, ".");
>  }
>
>  /*
> --
> 2.32.0.rc1.229.g3e70b5a671-goog