git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 4/5] submodule--helper: move config-sanitizing to submodule.c

From
Stefan Beller <sbeller@google.com>
Date
Apr 28, 2016, 15:30 UTC
Message-ID
<CAGZ79kbJvgseY6aBV6vh0aEFNmHDtSvQuM8xHvp7_pYZSXCq+A@mail.gmail.com>
In-Reply-To
<20160428133819.GD25319@sigill.intra.peff.net>
On Thu, Apr 28, 2016 at 6:38 AM, Jeff King <peff@peff.net> wrote:
Show 151 quoted lines
> These functions should be used by any code which spawns a
> submodule process, which may happen in submodule.c (e.g.,
> for spawning fetch). Let's move them there and make them
> public so that submodule--helper can continue to use them.
>
> Sine they're now public, let's also provide a basic overview
> of their intended use.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  builtin/submodule--helper.c | 48 --------------------------------------------
>  submodule.c                 | 49 +++++++++++++++++++++++++++++++++++++++++++++
>  submodule.h                 | 16 +++++++++++++++
>  3 files changed, 65 insertions(+), 48 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 3bd6883..de3ad5b 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -119,54 +119,6 @@ static int module_name(int argc, const char **argv, const char *prefix)
>         return 0;
>  }
>
> -/*
> - * Rules to sanitize configuration variables that are Ok to be passed into
> - * submodule operations from the parent project using "-c". Should only
> - * include keys which are both (a) safe and (b) necessary for proper
> - * operation.
> - */
> -static int submodule_config_ok(const char *var)
> -{
> -       if (starts_with(var, "credential."))
> -               return 1;
> -       return 0;
> -}
> -
> -static int sanitize_submodule_config(const char *var, const char *value, void *data)
> -{
> -       struct strbuf *out = data;
> -
> -       if (submodule_config_ok(var)) {
> -               if (out->len)
> -                       strbuf_addch(out, ' ');
> -
> -               if (value)
> -                       sq_quotef(out, "%s=%s", var, value);
> -               else
> -                       sq_quote_buf(out, var);
> -       }
> -
> -       return 0;
> -}
> -
> -static void prepare_submodule_repo_env(struct argv_array *out)
> -{
> -       const char * const *var;
> -
> -       for (var = local_repo_env; *var; var++) {
> -               if (!strcmp(*var, CONFIG_DATA_ENVIRONMENT)) {
> -                       struct strbuf sanitized_config = STRBUF_INIT;
> -                       git_config_from_parameters(sanitize_submodule_config,
> -                                                  &sanitized_config);
> -                       argv_array_pushf(out, "%s=%s", *var, sanitized_config.buf);
> -                       strbuf_release(&sanitized_config);
> -               } else {
> -                       argv_array_push(out, *var);
> -               }
> -       }
> -
> -}
> -
>  static int clone_submodule(const char *path, const char *gitdir, const char *url,
>                            const char *depth, const char *reference, int quiet)
>  {
> diff --git a/submodule.c b/submodule.c
> index 90825e1..02eaf0e 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -13,6 +13,7 @@
>  #include "argv-array.h"
>  #include "blob.h"
>  #include "thread-utils.h"
> +#include "quote.h"
>
>  static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
>  static int parallel_jobs = 1;
> @@ -1129,3 +1130,51 @@ int parallel_submodules(void)
>  {
>         return parallel_jobs;
>  }
> +
> +/*
> + * Rules to sanitize configuration variables that are Ok to be passed into
> + * submodule operations from the parent project using "-c". Should only
> + * include keys which are both (a) safe and (b) necessary for proper
> + * operation.
> + */
> +static int submodule_config_ok(const char *var)
> +{
> +       if (starts_with(var, "credential."))
> +               return 1;
> +       return 0;
> +}
> +
> +int sanitize_submodule_config(const char *var, const char *value, void *data)
> +{
> +       struct strbuf *out = data;
> +
> +       if (submodule_config_ok(var)) {
> +               if (out->len)
> +                       strbuf_addch(out, ' ');
> +
> +               if (value)
> +                       sq_quotef(out, "%s=%s", var, value);
> +               else
> +                       sq_quote_buf(out, var);
> +       }
> +
> +       return 0;
> +}
> +
> +void prepare_submodule_repo_env(struct argv_array *out)
> +{
> +       const char * const *var;
> +
> +       for (var = local_repo_env; *var; var++) {
> +               if (!strcmp(*var, CONFIG_DATA_ENVIRONMENT)) {
> +                       struct strbuf sanitized_config = STRBUF_INIT;
> +                       git_config_from_parameters(sanitize_submodule_config,
> +                                                  &sanitized_config);
> +                       argv_array_pushf(out, "%s=%s", *var, sanitized_config.buf);
> +                       strbuf_release(&sanitized_config);
> +               } else {
> +                       argv_array_push(out, *var);
> +               }
> +       }
> +
> +}
> diff --git a/submodule.h b/submodule.h
> index 7ef3775..7577b3b 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -61,4 +61,20 @@ int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam
>  void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
>  int parallel_submodules(void);
>
> +/*
> + * This function is intended as a callback for use with
> + * git_config_from_parameters(). It ignores any config options which
> + * are not suitable for passing along to a submodule, and accumulates the rest
> + * in "data", which must be a pointer to a strbuf.

So why is it a void* then? You could make it a strbuf* here, so you would not have to document it? Oh right, because of git_config_from_parameters(sanitize_submodule_config, ...

> The end result can
> + * be put into $GIT_CONFIG_PARAMETERS for passing to a sub-process.
s/sub-process/process operating on submodules/, maybe ?

While it is technically a sub-process, I started to have an aversion against "sub"-things unless strictly required. :)

Show 14 quoted lines
> + */
> +int sanitize_submodule_config(const char *var, const char *value, void *data);
> +
> +/*
> + * Prepare the "env_array" parameter of a "struct child_process" for executing
> + * a submodule by clearing any repo-specific envirionment variables, but
> + * retaining any config approved by sanitize_submodule_config().
> + */
> +void prepare_submodule_repo_env(struct argv_array *out);
> +
>  #endif
> --
> 2.8.1.617.gbdccc2d
>
Previous: Jeff KingNext: Jeff King
Message 28 of 36 in “[RFC] How to pass Git config command line instructions to Submodule commands?”
  1. Lars SchneiderApr 25, 2016
  2. Stefan BellerApr 25, 2016
  3. Jacob KellerApr 25, 2016
  4. Jeff KingApr 25, 2016
  5. Jeff KingApr 25, 2016
  6. Lars SchneiderApr 28, 2016
  7. Jeff KingApr 28, 2016
  8. Jeff KingApr 28, 2016
  9. Jeff KingApr 28, 2016
  10. 0/5 fixes for sanitized submodule configJeff King, Apr 28, 2016
  11. 1/5 t5550: fix typo in $HTTPD_URLJeff King, Apr 28, 2016
  12. Jacob KellerApr 28, 2016
  13. Jeff KingApr 28, 2016
  14. Jacob KellerApr 28, 2016
  15. 2/5 t5550: break submodule config test into multiple sub-testsJeff King, Apr 28, 2016
  16. Stefan BellerApr 28, 2016
  17. Jeff KingApr 28, 2016
  18. Jacob KellerApr 28, 2016
  19. 3/5 submodule: export sanitized GIT_CONFIG_PARAMETERSJeff King, Apr 28, 2016
  20. Stefan BellerApr 28, 2016
  21. Jeff KingApr 28, 2016
  22. Stefan BellerApr 28, 2016
  23. Johannes SchindelinApr 28, 2016
  24. Jacob KellerApr 28, 2016
  25. Jeff KingApr 28, 2016
  26. Jacob KellerApr 28, 2016
  27. 4/5 submodule--helper: move config-sanitizing to submodule.cJeff King, Apr 28, 2016
  28. Stefan BellerApr 28, 2016
  29. Jeff KingApr 28, 2016
  30. Lars SchneiderApr 28, 2016
  31. 5/5 submodule: use prepare_submodule_repo_env consistentlyJeff King, Apr 28, 2016
  32. Johannes SchindelinApr 28, 2016
  33. Stefan BellerApr 28, 2016
  34. Jacob KellerApr 28, 2016
  35. Lars SchneiderApr 28, 2016
  36. Jeff KingApr 28, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.