Skip to content

Commit 76e9bdc

Browse files
ao2gitster
ao2
authored and
gitster
committed
submodule: support reading .gitmodules when it's not in the working tree
When the .gitmodules file is not available in the working tree, try using the content from the index and from the current branch. This covers the case when the file is part of the repository but for some reason it is not checked out, for example because of a sparse checkout. This makes it possible to use at least the 'git submodule' commands which *read* the gitmodules configuration file without fully populating the working tree. Writing to .gitmodules will still require that the file is checked out, so check for that before calling config_set_in_gitmodules_file_gently. Add a similar check also in git-submodule.sh::cmd_add() to anticipate the eventual failure of the "git submodule add" command when .gitmodules is not safely writeable; this prevents the command from leaving the repository in a spurious state (e.g. the submodule repository was cloned but .gitmodules was not updated because config_set_in_gitmodules_file_gently failed). Moreover, since config_from_gitmodules() now accesses the global object store, it is necessary to protect all code paths which call the function against concurrent access to the global object store. Currently this only happens in builtin/grep.c::grep_submodules(), so call grep_read_lock() before invoking code involving config_from_gitmodules(). Finally, add t7418-submodule-sparse-gitmodules.sh to verify that reading from .gitmodules succeeds and that writing to it fails when the file is not checked out. NOTE: there is one rare case where this new feature does not work properly yet: nested submodules without .gitmodules in their working tree. This has been documented with a warning and a test_expect_failure item in t7814, and in this case the current behavior is not altered: no config is read. Signed-off-by: Antonio Ospite <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent b5c259f commit 76e9bdc

7 files changed

+216
-7
lines changed

‎builtin/grep.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -421,11 +421,23 @@ static int grep_submodule(struct grep_opt *opt, struct repository *superproject,
421421
struct repository submodule;
422422
int hit;
423423

424-
if (!is_submodule_active(superproject, path))
424+
/*
425+
* NEEDSWORK: submodules functions need to be protected because they
426+
* access the object store via config_from_gitmodules(): the latter
427+
* uses get_oid() which, for now, relies on the global the_repository
428+
* object.
429+
*/
430+
grep_read_lock();
431+
432+
if (!is_submodule_active(superproject, path)) {
433+
grep_read_unlock();
425434
return 0;
435+
}
426436

427-
if (repo_submodule_init(&submodule, superproject, path))
437+
if (repo_submodule_init(&submodule, superproject, path)) {
438+
grep_read_unlock();
428439
return 0;
440+
}
429441

430442
repo_read_gitmodules(&submodule);
431443

@@ -439,7 +451,6 @@ static int grep_submodule(struct grep_opt *opt, struct repository *superproject,
439451
* store is no longer global and instead is a member of the repository
440452
* object.
441453
*/
442-
grep_read_lock();
443454
add_to_alternates_memory(submodule.objects->objectdir);
444455
grep_read_unlock();
445456

‎builtin/submodule--helper.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2032,8 +2032,12 @@ static int module_config(int argc, const char **argv, const char *prefix)
20322032
return print_config_from_gitmodules(the_repository, argv[1]);
20332033

20342034
/* Equivalent to ACTION_SET in builtin/config.c */
2035-
if (argc == 3)
2035+
if (argc == 3) {
2036+
if (!is_writing_gitmodules_ok())
2037+
die(_("please make sure that the .gitmodules file is in the working tree"));
2038+
20362039
return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
2040+
}
20372041

20382042
usage_with_options(git_submodule_helper_usage, module_config_options);
20392043
}

‎git-submodule.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,11 @@ cmd_add()
159159
shift
160160
done
161161

162+
if ! git submodule--helper config --check-writeable >/dev/null 2>&1
163+
then
164+
die "$(eval_gettext "please make sure that the .gitmodules file is in the working tree")"
165+
fi
166+
162167
if test -n "$reference_path"
163168
then
164169
is_absolute_path "$reference_path" ||

‎submodule-config.c

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include "cache.h"
2+
#include "dir.h"
23
#include "repository.h"
34
#include "config.h"
45
#include "submodule-config.h"
@@ -603,8 +604,34 @@ static void submodule_cache_check_init(struct repository *repo)
603604
static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void *data)
604605
{
605606
if (repo->worktree) {
606-
char *file = repo_worktree_path(repo, GITMODULES_FILE);
607-
git_config_from_file(fn, file, data);
607+
struct git_config_source config_source = { 0 };
608+
const struct config_options opts = { 0 };
609+
struct object_id oid;
610+
char *file;
611+
612+
file = repo_worktree_path(repo, GITMODULES_FILE);
613+
if (file_exists(file)) {
614+
config_source.file = file;
615+
} else if (repo->submodule_prefix) {
616+
/*
617+
* When get_oid and config_with_options, used below,
618+
* become able to work on a specific repository, this
619+
* warning branch can be removed.
620+
*/
621+
warning("nested submodules without %s in the working tree are not supported yet",
622+
GITMODULES_FILE);
623+
goto out;
624+
} else if (get_oid(GITMODULES_INDEX, &oid) >= 0) {
625+
config_source.blob = GITMODULES_INDEX;
626+
} else if (get_oid(GITMODULES_HEAD, &oid) >= 0) {
627+
config_source.blob = GITMODULES_HEAD;
628+
} else {
629+
goto out;
630+
}
631+
632+
config_with_options(fn, data, &config_source, &opts);
633+
634+
out:
608635
free(file);
609636
}
610637
}

‎t/t7411-submodule-config.sh

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ test_expect_success 'error in history in fetchrecursesubmodule lets continue' '
134134
)
135135
'
136136

137-
test_expect_success 'reading submodules config with "submodule--helper config"' '
137+
test_expect_success 'reading submodules config from the working tree with "submodule--helper config"' '
138138
(cd super &&
139139
echo "../submodule" >expect &&
140140
git submodule--helper config submodule.submodule.url >actual &&
@@ -192,4 +192,28 @@ test_expect_success 'non-writeable .gitmodules when it is in the current branch
192192
)
193193
'
194194

195+
test_expect_success 'reading submodules config from the index when .gitmodules is not in the working tree' '
196+
ORIG=$(git -C super rev-parse HEAD) &&
197+
test_when_finished "git -C super reset --hard $ORIG" &&
198+
(cd super &&
199+
git submodule--helper config submodule.submodule.url "staged_url" &&
200+
git add .gitmodules &&
201+
rm -f .gitmodules &&
202+
echo "staged_url" >expect &&
203+
git submodule--helper config submodule.submodule.url >actual &&
204+
test_cmp expect actual
205+
)
206+
'
207+
208+
test_expect_success 'reading submodules config from the current branch when .gitmodules is not in the index' '
209+
ORIG=$(git -C super rev-parse HEAD) &&
210+
test_when_finished "git -C super reset --hard $ORIG" &&
211+
(cd super &&
212+
git rm .gitmodules &&
213+
echo "../submodule" >expect &&
214+
git submodule--helper config submodule.submodule.url >actual &&
215+
test_cmp expect actual
216+
)
217+
'
218+
195219
test_done

‎t/t7418-submodule-sparse-gitmodules.sh

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
#!/bin/sh
2+
#
3+
# Copyright (C) 2018 Antonio Ospite <[email protected]>
4+
#
5+
6+
test_description='Test reading/writing .gitmodules when not in the working tree
7+
8+
This test verifies that, when .gitmodules is in the current branch but is not
9+
in the working tree reading from it still works but writing to it does not.
10+
11+
The test setup uses a sparse checkout, however the same scenario can be set up
12+
also by committing .gitmodules and then just removing it from the filesystem.
13+
'
14+
15+
. ./test-lib.sh
16+
17+
test_expect_success 'sparse checkout setup which hides .gitmodules' '
18+
git init upstream &&
19+
git init submodule &&
20+
(cd submodule &&
21+
echo file >file &&
22+
git add file &&
23+
test_tick &&
24+
git commit -m "Add file"
25+
) &&
26+
(cd upstream &&
27+
git submodule add ../submodule &&
28+
test_tick &&
29+
git commit -m "Add submodule"
30+
) &&
31+
git clone upstream super &&
32+
(cd super &&
33+
cat >.git/info/sparse-checkout <<-\EOF &&
34+
/*
35+
!/.gitmodules
36+
EOF
37+
git config core.sparsecheckout true &&
38+
git read-tree -m -u HEAD &&
39+
test_path_is_missing .gitmodules
40+
)
41+
'
42+
43+
test_expect_success 'reading gitmodules config file when it is not checked out' '
44+
echo "../submodule" >expect &&
45+
git -C super submodule--helper config submodule.submodule.url >actual &&
46+
test_cmp expect actual
47+
'
48+
49+
test_expect_success 'not writing gitmodules config file when it is not checked out' '
50+
test_must_fail git -C super submodule--helper config submodule.submodule.url newurl &&
51+
test_path_is_missing super/.gitmodules
52+
'
53+
54+
test_expect_success 'initialising submodule when the gitmodules config is not checked out' '
55+
test_must_fail git -C super config submodule.submodule.url &&
56+
git -C super submodule init &&
57+
git -C super config submodule.submodule.url >actual &&
58+
echo "$(pwd)/submodule" >expect &&
59+
test_cmp expect actual
60+
'
61+
62+
test_expect_success 'updating submodule when the gitmodules config is not checked out' '
63+
test_path_is_missing super/submodule/file &&
64+
git -C super submodule update &&
65+
test_cmp submodule/file super/submodule/file
66+
'
67+
68+
ORIG_SUBMODULE=$(git -C submodule rev-parse HEAD)
69+
ORIG_UPSTREAM=$(git -C upstream rev-parse HEAD)
70+
ORIG_SUPER=$(git -C super rev-parse HEAD)
71+
72+
test_expect_success 're-updating submodule when the gitmodules config is not checked out' '
73+
test_when_finished "git -C submodule reset --hard $ORIG_SUBMODULE;
74+
git -C upstream reset --hard $ORIG_UPSTREAM;
75+
git -C super reset --hard $ORIG_SUPER;
76+
git -C upstream submodule update --remote;
77+
git -C super pull;
78+
git -C super submodule update --remote" &&
79+
(cd submodule &&
80+
echo file2 >file2 &&
81+
git add file2 &&
82+
test_tick &&
83+
git commit -m "Add file2 to submodule"
84+
) &&
85+
(cd upstream &&
86+
git submodule update --remote &&
87+
git add submodule &&
88+
test_tick &&
89+
git commit -m "Update submodule"
90+
) &&
91+
git -C super pull &&
92+
# The --for-status options reads the gitmodules config
93+
git -C super submodule summary --for-status >actual &&
94+
rev1=$(git -C submodule rev-parse --short HEAD) &&
95+
rev2=$(git -C submodule rev-parse --short HEAD^) &&
96+
cat >expect <<-EOF &&
97+
* submodule ${rev1}...${rev2} (1):
98+
< Add file2 to submodule
99+
100+
EOF
101+
test_cmp expect actual &&
102+
# Test that the update actually succeeds
103+
test_path_is_missing super/submodule/file2 &&
104+
git -C super submodule update &&
105+
test_cmp submodule/file2 super/submodule/file2 &&
106+
git -C super status --short >output &&
107+
test_must_be_empty output
108+
'
109+
110+
test_expect_success 'not adding submodules when the gitmodules config is not checked out' '
111+
git clone submodule new_submodule &&
112+
test_must_fail git -C super submodule add ../new_submodule &&
113+
test_path_is_missing .gitmodules
114+
'
115+
116+
# This test checks that the previous "git submodule add" did not leave the
117+
# repository in a spurious state when it failed.
118+
test_expect_success 'init submodule still works even after the previous add failed' '
119+
git -C super submodule init
120+
'
121+
122+
test_done

‎t/t7814-grep-recurse-submodules.sh

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,4 +380,20 @@ test_expect_success 'grep --recurse-submodules should pass the pattern type alon
380380
fi
381381
'
382382

383+
# Recursing down into nested submodules which do not have .gitmodules in their
384+
# working tree does not work yet. This is because config_from_gitmodules()
385+
# uses get_oid() and the latter is still not able to get objects from an
386+
# arbitrary repository (the nested submodule, in this case).
387+
test_expect_failure 'grep --recurse-submodules with submodules without .gitmodules in the working tree' '
388+
test_when_finished "git -C submodule checkout .gitmodules" &&
389+
rm submodule/.gitmodules &&
390+
git grep --recurse-submodules -e "(.|.)[\d]" >actual &&
391+
cat >expect <<-\EOF &&
392+
a:(1|2)d(3|4)
393+
submodule/a:(1|2)d(3|4)
394+
submodule/sub/a:(1|2)d(3|4)
395+
EOF
396+
test_cmp expect actual
397+
'
398+
383399
test_done

0 commit comments

Comments
 (0)