Re: [PATCH v6 0/2] Add support for sending additional HTTP headers

From: Jeff King <peff@peff.net>
Date: May 4, 2016, 07:45 UTC
Message-ID: <20160504074559.GA3077@sigill.intra.peff.net>
In-Reply-To: <20160504062618.GA9849@sigill.intra.peff.net>

On Wed, May 04, 2016 at 02:26:18AM -0400, Jeff King wrote:

Show 15 quoted lines

> >   submodule: pass on http.extraheader config settings
> 
> IMHO this should come on top of jk/submodule-config-sanitize-fix (I was
> surprised at first that your test worked at all, but that is because it
> is using "clone", which is the one code path that works).
> 
> But I think we are waiting on going one of two paths:
> 
>   1. drop sanitizing entirely
> 
>   2. fix sanitizing and add more variables to it
> 
> If we go the route of (2), then we'd want my fix topic and this patch.
> And if not, then we don't need any of it (just a patch dropping the
> filtering, which AFAIK nobody has written yet).

Actually, I think this last bit is not quite true. If we want to go back to "nothing gets passed to submodules", we can drop all of my patches, but I don't think anybody wants to do that.

But if we want "everything gets passed to submodules", then we do need something like my patch series, because every use of local_repo_env needs to be come "local_repo_env excluding GIT_CONFIG_PARAMETERS". I don't think we want to simply drop that variable from local_repo_env (which would also mean that it would be propagated to a local git-upload-pack, for example, along with any third-party scripts that use rev-parse --local-env-vars).

So I think we'd actually want my series as a preliminary fix, followed by dropping the whitelist entirely on top of that, and then probably simplifying the shell sanitize_submodule_env() on top of that (it would be correct without the whitelist, but you can also trivially implement it without having to call submodule--helper at all).

-Peff