Merge branch 'jk/submodule-c-credential' into js/http-custom-headers
authorJunio C Hamano <gitster@pobox.com>
Tue, 10 May 2016 17:38:31 +0000 (10:38 -0700)
committerJunio C Hamano <gitster@pobox.com>
Tue, 10 May 2016 17:38:31 +0000 (10:38 -0700)
* 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
git_config_push_parameter: handle empty GIT_CONFIG_PARAMETERS
git: submodule honor -c credential.* from command line
quote: implement sq_quotef()
submodule: fix segmentation fault in submodule--helper clone
submodule: fix submodule--helper clone usage
submodule: check argc count for git submodule--helper clone
submodule: don't pass empty string arguments to submodule--helper clone

builtin/submodule--helper.c
config.c
git-submodule.sh
quote.c
quote.h
submodule.c
submodule.h
t/t1300-repo-config.sh
t/t5550-http-fetch-dumb.sh
index 5295b727d4609fa33a08c31b2d051ce3f98244fb..45c97e6ae12dbd37ed4a7910f934ad9c2a40e540 100644 (file)
@@ -118,6 +118,7 @@ static int module_name(int argc, const char **argv, const char *prefix)
 
        return 0;
 }
+
 static int clone_submodule(const char *path, const char *gitdir, const char *url,
                           const char *depth, const char *reference, int quiet)
 {
@@ -139,7 +140,7 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
        argv_array_push(&cp.args, path);
 
        cp.git_cmd = 1;
-       cp.env = local_repo_env;
+       prepare_submodule_repo_env(&cp.env_array);
        cp.no_stdin = 1;
 
        return run_command(&cp);
@@ -180,14 +181,18 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 
        const char *const git_submodule_helper_usage[] = {
                N_("git submodule--helper clone [--prefix=<path>] [--quiet] "
-                  "[--reference <repository>] [--name <name>] [--url <url>]"
-                  "[--depth <depth>] [--] [<path>...]"),
+                  "[--reference <repository>] [--name <name>] [--depth <depth>] "
+                  "--url <url> --path <path>"),
                NULL
        };
 
        argc = parse_options(argc, argv, prefix, module_clone_options,
                             git_submodule_helper_usage, 0);
 
+       if (argc || !url || !path)
+               usage_with_options(git_submodule_helper_usage,
+                                  module_clone_options);
+
        strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name);
        sm_gitdir = strbuf_detach(&sb, NULL);
 
index 9ba40bc1b039b9b65425dc4fa1bd9c7f1fcb0868..8f66519e616ad9ebb6ee2540028cd2f04ce46103 100644 (file)
--- a/config.c
+++ b/config.c
@@ -162,7 +162,7 @@ void git_config_push_parameter(const char *text)
 {
        struct strbuf env = STRBUF_INIT;
        const char *old = getenv(CONFIG_DATA_ENVIRONMENT);
-       if (old) {
+       if (old && *old) {
                strbuf_addstr(&env, old);
                strbuf_addch(&env, ' ');
        }
index 43c68deee9db2b243d39eba704878eb7551ada2d..b9469e7db6ed3bb4844727897325f1dd5380303f 100755 (executable)
@@ -192,6 +192,17 @@ isnumber()
        n=$(($1 + 0)) 2>/dev/null && test "$n" = "$1"
 }
 
+# Sanitize the local git environment for use within a submodule. We
+# can't simply use clear_local_git_env since we want to preserve some
+# of the settings from GIT_CONFIG_PARAMETERS.
+sanitize_submodule_env()
+{
+       save_config=$GIT_CONFIG_PARAMETERS
+       clear_local_git_env
+       GIT_CONFIG_PARAMETERS=$save_config
+       export GIT_CONFIG_PARAMETERS
+}
+
 #
 # Add a new submodule to the working tree, .gitmodules and the index
 #
@@ -347,9 +358,9 @@ Use -f if you really want to add it." >&2
                                echo "$(eval_gettext "Reactivating local git directory for submodule '\$sm_name'.")"
                        fi
                fi
-               git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" "$reference" "$depth" || exit
+               git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
                (
-                       clear_local_git_env
+                       sanitize_submodule_env
                        cd "$sm_path" &&
                        # ash fails to wordsplit ${branch:+-b "$branch"...}
                        case "$branch" in
@@ -418,7 +429,7 @@ cmd_foreach()
                        name=$(git submodule--helper name "$sm_path")
                        (
                                prefix="$prefix$sm_path/"
-                               clear_local_git_env
+                               sanitize_submodule_env
                                cd "$sm_path" &&
                                sm_path=$(relative_path "$sm_path") &&
                                # we make $path available to scripts ...
@@ -592,14 +603,14 @@ cmd_deinit()
 }
 
 is_tip_reachable () (
-       clear_local_git_env
+       sanitize_submodule_env &&
        cd "$1" &&
        rev=$(git rev-list -n 1 "$2" --not --all 2>/dev/null) &&
        test -z "$rev"
 )
 
 fetch_in_submodule () (
-       clear_local_git_env
+       sanitize_submodule_env &&
        cd "$1" &&
        case "$2" in
        '')
@@ -727,11 +738,11 @@ Maybe you want to use 'update --init'?")"
 
                if ! test -d "$sm_path"/.git && ! test -f "$sm_path"/.git
                then
-                       git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$prefix" --path "$sm_path" --name "$name" --url "$url" "$reference" "$depth" || exit
+                       git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$prefix" --path "$sm_path" --name "$name" --url "$url" ${reference:+"$reference"} ${depth:+"$depth"} || exit
                        cloned_modules="$cloned_modules;$name"
                        subsha1=
                else
-                       subsha1=$(clear_local_git_env; cd "$sm_path" &&
+                       subsha1=$(sanitize_submodule_env; cd "$sm_path" &&
                                git rev-parse --verify HEAD) ||
                        die "$(eval_gettext "Unable to find current revision in submodule path '\$displaypath'")"
                fi
@@ -741,11 +752,11 @@ Maybe you want to use 'update --init'?")"
                        if test -z "$nofetch"
                        then
                                # Fetch remote before determining tracking $sha1
-                               (clear_local_git_env; cd "$sm_path" && git-fetch) ||
+                               (sanitize_submodule_env; cd "$sm_path" && git-fetch) ||
                                die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
                        fi
-                       remote_name=$(clear_local_git_env; cd "$sm_path" && get_default_remote)
-                       sha1=$(clear_local_git_env; cd "$sm_path" &&
+                       remote_name=$(sanitize_submodule_env; cd "$sm_path" && get_default_remote)
+                       sha1=$(sanitize_submodule_env; cd "$sm_path" &&
                                git rev-parse --verify "${remote_name}/${branch}") ||
                        die "$(eval_gettext "Unable to find current ${remote_name}/${branch} revision in submodule path '\$sm_path'")"
                fi
@@ -810,7 +821,7 @@ Maybe you want to use 'update --init'?")"
                                die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")"
                        esac
 
-                       if (clear_local_git_env; cd "$sm_path" && $command "$sha1")
+                       if (sanitize_submodule_env; cd "$sm_path" && $command "$sha1")
                        then
                                say "$say_msg"
                        elif test -n "$must_die_on_failure"
@@ -826,7 +837,7 @@ Maybe you want to use 'update --init'?")"
                then
                        (
                                prefix="$prefix$sm_path/"
-                               clear_local_git_env
+                               sanitize_submodule_env
                                cd "$sm_path" &&
                                eval cmd_update
                        )
@@ -864,7 +875,7 @@ Maybe you want to use 'update --init'?")"
 
 set_name_rev () {
        revname=$( (
-               clear_local_git_env
+               sanitize_submodule_env
                cd "$1" && {
                        git describe "$2" 2>/dev/null ||
                        git describe --tags "$2" 2>/dev/null ||
@@ -1148,7 +1159,7 @@ cmd_status()
                else
                        if test -z "$cached"
                        then
-                               sha1=$(clear_local_git_env; cd "$sm_path" && git rev-parse --verify HEAD)
+                               sha1=$(sanitize_submodule_env; cd "$sm_path" && git rev-parse --verify HEAD)
                        fi
                        set_name_rev "$sm_path" "$sha1"
                        say "+$sha1 $displaypath$revname"
@@ -1158,7 +1169,7 @@ cmd_status()
                then
                        (
                                prefix="$displaypath/"
-                               clear_local_git_env
+                               sanitize_submodule_env
                                cd "$sm_path" &&
                                eval cmd_status
                        ) ||
@@ -1232,7 +1243,7 @@ cmd_sync()
                        if test -e "$sm_path"/.git
                        then
                        (
-                               clear_local_git_env
+                               sanitize_submodule_env
                                cd "$sm_path"
                                remote=$(get_default_remote)
                                git config remote."$remote".url "$sub_origin_url"
diff --git a/quote.c b/quote.c
index fe884d24521f91a0f1f30b3cf5f08734bab7bd31..b281a8fe454e39728ce112915c7a5efb0f9c8bc0 100644 (file)
--- a/quote.c
+++ b/quote.c
@@ -43,6 +43,19 @@ void sq_quote_buf(struct strbuf *dst, const char *src)
        free(to_free);
 }
 
+void sq_quotef(struct strbuf *dst, const char *fmt, ...)
+{
+       struct strbuf src = STRBUF_INIT;
+
+       va_list ap;
+       va_start(ap, fmt);
+       strbuf_vaddf(&src, fmt, ap);
+       va_end(ap);
+
+       sq_quote_buf(dst, src.buf);
+       strbuf_release(&src);
+}
+
 void sq_quote_argv(struct strbuf *dst, const char** argv, size_t maxlen)
 {
        int i;
diff --git a/quote.h b/quote.h
index 99e04d34bf223a0147e65623322ef2c2fb01e01d..6c53a2cc66c44aaa12b787a5c16f739449753388 100644 (file)
--- a/quote.h
+++ b/quote.h
@@ -25,10 +25,13 @@ struct strbuf;
  * sq_quote_buf() writes to an existing buffer of specified size; it
  * will return the number of characters that would have been written
  * excluding the final null regardless of the buffer size.
+ *
+ * sq_quotef() quotes the entire formatted string as a single result.
  */
 
 extern void sq_quote_buf(struct strbuf *, const char *src);
 extern void sq_quote_argv(struct strbuf *, const char **argv, size_t maxlen);
+extern void sq_quotef(struct strbuf *, const char *fmt, ...);
 
 /* This unwraps what sq_quote() produces in place, but returns
  * NULL if the input does not look like what sq_quote would have
index 62c4356c50d4a41381336559f4e9af27e520ad0d..4b02da6987e12406d0825ee86c0313cb20da2420 100644 (file)
@@ -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 struct string_list changed_submodule_paths;
@@ -366,7 +367,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;
@@ -453,7 +454,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;
@@ -499,7 +500,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;
@@ -682,7 +683,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",
@@ -794,7 +795,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;
@@ -855,7 +856,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;
@@ -888,7 +889,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;
@@ -1094,3 +1095,13 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
        strbuf_release(&rel_path);
        free((void *)real_work_tree);
 }
+
+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);
+       }
+}
index e06eaa5ebb30e825fd0721c76e7d194b0b854706..869d259fac5ecc74532549c57add0b692a995728 100644 (file)
@@ -43,4 +43,11 @@ int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam
 int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);
 void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
 
+/*
+ * 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
index 6767da87cba9589222b033c80b605b832f708baf..1867a0fd419769195399864aa2ca7436c12005cd 100755 (executable)
@@ -1087,6 +1087,20 @@ test_expect_success 'git -c complains about empty key and value' '
        test_must_fail git -c "" rev-parse
 '
 
+test_expect_success 'multiple git -c appends config' '
+       test_config alias.x "!git -c x.two=2 config --get-regexp ^x\.*" &&
+       cat >expect <<-\EOF &&
+       x.one 1
+       x.two 2
+       EOF
+       git -c x.one=1 x >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success 'git -c is not confused by empty environment' '
+       GIT_CONFIG_PARAMETERS="" git -c x.one=1 config --list
+'
+
 test_expect_success 'git config --edit works' '
        git config -f tmp test.value no &&
        echo test.value=yes >expect &&
index 64146352ae20e9a5abdcda77be514d0f3264c3b8..3484b6f0f3cf04a9cf831a5d91c31486efebcc38 100755 (executable)
@@ -91,6 +91,55 @@ test_expect_success 'configured username does not override URL' '
        expect_askpass pass user@host
 '
 
+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.$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 &&