Re: [PATCH] submodule: stop sanitizing config options

Jeff King [off-list ref] writes:

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?

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

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

Jeff King [off-list ref] writes:

quoted

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