From 5ea50954d0632342d38bd1a4d86c5aa601ef5207 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Tue, 25 Jul 2017 14:39:14 -0700 Subject: [PATCH 01/15] t7411: check configuration parsing errors Check for configuration parsing errors in '.gitmodules' in t7411, which is explicitly testing the submodule-config subsystem, instead of in t7400. Also explicitly use the test helper instead of relying on the gitmodules file from being read in status. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- t/t7400-submodule-basic.sh | 10 ---------- t/t7411-submodule-config.sh | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index dcac364c5f..717447526e 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -46,16 +46,6 @@ test_expect_success 'submodule update aborts on missing gitmodules url' ' test_must_fail git submodule init ' -test_expect_success 'configuration parsing' ' - test_when_finished "rm -f .gitmodules" && - cat >.gitmodules <<-\EOF && - [submodule "s"] - path - ignore - EOF - test_must_fail git status -' - test_expect_success 'setup - repository in init subdirectory' ' mkdir init && ( diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh index eea36f1dbe..7d6b25ba23 100755 --- a/t/t7411-submodule-config.sh +++ b/t/t7411-submodule-config.sh @@ -31,6 +31,21 @@ test_expect_success 'submodule config cache setup' ' ) ' +test_expect_success 'configuration parsing with error' ' + test_when_finished "rm -rf repo" && + test_create_repo repo && + cat >repo/.gitmodules <<-\EOF && + [submodule "s"] + path + ignore + EOF + ( + cd repo && + test_must_fail test-submodule-config "" s 2>actual && + test_i18ngrep "bad config" actual + ) +' + cat >super/expect < Date: Tue, 25 Jul 2017 14:39:15 -0700 Subject: [PATCH 02/15] submodule: don't use submodule_from_name The function 'submodule_from_name()' is being used incorrectly here as a submodule path is being used instead of a submodule name. Since the correct function to use with a path to a submodule is already being used ('submodule_from_path()') let's remove the call to 'submodule_from_name()'. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- submodule.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/submodule.c b/submodule.c index 5139b9256b..19bd13bb2a 100644 --- a/submodule.c +++ b/submodule.c @@ -1177,8 +1177,6 @@ static int get_next_submodule(struct child_process *cp, continue; submodule = submodule_from_path(&null_oid, ce->name); - if (!submodule) - submodule = submodule_from_name(&null_oid, ce->name); default_argv = "yes"; if (spf->command_line_option == RECURSE_SUBMODULES_DEFAULT) { From 5556808690ea245708fb80383be5c1afee2fb3eb Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Tue, 25 Jul 2017 14:39:16 -0700 Subject: [PATCH 03/15] add, reset: ensure submodules can be added or reset Commit aee9c7d65 (Submodules: Add the new "ignore" config option for diff and status) introduced the ignore configuration option for submodules so that configured submodules could be omitted from the status and diff commands. Because this flag is respected in the diff machinery it has the unintended consequence of potentially prohibiting users from adding or resetting a submodule, even when a path to the submodule is explicitly given. Ensure that submodules can be added or set, even if they are configured to be ignored, by setting the `DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG` diff flag. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- builtin/add.c | 1 + builtin/reset.c | 1 + 2 files changed, 2 insertions(+) diff --git a/builtin/add.c b/builtin/add.c index e888fb8c5f..6f271512f8 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -116,6 +116,7 @@ int add_files_to_cache(const char *prefix, rev.diffopt.output_format = DIFF_FORMAT_CALLBACK; rev.diffopt.format_callback = update_callback; rev.diffopt.format_callback_data = &data; + rev.diffopt.flags |= DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG; rev.max_count = 0; /* do not compare unmerged paths with stage #2 */ run_diff_files(&rev, DIFF_RACY_IS_MODIFIED); return !!data.add_errors; diff --git a/builtin/reset.c b/builtin/reset.c index 046403ed68..772d078b85 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -156,6 +156,7 @@ static int read_from_tree(const struct pathspec *pathspec, opt.output_format = DIFF_FORMAT_CALLBACK; opt.format_callback = update_index_from_diff; opt.format_callback_data = &intent_to_add; + opt.flags |= DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG; if (do_diff_cache(tree_oid, &opt)) return 1; From 177257ccc733c1a363bfbb5de630a057804cefc3 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 3 Aug 2017 11:19:49 -0700 Subject: [PATCH 04/15] submodule--helper: don't overlay config in remote_submodule_branch Don't rely on overlaying the repository's config on top of the submodule-config, instead query the repository's config directly for the branch field. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 1e49ce580e..f71f4270d9 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1066,17 +1066,24 @@ static int resolve_relative_path(int argc, const char **argv, const char *prefix static const char *remote_submodule_branch(const char *path) { const struct submodule *sub; + const char *branch = NULL; + char *key; + gitmodules_config(); - git_config(submodule_config, NULL); sub = submodule_from_path(&null_oid, path); if (!sub) return NULL; - if (!sub->branch) + key = xstrfmt("submodule.%s.branch", sub->name); + if (repo_config_get_string_const(the_repository, key, &branch)) + branch = sub->branch; + free(key); + + if (!branch) return "master"; - if (!strcmp(sub->branch, ".")) { + if (!strcmp(branch, ".")) { unsigned char sha1[20]; const char *refname = resolve_ref_unsafe("HEAD", 0, sha1, NULL); @@ -1094,7 +1101,7 @@ static const char *remote_submodule_branch(const char *path) return refname; } - return sub->branch; + return branch; } static int resolve_remote_submodule_branch(int argc, const char **argv, From ec6141a0f290ba5a0cea2d15820be0223467e656 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 3 Aug 2017 11:19:50 -0700 Subject: [PATCH 05/15] submodule--helper: don't overlay config in update-clone Don't rely on overlaying the repository's config on top of the submodule-config, instead query the repository's config directly for the url and the update strategy configuration. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 23 ++++++++++++++++++---- submodule.c | 38 +++++++++++++++++++++++++------------ submodule.h | 1 + 3 files changed, 46 insertions(+), 16 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index f71f4270d9..36df7ab78f 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -780,6 +780,10 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce, struct strbuf *out) { const struct submodule *sub = NULL; + const char *url = NULL; + const char *update_string; + enum submodule_update_type update_type; + char *key; struct strbuf displaypath_sb = STRBUF_INIT; struct strbuf sb = STRBUF_INIT; const char *displaypath = NULL; @@ -808,9 +812,17 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce, goto cleanup; } + key = xstrfmt("submodule.%s.update", sub->name); + if (!repo_config_get_string_const(the_repository, key, &update_string)) { + update_type = parse_submodule_update_type(update_string); + } else { + update_type = sub->update_strategy.type; + } + free(key); + if (suc->update.type == SM_UPDATE_NONE || (suc->update.type == SM_UPDATE_UNSPECIFIED - && sub->update_strategy.type == SM_UPDATE_NONE)) { + && update_type == SM_UPDATE_NONE)) { strbuf_addf(out, _("Skipping submodule '%s'"), displaypath); strbuf_addch(out, '\n'); goto cleanup; @@ -822,6 +834,11 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce, goto cleanup; } + strbuf_reset(&sb); + strbuf_addf(&sb, "submodule.%s.url", sub->name); + if (repo_config_get_string_const(the_repository, sb.buf, &url)) + url = sub->url; + strbuf_reset(&sb); strbuf_addf(&sb, "%s/.git", ce->name); needs_cloning = !file_exists(sb.buf); @@ -851,7 +868,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce, argv_array_push(&child->args, "--depth=1"); argv_array_pushl(&child->args, "--path", sub->path, NULL); argv_array_pushl(&child->args, "--name", sub->name, NULL); - argv_array_pushl(&child->args, "--url", sub->url, NULL); + argv_array_pushl(&child->args, "--url", url, NULL); if (suc->references.nr) { struct string_list_item *item; for_each_string_list_item(item, &suc->references) @@ -1025,9 +1042,7 @@ static int update_clone(int argc, const char **argv, const char *prefix) if (pathspec.nr) suc.warn_if_uninitialized = 1; - /* Overlay the parsed .gitmodules file with .git/config */ gitmodules_config(); - git_config(submodule_config, NULL); run_processes_parallel(max_jobs, update_clone_get_next_task, diff --git a/submodule.c b/submodule.c index 19bd13bb2a..8a9b964ce8 100644 --- a/submodule.c +++ b/submodule.c @@ -398,24 +398,38 @@ void die_path_inside_submodule(const struct index_state *istate, } } +enum submodule_update_type parse_submodule_update_type(const char *value) +{ + if (!strcmp(value, "none")) + return SM_UPDATE_NONE; + else if (!strcmp(value, "checkout")) + return SM_UPDATE_CHECKOUT; + else if (!strcmp(value, "rebase")) + return SM_UPDATE_REBASE; + else if (!strcmp(value, "merge")) + return SM_UPDATE_MERGE; + else if (*value == '!') + return SM_UPDATE_COMMAND; + else + return SM_UPDATE_UNSPECIFIED; +} + int parse_submodule_update_strategy(const char *value, struct submodule_update_strategy *dst) { + enum submodule_update_type type; + free((void*)dst->command); dst->command = NULL; - if (!strcmp(value, "none")) - dst->type = SM_UPDATE_NONE; - else if (!strcmp(value, "checkout")) - dst->type = SM_UPDATE_CHECKOUT; - else if (!strcmp(value, "rebase")) - dst->type = SM_UPDATE_REBASE; - else if (!strcmp(value, "merge")) - dst->type = SM_UPDATE_MERGE; - else if (skip_prefix(value, "!", &value)) { - dst->type = SM_UPDATE_COMMAND; - dst->command = xstrdup(value); - } else + + type = parse_submodule_update_type(value); + if (type == SM_UPDATE_UNSPECIFIED) return -1; + + dst->type = type; + if (type == SM_UPDATE_COMMAND) + dst->command = xstrdup(value + 1); + return 0; } diff --git a/submodule.h b/submodule.h index e402b004ff..48586efe7d 100644 --- a/submodule.h +++ b/submodule.h @@ -62,6 +62,7 @@ extern void die_in_unpopulated_submodule(const struct index_state *istate, const char *prefix); extern void die_path_inside_submodule(const struct index_state *istate, const struct pathspec *ps); +extern enum submodule_update_type parse_submodule_update_type(const char *value); extern int parse_submodule_update_strategy(const char *value, struct submodule_update_strategy *dst); extern const char *submodule_strategy_to_string(const struct submodule_update_strategy *s); From 492c6c46da7847370d8ce0e6d369ae62215b6f8e Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 3 Aug 2017 11:19:51 -0700 Subject: [PATCH 06/15] fetch: don't overlay config with submodule-config Don't rely on overlaying the repository's config on top of the submodule-config, instead query the repository's config directly for the fetch_recurse field. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- builtin/fetch.c | 1 - submodule.c | 24 +++++++++++++++++------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index d84c26391c..3fe99073d3 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1362,7 +1362,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) if (recurse_submodules != RECURSE_SUBMODULES_OFF) { gitmodules_config(); - git_config(submodule_config, NULL); } if (all) { diff --git a/submodule.c b/submodule.c index 8a9b964ce8..59e3d0828f 100644 --- a/submodule.c +++ b/submodule.c @@ -1194,14 +1194,24 @@ static int get_next_submodule(struct child_process *cp, default_argv = "yes"; if (spf->command_line_option == RECURSE_SUBMODULES_DEFAULT) { - if (submodule && - submodule->fetch_recurse != - RECURSE_SUBMODULES_NONE) { - if (submodule->fetch_recurse == - RECURSE_SUBMODULES_OFF) + int fetch_recurse = RECURSE_SUBMODULES_NONE; + + if (submodule) { + char *key; + const char *value; + + fetch_recurse = submodule->fetch_recurse; + key = xstrfmt("submodule.%s.fetchRecurseSubmodules", submodule->name); + if (!repo_config_get_string_const(the_repository, key, &value)) { + fetch_recurse = parse_fetch_recurse_submodules_arg(key, value); + } + free(key); + } + + if (fetch_recurse != RECURSE_SUBMODULES_NONE) { + if (fetch_recurse == RECURSE_SUBMODULES_OFF) continue; - if (submodule->fetch_recurse == - RECURSE_SUBMODULES_ON_DEMAND) { + if (fetch_recurse == RECURSE_SUBMODULES_ON_DEMAND) { if (!unsorted_string_list_lookup(&changed_submodule_paths, ce->name)) continue; default_argv = "on-demand"; From fdfa9e97dbffdc6868a1a046456c0ad67dda9e29 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 3 Aug 2017 11:19:52 -0700 Subject: [PATCH 07/15] submodule: don't rely on overlayed config when setting diffopts Don't rely on overlaying the repository's config on top of the submodule-config, instead query the repository's config directory for the ignore field. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- submodule.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/submodule.c b/submodule.c index 59e3d0828f..a32043893b 100644 --- a/submodule.c +++ b/submodule.c @@ -165,8 +165,16 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt, { const struct submodule *submodule = submodule_from_path(&null_oid, path); if (submodule) { - if (submodule->ignore) - handle_ignore_submodules_arg(diffopt, submodule->ignore); + const char *ignore; + char *key; + + key = xstrfmt("submodule.%s.ignore", submodule->name); + if (repo_config_get_string_const(the_repository, key, &ignore)) + ignore = submodule->ignore; + free(key); + + if (ignore) + handle_ignore_submodules_arg(diffopt, ignore); else if (is_gitmodules_unmerged(&the_index)) DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES); } From 7463e2ec3e932707b70b5d5c82df51bfbb6aa77d Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 3 Aug 2017 11:19:53 -0700 Subject: [PATCH 08/15] unpack-trees: don't respect submodule.update The 'submodule.update' config was historically used and respected by the 'submodule update' command because update handled a variety of different ways it updated a submodule. As we begin teaching other commands about submodules it makes more sense for the different settings of 'submodule.update' to be handled by the individual commands themselves (checkout, rebase, merge, etc) so it shouldn't be respected by the native checkout command. Also remove the overlaying of the repository's config (via using 'submodule_config()') from the commands which use the unpack-trees logic (checkout, read-tree, reset). Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- builtin/checkout.c | 2 +- submodule.c | 1 - unpack-trees.c | 38 ++++++++------------------------------ 3 files changed, 9 insertions(+), 32 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 9661e1bcba..246e0cd166 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -858,7 +858,7 @@ static int git_checkout_config(const char *var, const char *value, void *cb) } if (starts_with(var, "submodule.")) - return submodule_config(var, value, NULL); + return git_default_submodule_config(var, value, NULL); return git_xmerge_config(var, value, NULL); } diff --git a/submodule.c b/submodule.c index a32043893b..f913c23415 100644 --- a/submodule.c +++ b/submodule.c @@ -235,7 +235,6 @@ void load_submodule_cache(void) return; gitmodules_config(); - git_config(submodule_config, NULL); } static int gitmodules_cb(const char *var, const char *value, void *data) diff --git a/unpack-trees.c b/unpack-trees.c index 05335fe5bf..5dce7ff7d4 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -255,28 +255,17 @@ static int check_submodule_move_head(const struct cache_entry *ce, { unsigned flags = SUBMODULE_MOVE_HEAD_DRY_RUN; const struct submodule *sub = submodule_from_ce(ce); + if (!sub) return 0; if (o->reset) flags |= SUBMODULE_MOVE_HEAD_FORCE; - switch (sub->update_strategy.type) { - case SM_UPDATE_UNSPECIFIED: - case SM_UPDATE_CHECKOUT: - if (submodule_move_head(ce->name, old_id, new_id, flags)) - return o->gently ? -1 : - add_rejected_path(o, ERROR_WOULD_LOSE_SUBMODULE, ce->name); - return 0; - case SM_UPDATE_NONE: - return 0; - case SM_UPDATE_REBASE: - case SM_UPDATE_MERGE: - case SM_UPDATE_COMMAND: - default: - warning(_("submodule update strategy not supported for submodule '%s'"), ce->name); - return -1; - } + if (submodule_move_head(ce->name, old_id, new_id, flags)) + return o->gently ? -1 : + add_rejected_path(o, ERROR_WOULD_LOSE_SUBMODULE, ce->name); + return 0; } static void reload_gitmodules_file(struct index_state *index, @@ -293,7 +282,6 @@ static void reload_gitmodules_file(struct index_state *index, submodule_free(); checkout_entry(ce, state, NULL); gitmodules_config(); - git_config(submodule_config, NULL); } else break; } @@ -308,19 +296,9 @@ static void unlink_entry(const struct cache_entry *ce) { const struct submodule *sub = submodule_from_ce(ce); if (sub) { - switch (sub->update_strategy.type) { - case SM_UPDATE_UNSPECIFIED: - case SM_UPDATE_CHECKOUT: - case SM_UPDATE_REBASE: - case SM_UPDATE_MERGE: - /* state.force is set at the caller. */ - submodule_move_head(ce->name, "HEAD", NULL, - SUBMODULE_MOVE_HEAD_FORCE); - break; - case SM_UPDATE_NONE: - case SM_UPDATE_COMMAND: - return; /* Do not touch the submodule. */ - } + /* state.force is set at the caller. */ + submodule_move_head(ce->name, "HEAD", NULL, + SUBMODULE_MOVE_HEAD_FORCE); } if (!check_leading_path(ce->name, ce_namelen(ce))) return; From 2cc67fe54a029842e71e49a676bf010e988d4063 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 3 Aug 2017 11:19:54 -0700 Subject: [PATCH 09/15] submodule: remove submodule_config callback routine Remove the last remaining caller of 'submodule_config()' as well as the function itself. With 'submodule_config()' being removed the submodule-config API can be a little simpler as callers don't need to worry about whether or not they need to overlay the repository's config on top of the submodule-config. This also makes it more difficult to accidentally add non-submodule specific configuration to the .gitmodules file. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 1 - submodule.c | 25 ++----------------------- submodule.h | 1 - 3 files changed, 2 insertions(+), 25 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 36df7ab78f..ba767c7048 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1205,7 +1205,6 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix) git_submodule_helper_usage, 0); gitmodules_config(); - git_config(submodule_config, NULL); if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0) return 1; diff --git a/submodule.c b/submodule.c index f913c23415..3b383d8c41 100644 --- a/submodule.c +++ b/submodule.c @@ -180,27 +180,6 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt, } } -/* For loading from the .gitmodules file. */ -static int git_modules_config(const char *var, const char *value, void *cb) -{ - if (starts_with(var, "submodule.")) - return parse_submodule_config_option(var, value); - return 0; -} - -/* Loads all submodule settings from the config. */ -int submodule_config(const char *var, const char *value, void *cb) -{ - if (!strcmp(var, "submodule.recurse")) { - int v = git_config_bool(var, value) ? - RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF; - config_update_recurse_submodules = v; - return 0; - } else { - return git_modules_config(var, value, cb); - } -} - /* Cheap function that only determines if we're interested in submodules at all */ int git_default_submodule_config(const char *var, const char *value, void *cb) { @@ -271,8 +250,8 @@ void gitmodules_config_oid(const struct object_id *commit_oid) struct object_id oid; if (gitmodule_oid_from_commit(commit_oid, &oid, &rev)) { - git_config_from_blob_oid(submodule_config, rev.buf, - &oid, NULL); + git_config_from_blob_oid(gitmodules_cb, rev.buf, + &oid, the_repository); } strbuf_release(&rev); } diff --git a/submodule.h b/submodule.h index 48586efe7d..4f70c49444 100644 --- a/submodule.h +++ b/submodule.h @@ -40,7 +40,6 @@ extern int remove_path_from_gitmodules(const char *path); extern void stage_updated_gitmodules(void); extern void set_diffopt_flags_from_submodule_config(struct diff_options *, const char *path); -extern int submodule_config(const char *var, const char *value, void *cb); extern int git_default_submodule_config(const char *var, const char *value, void *cb); struct option; From 078b75e99be00155eac3ec2d4eaa0ae0941ff54d Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 3 Aug 2017 11:19:55 -0700 Subject: [PATCH 10/15] diff: stop allowing diff to have submodules configured in .git/config Traditionally a submodule is comprised of a gitlink as well as a corresponding entry in the .gitmodules file. Diff doesn't follow this paradigm as its config callback routine falls back to populating the submodule-config if a config entry starts with 'submodule.'. Remove this behavior in order to be consistent with how the submodule-config is populated, via calling 'gitmodules_config()' or 'repo_read_gitmodules()'. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- diff.c | 3 -- t/t4027-diff-submodule.sh | 67 --------------------------------------- 2 files changed, 70 deletions(-) diff --git a/diff.c b/diff.c index 85e714f6c6..e43519b885 100644 --- a/diff.c +++ b/diff.c @@ -346,9 +346,6 @@ int git_diff_basic_config(const char *var, const char *value, void *cb) return 0; } - if (starts_with(var, "submodule.")) - return parse_submodule_config_option(var, value); - if (git_diff_heuristic_config(var, value, cb) < 0) return -1; diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh index 518bf9524e..2ffd11a142 100755 --- a/t/t4027-diff-submodule.sh +++ b/t/t4027-diff-submodule.sh @@ -113,35 +113,6 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)' ! test -s actual4 ' -test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match) [.git/config]' ' - git config diff.ignoreSubmodules all && - git diff HEAD >actual && - ! test -s actual && - git config submodule.subname.ignore none && - git config submodule.subname.path sub && - git diff HEAD >actual && - sed -e "1,/^@@/d" actual >actual.body && - expect_from_to >expect.body $subprev $subprev-dirty && - test_cmp expect.body actual.body && - git config submodule.subname.ignore all && - git diff HEAD >actual2 && - ! test -s actual2 && - git config submodule.subname.ignore untracked && - git diff HEAD >actual3 && - sed -e "1,/^@@/d" actual3 >actual3.body && - expect_from_to >expect.body $subprev $subprev-dirty && - test_cmp expect.body actual3.body && - git config submodule.subname.ignore dirty && - git diff HEAD >actual4 && - ! test -s actual4 && - git diff HEAD --ignore-submodules=none >actual && - sed -e "1,/^@@/d" actual >actual.body && - expect_from_to >expect.body $subprev $subprev-dirty && - test_cmp expect.body actual.body && - git config --remove-section submodule.subname && - git config --unset diff.ignoreSubmodules -' - test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match) [.gitmodules]' ' git config diff.ignoreSubmodules dirty && git diff HEAD >actual && @@ -208,24 +179,6 @@ test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match)' ! test -s actual4 ' -test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match) [.git/config]' ' - git config submodule.subname.ignore all && - git config submodule.subname.path sub && - git diff HEAD >actual2 && - ! test -s actual2 && - git config submodule.subname.ignore untracked && - git diff HEAD >actual3 && - ! test -s actual3 && - git config submodule.subname.ignore dirty && - git diff HEAD >actual4 && - ! test -s actual4 && - git diff --ignore-submodules=none HEAD >actual && - sed -e "1,/^@@/d" actual >actual.body && - expect_from_to >expect.body $subprev $subprev-dirty && - test_cmp expect.body actual.body && - git config --remove-section submodule.subname -' - test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match) [.gitmodules]' ' git config --add -f .gitmodules submodule.subname.ignore all && git config --add -f .gitmodules submodule.subname.path sub && @@ -261,26 +214,6 @@ test_expect_success 'git diff between submodule commits' ' ! test -s actual ' -test_expect_success 'git diff between submodule commits [.git/config]' ' - git diff HEAD^..HEAD >actual && - sed -e "1,/^@@/d" actual >actual.body && - expect_from_to >expect.body $subtip $subprev && - test_cmp expect.body actual.body && - git config submodule.subname.ignore dirty && - git config submodule.subname.path sub && - git diff HEAD^..HEAD >actual && - sed -e "1,/^@@/d" actual >actual.body && - expect_from_to >expect.body $subtip $subprev && - test_cmp expect.body actual.body && - git config submodule.subname.ignore all && - git diff HEAD^..HEAD >actual && - ! test -s actual && - git diff --ignore-submodules=dirty HEAD^..HEAD >actual && - sed -e "1,/^@@/d" actual >actual.body && - expect_from_to >expect.body $subtip $subprev && - git config --remove-section submodule.subname -' - test_expect_success 'git diff between submodule commits [.gitmodules]' ' git diff HEAD^..HEAD >actual && sed -e "1,/^@@/d" actual >actual.body && From 32bc548329d20d33fb25973d76440eccd260c433 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 3 Aug 2017 11:19:56 -0700 Subject: [PATCH 11/15] submodule-config: remove support for overlaying repository config All callers have been migrated to explicitly read any configuration they need. The support for handling it automatically in submodule-config is no longer needed. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- submodule-config.h | 1 - t/helper/test-submodule-config.c | 6 --- t/t7411-submodule-config.sh | 72 -------------------------------- 3 files changed, 79 deletions(-) diff --git a/submodule-config.h b/submodule-config.h index cccd34b929..84c2cf5152 100644 --- a/submodule-config.h +++ b/submodule-config.h @@ -34,7 +34,6 @@ extern int option_fetch_parse_recurse_submodules(const struct option *opt, const char *arg, int unset); extern int parse_update_recurse_submodules_arg(const char *opt, const char *arg); extern int parse_push_recurse_submodules_arg(const char *opt, const char *arg); -extern int parse_submodule_config_option(const char *var, const char *value); extern int submodule_config_option(struct repository *repo, const char *var, const char *value); extern const struct submodule *submodule_from_name( diff --git a/t/helper/test-submodule-config.c b/t/helper/test-submodule-config.c index e13fbcc1b5..f4a7c431c0 100644 --- a/t/helper/test-submodule-config.c +++ b/t/helper/test-submodule-config.c @@ -10,11 +10,6 @@ static void die_usage(int argc, const char **argv, const char *msg) exit(1); } -static int git_test_config(const char *var, const char *value, void *cb) -{ - return parse_submodule_config_option(var, value); -} - int cmd_main(int argc, const char **argv) { const char **arg = argv; @@ -38,7 +33,6 @@ int cmd_main(int argc, const char **argv) setup_git_directory(); gitmodules_config(); - git_config(git_test_config, NULL); while (*arg) { struct object_id commit_oid; diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh index 7d6b25ba23..46c09c7765 100755 --- a/t/t7411-submodule-config.sh +++ b/t/t7411-submodule-config.sh @@ -122,78 +122,6 @@ test_expect_success 'using different treeishs works' ' ) ' -cat >super/expect_url <super/expect_local_path <actual && - test_cmp expect_url actual && - git config submodule.a.path c && - test-submodule-config \ - "" c \ - "" submodule \ - >actual && - test_cmp expect_local_path actual && - git config submodule.a.url "$old_a" && - git config submodule.submodule.url "$old_submodule" && - git config --unset submodule.a.path c - ) -' - -cat >super/expect_url <actual && - test_cmp expect_url actual && - git config submodule.submodule.url "$old_submodule" && - git submodule init b - ) -' - -cat >super/expect_fetchrecurse_die.err <actual.out 2>actual.err && - touch expect_fetchrecurse_die.out && - test_cmp expect_fetchrecurse_die.out actual.out && - test_cmp expect_fetchrecurse_die.err actual.err && - git config --unset submodule.submodule.fetchrecursesubmodules - ) -' - test_expect_success 'error in history in fetchrecursesubmodule lets continue' ' (cd super && git config -f .gitmodules \ From 1b796ace7b5566d7cd2ed2ee56d3e5b1f7605272 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 3 Aug 2017 11:19:57 -0700 Subject: [PATCH 12/15] submodule-config: move submodule-config functions to submodule-config.c Migrate the functions used to initialize the submodule-config to submodule-config.c so that the callback routine used in the initialization process can be static and prevent it from being used outside of initializing the submodule-config through the main API. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- builtin/ls-files.c | 1 + submodule-config.c | 38 +++++++++++++++++++++++++++++++------- submodule-config.h | 7 ++----- submodule.c | 35 ----------------------------------- submodule.h | 2 -- 5 files changed, 34 insertions(+), 49 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index b8514a0029..d14612057e 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -19,6 +19,7 @@ #include "pathspec.h" #include "run-command.h" #include "submodule.h" +#include "submodule-config.h" static int abbrev; static int show_deleted; diff --git a/submodule-config.c b/submodule-config.c index 0b429e9426..86636654bd 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -449,9 +449,9 @@ static int parse_config(const char *var, const char *value, void *data) return ret; } -int gitmodule_oid_from_commit(const struct object_id *treeish_name, - struct object_id *gitmodules_oid, - struct strbuf *rev) +static int gitmodule_oid_from_commit(const struct object_id *treeish_name, + struct object_id *gitmodules_oid, + struct strbuf *rev) { int ret = 0; @@ -552,9 +552,9 @@ static void submodule_cache_check_init(struct repository *repo) submodule_cache_init(repo->submodule_cache); } -int submodule_config_option(struct repository *repo, - const char *var, const char *value) +static int gitmodules_cb(const char *var, const char *value, void *data) { + struct repository *repo = data; struct parse_config_parameter parameter; submodule_cache_check_init(repo); @@ -567,9 +567,33 @@ int submodule_config_option(struct repository *repo, return parse_config(var, value, ¶meter); } -int parse_submodule_config_option(const char *var, const char *value) +void repo_read_gitmodules(struct repository *repo) { - return submodule_config_option(the_repository, var, value); + if (repo->worktree) { + char *gitmodules; + + if (repo_read_index(repo) < 0) + return; + + gitmodules = repo_worktree_path(repo, GITMODULES_FILE); + + if (!is_gitmodules_unmerged(repo->index)) + git_config_from_file(gitmodules_cb, gitmodules, repo); + + free(gitmodules); + } +} + +void gitmodules_config_oid(const struct object_id *commit_oid) +{ + struct strbuf rev = STRBUF_INIT; + struct object_id oid; + + if (gitmodule_oid_from_commit(commit_oid, &oid, &rev)) { + git_config_from_blob_oid(gitmodules_cb, rev.buf, + &oid, the_repository); + } + strbuf_release(&rev); } const struct submodule *submodule_from_name(const struct object_id *treeish_name, diff --git a/submodule-config.h b/submodule-config.h index 84c2cf5152..e3845831f6 100644 --- a/submodule-config.h +++ b/submodule-config.h @@ -34,8 +34,8 @@ extern int option_fetch_parse_recurse_submodules(const struct option *opt, const char *arg, int unset); extern int parse_update_recurse_submodules_arg(const char *opt, const char *arg); extern int parse_push_recurse_submodules_arg(const char *opt, const char *arg); -extern int submodule_config_option(struct repository *repo, - const char *var, const char *value); +extern void repo_read_gitmodules(struct repository *repo); +extern void gitmodules_config_oid(const struct object_id *commit_oid); extern const struct submodule *submodule_from_name( const struct object_id *commit_or_tree, const char *name); extern const struct submodule *submodule_from_path( @@ -43,9 +43,6 @@ extern const struct submodule *submodule_from_path( extern const struct submodule *submodule_from_cache(struct repository *repo, const struct object_id *treeish_name, const char *key); -extern int gitmodule_oid_from_commit(const struct object_id *commit_oid, - struct object_id *gitmodules_oid, - struct strbuf *rev); extern void submodule_free(void); #endif /* SUBMODULE_CONFIG_H */ diff --git a/submodule.c b/submodule.c index 3b383d8c41..c1cef1c373 100644 --- a/submodule.c +++ b/submodule.c @@ -216,46 +216,11 @@ void load_submodule_cache(void) gitmodules_config(); } -static int gitmodules_cb(const char *var, const char *value, void *data) -{ - struct repository *repo = data; - return submodule_config_option(repo, var, value); -} - -void repo_read_gitmodules(struct repository *repo) -{ - if (repo->worktree) { - char *gitmodules; - - if (repo_read_index(repo) < 0) - return; - - gitmodules = repo_worktree_path(repo, GITMODULES_FILE); - - if (!is_gitmodules_unmerged(repo->index)) - git_config_from_file(gitmodules_cb, gitmodules, repo); - - free(gitmodules); - } -} - void gitmodules_config(void) { repo_read_gitmodules(the_repository); } -void gitmodules_config_oid(const struct object_id *commit_oid) -{ - struct strbuf rev = STRBUF_INIT; - struct object_id oid; - - if (gitmodule_oid_from_commit(commit_oid, &oid, &rev)) { - git_config_from_blob_oid(gitmodules_cb, rev.buf, - &oid, the_repository); - } - strbuf_release(&rev); -} - /* * Determine if a submodule has been initialized at a given 'path' */ diff --git a/submodule.h b/submodule.h index 4f70c49444..02195c24fc 100644 --- a/submodule.h +++ b/submodule.h @@ -47,8 +47,6 @@ int option_parse_recurse_submodules_worktree_updater(const struct option *opt, const char *arg, int unset); void load_submodule_cache(void); extern void gitmodules_config(void); -extern void repo_read_gitmodules(struct repository *repo); -extern void gitmodules_config_oid(const struct object_id *commit_oid); extern int is_submodule_active(struct repository *repo, const char *path); /* * Determine if a submodule has been populated at a given 'path' by checking if From ff6f1f564c48def1f8e1852826bab58af5044b06 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 3 Aug 2017 11:19:58 -0700 Subject: [PATCH 13/15] submodule-config: lazy-load a repository's .gitmodules file In order to use the submodule-config subsystem, callers first need to initialize it by calling 'repo_read_gitmodules()' or 'gitmodules_config()' (which just redirects to 'repo_read_gitmodules()'). There are a couple of callers who need to load an explicit revision of the repository's .gitmodules file (grep) or need to modify the .gitmodules file so they would need to load it before modify the file (checkout), but the majority of callers are simply reading the .gitmodules file present in the working tree. For the common case it would be nice to avoid the boilerplate of initializing the submodule-config system before using it, so instead let's perform lazy-loading of the submodule-config system. Remove the calls to reading the gitmodules file from ls-files to show that lazy-loading the .gitmodules file works. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- builtin/ls-files.c | 5 ----- submodule-config.c | 27 ++++++++++++++++++++++----- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index d14612057e..bd74ee07db 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -211,8 +211,6 @@ static void show_submodule(struct repository *superproject, if (repo_read_index(&submodule) < 0) die("index file corrupt"); - repo_read_gitmodules(&submodule); - show_files(&submodule, dir); repo_clear(&submodule); @@ -611,9 +609,6 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (require_work_tree && !is_inside_work_tree()) setup_work_tree(); - if (recurse_submodules) - repo_read_gitmodules(the_repository); - if (recurse_submodules && (show_stage || show_deleted || show_others || show_unmerged || show_killed || show_modified || show_resolve_undo || with_tree)) diff --git a/submodule-config.c b/submodule-config.c index 86636654bd..56d9d76d40 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -18,6 +18,7 @@ struct submodule_cache { struct hashmap for_path; struct hashmap for_name; unsigned initialized:1; + unsigned gitmodules_read:1; }; /* @@ -93,6 +94,7 @@ static void submodule_cache_clear(struct submodule_cache *cache) hashmap_free(&cache->for_path, 1); hashmap_free(&cache->for_name, 1); cache->initialized = 0; + cache->gitmodules_read = 0; } void submodule_cache_free(struct submodule_cache *cache) @@ -557,8 +559,6 @@ static int gitmodules_cb(const char *var, const char *value, void *data) struct repository *repo = data; struct parse_config_parameter parameter; - submodule_cache_check_init(repo); - parameter.cache = repo->submodule_cache; parameter.treeish_name = NULL; parameter.gitmodules_sha1 = null_sha1; @@ -569,6 +569,8 @@ static int gitmodules_cb(const char *var, const char *value, void *data) void repo_read_gitmodules(struct repository *repo) { + submodule_cache_check_init(repo); + if (repo->worktree) { char *gitmodules; @@ -582,6 +584,8 @@ void repo_read_gitmodules(struct repository *repo) free(gitmodules); } + + repo->submodule_cache->gitmodules_read = 1; } void gitmodules_config_oid(const struct object_id *commit_oid) @@ -589,24 +593,37 @@ void gitmodules_config_oid(const struct object_id *commit_oid) struct strbuf rev = STRBUF_INIT; struct object_id oid; + submodule_cache_check_init(the_repository); + if (gitmodule_oid_from_commit(commit_oid, &oid, &rev)) { git_config_from_blob_oid(gitmodules_cb, rev.buf, &oid, the_repository); } strbuf_release(&rev); + + the_repository->submodule_cache->gitmodules_read = 1; +} + +static void gitmodules_read_check(struct repository *repo) +{ + submodule_cache_check_init(repo); + + /* read the repo's .gitmodules file if it hasn't been already */ + if (!repo->submodule_cache->gitmodules_read) + repo_read_gitmodules(repo); } const struct submodule *submodule_from_name(const struct object_id *treeish_name, const char *name) { - submodule_cache_check_init(the_repository); + gitmodules_read_check(the_repository); return config_from(the_repository->submodule_cache, treeish_name, name, lookup_name); } const struct submodule *submodule_from_path(const struct object_id *treeish_name, const char *path) { - submodule_cache_check_init(the_repository); + gitmodules_read_check(the_repository); return config_from(the_repository->submodule_cache, treeish_name, path, lookup_path); } @@ -614,7 +631,7 @@ const struct submodule *submodule_from_cache(struct repository *repo, const struct object_id *treeish_name, const char *key) { - submodule_cache_check_init(repo); + gitmodules_read_check(repo); return config_from(repo->submodule_cache, treeish_name, key, lookup_path); } From 33028713206c3f59709617d8af5ba4212920a5f0 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 3 Aug 2017 11:19:59 -0700 Subject: [PATCH 14/15] unpack-trees: improve loading of .gitmodules When recursing submodules 'check_updates()' needs to have strict control over the submodule-config subsystem to ensure that the gitmodules file has been read before checking cache entries which are marked for removal as well ensuring the proper gitmodules file is read before updating cache entries. Because of this let's not rely on callers of 'check_updates()' to read the gitmodules file before calling 'check_updates()' and handle the reading explicitly. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- unpack-trees.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/unpack-trees.c b/unpack-trees.c index 5dce7ff7d4..3c7f464fae 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1,5 +1,6 @@ #define NO_THE_INDEX_COMPATIBILITY_MACROS #include "cache.h" +#include "repository.h" #include "config.h" #include "dir.h" #include "tree.h" @@ -268,22 +269,28 @@ static int check_submodule_move_head(const struct cache_entry *ce, return 0; } -static void reload_gitmodules_file(struct index_state *index, - struct checkout *state) +/* + * Preform the loading of the repository's gitmodules file. This function is + * used by 'check_update()' to perform loading of the gitmodules file in two + * differnt situations: + * (1) before removing entries from the working tree if the gitmodules file has + * been marked for removal. This situation is specified by 'state' == NULL. + * (2) before checking out entries to the working tree if the gitmodules file + * has been marked for update. This situation is specified by 'state' != NULL. + */ +static void load_gitmodules_file(struct index_state *index, + struct checkout *state) { - int i; - for (i = 0; i < index->cache_nr; i++) { - struct cache_entry *ce = index->cache[i]; - if (ce->ce_flags & CE_UPDATE) { - int r = strcmp(ce->name, GITMODULES_FILE); - if (r < 0) - continue; - else if (r == 0) { - submodule_free(); - checkout_entry(ce, state, NULL); - gitmodules_config(); - } else - break; + int pos = index_name_pos(index, GITMODULES_FILE, strlen(GITMODULES_FILE)); + + if (pos >= 0) { + struct cache_entry *ce = index->cache[pos]; + if (!state && ce->ce_flags & CE_WT_REMOVE) { + repo_read_gitmodules(the_repository); + } else if (state && (ce->ce_flags & CE_UPDATE)) { + submodule_free(); + checkout_entry(ce, state, NULL); + repo_read_gitmodules(the_repository); } } } @@ -343,6 +350,10 @@ static int check_updates(struct unpack_trees_options *o) if (o->update) git_attr_set_direction(GIT_ATTR_CHECKOUT, index); + + if (should_update_submodules() && o->update && !o->dry_run) + load_gitmodules_file(index, NULL); + for (i = 0; i < index->cache_nr; i++) { const struct cache_entry *ce = index->cache[i]; @@ -356,7 +367,7 @@ static int check_updates(struct unpack_trees_options *o) remove_scheduled_dirs(); if (should_update_submodules() && o->update && !o->dry_run) - reload_gitmodules_file(index, &state); + load_gitmodules_file(index, &state); for (i = 0; i < index->cache_nr; i++) { struct cache_entry *ce = index->cache[i]; From 557a5998df19faf8641acfc5b6b1c3c2ba64dca9 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 3 Aug 2017 11:20:00 -0700 Subject: [PATCH 15/15] submodule: remove gitmodules_config Now that the submodule-config subsystem can lazily read the gitmodules file we no longer need to explicitly pre-read the gitmodules by calling 'gitmodules_config()' so let's remove it. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- builtin/checkout.c | 1 - builtin/commit.c | 1 - builtin/diff-files.c | 1 - builtin/diff-index.c | 1 - builtin/diff-tree.c | 1 - builtin/diff.c | 2 -- builtin/fetch.c | 4 ---- builtin/grep.c | 4 ---- builtin/mv.c | 1 - builtin/read-tree.c | 2 -- builtin/reset.c | 2 -- builtin/rm.c | 1 - builtin/submodule--helper.c | 14 -------------- submodule.c | 15 --------------- submodule.h | 2 -- t/helper/test-submodule-config.c | 1 - 16 files changed, 53 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 246e0cd166..63ae16afcf 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1179,7 +1179,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) opts.prefix = prefix; opts.show_progress = -1; - gitmodules_config(); git_config(git_checkout_config, &opts); opts.track = BRANCH_TRACK_UNSPECIFIED; diff --git a/builtin/commit.c b/builtin/commit.c index 4bbac014ab..18ad714d95 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -195,7 +195,6 @@ static void determine_whence(struct wt_status *s) static void status_init_config(struct wt_status *s, config_fn_t fn) { wt_status_prepare(s); - gitmodules_config(); git_config(fn, s); determine_whence(s); init_diff_ui_defaults(); diff --git a/builtin/diff-files.c b/builtin/diff-files.c index 17bf84d18f..e88493ffe5 100644 --- a/builtin/diff-files.c +++ b/builtin/diff-files.c @@ -26,7 +26,6 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix) git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ init_revisions(&rev, prefix); - gitmodules_config(); rev.abbrev = 0; precompose_argv(argc, argv); diff --git a/builtin/diff-index.c b/builtin/diff-index.c index 185e6f9b58..9d772f8f27 100644 --- a/builtin/diff-index.c +++ b/builtin/diff-index.c @@ -23,7 +23,6 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix) git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ init_revisions(&rev, prefix); - gitmodules_config(); rev.abbrev = 0; precompose_argv(argc, argv); diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c index 31d2cb4107..d66499909e 100644 --- a/builtin/diff-tree.c +++ b/builtin/diff-tree.c @@ -110,7 +110,6 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix) git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ init_revisions(opt, prefix); - gitmodules_config(); opt->abbrev = 0; opt->diff = 1; opt->disable_stdin = 1; diff --git a/builtin/diff.c b/builtin/diff.c index 7cde6abbcf..7e3ebcea38 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -315,8 +315,6 @@ int cmd_diff(int argc, const char **argv, const char *prefix) no_index = DIFF_NO_INDEX_IMPLICIT; } - if (!no_index) - gitmodules_config(); init_diff_ui_defaults(); git_config(git_diff_ui_config, NULL); precompose_argv(argc, argv); diff --git a/builtin/fetch.c b/builtin/fetch.c index 3fe99073d3..132e3224ed 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1360,10 +1360,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) if (depth || deepen_since || deepen_not.nr) deepen = 1; - if (recurse_submodules != RECURSE_SUBMODULES_OFF) { - gitmodules_config(); - } - if (all) { if (argc == 1) die(_("fetch --all does not take a repository argument")); diff --git a/builtin/grep.c b/builtin/grep.c index ac06d2d33c..2d65f27d01 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -1048,10 +1048,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix) } #endif - if (recurse_submodules) { - gitmodules_config(); - } - if (show_in_pager && (cached || list.nr)) die(_("--open-files-in-pager only works on the worktree")); diff --git a/builtin/mv.c b/builtin/mv.c index 94fbaaa5da..ffdd5f01a1 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -131,7 +131,6 @@ int cmd_mv(int argc, const char **argv, const char *prefix) struct stat st; struct string_list src_for_dst = STRING_LIST_INIT_NODUP; - gitmodules_config(); git_config(git_default_config, NULL); argc = parse_options(argc, argv, prefix, builtin_mv_options, diff --git a/builtin/read-tree.c b/builtin/read-tree.c index d5f618d086..bf87a2710b 100644 --- a/builtin/read-tree.c +++ b/builtin/read-tree.c @@ -164,8 +164,6 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) argc = parse_options(argc, argv, unused_prefix, read_tree_options, read_tree_usage, 0); - load_submodule_cache(); - hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR); prefix_set = opts.prefix ? 1 : 0; diff --git a/builtin/reset.c b/builtin/reset.c index 772d078b85..50488d2738 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -309,8 +309,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix) PARSE_OPT_KEEP_DASHDASH); parse_args(&pathspec, argv, prefix, patch_mode, &rev); - load_submodule_cache(); - unborn = !strcmp(rev, "HEAD") && get_oid("HEAD", &oid); if (unborn) { /* reset on unborn branch: treat as reset to empty tree */ diff --git a/builtin/rm.c b/builtin/rm.c index 4057e73fa0..d91451fea1 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -255,7 +255,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix) struct pathspec pathspec; char *seen; - gitmodules_config(); git_config(git_default_config, NULL); argc = parse_options(argc, argv, prefix, builtin_rm_options, diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index ba767c7048..c97fde4396 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -275,8 +275,6 @@ static void module_list_active(struct module_list *list) int i; struct module_list active_modules = MODULE_LIST_INIT; - gitmodules_config(); - for (i = 0; i < list->nr; i++) { const struct cache_entry *ce = list->entries[i]; @@ -337,9 +335,6 @@ static void init_submodule(const char *path, const char *prefix, int quiet) struct strbuf sb = STRBUF_INIT; char *upd = NULL, *url = NULL, *displaypath; - /* Only loads from .gitmodules, no overlay with .git/config */ - gitmodules_config(); - if (prefix && get_super_prefix()) die("BUG: cannot have prefix and superprefix"); else if (prefix) @@ -475,7 +470,6 @@ static int module_name(int argc, const char **argv, const char *prefix) if (argc != 2) usage(_("git submodule--helper name ")); - gitmodules_config(); sub = submodule_from_path(&null_oid, argv[1]); if (!sub) @@ -1042,8 +1036,6 @@ static int update_clone(int argc, const char **argv, const char *prefix) if (pathspec.nr) suc.warn_if_uninitialized = 1; - gitmodules_config(); - run_processes_parallel(max_jobs, update_clone_get_next_task, update_clone_start_failure, @@ -1084,8 +1076,6 @@ static const char *remote_submodule_branch(const char *path) const char *branch = NULL; char *key; - gitmodules_config(); - sub = submodule_from_path(&null_oid, path); if (!sub) return NULL; @@ -1204,8 +1194,6 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, embed_gitdir_options, git_submodule_helper_usage, 0); - gitmodules_config(); - if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0) return 1; @@ -1221,8 +1209,6 @@ static int is_active(int argc, const char **argv, const char *prefix) if (argc != 2) die("submodule--helper is-active takes exactly 1 argument"); - gitmodules_config(); - return !is_submodule_active(the_repository, argv[1]); } diff --git a/submodule.c b/submodule.c index c1cef1c373..77346da886 100644 --- a/submodule.c +++ b/submodule.c @@ -208,19 +208,6 @@ int option_parse_recurse_submodules_worktree_updater(const struct option *opt, return 0; } -void load_submodule_cache(void) -{ - if (config_update_recurse_submodules == RECURSE_SUBMODULES_OFF) - return; - - gitmodules_config(); -} - -void gitmodules_config(void) -{ - repo_read_gitmodules(the_repository); -} - /* * Determine if a submodule has been initialized at a given 'path' */ @@ -1093,7 +1080,6 @@ int submodule_touches_in_range(struct object_id *excl_oid, struct argv_array args = ARGV_ARRAY_INIT; int ret; - gitmodules_config(); /* No need to check if there are no submodules configured */ if (!submodule_from_path(NULL, NULL)) return 0; @@ -2000,7 +1986,6 @@ int submodule_to_gitdir(struct strbuf *buf, const char *submodule) strbuf_addstr(buf, git_dir); } if (!is_git_directory(buf->buf)) { - gitmodules_config(); sub = submodule_from_path(&null_oid, submodule); if (!sub) { ret = -1; diff --git a/submodule.h b/submodule.h index 02195c24fc..be103ad9d9 100644 --- a/submodule.h +++ b/submodule.h @@ -45,8 +45,6 @@ extern int git_default_submodule_config(const char *var, const char *value, void struct option; int option_parse_recurse_submodules_worktree_updater(const struct option *opt, const char *arg, int unset); -void load_submodule_cache(void); -extern void gitmodules_config(void); extern int is_submodule_active(struct repository *repo, const char *path); /* * Determine if a submodule has been populated at a given 'path' by checking if diff --git a/t/helper/test-submodule-config.c b/t/helper/test-submodule-config.c index f4a7c431c0..f23db3b19a 100644 --- a/t/helper/test-submodule-config.c +++ b/t/helper/test-submodule-config.c @@ -32,7 +32,6 @@ int cmd_main(int argc, const char **argv) die_usage(argc, argv, "Wrong number of arguments."); setup_git_directory(); - gitmodules_config(); while (*arg) { struct object_id commit_oid;