{"thread":{"id":"41520","subject":"[PATCH v3 1/2] t/lib-http/apache.conf: load mod_unixd module in apache 2.4","startedAt":"2016-02-25T18:57:11Z","lastAt":"2016-02-26T07:32:56Z","messageCount":15,"participants":["Jacob Keller","Jeff King","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"279362","messageId":"1456426632-23257-1-git-send-email-jacob.e.keller@intel.com","threadId":"41520","inReplyTo":null,"subject":"[PATCH v3 1/2] t/lib-http/apache.conf: load mod_unixd module in apache 2.4","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-02-25T18:57:11Z","receivedAt":"2016-02-25T18:57:11Z","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\nSince 2.4, apache will fail to load unless mod_unixd is loaded in order\nto drop privileges.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n\nI am not sure why this wasn't there already, but I am unable to run\nhttpd 2.4.18 without it, on Fedora 23.\n\n t/lib-httpd/apache.conf | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf\nindex 7d15e6d44c83..f667e7ce2f33 100644\n--- a/t/lib-httpd/apache.conf\n+++ b/t/lib-httpd/apache.conf\n@@ -64,6 +64,9 @@ LockFile accept.lock\n <IfModule !mod_mpm_prefork.c>\n \tLoadModule mpm_prefork_module modules/mod_mpm_prefork.so\n </IfModule>\n+<IfModule !mod_unixd.c>\n+\tLoadModule unixd_module modules/mod_unixd.so\n+</IfModule>\n </IfVersion>\n \n PassEnv GIT_VALGRIND\n-- \n2.7.1.429.g45cd78e\n"},{"id":"279363","messageId":"1456426632-23257-2-git-send-email-jacob.e.keller@intel.com","threadId":"41520","inReplyTo":"1456426632-23257-1-git-send-email-jacob.e.keller@intel.com","subject":"[PATCH v3 2/2] git: submodule honor -c credential.* from command line","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-02-25T18:57:12Z","receivedAt":"2016-02-25T18:57:12Z","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, sanitize-config, which shall be\nused to sanitize GIT_CONFIG_PARAMETERS, removing all key/value pairs\nexcept a small subset that are known to be safe and necessary.\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---\n\nThis version incorporates a bunch of suggestions from the review on v2,\nincluding a new test using httpd to verify the credentials. In addition,\nit includes a fix so that the C code in submodule--helper works as well.\nPreviously we used cp.env = local_repo_env to clear all the variables.\nInstead, we want to clear everything except use the new filter. This is\nsomewhat subtle but makes sense once you read how the child_process env\ncode works.\n\nNotes:\n    - v2\n    * Clarify which paramaters are left after the sanitization, and don't seem to\n      indicate it is our goal to extend the list.\n    * add a comment in the submodule_config_ok function indicating the same\n    \n    - v3\n    * Remove extraneous comments\n    * add strbuf_release calls\n    * rename sanitize_local_git_env\n    * remove use of local\n    * add C equivalent of sanitize_config\n    * add a test for the credential passing\n\n builtin/submodule--helper.c  | 86 +++++++++++++++++++++++++++++++++++++++++++-\n git-submodule.sh             | 36 ++++++++++++-------\n t/t5550-http-fetch-dumb.sh   | 17 +++++++++\n t/t7412-submodule--helper.sh | 25 +++++++++++++\n 4 files changed, 150 insertions(+), 14 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..4b43f5dc50e5 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -124,6 +124,64 @@ static int module_name(int argc, const char **argv, const char *prefix)\n \n \treturn 0;\n }\n+\n+/*\n+ * Rules to sanitize configuration variables that are Ok to be passed into\n+ * submodule operations from the parent project using \"-c\". Should only\n+ * include keys which are both (a) safe and (b) necessary for proper\n+ * operation.\n+ */\n+static int submodule_config_ok(const char *var)\n+{\n+\tif (starts_with(var, \"credential.\"))\n+\t\treturn 1;\n+\treturn 0;\n+}\n+\n+static 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/*\n+\t\t * The GIT_CONFIG_PARAMETERS parser is a bit picky and\n+\t\t * requires that each var=value pair be quoted as a single\n+\t\t * unit.\n+\t\t */\n+\t\tstrbuf_addstr(&quoted, var);\n+\t\tstrbuf_addch(&quoted, '=');\n+\t\tstrbuf_addstr(&quoted, value);\n+\n+\t\tsq_quote_buf(out, quoted.buf);\n+\t}\n+\n+\tstrbuf_release(&quoted);\n+\n+\treturn 0;\n+}\n+\n+static void prepare_submodule_repo_env(struct argv_array *out)\n+{\n+\tconst char * const *var;\n+\n+\tfor (var = local_repo_env; *var; var++) {\n+\t\tif (!strcmp(*var, CONFIG_DATA_ENVIRONMENT)) {\n+\t\t\tstruct strbuf sanitized_config = STRBUF_INIT;\n+\t\t\tgit_config_from_parameters(sanitize_submodule_config,\n+\t\t\t\t\t\t   &sanitized_config);\n+\t\t\targv_array_pushf(out, \"%s=%s\", *var, sanitized_config.buf);\n+\t\t\tstrbuf_release(&sanitized_config);\n+\t\t} else {\n+\t\t\targv_array_push(out, *var);\n+\t\t}\n+\t}\n+\n+}\n+\n static int clone_submodule(const char *path, const char *gitdir, const char *url,\n \t\t\t   const char *depth, const char *reference, int quiet)\n {\n@@ -145,7 +203,7 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url\n \targv_array_push(&cp.args, path);\n \n \tcp.git_cmd = 1;\n-\tcp.env = local_repo_env;\n+\tprepare_submodule_repo_env(&cp.env_array);\n \tcp.no_stdin = 1;\n \n \treturn run_command(&cp);\n@@ -255,6 +313,31 @@ static int module_clone(int argc, const char **argv, const char *prefix)\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+\tstrbuf_release(&sanitized_config);\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 +347,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..7c79e87fb7c4 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_submodule_env()\n+{\n+\tsanitized_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_submodule_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_submodule_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_submodule_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_submodule_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_submodule_env; cd \"$sm_path\" && get_default_remote)\n+\t\t\tsha1=$(sanitize_submodule_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_submodule_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_submodule_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_submodule_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_submodule_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_submodule_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_submodule_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_submodule_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/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\nindex 64146352ae20..48e2ab62da7a 100755\n--- a/t/t5550-http-fetch-dumb.sh\n+++ b/t/t5550-http-fetch-dumb.sh\n@@ -91,6 +91,23 @@ test_expect_success 'configured username does not override URL' '\n \texpect_askpass pass user@host\n '\n \n+test_expect_success 'cmdline credential config passes into submodules' '\n+\tgit init super &&\n+\tset_askpass user@host pass@host &&\n+\t(\n+\t\tcd super &&\n+\t\tgit submodule add \"$HTTPD_URL/auth/dumb/repo.git\" sub &&\n+\t\tgit commit -m \"add submodule\"\n+\t) &&\n+\tset_askpass wrong pass@host &&\n+\ttest_must_fail git clone --recursive super super-clone &&\n+\trm -rf super-clone &&\n+\tset_askpass wrong pass@host &&\n+\tgit -c \"credential.$HTTP_URL.username=user@host\" \\\n+\t\tclone --recursive super super-clone &&\n+\texpect_askpass pass user@host\n+'\n+\n test_expect_success 'fetch changes via http' '\n \techo content >>file &&\n \tgit commit -a -m two &&\ndiff --git a/t/t7412-submodule--helper.sh b/t/t7412-submodule--helper.sh\nnew file mode 100755\nindex 000000000000..0c68826c0a6f\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 verifies the submodule--helper plumbing command used to implement\n+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"},{"id":"279391","messageId":"20160225220045.GA10267@sigill.intra.peff.net","threadId":"41520","inReplyTo":"1456426632-23257-1-git-send-email-jacob.e.keller@intel.com","subject":"Re: [PATCH v3 1/2] t/lib-http/apache.conf: load mod_unixd module in apache 2.4","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-25T22:00:45Z","receivedAt":"2016-02-25T22:00:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 25, 2016 at 10:57:11AM -0800, Jacob Keller wrote:\n\n> From: Jacob Keller <jacob.keller@gmail.com>\n> \n> Since 2.4, apache will fail to load unless mod_unixd is loaded in order\n> to drop privileges.\n> \n> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>\n> ---\n> \n> I am not sure why this wasn't there already, but I am unable to run\n> httpd 2.4.18 without it, on Fedora 23.\n> \n>  t/lib-httpd/apache.conf | 3 +++\n>  1 file changed, 3 insertions(+)\n\nMichael (cc'd) posted an identical patch with some more discussion back\nin May:\n\n  http://article.gmane.org/gmane.comp.version-control.git/268770\n\nThe series languished because none of the reviewers had systems that\nreproduced the problem, and I think there's some additional work needed\nto get all of the svn-over-http tests running[1].\n\nI think this bit should be OK to take without the rest (though I like\nthe extra discussion in the original).\n\n-Peff\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/284349\n"},{"id":"279434","messageId":"xmqqbn74mlco.fsf@gitster.mtv.corp.google.com","threadId":"41520","inReplyTo":"20160225220045.GA10267@sigill.intra.peff.net","subject":"Re: [PATCH v3 1/2] t/lib-http/apache.conf: load mod_unixd module in apache 2.4","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-25T23:16:55Z","receivedAt":"2016-02-25T23:16:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Feb 25, 2016 at 10:57:11AM -0800, Jacob Keller wrote:\n>\n>> From: Jacob Keller <jacob.keller@gmail.com>\n>> \n>> Since 2.4, apache will fail to load unless mod_unixd is loaded in order\n>> to drop privileges.\n>> \n>> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>\n>> ---\n>> \n>> I am not sure why this wasn't there already, but I am unable to run\n>> httpd 2.4.18 without it, on Fedora 23.\n>> \n>>  t/lib-httpd/apache.conf | 3 +++\n>>  1 file changed, 3 insertions(+)\n>\n> Michael (cc'd) posted an identical patch with some more discussion back\n> in May:\n>\n>   http://article.gmane.org/gmane.comp.version-control.git/268770\n>\n> The series languished because none of the reviewers had systems that\n> reproduced the problem, and I think there's some additional work needed\n> to get all of the svn-over-http tests running[1].\n>\n> I think this bit should be OK to take without the rest (though I like\n> the extra discussion in the original).\n\nI can resurrect 745a5487 (t/lib-httpd: load mod_unixd, 2015-04-08),\nwhich is still on 'pu', and apply 2/2 from Jacov on top of it.\n\nThanks.\n\n>\n> -Peff\n>\n> [1] http://article.gmane.org/gmane.comp.version-control.git/284349\n"},{"id":"298979","messageId":"xmqq7fhsmlba.fsf@gitster.mtv.corp.google.com","threadId":"41520","inReplyTo":"xmqqbn74mlco.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 1/2] t/lib-http/apache.conf: load mod_unixd module in apache 2.4","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-25T23:17:45Z","receivedAt":"2016-02-25T23:17:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":""},{"id":"279435","messageId":"xmqq37sgml9f.fsf@gitster.mtv.corp.google.com","threadId":"41520","inReplyTo":"xmqqbn74mlco.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 1/2] t/lib-http/apache.conf: load mod_unixd module in apache 2.4","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-25T23:18:52Z","receivedAt":"2016-02-25T23:18:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> Michael (cc'd) posted an identical patch with some more discussion back\n>> in May:\n>>\n>>   http://article.gmane.org/gmane.comp.version-control.git/268770\n>>\n>> The series languished because none of the reviewers had systems that\n>> reproduced the problem, and I think there's some additional work needed\n>> to get all of the svn-over-http tests running[1].\n>>\n>> I think this bit should be OK to take without the rest (though I like\n>> the extra discussion in the original).\n>\n> I can resurrect 745a5487 (t/lib-httpd: load mod_unixd, 2015-04-08),\n> which is still on 'pu', and apply 2/2 from Jacov on top of it.\n\nAhh, the one you quoted does have a better log message.  Let's\nreplace the ancieint one I have and use Jacob's 2/2 on top of it.\n"},{"id":"279440","messageId":"CA+P7+xrtkgAv3oNwSvO1hfSAz_VDNGSa4GQWWSPDnZrZdsDokg@mail.gmail.com","threadId":"41520","inReplyTo":"xmqq37sgml9f.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 1/2] t/lib-http/apache.conf: load mod_unixd module in apache 2.4","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-02-25T23:39:37Z","receivedAt":"2016-02-25T23:39:37Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Feb 25, 2016 at 3:18 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>>> Michael (cc'd) posted an identical patch with some more discussion back\n>>> in May:\n>>>\n>>>   http://article.gmane.org/gmane.comp.version-control.git/268770\n>>>\n>>> The series languished because none of the reviewers had systems that\n>>> reproduced the problem, and I think there's some additional work needed\n>>> to get all of the svn-over-http tests running[1].\n>>>\n>>> I think this bit should be OK to take without the rest (though I like\n>>> the extra discussion in the original).\n>>\n>> I can resurrect 745a5487 (t/lib-httpd: load mod_unixd, 2015-04-08),\n>> which is still on 'pu', and apply 2/2 from Jacov on top of it.\n>\n> Ahh, the one you quoted does have a better log message.  Let's\n> replace the ancieint one I have and use Jacob's 2/2 on top of it.\n>\n\nAgreed. I did not know the source of the issue, only what fixed it.\n\nThanks,\nJake\n"},{"id":"279442","messageId":"CAPc5daUEqb2xuJFGj2xCPn-uqJ5XDcpP7VmiZr0VjZQ48m=ELw@mail.gmail.com","threadId":"41520","inReplyTo":"CA+P7+xrtkgAv3oNwSvO1hfSAz_VDNGSa4GQWWSPDnZrZdsDokg@mail.gmail.com","subject":"Re: [PATCH v3 1/2] t/lib-http/apache.conf: load mod_unixd module in apache 2.4","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-25T23:41:54Z","receivedAt":"2016-02-25T23:41:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Thu, Feb 25, 2016 at 3:39 PM, Jacob Keller <jacob.keller@gmail.com> wrote:\n>>\n>> Ahh, the one you quoted does have a better log message.  Let's\n>> replace the ancieint one I have and use Jacob's 2/2 on top of it.\n>>\n>\n> Agreed. I did not know the source of the issue, only what fixed it.\n\nActually, let's wait a bit to avoid unnecessary conflicts between topics.\nsb/submodule-fetch-nontip changes the way how these calls to \"clean env\"\nhelper function are made.\n"},{"id":"279443","messageId":"CA+P7+xpBtzQ6vd-aTqqrcBVwjXi2UKunnMacrPCHSKiQbX0BuA@mail.gmail.com","threadId":"41520","inReplyTo":"CAPc5daUEqb2xuJFGj2xCPn-uqJ5XDcpP7VmiZr0VjZQ48m=ELw@mail.gmail.com","subject":"Re: [PATCH v3 1/2] t/lib-http/apache.conf: load mod_unixd module in apache 2.4","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-02-25T23:43:42Z","receivedAt":"2016-02-25T23:43:42Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Feb 25, 2016 at 3:41 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Actually, let's wait a bit to avoid unnecessary conflicts between topics.\n> sb/submodule-fetch-nontip changes the way how these calls to \"clean env\"\n> helper function are made.\n\nYa there will be some superficial conflicts with some of Stefan's work.\n\nRegards,\nJake\n"},{"id":"279459","messageId":"20160226015510.GA5347@flurp.local","threadId":"41520","inReplyTo":"1456426632-23257-2-git-send-email-jacob.e.keller@intel.com","subject":"Re: [PATCH v3 2/2] git: submodule honor -c credential.* from command line","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-02-26T01:55:10Z","receivedAt":"2016-02-26T01:55:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Feb 25, 2016 at 10:57:12AM -0800, Jacob Keller wrote:\n> [...]\n> Replace all the calls to clear_local_git_env with a wrapped function\n> that filters GIT_CONFIG_PARAMETERS using the new helper and then\n> restores it to the filtered subset after clearing the rest of the\n> environment.\n> \n> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>\n> ---\n> diff --git a/t/t7412-submodule--helper.sh b/t/t7412-submodule--helper.sh\n> @@ -0,0 +1,25 @@\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\nNot worth a re-roll, but these quote sequences are brain-melting.\nEasier would have been to double-quote the second argument of\ntest_expect_success() and then do either:\n\n    test_expect_success 'sanitize-config keeps credential.helper' \"\n        git -c [...] submodule--helper sanitize-config >actual &&\n        echo \\'credential.helper=helper\\' >expect &&\n        test_cmp expect actual\n    \"\n\nor:\n\n    test_expect_success 'sanitize-config keeps credential.helper' \"\n        git -c [...] submodule--helper sanitize-config >actual &&\n        cat >expect <<-\\EOF &&\n        'credential.helper=helper'\n        EOF\n        test_cmp expect actual\n    \"\n\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_done\n"},{"id":"279478","messageId":"CA+P7+xqyTUh60BOmY03JHE6HyVqY7iidVkUf3ji95_s3uE32cg@mail.gmail.com","threadId":"41520","inReplyTo":"20160226015510.GA5347@flurp.local","subject":"Re: [PATCH v3 2/2] git: submodule honor -c credential.* from command line","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-02-26T02:19:17Z","receivedAt":"2016-02-26T02:19:17Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Feb 25, 2016 at 5:55 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> Not worth a re-roll, but these quote sequences are brain-melting.\n> Easier would have been to double-quote the second argument of\n> test_expect_success() and then do either:\n>\n\nThey are. I fiddled with things till I got it working. I wasn't sure\nif double quotes would cause a problem or not, since most other tests\nseemed to avoid it.\n\nHowever this re-roll forgot to check argc in submodule--helper so\nmaybe worth reroll to fix both things?\n\nThanks\nJake\n"},{"id":"279479","messageId":"CAPig+cRM_1JLfBiOqo+-EdKbXp--4VUbddKN_zpSCSLK8PT09w@mail.gmail.com","threadId":"41520","inReplyTo":"CA+P7+xqyTUh60BOmY03JHE6HyVqY7iidVkUf3ji95_s3uE32cg@mail.gmail.com","subject":"Re: [PATCH v3 2/2] git: submodule honor -c credential.* from command line","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-02-26T02:26:14Z","receivedAt":"2016-02-26T02:26:14Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Feb 25, 2016 at 9:19 PM, Jacob Keller <jacob.keller@gmail.com> wrote:\n> On Thu, Feb 25, 2016 at 5:55 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n>> Not worth a re-roll, but these quote sequences are brain-melting.\n>> Easier would have been to double-quote the second argument of\n>> test_expect_success() and then do either:\n>\n> They are. I fiddled with things till I got it working. I wasn't sure\n> if double quotes would cause a problem or not, since most other tests\n> seemed to avoid it.\n\nYou normally want variable interpolations within the test to happen\nwhen the test is actually run rather than when it is defined, which is\nwhy single quotes are normally used. But this test doesn't use any\nvariable interpolations, so double quotes won't hurt it.\n\n> However this re-roll forgot to check argc in submodule--helper so\n> maybe worth reroll to fix both things?\n\nPossibly.\n\nOne thing I elided accidentally was that when changing the second\nargument of test_expect_success to double-quotes, you would altogether\ndrop the double-quotes around \"helper\" in:\n\n    git -c credential.helper=\"helper\" submodule--helper [...] &&\n\nto become:\n\n    git -c credential.helper=helper submodule--helper [...] &&\n"},{"id":"279480","messageId":"CA+P7+xpdZRMPgUtP3Hu1DMN74S5rVPT6La7Pa0=LsLJHaO7bxw@mail.gmail.com","threadId":"41520","inReplyTo":"CAPig+cRM_1JLfBiOqo+-EdKbXp--4VUbddKN_zpSCSLK8PT09w@mail.gmail.com","subject":"Re: [PATCH v3 2/2] git: submodule honor -c credential.* from command line","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-02-26T02:31:42Z","receivedAt":"2016-02-26T02:31:42Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Feb 25, 2016 at 6:26 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> One thing I elided accidentally was that when changing the second\n> argument of test_expect_success to double-quotes, you would altogether\n> drop the double-quotes around \"helper\" in:\n>\n>     git -c credential.helper=\"helper\" submodule--helper [...] &&\n>\n> to become:\n>\n>     git -c credential.helper=helper submodule--helper [...] &&\n\n\nYes I figured that :)\n\nThanks,\nJake\n"},{"id":"279484","messageId":"20160226060743.GB24517@sigill.intra.peff.net","threadId":"41520","inReplyTo":"CAPig+cRM_1JLfBiOqo+-EdKbXp--4VUbddKN_zpSCSLK8PT09w@mail.gmail.com","subject":"Re: [PATCH v3 2/2] git: submodule honor -c credential.* from command line","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-26T06:07:43Z","receivedAt":"2016-02-26T06:07:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 25, 2016 at 09:26:14PM -0500, Eric Sunshine wrote:\n\n> On Thu, Feb 25, 2016 at 9:19 PM, Jacob Keller <jacob.keller@gmail.com> wrote:\n> > On Thu, Feb 25, 2016 at 5:55 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> >> Not worth a re-roll, but these quote sequences are brain-melting.\n> >> Easier would have been to double-quote the second argument of\n> >> test_expect_success() and then do either:\n> >\n> > They are. I fiddled with things till I got it working. I wasn't sure\n> > if double quotes would cause a problem or not, since most other tests\n> > seemed to avoid it.\n> \n> You normally want variable interpolations within the test to happen\n> when the test is actually run rather than when it is defined, which is\n> why single quotes are normally used. But this test doesn't use any\n> variable interpolations, so double quotes won't hurt it.\n\nOf the two you suggested, I think I like the here-doc better, as it does\nnot leave any question to a reader that there is other interpolation\ngoing on.\n\n-Peff\n"},{"id":"279488","messageId":"CA+P7+xqUZeSCqXGB_E-P5NkQaqYEdF0vXmzQSTNo64R3GRHdoQ@mail.gmail.com","threadId":"41520","inReplyTo":"20160226060743.GB24517@sigill.intra.peff.net","subject":"Re: [PATCH v3 2/2] git: submodule honor -c credential.* from command line","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-02-26T07:32:56Z","receivedAt":"2016-02-26T07:32:56Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Feb 25, 2016 at 10:07 PM, Jeff King <peff@peff.net> wrote:\n> Of the two you suggested, I think I like the here-doc better, as it does\n> not leave any question to a reader that there is other interpolation\n> going on.\n>\n> -Peff\n\nAgreed. Since I forgot the parts about argc checking, I will do a\nreroll with the heredoc rolled in as well.\n\nThanks,\nJake\n"}]}