Merge branch 'jk/submodule-c-credential'

An earlier addition of "sanitize_submodule_env" with 14111fc4 (git:
submodule honor -c credential.* from command line, 2016-02-29)
turned out to be a convoluted no-op; implement what it wanted to do
correctly, and stop filtering settings given via "git -c var=val".

* jk/submodule-c-credential:
submodule: stop sanitizing config options
submodule: use prepare_submodule_repo_env consistently
submodule--helper: move config-sanitizing to submodule.c
submodule: export sanitized GIT_CONFIG_PARAMETERS
t5550: break submodule config test into multiple sub-tests
t5550: fix typo in $HTTPD_URL

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 825a421fc928e108fd9b2972c89383533bc0d978..8da263f0b0b086a74e23c2a5854bfa66623f70df 100644 (file)
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -443,54 +443,6 @@ static int module_name(int argc, const char **argv, const char *prefix)
        return 0;
 }
 
-/*
- * Rules to sanitize configuration variables that are Ok to be passed into
- * submodule operations from the parent project using "-c". Should only
- * include keys which are both (a) safe and (b) necessary for proper
- * operation.
- */
-static int submodule_config_ok(const char *var)
-{
-       if (starts_with(var, "credential."))
-               return 1;
-       return 0;
-}
-
-static int sanitize_submodule_config(const char *var, const char *value, void *data)
-{
-       struct strbuf *out = data;
-
-       if (submodule_config_ok(var)) {
-               if (out->len)
-                       strbuf_addch(out, ' ');
-
-               if (value)
-                       sq_quotef(out, "%s=%s", var, value);
-               else
-                       sq_quote_buf(out, var);
-       }
-
-       return 0;
-}
-
-static void prepare_submodule_repo_env(struct argv_array *out)
-{
-       const char * const *var;
-
-       for (var = local_repo_env; *var; var++) {
-               if (!strcmp(*var, CONFIG_DATA_ENVIRONMENT)) {
-                       struct strbuf sanitized_config = STRBUF_INIT;
-                       git_config_from_parameters(sanitize_submodule_config,
-                                                  &sanitized_config);
-                       argv_array_pushf(out, "%s=%s", *var, sanitized_config.buf);
-                       strbuf_release(&sanitized_config);
-               } else {
-                       argv_array_push(out, *var);
-               }
-       }
-
-}
-
 static int clone_submodule(const char *path, const char *gitdir, const char *url,
                           const char *depth, const char *reference, int quiet)
 {
@@ -618,22 +570,6 @@ static int module_clone(int argc, const char **argv, const char *prefix)
        return 0;
 }
 
-static int module_sanitize_config(int argc, const char **argv, const char *prefix)
-{
-       struct strbuf sanitized_config = STRBUF_INIT;
-
-       if (argc > 1)
-               usage(_("git submodule--helper sanitize-config"));
-
-       git_config_from_parameters(sanitize_submodule_config, &sanitized_config);
-       if (sanitized_config.len)
-               printf("%s\n", sanitized_config.buf);
-
-       strbuf_release(&sanitized_config);
-
-       return 0;
-}
-
 struct submodule_update_clone {
        /* index into 'list', the list of submodules to look into for cloning */
        int current;
@@ -906,7 +842,6 @@ static struct cmd_struct commands[] = {
        {"list", module_list},
        {"name", module_name},
        {"clone", module_clone},
-       {"sanitize-config", module_sanitize_config},
        {"update-clone", update_clone},
        {"resolve-relative-url", resolve_relative_url},
        {"resolve-relative-url-test", resolve_relative_url_test},

diff --git a/git-submodule.sh b/git-submodule.sh
index 72fa3912831e3e981012fca5f4d6a3309ac7606b..5a4dec050b2ebdbe8f9674666ddaabb4c0e193f2 100755 (executable)
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -124,9 +124,10 @@ 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
 }
 
 #

diff --git a/submodule.c b/submodule.c
index 4cc1c27931ee820e902e31e53112ef047ec821c0..4532b11d66f8cdc0abdf003b3a16a632bd923890 100644 (file)
--- a/submodule.c
+++ b/submodule.c
@@ -13,6 +13,7 @@
 #include "argv-array.h"
 #include "blob.h"
 #include "thread-utils.h"
+#include "quote.h"
 
 static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
 static int parallel_jobs = 1;
@@ -414,7 +415,7 @@ static int submodule_needs_pushing(const char *path, const unsigned char sha1[20
 
                argv[1] = sha1_to_hex(sha1);
                cp.argv = argv;
-               cp.env = local_repo_env;
+               prepare_submodule_repo_env(&cp.env_array);
                cp.git_cmd = 1;
                cp.no_stdin = 1;
                cp.out = -1;
@@ -501,7 +502,7 @@ static int push_submodule(const char *path)
                const char *argv[] = {"push", NULL};
 
                cp.argv = argv;
-               cp.env = local_repo_env;
+               prepare_submodule_repo_env(&cp.env_array);
                cp.git_cmd = 1;
                cp.no_stdin = 1;
                cp.dir = path;
@@ -547,7 +548,7 @@ static int is_submodule_commit_present(const char *path, unsigned char sha1[20])
 
                argv[3] = sha1_to_hex(sha1);
                cp.argv = argv;
-               cp.env = local_repo_env;
+               prepare_submodule_repo_env(&cp.env_array);
                cp.git_cmd = 1;
                cp.no_stdin = 1;
                cp.dir = path;
@@ -730,7 +731,7 @@ static int get_next_submodule(struct child_process *cp,
                if (is_directory(git_dir)) {
                        child_process_init(cp);
                        cp->dir = strbuf_detach(&submodule_path, NULL);
-                       cp->env = local_repo_env;
+                       prepare_submodule_repo_env(&cp->env_array);
                        cp->git_cmd = 1;
                        if (!spf->quiet)
                                strbuf_addf(err, "Fetching submodule %s%s\n",
@@ -845,7 +846,7 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
                argv[2] = "-uno";
 
        cp.argv = argv;
-       cp.env = local_repo_env;
+       prepare_submodule_repo_env(&cp.env_array);
        cp.git_cmd = 1;
        cp.no_stdin = 1;
        cp.out = -1;
@@ -906,7 +907,7 @@ int submodule_uses_gitfile(const char *path)
 
        /* Now test that all nested submodules use a gitfile too */
        cp.argv = argv;
-       cp.env = local_repo_env;
+       prepare_submodule_repo_env(&cp.env_array);
        cp.git_cmd = 1;
        cp.no_stdin = 1;
        cp.no_stderr = 1;
@@ -939,7 +940,7 @@ int ok_to_remove_submodule(const char *path)
                return 0;
 
        cp.argv = argv;
-       cp.env = local_repo_env;
+       prepare_submodule_repo_env(&cp.env_array);
        cp.git_cmd = 1;
        cp.no_stdin = 1;
        cp.out = -1;
@@ -1150,3 +1151,13 @@ int parallel_submodules(void)
 {
        return parallel_jobs;
 }
+
+void prepare_submodule_repo_env(struct argv_array *out)
+{
+       const char * const *var;
+
+       for (var = local_repo_env; *var; var++) {
+               if (strcmp(*var, CONFIG_DATA_ENVIRONMENT))
+                       argv_array_push(out, *var);
+       }
+}

diff --git a/submodule.h b/submodule.h
index ff4c4f33a5584d66e50f19739be4ca5055874c6c..2af9390998194c7d51a3e23b14723574d6696926 100644 (file)
--- a/submodule.h
+++ b/submodule.h
@@ -62,4 +62,11 @@ int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam
 void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
 int parallel_submodules(void);
 
+/*
+ * Prepare the "env_array" parameter of a "struct child_process" for executing
+ * a submodule by clearing any repo-specific envirionment variables, but
+ * retaining any config in the environment.
+ */
+void prepare_submodule_repo_env(struct argv_array *out);
+
 #endif

diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index 48e2ab62da7a4293dd647524f75f3590783bcb02..3484b6f0f3cf04a9cf831a5d91c31486efebcc38 100755 (executable)
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -91,23 +91,55 @@ test_expect_success 'configured username does not override URL' '
        expect_askpass pass user@host
 '
 
-test_expect_success 'cmdline credential config passes into submodules' '
+test_expect_success 'set up repo with http submodules' '
        git init super &&
        set_askpass user@host pass@host &&
        (
                cd super &&
                git submodule add "$HTTPD_URL/auth/dumb/repo.git" sub &&
                git commit -m "add submodule"
-       ) &&
+       )
+'
+
+test_expect_success 'cmdline credential config passes to submodule via clone' '
        set_askpass wrong pass@host &&
        test_must_fail git clone --recursive super super-clone &&
        rm -rf super-clone &&
+
        set_askpass wrong pass@host &&
-       git -c "credential.$HTTP_URL.username=user@host" \
+       git -c "credential.$HTTPD_URL.username=user@host" \
                clone --recursive super super-clone &&
        expect_askpass pass user@host
 '
 
+test_expect_success 'cmdline credential config passes submodule via fetch' '
+       set_askpass wrong pass@host &&
+       test_must_fail git -C super-clone fetch --recurse-submodules &&
+
+       set_askpass wrong pass@host &&
+       git -C super-clone \
+           -c "credential.$HTTPD_URL.username=user@host" \
+           fetch --recurse-submodules &&
+       expect_askpass pass user@host
+'
+
+test_expect_success 'cmdline credential config passes submodule update' '
+       # advance the submodule HEAD so that a fetch is required
+       git commit --allow-empty -m foo &&
+       git push "$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/repo.git" HEAD &&
+       sha1=$(git rev-parse HEAD) &&
+       git -C super-clone update-index --cacheinfo 160000,$sha1,sub &&
+
+       set_askpass wrong pass@host &&
+       test_must_fail git -C super-clone submodule update &&
+
+       set_askpass wrong pass@host &&
+       git -C super-clone \
+           -c "credential.$HTTPD_URL.username=user@host" \
+           submodule update &&
+       expect_askpass pass user@host
+'
+
 test_expect_success 'fetch changes via http' '
        echo content >>file &&
        git commit -a -m two &&

diff --git a/t/t7412-submodule--helper.sh b/t/t7412-submodule--helper.sh
deleted file mode 100755 (executable)
index 149d428..0000000
--- a/t/t7412-submodule--helper.sh
+++ /dev/null
@@ -1,26 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2016 Jacob Keller
-#
-
-test_description='Basic plumbing support of submodule--helper
-
-This test verifies the submodule--helper plumbing command used to implement
-git-submodule.
-'
-
-. ./test-lib.sh
-
-test_expect_success 'sanitize-config clears configuration' '
-       git -c user.name="Some User" submodule--helper sanitize-config >actual &&
-       test_must_be_empty actual
-'
-
-sq="'"
-test_expect_success 'sanitize-config keeps credential.helper' '
-       git -c credential.helper=helper submodule--helper sanitize-config >actual &&
-       echo "${sq}credential.helper=helper${sq}" >expect &&
-       test_cmp expect actual
-'
-
-test_done