Skip to content

Commit 1414f48

Browse files
dschodscho
committed
Merge branch 'jk/submodule-c-credential' into js/http-custom-headers
* 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 Signed-off-by: Johannes Schindelin <[email protected]>
2 parents 64524be + c7aea25 commit 1414f48

File tree

9 files changed

+140
-27
lines changed

9 files changed

+140
-27
lines changed

‎builtin/submodule--helper.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ static int module_name(int argc, const char **argv, const char *prefix)
118118

119119
return 0;
120120
}
121+
121122
static int clone_submodule(const char *path, const char *gitdir, const char *url,
122123
const char *depth, const char *reference, int quiet)
123124
{
@@ -139,7 +140,7 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
139140
argv_array_push(&cp.args, path);
140141

141142
cp.git_cmd = 1;
142-
cp.env = local_repo_env;
143+
prepare_submodule_repo_env(&cp.env_array);
143144
cp.no_stdin = 1;
144145

145146
return run_command(&cp);
@@ -180,8 +181,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
180181

181182
const char *const git_submodule_helper_usage[] = {
182183
N_("git submodule--helper clone [--prefix=<path>] [--quiet] "
183-
"[--reference <repository>] [--name <name>] [--url <url>]"
184-
"[--depth <depth>] [--] [<path>...]"),
184+
"[--reference <repository>] [--name <name>] [--depth <depth>] "
185+
"--url <url> --path <path>"),
185186
NULL
186187
};
187188

@@ -191,6 +192,10 @@ static int module_clone(int argc, const char **argv, const char *prefix)
191192
if (!path || !*path)
192193
die(_("submodule--helper: unspecified or empty --path"));
193194

195+
if (argc || !url)
196+
usage_with_options(git_submodule_helper_usage,
197+
module_clone_options);
198+
194199
strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name);
195200
sm_gitdir = xstrdup(absolute_path(sb.buf));
196201
strbuf_reset(&sb);

‎config.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ void git_config_push_parameter(const char *text)
162162
{
163163
struct strbuf env = STRBUF_INIT;
164164
const char *old = getenv(CONFIG_DATA_ENVIRONMENT);
165-
if (old) {
165+
if (old && *old) {
166166
strbuf_addstr(&env, old);
167167
strbuf_addch(&env, ' ');
168168
}

‎git-submodule.sh

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,17 @@ isnumber()
192192
n=$(($1 + 0)) 2>/dev/null && test "$n" = "$1"
193193
}
194194

195+
# Sanitize the local git environment for use within a submodule. We
196+
# can't simply use clear_local_git_env since we want to preserve some
197+
# of the settings from GIT_CONFIG_PARAMETERS.
198+
sanitize_submodule_env()
199+
{
200+
save_config=$GIT_CONFIG_PARAMETERS
201+
clear_local_git_env
202+
GIT_CONFIG_PARAMETERS=$save_config
203+
export GIT_CONFIG_PARAMETERS
204+
}
205+
195206
#
196207
# Add a new submodule to the working tree, .gitmodules and the index
197208
#
@@ -347,9 +358,9 @@ Use -f if you really want to add it." >&2
347358
echo "$(eval_gettext "Reactivating local git directory for submodule '\$sm_name'.")"
348359
fi
349360
fi
350-
git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" "$reference" "$depth" || exit
361+
git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
351362
(
352-
clear_local_git_env
363+
sanitize_submodule_env
353364
cd "$sm_path" &&
354365
# ash fails to wordsplit ${branch:+-b "$branch"...}
355366
case "$branch" in
@@ -418,7 +429,7 @@ cmd_foreach()
418429
name=$(git submodule--helper name "$sm_path")
419430
(
420431
prefix="$prefix$sm_path/"
421-
clear_local_git_env
432+
sanitize_submodule_env
422433
cd "$sm_path" &&
423434
sm_path=$(relative_path "$sm_path") &&
424435
# we make $path available to scripts ...
@@ -601,14 +612,14 @@ cmd_deinit()
601612
}
602613

603614
is_tip_reachable () (
604-
clear_local_git_env
615+
sanitize_submodule_env &&
605616
cd "$1" &&
606617
rev=$(git rev-list -n 1 "$2" --not --all 2>/dev/null) &&
607618
test -z "$rev"
608619
)
609620

610621
fetch_in_submodule () (
611-
clear_local_git_env
622+
sanitize_submodule_env &&
612623
cd "$1" &&
613624
case "$2" in
614625
'')
@@ -736,11 +747,11 @@ Maybe you want to use 'update --init'?")"
736747

737748
if ! test -d "$sm_path"/.git && ! test -f "$sm_path"/.git
738749
then
739-
git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$prefix" --path "$sm_path" --name "$name" --url "$url" "$reference" "$depth" || exit
750+
git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$prefix" --path "$sm_path" --name "$name" --url "$url" ${reference:+"$reference"} ${depth:+"$depth"} || exit
740751
cloned_modules="$cloned_modules;$name"
741752
subsha1=
742753
else
743-
subsha1=$(clear_local_git_env; cd "$sm_path" &&
754+
subsha1=$(sanitize_submodule_env; cd "$sm_path" &&
744755
git rev-parse --verify HEAD) ||
745756
die "$(eval_gettext "Unable to find current revision in submodule path '\$displaypath'")"
746757
fi
@@ -750,11 +761,11 @@ Maybe you want to use 'update --init'?")"
750761
if test -z "$nofetch"
751762
then
752763
# Fetch remote before determining tracking $sha1
753-
(clear_local_git_env; cd "$sm_path" && git-fetch) ||
764+
(sanitize_submodule_env; cd "$sm_path" && git-fetch) ||
754765
die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
755766
fi
756-
remote_name=$(clear_local_git_env; cd "$sm_path" && get_default_remote)
757-
sha1=$(clear_local_git_env; cd "$sm_path" &&
767+
remote_name=$(sanitize_submodule_env; cd "$sm_path" && get_default_remote)
768+
sha1=$(sanitize_submodule_env; cd "$sm_path" &&
758769
git rev-parse --verify "${remote_name}/${branch}") ||
759770
die "$(eval_gettext "Unable to find current ${remote_name}/${branch} revision in submodule path '\$sm_path'")"
760771
fi
@@ -819,7 +830,7 @@ Maybe you want to use 'update --init'?")"
819830
die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")"
820831
esac
821832

822-
if (clear_local_git_env; cd "$sm_path" && $command "$sha1")
833+
if (sanitize_submodule_env; cd "$sm_path" && $command "$sha1")
823834
then
824835
say "$say_msg"
825836
elif test -n "$must_die_on_failure"
@@ -835,7 +846,7 @@ Maybe you want to use 'update --init'?")"
835846
then
836847
(
837848
prefix="$prefix$sm_path/"
838-
clear_local_git_env
849+
sanitize_submodule_env
839850
cd "$sm_path" &&
840851
eval cmd_update
841852
)
@@ -873,7 +884,7 @@ Maybe you want to use 'update --init'?")"
873884

874885
set_name_rev () {
875886
revname=$( (
876-
clear_local_git_env
887+
sanitize_submodule_env
877888
cd "$1" && {
878889
git describe "$2" 2>/dev/null ||
879890
git describe --tags "$2" 2>/dev/null ||
@@ -1157,7 +1168,7 @@ cmd_status()
11571168
else
11581169
if test -z "$cached"
11591170
then
1160-
sha1=$(clear_local_git_env; cd "$sm_path" && git rev-parse --verify HEAD)
1171+
sha1=$(sanitize_submodule_env; cd "$sm_path" && git rev-parse --verify HEAD)
11611172
fi
11621173
set_name_rev "$sm_path" "$sha1"
11631174
say "+$sha1 $displaypath$revname"
@@ -1167,7 +1178,7 @@ cmd_status()
11671178
then
11681179
(
11691180
prefix="$displaypath/"
1170-
clear_local_git_env
1181+
sanitize_submodule_env
11711182
wt_prefix=
11721183
cd "$sm_path" &&
11731184
eval cmd_status
@@ -1242,7 +1253,7 @@ cmd_sync()
12421253
if test -e "$sm_path"/.git
12431254
then
12441255
(
1245-
clear_local_git_env
1256+
sanitize_submodule_env
12461257
cd "$sm_path"
12471258
remote=$(get_default_remote)
12481259
git config remote."$remote".url "$sub_origin_url"

‎quote.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,19 @@ void sq_quote_buf(struct strbuf *dst, const char *src)
4343
free(to_free);
4444
}
4545

46+
void sq_quotef(struct strbuf *dst, const char *fmt, ...)
47+
{
48+
struct strbuf src = STRBUF_INIT;
49+
50+
va_list ap;
51+
va_start(ap, fmt);
52+
strbuf_vaddf(&src, fmt, ap);
53+
va_end(ap);
54+
55+
sq_quote_buf(dst, src.buf);
56+
strbuf_release(&src);
57+
}
58+
4659
void sq_quote_argv(struct strbuf *dst, const char** argv, size_t maxlen)
4760
{
4861
int i;

‎quote.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,13 @@ struct strbuf;
2525
* sq_quote_buf() writes to an existing buffer of specified size; it
2626
* will return the number of characters that would have been written
2727
* excluding the final null regardless of the buffer size.
28+
*
29+
* sq_quotef() quotes the entire formatted string as a single result.
2830
*/
2931

3032
extern void sq_quote_buf(struct strbuf *, const char *src);
3133
extern void sq_quote_argv(struct strbuf *, const char **argv, size_t maxlen);
34+
extern void sq_quotef(struct strbuf *, const char *fmt, ...);
3235

3336
/* This unwraps what sq_quote() produces in place, but returns
3437
* NULL if the input does not look like what sq_quote would have

‎submodule.c

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "argv-array.h"
1414
#include "blob.h"
1515
#include "thread-utils.h"
16+
#include "quote.h"
1617

1718
static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
1819
static struct string_list changed_submodule_paths;
@@ -366,7 +367,7 @@ static int submodule_needs_pushing(const char *path, const unsigned char sha1[20
366367

367368
argv[1] = sha1_to_hex(sha1);
368369
cp.argv = argv;
369-
cp.env = local_repo_env;
370+
prepare_submodule_repo_env(&cp.env_array);
370371
cp.git_cmd = 1;
371372
cp.no_stdin = 1;
372373
cp.out = -1;
@@ -453,7 +454,7 @@ static int push_submodule(const char *path)
453454
const char *argv[] = {"push", NULL};
454455

455456
cp.argv = argv;
456-
cp.env = local_repo_env;
457+
prepare_submodule_repo_env(&cp.env_array);
457458
cp.git_cmd = 1;
458459
cp.no_stdin = 1;
459460
cp.dir = path;
@@ -499,7 +500,7 @@ static int is_submodule_commit_present(const char *path, unsigned char sha1[20])
499500

500501
argv[3] = sha1_to_hex(sha1);
501502
cp.argv = argv;
502-
cp.env = local_repo_env;
503+
prepare_submodule_repo_env(&cp.env_array);
503504
cp.git_cmd = 1;
504505
cp.no_stdin = 1;
505506
cp.dir = path;
@@ -682,7 +683,7 @@ static int get_next_submodule(struct child_process *cp,
682683
if (is_directory(git_dir)) {
683684
child_process_init(cp);
684685
cp->dir = strbuf_detach(&submodule_path, NULL);
685-
cp->env = local_repo_env;
686+
prepare_submodule_repo_env(&cp->env_array);
686687
cp->git_cmd = 1;
687688
if (!spf->quiet)
688689
strbuf_addf(err, "Fetching submodule %s%s\n",
@@ -794,7 +795,7 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
794795
argv[2] = "-uno";
795796

796797
cp.argv = argv;
797-
cp.env = local_repo_env;
798+
prepare_submodule_repo_env(&cp.env_array);
798799
cp.git_cmd = 1;
799800
cp.no_stdin = 1;
800801
cp.out = -1;
@@ -855,7 +856,7 @@ int submodule_uses_gitfile(const char *path)
855856

856857
/* Now test that all nested submodules use a gitfile too */
857858
cp.argv = argv;
858-
cp.env = local_repo_env;
859+
prepare_submodule_repo_env(&cp.env_array);
859860
cp.git_cmd = 1;
860861
cp.no_stdin = 1;
861862
cp.no_stderr = 1;
@@ -888,7 +889,7 @@ int ok_to_remove_submodule(const char *path)
888889
return 0;
889890

890891
cp.argv = argv;
891-
cp.env = local_repo_env;
892+
prepare_submodule_repo_env(&cp.env_array);
892893
cp.git_cmd = 1;
893894
cp.no_stdin = 1;
894895
cp.out = -1;
@@ -1094,3 +1095,13 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
10941095
strbuf_release(&rel_path);
10951096
free((void *)real_work_tree);
10961097
}
1098+
1099+
void prepare_submodule_repo_env(struct argv_array *out)
1100+
{
1101+
const char * const *var;
1102+
1103+
for (var = local_repo_env; *var; var++) {
1104+
if (strcmp(*var, CONFIG_DATA_ENVIRONMENT))
1105+
argv_array_push(out, *var);
1106+
}
1107+
}

‎submodule.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,11 @@ int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam
4343
int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);
4444
void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
4545

46+
/*
47+
* Prepare the "env_array" parameter of a "struct child_process" for executing
48+
* a submodule by clearing any repo-specific envirionment variables, but
49+
* retaining any config in the environment.
50+
*/
51+
void prepare_submodule_repo_env(struct argv_array *out);
52+
4653
#endif

‎t/t1300-repo-config.sh

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,6 +1087,20 @@ test_expect_success 'git -c complains about empty key and value' '
10871087
test_must_fail git -c "" rev-parse
10881088
'
10891089

1090+
test_expect_success 'multiple git -c appends config' '
1091+
test_config alias.x "!git -c x.two=2 config --get-regexp ^x\.*" &&
1092+
cat >expect <<-\EOF &&
1093+
x.one 1
1094+
x.two 2
1095+
EOF
1096+
git -c x.one=1 x >actual &&
1097+
test_cmp expect actual
1098+
'
1099+
1100+
test_expect_success 'git -c is not confused by empty environment' '
1101+
GIT_CONFIG_PARAMETERS="" git -c x.one=1 config --list
1102+
'
1103+
10901104
test_expect_success 'git config --edit works' '
10911105
git config -f tmp test.value no &&
10921106
echo test.value=yes >expect &&

‎t/t5550-http-fetch-dumb.sh

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,55 @@ test_expect_success 'configured username does not override URL' '
9191
expect_askpass pass user@host
9292
'
9393

94+
test_expect_success 'set up repo with http submodules' '
95+
git init super &&
96+
set_askpass user@host pass@host &&
97+
(
98+
cd super &&
99+
git submodule add "$HTTPD_URL/auth/dumb/repo.git" sub &&
100+
git commit -m "add submodule"
101+
)
102+
'
103+
104+
test_expect_success 'cmdline credential config passes to submodule via clone' '
105+
set_askpass wrong pass@host &&
106+
test_must_fail git clone --recursive super super-clone &&
107+
rm -rf super-clone &&
108+
109+
set_askpass wrong pass@host &&
110+
git -c "credential.$HTTPD_URL.username=user@host" \
111+
clone --recursive super super-clone &&
112+
expect_askpass pass user@host
113+
'
114+
115+
test_expect_success 'cmdline credential config passes submodule via fetch' '
116+
set_askpass wrong pass@host &&
117+
test_must_fail git -C super-clone fetch --recurse-submodules &&
118+
119+
set_askpass wrong pass@host &&
120+
git -C super-clone \
121+
-c "credential.$HTTPD_URL.username=user@host" \
122+
fetch --recurse-submodules &&
123+
expect_askpass pass user@host
124+
'
125+
126+
test_expect_success 'cmdline credential config passes submodule update' '
127+
# advance the submodule HEAD so that a fetch is required
128+
git commit --allow-empty -m foo &&
129+
git push "$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/repo.git" HEAD &&
130+
sha1=$(git rev-parse HEAD) &&
131+
git -C super-clone update-index --cacheinfo 160000,$sha1,sub &&
132+
133+
set_askpass wrong pass@host &&
134+
test_must_fail git -C super-clone submodule update &&
135+
136+
set_askpass wrong pass@host &&
137+
git -C super-clone \
138+
-c "credential.$HTTPD_URL.username=user@host" \
139+
submodule update &&
140+
expect_askpass pass user@host
141+
'
142+
94143
test_expect_success 'fetch changes via http' '
95144
echo content >>file &&
96145
git commit -a -m two &&

0 commit comments

Comments
 (0)