Re: [PATCH v3 3/3] git: submodule honor -c credential.* from command line

From: Stefan Beller <sbeller@google.com>
Date: Feb 26, 2016, 19:41 UTC
Message-ID: <CAGZ79kZnR3EsESCFdORq84awWnRZZ9A_+mcH_54ULZZJfD8xyQ@mail.gmail.com>
In-Reply-To: <1456514268-8241-3-git-send-email-jacob.e.keller@intel.com>

On Fri, Feb 26, 2016 at 11:17 AM, Jacob Keller <jacob.e.keller@intel.com> wrote:

Show 40 quoted lines

> From: Jacob Keller <jacob.keller@gmail.com>
>
> Due to the way that the git-submodule code works, it clears all local
> git environment variables before entering submodules. This is normally
> a good thing since we want to clear settings such as GIT_WORKTREE and
> other variables which would affect the operation of submodule commands.
> However, GIT_CONFIG_PARAMETERS is special, and we actually do want to
> preserve these settings. However, we do not want to preserve all
> configuration as many things should be left specific to the parent
> project.
>
> Add a git submodule--helper function, sanitize-config, which shall be
> used to sanitize GIT_CONFIG_PARAMETERS, removing all key/value pairs
> except a small subset that are known to be safe and necessary.
>
> Replace all the calls to clear_local_git_env with a wrapped function
> that filters GIT_CONFIG_PARAMETERS using the new helper and then
> restores it to the filtered subset after clearing the rest of the
> environment.
>
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
>
> Notes:
>     - v2
>     * Clarify which paramaters are left after the sanitization, and don't seem to
>       indicate it is our goal to extend the list.
>     * add a comment in the submodule_config_ok function indicating the same
>
>     - v3
>     * Remove extraneous comments
>     * add strbuf_release calls
>     * rename sanitize_local_git_env
>     * remove use of local
>     * add C equivalent of sanitize_config
>     * add a test for the credential passing
>
>     - v3
>     * use argc check instead of empty options check
>     * fix brain-melting quotes in t7412-submodule--helper.sh

In the future, would you mind to try interdiffs in a cover letter?

(I do git diff HEAD...$(previousSeries) with previousSerier either local branch or rather what Junio picked up already. There is also tbdiff, which should be better and easier than this work flow)

> +                */
> +               strbuf_addstr(&quoted, var);
> +               strbuf_addch(&quoted, '=');
> +               strbuf_addstr(&quoted, value);

This could be `strbuf_addf("%s=%s", var, value);` (?) which then gets quoted below

> +                       git_config_from_parameters(sanitize_submodule_config,
> +                                                  &sanitized_config);
> +                       argv_array_pushf(out, "%s=%s", *var, sanitized_config.buf);

like here?

> -                       subsha1=$(clear_local_git_env; cd "$sm_path" &&
> +                       subsha1=$(sanitize_submodule_env; cd "$sm_path" &&
>                                 git rev-parse --verify HEAD) ||

While at it, we could discuss if we want to replace the pattern cd <somewhere> && git-command by `git -C <somewhere> <command>` eventually (not in this patch) ?