Skip to content

Commit b32f5b6

Browse files
gitstergitster
committed
Merge branch 'js/submodule-fix-misuse-of-path-and-name'
In .gitmodules files, submodules are keyed by their names, and the path to the submodule whose name is $name is specified by the submodule.$name.path variable. There were a few codepaths that mixed the name and path up when consulting the submodule database, which have been corrected. It took long for these bugs to be found as the name of a submodule initially is the same as its path, and the problem does not surface until it is moved to a different path, which apparently happens very rarely. * js/submodule-fix-misuse-of-path-and-name: t7420: test that we correctly handle renamed submodules t7419: test that we correctly handle renamed submodules t7419, t7420: use test_cmp_config instead of grepping .gitmodules t7419: actually test the branch switching submodule--helper: return error from set-url when modifying failed submodule--helper: use submodule_from_path in set-{url,branch}
2 parents a45edde + bd1c20c commit b32f5b6

File tree

3 files changed

+90
-22
lines changed

3 files changed

+90
-22
lines changed

‎builtin/submodule--helper.c

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2889,7 +2889,7 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
28892889

28902890
static int module_set_url(int argc, const char **argv, const char *prefix)
28912891
{
2892-
int quiet = 0;
2892+
int quiet = 0, ret;
28932893
const char *newurl;
28942894
const char *path;
28952895
char *config_name;
@@ -2901,20 +2901,29 @@ static int module_set_url(int argc, const char **argv, const char *prefix)
29012901
N_("git submodule set-url [--quiet] <path> <newurl>"),
29022902
NULL
29032903
};
2904+
const struct submodule *sub;
29042905

29052906
argc = parse_options(argc, argv, prefix, options, usage, 0);
29062907

29072908
if (argc != 2 || !(path = argv[0]) || !(newurl = argv[1]))
29082909
usage_with_options(usage, options);
29092910

2910-
config_name = xstrfmt("submodule.%s.url", path);
2911+
sub = submodule_from_path(the_repository, null_oid(), path);
29112912

2912-
config_set_in_gitmodules_file_gently(config_name, newurl);
2913-
sync_submodule(path, prefix, NULL, quiet ? OPT_QUIET : 0);
2913+
if (!sub)
2914+
die(_("no submodule mapping found in .gitmodules for path '%s'"),
2915+
path);
29142916

2915-
free(config_name);
2917+
config_name = xstrfmt("submodule.%s.url", sub->name);
2918+
ret = config_set_in_gitmodules_file_gently(config_name, newurl);
29162919

2917-
return 0;
2920+
if (!ret) {
2921+
repo_read_gitmodules(the_repository, 0);
2922+
sync_submodule(sub->path, prefix, NULL, quiet ? OPT_QUIET : 0);
2923+
}
2924+
2925+
free(config_name);
2926+
return !!ret;
29182927
}
29192928

29202929
static int module_set_branch(int argc, const char **argv, const char *prefix)
@@ -2941,6 +2950,7 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)
29412950
N_("git submodule set-branch [-q|--quiet] (-b|--branch) <branch> <path>"),
29422951
NULL
29432952
};
2953+
const struct submodule *sub;
29442954

29452955
argc = parse_options(argc, argv, prefix, options, usage, 0);
29462956

@@ -2953,7 +2963,13 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)
29532963
if (argc != 1 || !(path = argv[0]))
29542964
usage_with_options(usage, options);
29552965

2956-
config_name = xstrfmt("submodule.%s.branch", path);
2966+
sub = submodule_from_path(the_repository, null_oid(), path);
2967+
2968+
if (!sub)
2969+
die(_("no submodule mapping found in .gitmodules for path '%s'"),
2970+
path);
2971+
2972+
config_name = xstrfmt("submodule.%s.branch", sub->name);
29572973
ret = config_set_in_gitmodules_file_gently(config_name, opt_branch);
29582974

29592975
free(config_name);

‎t/t7419-submodule-set-branch.sh

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ as expected.
1111

1212
TEST_PASSES_SANITIZE_LEAK=true
1313
TEST_NO_CREATE_REPO=1
14+
15+
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
16+
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
17+
1418
. ./test-lib.sh
1519

1620
test_expect_success 'setup' '
@@ -27,26 +31,28 @@ test_expect_success 'submodule config cache setup' '
2731
git checkout -b topic &&
2832
echo b >a &&
2933
git add . &&
30-
git commit -mb
34+
git commit -mb &&
35+
git checkout main
3136
) &&
3237
mkdir super &&
3338
(cd super &&
3439
git init &&
3540
git submodule add ../submodule &&
36-
git commit -m "add submodule"
41+
git submodule add --name thename ../submodule thepath &&
42+
git commit -m "add submodules"
3743
)
3844
'
3945

4046
test_expect_success 'ensure submodule branch is unset' '
4147
(cd super &&
42-
! grep branch .gitmodules
48+
test_cmp_config "" -f .gitmodules --default "" submodule.submodule.branch
4349
)
4450
'
4551

4652
test_expect_success 'test submodule set-branch --branch' '
4753
(cd super &&
4854
git submodule set-branch --branch topic submodule &&
49-
grep "branch = topic" .gitmodules &&
55+
test_cmp_config topic -f .gitmodules submodule.submodule.branch &&
5056
git submodule update --remote &&
5157
cat <<-\EOF >expect &&
5258
b
@@ -57,24 +63,22 @@ test_expect_success 'test submodule set-branch --branch' '
5763
'
5864

5965
test_expect_success 'test submodule set-branch --default' '
60-
test_commit -C submodule c &&
6166
(cd super &&
6267
git submodule set-branch --default submodule &&
63-
! grep branch .gitmodules &&
68+
test_cmp_config "" -f .gitmodules --default "" submodule.submodule.branch &&
6469
git submodule update --remote &&
6570
cat <<-\EOF >expect &&
66-
c
71+
a
6772
EOF
6873
git -C submodule show -s --pretty=%s >actual &&
6974
test_cmp expect actual
7075
)
7176
'
7277

7378
test_expect_success 'test submodule set-branch -b' '
74-
test_commit -C submodule b &&
7579
(cd super &&
7680
git submodule set-branch -b topic submodule &&
77-
grep "branch = topic" .gitmodules &&
81+
test_cmp_config topic -f .gitmodules submodule.submodule.branch &&
7882
git submodule update --remote &&
7983
cat <<-\EOF >expect &&
8084
b
@@ -85,17 +89,43 @@ test_expect_success 'test submodule set-branch -b' '
8589
'
8690

8791
test_expect_success 'test submodule set-branch -d' '
88-
test_commit -C submodule d &&
8992
(cd super &&
9093
git submodule set-branch -d submodule &&
91-
! grep branch .gitmodules &&
94+
test_cmp_config "" -f .gitmodules --default "" submodule.submodule.branch &&
9295
git submodule update --remote &&
9396
cat <<-\EOF >expect &&
94-
d
97+
a
9598
EOF
9699
git -C submodule show -s --pretty=%s >actual &&
97100
test_cmp expect actual
98101
)
99102
'
100103

104+
test_expect_success 'test submodule set-branch --branch with named submodule' '
105+
(cd super &&
106+
git submodule set-branch --branch topic thepath &&
107+
test_cmp_config topic -f .gitmodules submodule.thename.branch &&
108+
test_cmp_config "" -f .gitmodules --default "" submodule.thepath.branch &&
109+
git submodule update --remote &&
110+
cat <<-\EOF >expect &&
111+
b
112+
EOF
113+
git -C thepath show -s --pretty=%s >actual &&
114+
test_cmp expect actual
115+
)
116+
'
117+
118+
test_expect_success 'test submodule set-branch --default with named submodule' '
119+
(cd super &&
120+
git submodule set-branch --default thepath &&
121+
test_cmp_config "" -f .gitmodules --default "" submodule.thename.branch &&
122+
git submodule update --remote &&
123+
cat <<-\EOF >expect &&
124+
a
125+
EOF
126+
git -C thepath show -s --pretty=%s >actual &&
127+
test_cmp expect actual
128+
)
129+
'
130+
101131
test_done

‎t/t7420-submodule-set-url.sh

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,26 @@ test_expect_success 'submodule config cache setup' '
2525
git add file &&
2626
git commit -ma
2727
) &&
28+
mkdir namedsubmodule &&
29+
(
30+
cd namedsubmodule &&
31+
git init &&
32+
echo 1 >file &&
33+
git add file &&
34+
git commit -m1
35+
) &&
2836
mkdir super &&
2937
(
3038
cd super &&
3139
git init &&
3240
git submodule add ../submodule &&
33-
git commit -m "add submodule"
41+
git submodule add --name thename ../namedsubmodule thepath &&
42+
git commit -m "add submodules"
3443
)
3544
'
3645

3746
test_expect_success 'test submodule set-url' '
38-
# add a commit and move the submodule (change the url)
47+
# add commits and move the submodules (change the urls)
3948
(
4049
cd submodule &&
4150
echo b >>file &&
@@ -44,15 +53,28 @@ test_expect_success 'test submodule set-url' '
4453
) &&
4554
mv submodule newsubmodule &&
4655
56+
(
57+
cd namedsubmodule &&
58+
echo 2 >>file &&
59+
git add file &&
60+
git commit -m2
61+
) &&
62+
mv namedsubmodule newnamedsubmodule &&
63+
4764
git -C newsubmodule show >expect &&
65+
git -C newnamedsubmodule show >>expect &&
4866
(
4967
cd super &&
5068
test_must_fail git submodule update --remote &&
5169
git submodule set-url submodule ../newsubmodule &&
52-
grep -F "url = ../newsubmodule" .gitmodules &&
70+
test_cmp_config ../newsubmodule -f .gitmodules submodule.submodule.url &&
71+
git submodule set-url thepath ../newnamedsubmodule &&
72+
test_cmp_config ../newnamedsubmodule -f .gitmodules submodule.thename.url &&
73+
test_cmp_config "" -f .gitmodules --default "" submodule.thepath.url &&
5374
git submodule update --remote
5475
) &&
5576
git -C super/submodule show >actual &&
77+
git -C super/thepath show >>actual &&
5678
test_cmp expect actual
5779
'
5880

0 commit comments

Comments
 (0)