Re: [PATCH] submodule: stop sanitizing config options

[Jeff King <peff@peff.net>]

On Wed, May 04, 2016 at 11:43:47AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index 3a40d4b..c9d53e1 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
> > @@ -197,9 +197,9 @@ isnumber()
> >  # of the settings from GIT_CONFIG_PARAMETERS.
> >  sanitize_submodule_env()
> >  {
> > -	sanitized_config=$(git submodule--helper sanitize-config)
> > +	save_config=$GIT_CONFIG_PARAMETERS
> >  	clear_local_git_env
> > -	GIT_CONFIG_PARAMETERS=$sanitized_config
> > +	GIT_CONFIG_PARAMETERS=$save_config
> >  	export GIT_CONFIG_PARAMETERS
> >  }
> 
> This does "clear the obviously per-repository stuff, but add back in
> anything that came from -c".  If it is easy to do "add anything that
> came from -c, and then clear the obviously per-repository stuff", we
> don't even have to say "exporting core.worktree down may hurt; do
> not do it then", which may be the best of both worlds?

It's not easy at this level. clear_local_git_env() is just clearing
whole variables like $GIT_WORK_TREE. But if you say "-c core.worktree",
that is still live in $GIT_CONFIG_PARAMETERS, and gets fed to
sub-processes on each invocation. Nobody ever removes individual content
from $GIT_CONFIG_PARAMETERS once something is in it.

> Or have we decided that even sharing core.worktree may have a valid
> use case and it is better not to filter them?

I do not personally consider it a valid use case. The sentiment just
seemed to be that it was better to make the rule simple and easy to
explain ("everything gets passed through; if it hurts, don't do it")
than to keep sanitizing.

If you want "pass through everything except core.worktree", then that is
the blacklist approach (which is easy to do if we scrap this patch and
just teak submodule_config_ok() from a whitelist into a blacklist).

-Peff