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

[Stefan Beller <sbeller@xxxxxxxxxx>]

On Fri, Feb 26, 2016 at 11:17 AM, Jacob Keller <jacob.e.keller@xxxxxxxxx> wrote:
> From: Jacob Keller <jacob.keller@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> ---
>
> 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) ?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html