{"thread":{"id":"41506","subject":"[PATCH] git: submodule honor -c credential.* from command line","startedAt":"2016-02-24T23:12:38Z","lastAt":"2016-02-24T23:12:38Z","messageCount":1,"participants":["Jacob Keller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"279225","messageId":"1456355558-5151-1-git-send-email-jacob.e.keller@intel.com","threadId":"41506","inReplyTo":null,"subject":"[PATCH] git: submodule honor -c credential.* from command line","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-02-24T23:12:38Z","receivedAt":"2016-02-24T23:12:38Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nDue to the way that the git-submodule code works, it clears all local\ngit environment variables before entering submodules. This is normally\na good thing since we want to clear settings such as GIT_WORKTREE and\nother variables which would affect the operation of submodule commands.\nHowever, GIT_CONFIG_PARAMETERS is special, and we actually do want to\npreserve these settings. However, we do not want to preserve all\nconfiguration as many things should be left specific to the parent\nproject.\n\nAdd a git submodule--helper function which can be used to sanitize the\nGIT_CONFIG_PARAMETERS value to only allow certain settings. For now,\nrestrict this to only credential.* settings.\n\nReplace all the calls to clear_local_git_env with a wrapped function\nthat filters GIT_CONFIG_PARAMETERS using the new helper and then\nrestores it to the filtered subset after clearing the rest of the\nenvironment.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\nThis is a re-send with +Cc Stefan Beller since he has handled a lot of\nthe git-submodule--helper code.\n\n builtin/submodule--helper.c  | 51 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh             | 36 ++++++++++++++++++++-----------\n t/t7412-submodule--helper.sh | 25 ++++++++++++++++++++++\n 3 files changed, 99 insertions(+), 13 deletions(-)\n create mode 100755 t/t7412-submodule--helper.sh\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f4c3eff179b5..8194d3b3d1d5 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -255,6 +255,56 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+int submodule_config_ok(const char *var)\n+{\n+\tif (starts_with(var, \"credential.\"))\n+\t\treturn 1;\n+\treturn 0;\n+}\n+\n+int sanitize_submodule_config(const char *var, const char *value, void *data)\n+{\n+\tstruct strbuf quoted = STRBUF_INIT;\n+\tstruct strbuf *out = data;\n+\n+\tif (submodule_config_ok(var)) {\n+\t\tif (out->len)\n+\t\t\tstrbuf_addch(out, ' ');\n+\n+\t\t/* combined all the values before we quote them */\n+\t\tstrbuf_addstr(&quoted, var);\n+\t\tstrbuf_addch(&quoted, '=');\n+\t\tstrbuf_addstr(&quoted, value);\n+\n+\t\t/* safely quote them for shell use */\n+\t\tsq_quote_buf(out, quoted.buf);\n+\t}\n+\treturn 0;\n+}\n+\n+static int module_sanitize_config(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct strbuf sanitized_config = STRBUF_INIT;\n+\n+\tstruct option module_sanitize_config_options[] = {\n+\t\tOPT_END()\n+\t};\n+\n+\tconst char *const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper sanitize-config\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_sanitize_config_options,\n+\t\t\t     git_submodule_helper_usage, 0);\n+\n+\tgit_config_from_parameters(sanitize_submodule_config, &sanitized_config);\n+\tif (sanitized_config.len)\n+\t\tprintf(\"%s\\n\", sanitized_config.buf);\n+\n+\treturn 0;\n+}\n+\n struct cmd_struct {\n \tconst char *cmd;\n \tint (*fn)(int, const char **, const char *);\n@@ -264,6 +314,7 @@ static struct cmd_struct commands[] = {\n \t{\"list\", module_list},\n \t{\"name\", module_name},\n \t{\"clone\", module_clone},\n+\t{\"sanitize-config\", module_sanitize_config},\n };\n \n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 9bc5c5f94d1d..dd469ecb2065 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -192,6 +192,16 @@ isnumber()\n \tn=$(($1 + 0)) 2>/dev/null && test \"$n\" = \"$1\"\n }\n \n+# Sanitize the local git environment for use within a submodule. We\n+# can't simply use clear_local_git_env since we want to preserve some\n+# of the settings from GIT_CONFIG_PARAMETERS.\n+sanitize_local_git_env()\n+{\n+\tlocal sanitized_config = $(git submodule--helper sanitize-config)\n+\tclear_local_git_env\n+\tGIT_CONFIG_PARAMETERS=$sanitized_config\n+}\n+\n #\n # Add a new submodule to the working tree, .gitmodules and the index\n #\n@@ -349,7 +359,7 @@ Use -f if you really want to add it.\" >&2\n \t\tfi\n \t\tgit submodule--helper clone ${GIT_QUIET:+--quiet} --prefix \"$wt_prefix\" --path \"$sm_path\" --name \"$sm_name\" --url \"$realrepo\" \"$reference\" \"$depth\" || exit\n \t\t(\n-\t\t\tclear_local_git_env\n+\t\t\tsanitize_local_git_env\n \t\t\tcd \"$sm_path\" &&\n \t\t\t# ash fails to wordsplit ${branch:+-b \"$branch\"...}\n \t\t\tcase \"$branch\" in\n@@ -418,7 +428,7 @@ cmd_foreach()\n \t\t\tname=$(git submodule--helper name \"$sm_path\")\n \t\t\t(\n \t\t\t\tprefix=\"$prefix$sm_path/\"\n-\t\t\t\tclear_local_git_env\n+\t\t\t\tsanitize_local_git_env\n \t\t\t\tcd \"$sm_path\" &&\n \t\t\t\tsm_path=$(relative_path \"$sm_path\") &&\n \t\t\t\t# we make $path available to scripts ...\n@@ -713,7 +723,7 @@ Maybe you want to use 'update --init'?\")\"\n \t\t\tcloned_modules=\"$cloned_modules;$name\"\n \t\t\tsubsha1=\n \t\telse\n-\t\t\tsubsha1=$(clear_local_git_env; cd \"$sm_path\" &&\n+\t\t\tsubsha1=$(sanitize_local_git_env; cd \"$sm_path\" &&\n \t\t\t\tgit rev-parse --verify HEAD) ||\n \t\t\tdie \"$(eval_gettext \"Unable to find current revision in submodule path '\\$displaypath'\")\"\n \t\tfi\n@@ -723,11 +733,11 @@ Maybe you want to use 'update --init'?\")\"\n \t\t\tif test -z \"$nofetch\"\n \t\t\tthen\n \t\t\t\t# Fetch remote before determining tracking $sha1\n-\t\t\t\t(clear_local_git_env; cd \"$sm_path\" && git-fetch) ||\n+\t\t\t\t(sanitize_local_git_env; cd \"$sm_path\" && git-fetch) ||\n \t\t\t\tdie \"$(eval_gettext \"Unable to fetch in submodule path '\\$sm_path'\")\"\n \t\t\tfi\n-\t\t\tremote_name=$(clear_local_git_env; cd \"$sm_path\" && get_default_remote)\n-\t\t\tsha1=$(clear_local_git_env; cd \"$sm_path\" &&\n+\t\t\tremote_name=$(sanitize_local_git_env; cd \"$sm_path\" && get_default_remote)\n+\t\t\tsha1=$(sanitize_local_git_env; cd \"$sm_path\" &&\n \t\t\t\tgit rev-parse --verify \"${remote_name}/${branch}\") ||\n \t\t\tdie \"$(eval_gettext \"Unable to find current ${remote_name}/${branch} revision in submodule path '\\$sm_path'\")\"\n \t\tfi\n@@ -745,7 +755,7 @@ Maybe you want to use 'update --init'?\")\"\n \t\t\tthen\n \t\t\t\t# Run fetch only if $sha1 isn't present or it\n \t\t\t\t# is not reachable from a ref.\n-\t\t\t\t(clear_local_git_env; cd \"$sm_path\" &&\n+\t\t\t\t(sanitize_local_git_env; cd \"$sm_path\" &&\n \t\t\t\t\t( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&\n \t\t\t\t\t test -z \"$rev\") || git-fetch)) ||\n \t\t\t\tdie \"$(eval_gettext \"Unable to fetch in submodule path '\\$displaypath'\")\"\n@@ -787,7 +797,7 @@ Maybe you want to use 'update --init'?\")\"\n \t\t\t\tdie \"$(eval_gettext \"Invalid update mode '$update_module' for submodule '$name'\")\"\n \t\t\tesac\n \n-\t\t\tif (clear_local_git_env; cd \"$sm_path\" && $command \"$sha1\")\n+\t\t\tif (sanitize_local_git_env; cd \"$sm_path\" && $command \"$sha1\")\n \t\t\tthen\n \t\t\t\tsay \"$say_msg\"\n \t\t\telif test -n \"$must_die_on_failure\"\n@@ -803,7 +813,7 @@ Maybe you want to use 'update --init'?\")\"\n \t\tthen\n \t\t\t(\n \t\t\t\tprefix=\"$prefix$sm_path/\"\n-\t\t\t\tclear_local_git_env\n+\t\t\t\tsanitize_local_git_env\n \t\t\t\tcd \"$sm_path\" &&\n \t\t\t\teval cmd_update\n \t\t\t)\n@@ -841,7 +851,7 @@ Maybe you want to use 'update --init'?\")\"\n \n set_name_rev () {\n \trevname=$( (\n-\t\tclear_local_git_env\n+\t\tsanitize_local_git_env\n \t\tcd \"$1\" && {\n \t\t\tgit describe \"$2\" 2>/dev/null ||\n \t\t\tgit describe --tags \"$2\" 2>/dev/null ||\n@@ -1125,7 +1135,7 @@ cmd_status()\n \t\telse\n \t\t\tif test -z \"$cached\"\n \t\t\tthen\n-\t\t\t\tsha1=$(clear_local_git_env; cd \"$sm_path\" && git rev-parse --verify HEAD)\n+\t\t\t\tsha1=$(sanitize_local_git_env; cd \"$sm_path\" && git rev-parse --verify HEAD)\n \t\t\tfi\n \t\t\tset_name_rev \"$sm_path\" \"$sha1\"\n \t\t\tsay \"+$sha1 $displaypath$revname\"\n@@ -1135,7 +1145,7 @@ cmd_status()\n \t\tthen\n \t\t\t(\n \t\t\t\tprefix=\"$displaypath/\"\n-\t\t\t\tclear_local_git_env\n+\t\t\t\tsanitize_local_git_env\n \t\t\t\tcd \"$sm_path\" &&\n \t\t\t\teval cmd_status\n \t\t\t) ||\n@@ -1209,7 +1219,7 @@ cmd_sync()\n \t\t\tif test -e \"$sm_path\"/.git\n \t\t\tthen\n \t\t\t(\n-\t\t\t\tclear_local_git_env\n+\t\t\t\tsanitize_local_git_env\n \t\t\t\tcd \"$sm_path\"\n \t\t\t\tremote=$(get_default_remote)\n \t\t\t\tgit config remote.\"$remote\".url \"$sub_origin_url\"\ndiff --git a/t/t7412-submodule--helper.sh b/t/t7412-submodule--helper.sh\nnew file mode 100755\nindex 000000000000..376f58afe967\n--- /dev/null\n+++ b/t/t7412-submodule--helper.sh\n@@ -0,0 +1,25 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2016 Jacob Keller\n+#\n+\n+test_description='Basic plumbing support of submodule--helper\n+\n+This test tries to verify the submodule--helper plumbing command used\n+to implement git-submodule.\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'sanitize-config clears configuration' '\n+\tgit -c user.name=\"Some User\" submodule--helper sanitize-config >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n+test_expect_success 'sanitize-config keeps credential.helper' '\n+\tgit -c credential.helper=\"helper\" submodule--helper sanitize-config >actual &&\n+\techo \"'\\''credential.helper=helper'\\''\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n2.7.1.429.g45cd78e\n"}]}