diff --git a/builtin/grep.c b/builtin/grep.c index 601f801158..7da8fef31a 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -421,11 +421,23 @@ static int grep_submodule(struct grep_opt *opt, struct repository *superproject, struct repository submodule; int hit; - if (!is_submodule_active(superproject, path)) - return 0; + /* + * NEEDSWORK: submodules functions need to be protected because they + * access the object store via config_from_gitmodules(): the latter + * uses get_oid() which, for now, relies on the global the_repository + * object. + */ + grep_read_lock(); - if (repo_submodule_init(&submodule, superproject, path)) + if (!is_submodule_active(superproject, path)) { + grep_read_unlock(); return 0; + } + + if (repo_submodule_init(&submodule, superproject, path)) { + grep_read_unlock(); + return 0; + } repo_read_gitmodules(&submodule); @@ -439,7 +451,6 @@ static int grep_submodule(struct grep_opt *opt, struct repository *superproject, * store is no longer global and instead is a member of the repository * object. */ - grep_read_lock(); add_to_alternates_memory(submodule.objects->objectdir); grep_read_unlock(); diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 8699515e81..43d2a132a9 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2032,8 +2032,12 @@ static int module_config(int argc, const char **argv, const char *prefix) return print_config_from_gitmodules(the_repository, argv[1]); /* Equivalent to ACTION_SET in builtin/config.c */ - if (argc == 3) + if (argc == 3) { + if (!is_writing_gitmodules_ok()) + die(_("please make sure that the .gitmodules file is in the working tree")); + return config_set_in_gitmodules_file_gently(argv[1], argv[2]); + } usage_with_options(git_submodule_helper_usage, module_config_options); } diff --git a/git-submodule.sh b/git-submodule.sh index 25b9bc58cb..bff855f54a 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -159,6 +159,11 @@ cmd_add() shift done + if ! git submodule--helper config --check-writeable >/dev/null 2>&1 + then + die "$(eval_gettext "please make sure that the .gitmodules file is in the working tree")" + fi + if test -n "$reference_path" then is_absolute_path "$reference_path" || diff --git a/submodule-config.c b/submodule-config.c index 9faf141e47..6869394c32 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "dir.h" #include "repository.h" #include "config.h" #include "submodule-config.h" @@ -603,8 +604,34 @@ static void submodule_cache_check_init(struct repository *repo) static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void *data) { if (repo->worktree) { - char *file = repo_worktree_path(repo, GITMODULES_FILE); - git_config_from_file(fn, file, data); + struct git_config_source config_source = { 0 }; + const struct config_options opts = { 0 }; + struct object_id oid; + char *file; + + file = repo_worktree_path(repo, GITMODULES_FILE); + if (file_exists(file)) { + config_source.file = file; + } else if (repo->submodule_prefix) { + /* + * When get_oid and config_with_options, used below, + * become able to work on a specific repository, this + * warning branch can be removed. + */ + warning("nested submodules without %s in the working tree are not supported yet", + GITMODULES_FILE); + goto out; + } else if (get_oid(GITMODULES_INDEX, &oid) >= 0) { + config_source.blob = GITMODULES_INDEX; + } else if (get_oid(GITMODULES_HEAD, &oid) >= 0) { + config_source.blob = GITMODULES_HEAD; + } else { + goto out; + } + + config_with_options(fn, data, &config_source, &opts); + +out: free(file); } } diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh index 45953f9300..2cfabb18bc 100755 --- a/t/t7411-submodule-config.sh +++ b/t/t7411-submodule-config.sh @@ -134,7 +134,7 @@ test_expect_success 'error in history in fetchrecursesubmodule lets continue' ' ) ' -test_expect_success 'reading submodules config with "submodule--helper config"' ' +test_expect_success 'reading submodules config from the working tree with "submodule--helper config"' ' (cd super && echo "../submodule" >expect && 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 ) ' +test_expect_success 'reading submodules config from the index when .gitmodules is not in the working tree' ' + ORIG=$(git -C super rev-parse HEAD) && + test_when_finished "git -C super reset --hard $ORIG" && + (cd super && + git submodule--helper config submodule.submodule.url "staged_url" && + git add .gitmodules && + rm -f .gitmodules && + echo "staged_url" >expect && + git submodule--helper config submodule.submodule.url >actual && + test_cmp expect actual + ) +' + +test_expect_success 'reading submodules config from the current branch when .gitmodules is not in the index' ' + ORIG=$(git -C super rev-parse HEAD) && + test_when_finished "git -C super reset --hard $ORIG" && + (cd super && + git rm .gitmodules && + echo "../submodule" >expect && + git submodule--helper config submodule.submodule.url >actual && + test_cmp expect actual + ) +' + test_done diff --git a/t/t7418-submodule-sparse-gitmodules.sh b/t/t7418-submodule-sparse-gitmodules.sh new file mode 100755 index 0000000000..3f7f271883 --- /dev/null +++ b/t/t7418-submodule-sparse-gitmodules.sh @@ -0,0 +1,122 @@ +#!/bin/sh +# +# Copyright (C) 2018 Antonio Ospite +# + +test_description='Test reading/writing .gitmodules when not in the working tree + +This test verifies that, when .gitmodules is in the current branch but is not +in the working tree reading from it still works but writing to it does not. + +The test setup uses a sparse checkout, however the same scenario can be set up +also by committing .gitmodules and then just removing it from the filesystem. +' + +. ./test-lib.sh + +test_expect_success 'sparse checkout setup which hides .gitmodules' ' + git init upstream && + git init submodule && + (cd submodule && + echo file >file && + git add file && + test_tick && + git commit -m "Add file" + ) && + (cd upstream && + git submodule add ../submodule && + test_tick && + git commit -m "Add submodule" + ) && + git clone upstream super && + (cd super && + cat >.git/info/sparse-checkout <<-\EOF && + /* + !/.gitmodules + EOF + git config core.sparsecheckout true && + git read-tree -m -u HEAD && + test_path_is_missing .gitmodules + ) +' + +test_expect_success 'reading gitmodules config file when it is not checked out' ' + echo "../submodule" >expect && + git -C super submodule--helper config submodule.submodule.url >actual && + test_cmp expect actual +' + +test_expect_success 'not writing gitmodules config file when it is not checked out' ' + test_must_fail git -C super submodule--helper config submodule.submodule.url newurl && + test_path_is_missing super/.gitmodules +' + +test_expect_success 'initialising submodule when the gitmodules config is not checked out' ' + test_must_fail git -C super config submodule.submodule.url && + git -C super submodule init && + git -C super config submodule.submodule.url >actual && + echo "$(pwd)/submodule" >expect && + test_cmp expect actual +' + +test_expect_success 'updating submodule when the gitmodules config is not checked out' ' + test_path_is_missing super/submodule/file && + git -C super submodule update && + test_cmp submodule/file super/submodule/file +' + +ORIG_SUBMODULE=$(git -C submodule rev-parse HEAD) +ORIG_UPSTREAM=$(git -C upstream rev-parse HEAD) +ORIG_SUPER=$(git -C super rev-parse HEAD) + +test_expect_success 're-updating submodule when the gitmodules config is not checked out' ' + test_when_finished "git -C submodule reset --hard $ORIG_SUBMODULE; + git -C upstream reset --hard $ORIG_UPSTREAM; + git -C super reset --hard $ORIG_SUPER; + git -C upstream submodule update --remote; + git -C super pull; + git -C super submodule update --remote" && + (cd submodule && + echo file2 >file2 && + git add file2 && + test_tick && + git commit -m "Add file2 to submodule" + ) && + (cd upstream && + git submodule update --remote && + git add submodule && + test_tick && + git commit -m "Update submodule" + ) && + git -C super pull && + # The --for-status options reads the gitmodules config + git -C super submodule summary --for-status >actual && + rev1=$(git -C submodule rev-parse --short HEAD) && + rev2=$(git -C submodule rev-parse --short HEAD^) && + cat >expect <<-EOF && + * submodule ${rev1}...${rev2} (1): + < Add file2 to submodule + + EOF + test_cmp expect actual && + # Test that the update actually succeeds + test_path_is_missing super/submodule/file2 && + git -C super submodule update && + test_cmp submodule/file2 super/submodule/file2 && + git -C super status --short >output && + test_must_be_empty output +' + +test_expect_success 'not adding submodules when the gitmodules config is not checked out' ' + git clone submodule new_submodule && + test_must_fail git -C super submodule add ../new_submodule && + test_path_is_missing .gitmodules +' + +# This test checks that the previous "git submodule add" did not leave the +# repository in a spurious state when it failed. +test_expect_success 'init submodule still works even after the previous add failed' ' + git -C super submodule init +' + +test_done diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh index 7184113b9b..fa475d52fa 100755 --- a/t/t7814-grep-recurse-submodules.sh +++ b/t/t7814-grep-recurse-submodules.sh @@ -380,4 +380,20 @@ test_expect_success 'grep --recurse-submodules should pass the pattern type alon fi ' +# Recursing down into nested submodules which do not have .gitmodules in their +# working tree does not work yet. This is because config_from_gitmodules() +# uses get_oid() and the latter is still not able to get objects from an +# arbitrary repository (the nested submodule, in this case). +test_expect_failure 'grep --recurse-submodules with submodules without .gitmodules in the working tree' ' + test_when_finished "git -C submodule checkout .gitmodules" && + rm submodule/.gitmodules && + git grep --recurse-submodules -e "(.|.)[\d]" >actual && + cat >expect <<-\EOF && + a:(1|2)d(3|4) + submodule/a:(1|2)d(3|4) + submodule/sub/a:(1|2)d(3|4) + EOF + test_cmp expect actual +' + test_done