From d7a3803f9e83242adac0f02af843ef0520c71f0a Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Fri, 26 May 2017 12:10:12 -0700 Subject: reset/checkout/read-tree: unify config callback for submodule recursion The callback function is essentially duplicated 3 times. Remove all of them and offer a new callback function, that lives in submodule.c By putting the callback function there, we no longer need the function 'set_config_update_recurse_submodules', nor duplicate the global variable in each builtin as well as submodule.c In the three builtins we have different 2 ways how to load the .gitmodules and config file, which are slightly different. git-checkout has to load the submodule config all the time due to 23b4c7bcc5 (checkout: Use submodule.*.ignore settings from .git/config and .gitmodules, 2010-08-28) git-reset and git-read-tree do not respect these diff settings, so loading the submodule configuration is optional. Also put that into submodule.c for code deduplication. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- submodule.c | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) (limited to 'submodule.c') diff --git a/submodule.c b/submodule.c index 54825100b2..c9e764b519 100644 --- a/submodule.c +++ b/submodule.c @@ -18,7 +18,7 @@ #include "worktree.h" static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND; -static int config_update_recurse_submodules = RECURSE_SUBMODULES_DEFAULT; +static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF; static int parallel_jobs = 1; static struct string_list changed_submodule_paths = STRING_LIST_INIT_DUP; static int initialized_fetch_ref_tips; @@ -169,6 +169,32 @@ int submodule_config(const char *var, const char *value, void *cb) return 0; } +int option_parse_recurse_submodules_worktree_updater(const struct option *opt, + const char *arg, int unset) +{ + if (unset) { + config_update_recurse_submodules = RECURSE_SUBMODULES_OFF; + return 0; + } + if (arg) + config_update_recurse_submodules = + parse_update_recurse_submodules_arg(opt->long_name, + arg); + else + config_update_recurse_submodules = RECURSE_SUBMODULES_ON; + + return 0; +} + +void load_submodule_cache(void) +{ + if (config_update_recurse_submodules == RECURSE_SUBMODULES_OFF) + return; + + gitmodules_config(); + git_config(submodule_config, NULL); +} + void gitmodules_config(void) { const char *work_tree = get_git_work_tree(); @@ -596,11 +622,6 @@ void set_config_fetch_recurse_submodules(int value) config_fetch_recurse_submodules = value; } -void set_config_update_recurse_submodules(int value) -{ - config_update_recurse_submodules = value; -} - int should_update_submodules(void) { return config_update_recurse_submodules == RECURSE_SUBMODULES_ON; -- cgit v1.2.3 From 1d789d089280539ca39b83aabb67860929d39b75 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Fri, 26 May 2017 12:10:13 -0700 Subject: submodule loading: separate code path for .gitmodules and config overlay The .gitmodules file is not supposed to have all the options available, that are available in the configuration so separate it out. A configuration option such as the hypothetical submodule.color.diff that determines in which color a submodule change is printed, is a very user specific thing, that the .gitmodules file should not tamper with. The .gitmodules file should only be used for settings that required to setup the project in which the .gitmodules file is tracked. As the minimum this would only include the name<->path mapping of the submodule and its URL and branch. Any further setting (such as 'fetch.recursesubmodules' or 'submodule..{update, ignore, shallow}') is not specific to the project setup requirements, but rather is a distribution of suggested developer configurations. In other areas of Git a suggested developer configuration is not transported in-tree but via other means. In an organisation this could be done by deploying an opinionated system wide config (/etc/gitconfig) or by putting the settings in the users home directory when they start at the organisation. In open source projects this is often accomplished via extensive READMEs (cf. our SubmittingPatches/CodingGuidlines). As a later patch in this series wants to introduce a generic submodule recursion option, we want to make sure that switch is not exposed via the gitmodules file. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- submodule.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) (limited to 'submodule.c') diff --git a/submodule.c b/submodule.c index c9e764b519..78cccb7563 100644 --- a/submodule.c +++ b/submodule.c @@ -153,7 +153,8 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt, } } -int submodule_config(const char *var, const char *value, void *cb) +/* For loading from the .gitmodules file. */ +static int git_modules_config(const char *var, const char *value, void *cb) { if (!strcmp(var, "submodule.fetchjobs")) { parallel_jobs = git_config_int(var, value); @@ -169,6 +170,12 @@ int submodule_config(const char *var, const char *value, void *cb) return 0; } +/* Loads all submodule settings from the config */ +int submodule_config(const char *var, const char *value, void *cb) +{ + return git_modules_config(var, value, cb); +} + int option_parse_recurse_submodules_worktree_updater(const struct option *opt, const char *arg, int unset) { @@ -222,7 +229,8 @@ void gitmodules_config(void) } if (!gitmodules_is_unmerged) - git_config_from_file(submodule_config, gitmodules_path.buf, NULL); + git_config_from_file(git_modules_config, + gitmodules_path.buf, NULL); strbuf_release(&gitmodules_path); } } @@ -233,7 +241,7 @@ void gitmodules_config_sha1(const unsigned char *commit_sha1) unsigned char sha1[20]; if (gitmodule_sha1_from_commit(commit_sha1, sha1, &rev)) { - git_config_from_blob_sha1(submodule_config, rev.buf, + git_config_from_blob_sha1(git_modules_config, rev.buf, sha1, NULL); } strbuf_release(&rev); -- cgit v1.2.3 From 046b48239eca34425909330e59da57f5fd421bdc Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Wed, 31 May 2017 17:30:47 -0700 Subject: Introduce 'submodule.recurse' option for worktree manipulators Any command that understands '--recurse-submodules' can have its default changed to true, by setting the new 'submodule.recurse' option. This patch includes read-tree/checkout/reset for working tree manipulating commands. Later patches will cover other commands. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- Documentation/config.txt | 5 +++++ builtin/checkout.c | 2 +- builtin/read-tree.c | 10 +++++++++- builtin/reset.c | 10 +++++++++- submodule.c | 23 +++++++++++++++++++++-- submodule.h | 1 + t/lib-submodule-update.sh | 12 ++++++++++++ 7 files changed, 58 insertions(+), 5 deletions(-) (limited to 'submodule.c') diff --git a/Documentation/config.txt b/Documentation/config.txt index e0b9fd0bc3..f60c504e86 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -3065,6 +3065,11 @@ submodule.active:: submodule's path to determine if the submodule is of interest to git commands. +submodule.recurse:: + Specifies if commands recurse into submodules by default. This + applies to all commands that have a `--recurse-submodules` option. + Defaults to false. + submodule.fetchJobs:: Specifies how many submodules are fetched/cloned at the same time. A positive integer allows up to that number of submodules fetched diff --git a/builtin/checkout.c b/builtin/checkout.c index 56ea723b75..e289b7d477 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -855,7 +855,7 @@ static int git_checkout_config(const char *var, const char *value, void *cb) } if (starts_with(var, "submodule.")) - return parse_submodule_config_option(var, value); + return submodule_config(var, value, NULL); return git_xmerge_config(var, value, NULL); } diff --git a/builtin/read-tree.c b/builtin/read-tree.c index 8a889ef4c3..7fd55140db 100644 --- a/builtin/read-tree.c +++ b/builtin/read-tree.c @@ -98,6 +98,14 @@ static int debug_merge(const struct cache_entry * const *stages, return 0; } +static int git_read_tree_config(const char *var, const char *value, void *cb) +{ + if (!strcmp(var, "submodule.recurse")) + return git_default_submodule_config(var, value, cb); + + return git_default_config(var, value, cb); +} + static struct lock_file lock_file; int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) @@ -150,7 +158,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) opts.src_index = &the_index; opts.dst_index = &the_index; - git_config(git_default_config, NULL); + git_config(git_read_tree_config, NULL); argc = parse_options(argc, argv, unused_prefix, read_tree_options, read_tree_usage, 0); diff --git a/builtin/reset.c b/builtin/reset.c index 6f89dc5494..585cfe0745 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -266,6 +266,14 @@ static int reset_refs(const char *rev, const struct object_id *oid) return update_ref_status; } +static int git_reset_config(const char *var, const char *value, void *cb) +{ + if (!strcmp(var, "submodule.recurse")) + return git_default_submodule_config(var, value, cb); + + return git_default_config(var, value, cb); +} + int cmd_reset(int argc, const char **argv, const char *prefix) { int reset_type = NONE, update_ref_status = 0, quiet = 0; @@ -294,7 +302,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) OPT_END() }; - git_config(git_default_config, NULL); + git_config(git_reset_config, NULL); argc = parse_options(argc, argv, prefix, options, git_reset_usage, PARSE_OPT_KEEP_DASHDASH); diff --git a/submodule.c b/submodule.c index 78cccb7563..2b157dc995 100644 --- a/submodule.c +++ b/submodule.c @@ -16,6 +16,7 @@ #include "quote.h" #include "remote.h" #include "worktree.h" +#include "parse-options.h" static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND; static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF; @@ -170,10 +171,28 @@ static int git_modules_config(const char *var, const char *value, void *cb) return 0; } -/* Loads all submodule settings from the config */ +/* Loads all submodule settings from the config. */ int submodule_config(const char *var, const char *value, void *cb) { - return git_modules_config(var, value, 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) +{ + 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; } int option_parse_recurse_submodules_worktree_updater(const struct option *opt, diff --git a/submodule.h b/submodule.h index b13f120f76..d920ca1d5a 100644 --- a/submodule.h +++ b/submodule.h @@ -39,6 +39,7 @@ 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; int option_parse_recurse_submodules_worktree_updater(const struct option *opt, diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh index 0272c4d8ca..52beadad96 100755 --- a/t/lib-submodule-update.sh +++ b/t/lib-submodule-update.sh @@ -990,6 +990,18 @@ test_submodule_switch_recursing_with_args () { ) ' + test_expect_success "git -c submodule.recurse=true $cmd_args: modified submodule updates submodule work tree" ' + prolog && + reset_work_tree_to_interested add_sub1 && + ( + cd submodule_update && + git branch -t modify_sub1 origin/modify_sub1 && + git -c submodule.recurse=true $cmd_args modify_sub1 && + test_superproject_content origin/modify_sub1 && + test_submodule_content sub1 origin/modify_sub1 + ) + ' + # Updating a submodule to an invalid sha1 doesn't update the # superproject nor the submodule's work tree. test_expect_success "$command: updating to a missing submodule commit fails" ' -- cgit v1.2.3