{"thread":{"id":"45992","subject":"[PATCH 2/3] submodule test invocation: only pass additional arguments","startedAt":"2017-05-17T21:32:11Z","lastAt":"2017-05-18T19:15:38Z","messageCount":13,"participants":["Stefan Beller","Junio C Hamano","Brandon Williams"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"320107","messageId":"20170517213135.20988-3-sbeller@google.com","threadId":"45992","inReplyTo":"20170517213135.20988-1-sbeller@google.com","subject":"[PATCH 2/3] submodule test invocation: only pass additional arguments","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-05-17T21:31:34Z","receivedAt":"2017-05-17T21:32:11Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In a later patch we want to test a command without the\n'--recurse-submodules' given, but instead we'd give a '-c <option>.\n\nTo enable that we'll just pass the minimum required to the submodule\ntesting, such that we can construct the command with the option easier.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n t/lib-submodule-update.sh      | 6 ++++--\n t/t1013-read-tree-submodule.sh | 4 ++--\n t/t2013-checkout-submodule.sh  | 4 ++--\n t/t7112-reset-submodule.sh     | 4 ++--\n 4 files changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh\nindex f0b1b18206..0f70b5ec7b 100755\n--- a/t/lib-submodule-update.sh\n+++ b/t/lib-submodule-update.sh\n@@ -782,7 +782,8 @@ test_submodule_forced_switch () {\n #   git directory first into the superproject.\n \n test_submodule_switch_recursing () {\n-\tcommand=\"$1\"\n+\tcmd_args=\"$1\"\n+\tcommand=\"git $cmd_args --recurse-submodules\"\n \tRESULTDS=success\n \tif test \"$KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS\" = 1\n \tthen\n@@ -1022,7 +1023,8 @@ test_submodule_switch_recursing () {\n # that change a submodule, but throwing away local changes in\n # the superproject as well as the submodule is allowed.\n test_submodule_forced_switch_recursing () {\n-\tcommand=\"$1\"\n+\tcmd_args=\"$1\"\n+\tcommand=\"git $cmd_args --recurse-submodules\"\n \tRESULT=success\n \tif test \"$KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS\" = 1\n \tthen\ndiff --git a/t/t1013-read-tree-submodule.sh b/t/t1013-read-tree-submodule.sh\nindex de1ba02dc5..a779e6917c 100755\n--- a/t/t1013-read-tree-submodule.sh\n+++ b/t/t1013-read-tree-submodule.sh\n@@ -9,9 +9,9 @@ KNOWN_FAILURE_SUBMODULE_RECURSIVE_NESTED=1\n KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1\n KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED=1\n \n-test_submodule_switch_recursing \"git read-tree --recurse-submodules -u -m\"\n+test_submodule_switch_recursing \"read-tree -u -m\"\n \n-test_submodule_forced_switch_recursing \"git read-tree --recurse-submodules -u --reset\"\n+test_submodule_forced_switch_recursing \"read-tree -u --reset\"\n \n test_submodule_switch \"git read-tree -u -m\"\n \ndiff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh\nindex e8f70b806f..2672f104cf 100755\n--- a/t/t2013-checkout-submodule.sh\n+++ b/t/t2013-checkout-submodule.sh\n@@ -65,9 +65,9 @@ test_expect_success '\"checkout <submodule>\" honors submodule.*.ignore from .git/\n \n KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1\n KNOWN_FAILURE_SUBMODULE_RECURSIVE_NESTED=1\n-test_submodule_switch_recursing \"git checkout --recurse-submodules\"\n+test_submodule_switch_recursing \"checkout\"\n \n-test_submodule_forced_switch_recursing \"git checkout -f --recurse-submodules\"\n+test_submodule_forced_switch_recursing \"checkout -f\"\n \n test_submodule_switch \"git checkout\"\n \ndiff --git a/t/t7112-reset-submodule.sh b/t/t7112-reset-submodule.sh\nindex f86ccdf215..a000304221 100755\n--- a/t/t7112-reset-submodule.sh\n+++ b/t/t7112-reset-submodule.sh\n@@ -9,9 +9,9 @@ KNOWN_FAILURE_SUBMODULE_RECURSIVE_NESTED=1\n KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1\n KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED=1\n \n-test_submodule_switch_recursing \"git reset --recurse-submodules --keep\"\n+test_submodule_switch_recursing \"reset --keep\"\n \n-test_submodule_forced_switch_recursing \"git reset --hard --recurse-submodules\"\n+test_submodule_forced_switch_recursing \"reset --hard\"\n \n test_submodule_switch \"git reset --keep\"\n \n-- \n2.13.0.18.g7d86cc8ba0\n\n"},{"id":"320108","messageId":"20170517213135.20988-2-sbeller@google.com","threadId":"45992","inReplyTo":"20170517213135.20988-1-sbeller@google.com","subject":"[PATCH 1/3] submodule.c: add has_submodules to check if we have any submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-05-17T21:31:33Z","receivedAt":"2017-05-17T21:32:13Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When submodules are involved, it often slows down the process, as most\nsubmodule related handling is either done via a child process or by\niterating over the index finding all gitlinks.\n\nFor most commands that may interact with submodules, we need have a\nquick check if we do have any submodules at all, such that we can\nbe fast in the case when no submodules are in use.  For this quick\ncheck introduce a function that checks with different heuristics if\nwe do have submodules around, checking if\n* anything related to submodules is configured,\n* absorbed git dirs for submodules are present,\n* the '.gitmodules' file exists\n* gitlinks are recorded in the index.\n\nEach heuristic has advantages and disadvantages.\nFor example in a later patch, when we first use this function in\ngit-clone, we'll just check for the existence of the '.gitmodules'\nfile, because at the time of running the clone command there will\nbe no absorbed git dirs or submodule configuration around.\n\nChecking for any configuration related to submodules would be useful\nin a later stage (after cloning) to see if the submodules are actually\nin use.\n\nChecking for absorbed git directories is good to see if the user has\nactually cloned submodules already (i.e. not just initialized them by\nconfiguring them).\n\nThe heuristic for checking the configuration requires this patch\nto have have a global state, whether the submodule config has already\nbeen read, and if there were any submodule related keys. Make\n'submodule_config' private to the submodule code, and introduce\n'load_submodule_config' that will take care of this global state.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/checkout.c          |  2 +-\n builtin/fetch.c             |  2 +-\n builtin/read-tree.c         |  2 +-\n builtin/reset.c             |  2 +-\n builtin/submodule--helper.c |  6 ++--\n submodule.c                 | 78 +++++++++++++++++++++++++++++++++++----------\n submodule.h                 |  8 ++++-\n unpack-trees.c              |  2 +-\n 8 files changed, 77 insertions(+), 25 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex bfa5419f33..2787b343b1 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1215,7 +1215,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (recurse_submodules != RECURSE_SUBMODULES_OFF) {\n-\t\tgit_config(submodule_config, NULL);\n+\t\tload_submodule_config();\n \t\tif (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)\n \t\t\tset_config_update_recurse_submodules(recurse_submodules);\n \t}\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 4ef7a08afc..510ef1c9de 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1344,7 +1344,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t\t\tset_config_fetch_recurse_submodules(arg);\n \t\t}\n \t\tgitmodules_config();\n-\t\tgit_config(submodule_config, NULL);\n+\t\tload_submodule_config();\n \t}\n \n \tif (all) {\ndiff --git a/builtin/read-tree.c b/builtin/read-tree.c\nindex 23e212ee8c..fe2ec60a51 100644\n--- a/builtin/read-tree.c\n+++ b/builtin/read-tree.c\n@@ -177,7 +177,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \n \tif (recurse_submodules != RECURSE_SUBMODULES_DEFAULT) {\n \t\tgitmodules_config();\n-\t\tgit_config(submodule_config, NULL);\n+\t\tload_submodule_config();\n \t\tset_config_update_recurse_submodules(RECURSE_SUBMODULES_ON);\n \t}\n \ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 5ce27fcaed..f3aca487d9 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -321,7 +321,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \n \tif (recurse_submodules != RECURSE_SUBMODULES_DEFAULT) {\n \t\tgitmodules_config();\n-\t\tgit_config(submodule_config, NULL);\n+\t\tload_submodule_config();\n \t\tset_config_update_recurse_submodules(RECURSE_SUBMODULES_ON);\n \t}\n \ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 85aafe46a4..770a95ca14 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1015,7 +1015,7 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \n \t/* Overlay the parsed .gitmodules file with .git/config */\n \tgitmodules_config();\n-\tgit_config(submodule_config, NULL);\n+\tload_submodule_config();\n \n \tif (max_jobs < 0)\n \t\tmax_jobs = parallel_submodules();\n@@ -1058,7 +1058,7 @@ static const char *remote_submodule_branch(const char *path)\n {\n \tconst struct submodule *sub;\n \tgitmodules_config();\n-\tgit_config(submodule_config, NULL);\n+\tload_submodule_config();\n \n \tsub = submodule_from_path(null_sha1, path);\n \tif (!sub)\n@@ -1130,7 +1130,7 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)\n \t\t\t     git_submodule_helper_usage, 0);\n \n \tgitmodules_config();\n-\tgit_config(submodule_config, NULL);\n+\tload_submodule_config();\n \n \tif (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)\n \t\treturn 1;\ndiff --git a/submodule.c b/submodule.c\nindex 20ed5b5681..14ea405048 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -24,6 +24,12 @@ static int initialized_fetch_ref_tips;\n static struct sha1_array ref_tips_before_fetch;\n static struct sha1_array ref_tips_after_fetch;\n \n+static enum {\n+\tSUBMODULE_CONFIG_NOT_READ = 0,\n+\tSUBMODULE_CONFIG_NO_CONFIG,\n+\tSUBMODULE_CONFIG_EXISTS,\n+} submodule_config_reading;\n+\n /*\n  * The following flag is set if the .gitmodules file is unmerged. We then\n  * disable recursion for all submodules where .git/config doesn't have a\n@@ -83,6 +89,62 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n \treturn 0;\n }\n \n+static int submodule_config(const char *var, const char *value, void *cb)\n+{\n+\tif (!strcmp(var, \"submodule.fetchjobs\")) {\n+\t\tsubmodule_config_reading = SUBMODULE_CONFIG_EXISTS;\n+\t\tparallel_jobs = git_config_int(var, value);\n+\t\tif (parallel_jobs < 0)\n+\t\t\tdie(_(\"negative values not allowed for submodule.fetchJobs\"));\n+\t\treturn 0;\n+\t} else if (starts_with(var, \"submodule.\")) {\n+\t\tsubmodule_config_reading = SUBMODULE_CONFIG_EXISTS;\n+\t\treturn parse_submodule_config_option(var, value);\n+\t} else if (!strcmp(var, \"fetch.recursesubmodules\")) {\n+\t\tsubmodule_config_reading = SUBMODULE_CONFIG_EXISTS;\n+\t\tconfig_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(var, value);\n+\t\treturn 0;\n+\t}\n+\treturn 0;\n+}\n+\n+void load_submodule_config(void)\n+{\n+\tsubmodule_config_reading = SUBMODULE_CONFIG_NO_CONFIG;\n+\tgit_config(submodule_config, NULL);\n+}\n+\n+int has_submodules(unsigned what_to_check)\n+{\n+\tif (what_to_check & SUBMODULE_CHECK_ANY_CONFIG) {\n+\t\tif (submodule_config_reading == SUBMODULE_CONFIG_NOT_READ)\n+\t\t\tload_submodule_config();\n+\t\tif (submodule_config_reading == SUBMODULE_CONFIG_EXISTS)\n+\t\t\treturn 1;\n+\t}\n+\n+\tif ((what_to_check & SUBMODULE_CHECK_ABSORBED_GIT_DIRS) &&\n+\t    file_exists(git_path(\"modules\")))\n+\t\treturn 1;\n+\n+\tif ((what_to_check & SUBMODULE_CHECK_GITMODULES_IN_WT) &&\n+\t    (!is_bare_repository() && file_exists(\".gitmodules\")))\n+\t\treturn 1;\n+\n+\tif (what_to_check & SUBMODULE_CHECK_GITLINKS_IN_TREE) {\n+\t\tint i;\n+\n+\t\tif (read_cache() < 0)\n+\t\t\tdie(_(\"index file corrupt\"));\n+\n+\t\tfor (i = 0; i < active_nr; i++)\n+\t\t\tif (S_ISGITLINK(active_cache[i]->ce_mode))\n+\t\t\t\treturn 1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n /*\n  * Try to remove the \"submodule.<name>\" section from .gitmodules where the given\n  * path is configured. Return 0 only if a .gitmodules file was found, a section\n@@ -152,22 +214,6 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \t}\n }\n \n-int submodule_config(const char *var, const char *value, void *cb)\n-{\n-\tif (!strcmp(var, \"submodule.fetchjobs\")) {\n-\t\tparallel_jobs = git_config_int(var, value);\n-\t\tif (parallel_jobs < 0)\n-\t\t\tdie(_(\"negative values not allowed for submodule.fetchJobs\"));\n-\t\treturn 0;\n-\t} else if (starts_with(var, \"submodule.\"))\n-\t\treturn parse_submodule_config_option(var, value);\n-\telse if (!strcmp(var, \"fetch.recursesubmodules\")) {\n-\t\tconfig_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(var, value);\n-\t\treturn 0;\n-\t}\n-\treturn 0;\n-}\n-\n void gitmodules_config(void)\n {\n \tconst char *work_tree = get_git_work_tree();\ndiff --git a/submodule.h b/submodule.h\nindex 8a8bc49dc9..5ec72fbb16 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -1,6 +1,12 @@\n #ifndef SUBMODULE_H\n #define SUBMODULE_H\n \n+#define SUBMODULE_CHECK_ANY_CONFIG\t\t(1<<0)\n+#define SUBMODULE_CHECK_ABSORBED_GIT_DIRS\t(1<<1)\n+#define SUBMODULE_CHECK_GITMODULES_IN_WT\t(1<<2)\n+#define SUBMODULE_CHECK_GITLINKS_IN_TREE \t(1<<3)\n+int has_submodules(unsigned what_to_check);\n+\n struct diff_options;\n struct argv_array;\n struct sha1_array;\n@@ -37,7 +43,7 @@ extern int remove_path_from_gitmodules(const char *path);\n extern void stage_updated_gitmodules(void);\n extern void set_diffopt_flags_from_submodule_config(struct diff_options *,\n \t\tconst char *path);\n-extern int submodule_config(const char *var, const char *value, void *cb);\n+extern void load_submodule_config(void);\n extern void gitmodules_config(void);\n extern void gitmodules_config_sha1(const unsigned char *commit_sha1);\n extern int is_submodule_initialized(const char *path);\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 4b3f9518e5..fe42d03a74 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -292,7 +292,7 @@ static void reload_gitmodules_file(struct index_state *index,\n \t\t\t\tsubmodule_free();\n \t\t\t\tcheckout_entry(ce, state, NULL);\n \t\t\t\tgitmodules_config();\n-\t\t\t\tgit_config(submodule_config, NULL);\n+\t\t\t\tload_submodule_config();\n \t\t\t} else\n \t\t\t\tbreak;\n \t\t}\n-- \n2.13.0.18.g7d86cc8ba0\n\n"},{"id":"320109","messageId":"20170517213135.20988-1-sbeller@google.com","threadId":"45992","inReplyTo":null,"subject":"[PATCH 0/3] Add option to recurse into submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-05-17T21:31:32Z","receivedAt":"2017-05-17T21:32:14Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In a submodule heavy workflow it becomes tedious to pass in\n--recurse-submodules all the time, so make an option for it.\n\nThanks,\nStefan\n\nStefan Beller (3):\n  submodule.c: add has_submodules to check if we have any submodules\n  submodule test invocation: only pass additional arguments\n  Introduce submodule.recurse option\n\n Documentation/config.txt       |  5 +++\n builtin/checkout.c             |  8 ++--\n builtin/fetch.c                |  2 +-\n builtin/read-tree.c            |  2 +-\n builtin/reset.c                |  2 +-\n builtin/submodule--helper.c    |  6 +--\n submodule.c                    | 83 ++++++++++++++++++++++++++++++++++--------\n submodule.h                    |  8 +++-\n t/lib-submodule-update.sh      | 18 ++++++++-\n t/t1013-read-tree-submodule.sh |  4 +-\n t/t2013-checkout-submodule.sh  |  4 +-\n t/t5526-fetch-submodules.sh    | 10 +++++\n t/t7112-reset-submodule.sh     |  4 +-\n unpack-trees.c                 |  2 +-\n 14 files changed, 121 insertions(+), 37 deletions(-)\n\n-- \n2.13.0.18.g7d86cc8ba0\n\n"},{"id":"320110","messageId":"20170517213135.20988-4-sbeller@google.com","threadId":"45992","inReplyTo":"20170517213135.20988-1-sbeller@google.com","subject":"[PATCH 3/3] Introduce submodule.recurse option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-05-17T21:31:35Z","receivedAt":"2017-05-17T21:32:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Any command that understands the boolean --recurse-submodule=[true/false]\ncan have its default changed to true, by setting the submodule.recurse\noption.\n\ngit-push takes a --recurse-submodule argument but it is not boolean,\nhence it is not included (yet?).\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n Documentation/config.txt    |  5 +++++\n builtin/checkout.c          |  8 +++-----\n submodule.c                 |  7 ++++++-\n t/lib-submodule-update.sh   | 12 ++++++++++++\n t/t5526-fetch-submodules.sh | 10 ++++++++++\n 5 files changed, 36 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 475e874d51..e367becf72 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -3063,6 +3063,11 @@ submodule.active::\n \tsubmodule's path to determine if the submodule is of interest to git\n \tcommands.\n \n+submodule.recurse::\n+\tSpecifies if commands recurse into submodules by default. This\n+\tapplies to all commands that have a `--recurse-submodules` option.\n+\tDefaults to false.\n+\n submodule.fetchJobs::\n \tSpecifies how many submodules are fetched/cloned at the same time.\n \tA positive integer allows up to that number of submodules fetched\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 2787b343b1..e4bd93c9cd 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1195,6 +1195,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \topts.show_progress = -1;\n \n \tgitmodules_config();\n+\tload_submodule_config();\n \tgit_config(git_checkout_config, &opts);\n \n \topts.track = BRANCH_TRACK_UNSPECIFIED;\n@@ -1214,11 +1215,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\tgit_xmerge_config(\"merge.conflictstyle\", conflict_style, NULL);\n \t}\n \n-\tif (recurse_submodules != RECURSE_SUBMODULES_OFF) {\n-\t\tload_submodule_config();\n-\t\tif (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)\n-\t\t\tset_config_update_recurse_submodules(recurse_submodules);\n-\t}\n+\tif (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)\n+\t\tset_config_update_recurse_submodules(recurse_submodules);\n \n \tif ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1)\n \t\tdie(_(\"-b, -B and --orphan are mutually exclusive\"));\ndiff --git a/submodule.c b/submodule.c\nindex 14ea405048..000060a253 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -91,7 +91,12 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n \n static int submodule_config(const char *var, const char *value, void *cb)\n {\n-\tif (!strcmp(var, \"submodule.fetchjobs\")) {\n+\tif (!strcmp(var, \"submodule.recurse\")) {\n+\t\tint v = git_config_bool(var, value) ?\n+\t\t\tRECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;\n+\t\tconfig_fetch_recurse_submodules = v;\n+\t\tconfig_update_recurse_submodules = v;\n+\t} else if (!strcmp(var, \"submodule.fetchjobs\")) {\n \t\tsubmodule_config_reading = SUBMODULE_CONFIG_EXISTS;\n \t\tparallel_jobs = git_config_int(var, value);\n \t\tif (parallel_jobs < 0)\ndiff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh\nindex 0f70b5ec7b..b30164339e 100755\n--- a/t/lib-submodule-update.sh\n+++ b/t/lib-submodule-update.sh\n@@ -990,6 +990,18 @@ test_submodule_switch_recursing () {\n \t\t)\n \t'\n \n+\ttest_expect_success \"git -c submodule.recurse=true $cmd_args --recurse-submodules: modified submodule updates submodule work tree\" '\n+\t\tprolog &&\n+\t\treset_work_tree_to_interested add_sub1 &&\n+\t\t(\n+\t\t\tcd submodule_update &&\n+\t\t\tgit branch -t modify_sub1 origin/modify_sub1 &&\n+\t\t\tgit -c submodule.recurse=true $cmd_args --recurse-submodules modify_sub1 &&\n+\t\t\ttest_superproject_content origin/modify_sub1 &&\n+\t\t\ttest_submodule_content sub1 origin/modify_sub1\n+\t\t)\n+\t'\n+\n \t# Updating a submodule to an invalid sha1 doesn't update the\n \t# superproject nor the submodule's work tree.\n \ttest_expect_success \"$command: updating to a missing submodule commit fails\" '\ndiff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\nindex f3b0a8d30a..162baf101f 100755\n--- a/t/t5526-fetch-submodules.sh\n+++ b/t/t5526-fetch-submodules.sh\n@@ -71,6 +71,16 @@ test_expect_success \"fetch --recurse-submodules recurses into submodules\" '\n \ttest_i18ncmp expect.err actual.err\n '\n \n+test_expect_success \"submodule.recurse option triggers recursive fetch\" '\n+\tadd_upstream_commit &&\n+\t(\n+\t\tcd downstream &&\n+\t\tgit -c submodule.recurse fetch >../actual.out 2>../actual.err\n+\t) &&\n+\ttest_must_be_empty actual.out &&\n+\ttest_i18ncmp expect.err actual.err\n+'\n+\n test_expect_success \"fetch --recurse-submodules -j2 has the same output behaviour\" '\n \tadd_upstream_commit &&\n \t(\n-- \n2.13.0.18.g7d86cc8ba0\n\n"},{"id":"320140","messageId":"xmqqshk2u3kz.fsf@gitster.mtv.corp.google.com","threadId":"45992","inReplyTo":"20170517213135.20988-2-sbeller@google.com","subject":"Re: [PATCH 1/3] submodule.c: add has_submodules to check if we have any submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-18T05:38:36Z","receivedAt":"2017-05-18T05:38:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index 4ef7a08afc..510ef1c9de 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -1344,7 +1344,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n>  \t\t\tset_config_fetch_recurse_submodules(arg);\n>  \t\t}\n>  \t\tgitmodules_config();\n> -\t\tgit_config(submodule_config, NULL);\n> +\t\tload_submodule_config();\n>  \t}\n>  \n> ...\n> +static enum {\n> +\tSUBMODULE_CONFIG_NOT_READ = 0,\n> +\tSUBMODULE_CONFIG_NO_CONFIG,\n> +\tSUBMODULE_CONFIG_EXISTS,\n> +} submodule_config_reading;\n> +\n>  /*\n>   * The following flag is set if the .gitmodules file is unmerged. We then\n>   * disable recursion for all submodules where .git/config doesn't have a\n> @@ -83,6 +89,62 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n>  \treturn 0;\n>  }\n>  \n> +static int submodule_config(const char *var, const char *value, void *cb)\n> +{\n> +\tif (!strcmp(var, \"submodule.fetchjobs\")) {\n> +...\n> +\t}\n> +\treturn 0;\n> +}\n> +\n> +void load_submodule_config(void)\n> +{\n> +\tsubmodule_config_reading = SUBMODULE_CONFIG_NO_CONFIG;\n> +\tgit_config(submodule_config, NULL);\n> +}\n\nOK, so anybody who does the git_config(submodule_config) must\ninstead call this one, so that we can notice there is some\n\"submodule\" stuff configured.  And that is ensured by making\nsubmodule_config private to this module.\n\nNicely done.\n\nOn a possibly related tangent, I've often found it somewhat\nirritating that that these two calls have to go hand-in-hand.\n\n>  \t\tgitmodules_config();\n> -\t\tgit_config(submodule_config, NULL);\n\nI wonder if it makes sense to roll that gitmodule_config() thing\ninto this function as well?  \n\nOr do some callsites of load_submodule_config() need to omit call to\ngitmodules_config()?  If so please disregard.\n\nThanks.\n\n\n\n"},{"id":"320163","messageId":"20170518153508.GA112091@google.com","threadId":"45992","inReplyTo":"20170517213135.20988-2-sbeller@google.com","subject":"Re: [PATCH 1/3] submodule.c: add has_submodules to check if we have any submodules","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-18T15:35:08Z","receivedAt":"2017-05-18T15:35:18Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 05/17, Stefan Beller wrote:\n> When submodules are involved, it often slows down the process, as most\n> submodule related handling is either done via a child process or by\n> iterating over the index finding all gitlinks.\n> \n> For most commands that may interact with submodules, we need have a\n> quick check if we do have any submodules at all, such that we can\n> be fast in the case when no submodules are in use.  For this quick\n> check introduce a function that checks with different heuristics if\n> we do have submodules around, checking if\n> * anything related to submodules is configured,\n> * absorbed git dirs for submodules are present,\n> * the '.gitmodules' file exists\n> * gitlinks are recorded in the index.\n> \n> Each heuristic has advantages and disadvantages.\n> For example in a later patch, when we first use this function in\n> git-clone, we'll just check for the existence of the '.gitmodules'\n> file, because at the time of running the clone command there will\n> be no absorbed git dirs or submodule configuration around.\n> \n> Checking for any configuration related to submodules would be useful\n> in a later stage (after cloning) to see if the submodules are actually\n> in use.\n> \n> Checking for absorbed git directories is good to see if the user has\n> actually cloned submodules already (i.e. not just initialized them by\n> configuring them).\n> \n> The heuristic for checking the configuration requires this patch\n> to have have a global state, whether the submodule config has already\n> been read, and if there were any submodule related keys. Make\n> 'submodule_config' private to the submodule code, and introduce\n> 'load_submodule_config' that will take care of this global state.\n> \n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>  builtin/checkout.c          |  2 +-\n>  builtin/fetch.c             |  2 +-\n>  builtin/read-tree.c         |  2 +-\n>  builtin/reset.c             |  2 +-\n>  builtin/submodule--helper.c |  6 ++--\n>  submodule.c                 | 78 +++++++++++++++++++++++++++++++++++----------\n>  submodule.h                 |  8 ++++-\n>  unpack-trees.c              |  2 +-\n>  8 files changed, 77 insertions(+), 25 deletions(-)\n> \n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index bfa5419f33..2787b343b1 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -1215,7 +1215,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n>  \t}\n>  \n>  \tif (recurse_submodules != RECURSE_SUBMODULES_OFF) {\n> -\t\tgit_config(submodule_config, NULL);\n> +\t\tload_submodule_config();\n>  \t\tif (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)\n>  \t\t\tset_config_update_recurse_submodules(recurse_submodules);\n>  \t}\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index 4ef7a08afc..510ef1c9de 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -1344,7 +1344,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n>  \t\t\tset_config_fetch_recurse_submodules(arg);\n>  \t\t}\n>  \t\tgitmodules_config();\n> -\t\tgit_config(submodule_config, NULL);\n> +\t\tload_submodule_config();\n>  \t}\n>  \n>  \tif (all) {\n> diff --git a/builtin/read-tree.c b/builtin/read-tree.c\n> index 23e212ee8c..fe2ec60a51 100644\n> --- a/builtin/read-tree.c\n> +++ b/builtin/read-tree.c\n> @@ -177,7 +177,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n>  \n>  \tif (recurse_submodules != RECURSE_SUBMODULES_DEFAULT) {\n>  \t\tgitmodules_config();\n> -\t\tgit_config(submodule_config, NULL);\n> +\t\tload_submodule_config();\n>  \t\tset_config_update_recurse_submodules(RECURSE_SUBMODULES_ON);\n>  \t}\n>  \n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 5ce27fcaed..f3aca487d9 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -321,7 +321,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n>  \n>  \tif (recurse_submodules != RECURSE_SUBMODULES_DEFAULT) {\n>  \t\tgitmodules_config();\n> -\t\tgit_config(submodule_config, NULL);\n> +\t\tload_submodule_config();\n>  \t\tset_config_update_recurse_submodules(RECURSE_SUBMODULES_ON);\n>  \t}\n>  \n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index 85aafe46a4..770a95ca14 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -1015,7 +1015,7 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n>  \n>  \t/* Overlay the parsed .gitmodules file with .git/config */\n>  \tgitmodules_config();\n> -\tgit_config(submodule_config, NULL);\n> +\tload_submodule_config();\n>  \n>  \tif (max_jobs < 0)\n>  \t\tmax_jobs = parallel_submodules();\n> @@ -1058,7 +1058,7 @@ static const char *remote_submodule_branch(const char *path)\n>  {\n>  \tconst struct submodule *sub;\n>  \tgitmodules_config();\n> -\tgit_config(submodule_config, NULL);\n> +\tload_submodule_config();\n>  \n>  \tsub = submodule_from_path(null_sha1, path);\n>  \tif (!sub)\n> @@ -1130,7 +1130,7 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)\n>  \t\t\t     git_submodule_helper_usage, 0);\n>  \n>  \tgitmodules_config();\n> -\tgit_config(submodule_config, NULL);\n> +\tload_submodule_config();\n>  \n>  \tif (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)\n>  \t\treturn 1;\n> diff --git a/submodule.c b/submodule.c\n> index 20ed5b5681..14ea405048 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -24,6 +24,12 @@ static int initialized_fetch_ref_tips;\n>  static struct sha1_array ref_tips_before_fetch;\n>  static struct sha1_array ref_tips_after_fetch;\n>  \n> +static enum {\n> +\tSUBMODULE_CONFIG_NOT_READ = 0,\n> +\tSUBMODULE_CONFIG_NO_CONFIG,\n> +\tSUBMODULE_CONFIG_EXISTS,\n> +} submodule_config_reading;\n\nAny way we can have this not be a global, but rather a parameter?  You\ncould pass in a pointer to this value via the callback data parameter in\nthe submodule_config function.\n\n> +\n>  /*\n>   * The following flag is set if the .gitmodules file is unmerged. We then\n>   * disable recursion for all submodules where .git/config doesn't have a\n> @@ -83,6 +89,62 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n>  \treturn 0;\n>  }\n>  \n> +static int submodule_config(const char *var, const char *value, void *cb)\n> +{\n> +\tif (!strcmp(var, \"submodule.fetchjobs\")) {\n> +\t\tsubmodule_config_reading = SUBMODULE_CONFIG_EXISTS;\n> +\t\tparallel_jobs = git_config_int(var, value);\n> +\t\tif (parallel_jobs < 0)\n> +\t\t\tdie(_(\"negative values not allowed for submodule.fetchJobs\"));\n> +\t\treturn 0;\n> +\t} else if (starts_with(var, \"submodule.\")) {\n> +\t\tsubmodule_config_reading = SUBMODULE_CONFIG_EXISTS;\n> +\t\treturn parse_submodule_config_option(var, value);\n> +\t} else if (!strcmp(var, \"fetch.recursesubmodules\")) {\n> +\t\tsubmodule_config_reading = SUBMODULE_CONFIG_EXISTS;\n> +\t\tconfig_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(var, value);\n> +\t\treturn 0;\n> +\t}\n> +\treturn 0;\n> +}\n> +\n> +void load_submodule_config(void)\n> +{\n> +\tsubmodule_config_reading = SUBMODULE_CONFIG_NO_CONFIG;\n> +\tgit_config(submodule_config, NULL);\n> +}\n> +\n> +int has_submodules(unsigned what_to_check)\n> +{\n> +\tif (what_to_check & SUBMODULE_CHECK_ANY_CONFIG) {\n> +\t\tif (submodule_config_reading == SUBMODULE_CONFIG_NOT_READ)\n> +\t\t\tload_submodule_config();\n> +\t\tif (submodule_config_reading == SUBMODULE_CONFIG_EXISTS)\n> +\t\t\treturn 1;\n> +\t}\n> +\n> +\tif ((what_to_check & SUBMODULE_CHECK_ABSORBED_GIT_DIRS) &&\n> +\t    file_exists(git_path(\"modules\")))\n> +\t\treturn 1;\n> +\n> +\tif ((what_to_check & SUBMODULE_CHECK_GITMODULES_IN_WT) &&\n> +\t    (!is_bare_repository() && file_exists(\".gitmodules\")))\n> +\t\treturn 1;\n> +\n> +\tif (what_to_check & SUBMODULE_CHECK_GITLINKS_IN_TREE) {\n> +\t\tint i;\n> +\n> +\t\tif (read_cache() < 0)\n> +\t\t\tdie(_(\"index file corrupt\"));\n> +\n> +\t\tfor (i = 0; i < active_nr; i++)\n> +\t\t\tif (S_ISGITLINK(active_cache[i]->ce_mode))\n> +\t\t\t\treturn 1;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n>  /*\n>   * Try to remove the \"submodule.<name>\" section from .gitmodules where the given\n>   * path is configured. Return 0 only if a .gitmodules file was found, a section\n> @@ -152,22 +214,6 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n>  \t}\n>  }\n>  \n> -int submodule_config(const char *var, const char *value, void *cb)\n> -{\n> -\tif (!strcmp(var, \"submodule.fetchjobs\")) {\n> -\t\tparallel_jobs = git_config_int(var, value);\n> -\t\tif (parallel_jobs < 0)\n> -\t\t\tdie(_(\"negative values not allowed for submodule.fetchJobs\"));\n> -\t\treturn 0;\n> -\t} else if (starts_with(var, \"submodule.\"))\n> -\t\treturn parse_submodule_config_option(var, value);\n> -\telse if (!strcmp(var, \"fetch.recursesubmodules\")) {\n> -\t\tconfig_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(var, value);\n> -\t\treturn 0;\n> -\t}\n> -\treturn 0;\n> -}\n> -\n>  void gitmodules_config(void)\n>  {\n>  \tconst char *work_tree = get_git_work_tree();\n> diff --git a/submodule.h b/submodule.h\n> index 8a8bc49dc9..5ec72fbb16 100644\n> --- a/submodule.h\n> +++ b/submodule.h\n> @@ -1,6 +1,12 @@\n>  #ifndef SUBMODULE_H\n>  #define SUBMODULE_H\n>  \n> +#define SUBMODULE_CHECK_ANY_CONFIG\t\t(1<<0)\n> +#define SUBMODULE_CHECK_ABSORBED_GIT_DIRS\t(1<<1)\n> +#define SUBMODULE_CHECK_GITMODULES_IN_WT\t(1<<2)\n> +#define SUBMODULE_CHECK_GITLINKS_IN_TREE \t(1<<3)\n> +int has_submodules(unsigned what_to_check);\n> +\n>  struct diff_options;\n>  struct argv_array;\n>  struct sha1_array;\n> @@ -37,7 +43,7 @@ extern int remove_path_from_gitmodules(const char *path);\n>  extern void stage_updated_gitmodules(void);\n>  extern void set_diffopt_flags_from_submodule_config(struct diff_options *,\n>  \t\tconst char *path);\n> -extern int submodule_config(const char *var, const char *value, void *cb);\n> +extern void load_submodule_config(void);\n>  extern void gitmodules_config(void);\n>  extern void gitmodules_config_sha1(const unsigned char *commit_sha1);\n>  extern int is_submodule_initialized(const char *path);\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index 4b3f9518e5..fe42d03a74 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -292,7 +292,7 @@ static void reload_gitmodules_file(struct index_state *index,\n>  \t\t\t\tsubmodule_free();\n>  \t\t\t\tcheckout_entry(ce, state, NULL);\n>  \t\t\t\tgitmodules_config();\n> -\t\t\t\tgit_config(submodule_config, NULL);\n> +\t\t\t\tload_submodule_config();\n>  \t\t\t} else\n>  \t\t\t\tbreak;\n>  \t\t}\n> -- \n> 2.13.0.18.g7d86cc8ba0\n> \n\n-- \nBrandon Williams\n"},{"id":"320164","messageId":"20170518153901.GB112091@google.com","threadId":"45992","inReplyTo":"20170517213135.20988-4-sbeller@google.com","subject":"Re: [PATCH 3/3] Introduce submodule.recurse option","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-18T15:39:01Z","receivedAt":"2017-05-18T15:39:08Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 05/17, Stefan Beller wrote:\n> Any command that understands the boolean --recurse-submodule=[true/false]\n> can have its default changed to true, by setting the submodule.recurse\n> option.\n> \n> git-push takes a --recurse-submodule argument but it is not boolean,\n> hence it is not included (yet?).\n\nls-files and grep could also be added.  They don't need to be added in\nthis patch though.\n\n> \n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>  Documentation/config.txt    |  5 +++++\n>  builtin/checkout.c          |  8 +++-----\n>  submodule.c                 |  7 ++++++-\n>  t/lib-submodule-update.sh   | 12 ++++++++++++\n>  t/t5526-fetch-submodules.sh | 10 ++++++++++\n>  5 files changed, 36 insertions(+), 6 deletions(-)\n> \n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 475e874d51..e367becf72 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -3063,6 +3063,11 @@ submodule.active::\n>  \tsubmodule's path to determine if the submodule is of interest to git\n>  \tcommands.\n>  \n> +submodule.recurse::\n> +\tSpecifies if commands recurse into submodules by default. This\n> +\tapplies to all commands that have a `--recurse-submodules` option.\n> +\tDefaults to false.\n> +\n>  submodule.fetchJobs::\n>  \tSpecifies how many submodules are fetched/cloned at the same time.\n>  \tA positive integer allows up to that number of submodules fetched\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 2787b343b1..e4bd93c9cd 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -1195,6 +1195,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n>  \topts.show_progress = -1;\n>  \n>  \tgitmodules_config();\n> +\tload_submodule_config();\n>  \tgit_config(git_checkout_config, &opts);\n>  \n>  \topts.track = BRANCH_TRACK_UNSPECIFIED;\n> @@ -1214,11 +1215,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n>  \t\tgit_xmerge_config(\"merge.conflictstyle\", conflict_style, NULL);\n>  \t}\n>  \n> -\tif (recurse_submodules != RECURSE_SUBMODULES_OFF) {\n> -\t\tload_submodule_config();\n> -\t\tif (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)\n> -\t\t\tset_config_update_recurse_submodules(recurse_submodules);\n> -\t}\n> +\tif (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)\n> +\t\tset_config_update_recurse_submodules(recurse_submodules);\n>  \n>  \tif ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1)\n>  \t\tdie(_(\"-b, -B and --orphan are mutually exclusive\"));\n> diff --git a/submodule.c b/submodule.c\n> index 14ea405048..000060a253 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -91,7 +91,12 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n>  \n>  static int submodule_config(const char *var, const char *value, void *cb)\n>  {\n> -\tif (!strcmp(var, \"submodule.fetchjobs\")) {\n> +\tif (!strcmp(var, \"submodule.recurse\")) {\n> +\t\tint v = git_config_bool(var, value) ?\n> +\t\t\tRECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;\n> +\t\tconfig_fetch_recurse_submodules = v;\n> +\t\tconfig_update_recurse_submodules = v;\n> +\t} else if (!strcmp(var, \"submodule.fetchjobs\")) {\n>  \t\tsubmodule_config_reading = SUBMODULE_CONFIG_EXISTS;\n>  \t\tparallel_jobs = git_config_int(var, value);\n>  \t\tif (parallel_jobs < 0)\n> diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh\n> index 0f70b5ec7b..b30164339e 100755\n> --- a/t/lib-submodule-update.sh\n> +++ b/t/lib-submodule-update.sh\n> @@ -990,6 +990,18 @@ test_submodule_switch_recursing () {\n>  \t\t)\n>  \t'\n>  \n> +\ttest_expect_success \"git -c submodule.recurse=true $cmd_args --recurse-submodules: modified submodule updates submodule work tree\" '\n> +\t\tprolog &&\n> +\t\treset_work_tree_to_interested add_sub1 &&\n> +\t\t(\n> +\t\t\tcd submodule_update &&\n> +\t\t\tgit branch -t modify_sub1 origin/modify_sub1 &&\n> +\t\t\tgit -c submodule.recurse=true $cmd_args --recurse-submodules modify_sub1 &&\n> +\t\t\ttest_superproject_content origin/modify_sub1 &&\n> +\t\t\ttest_submodule_content sub1 origin/modify_sub1\n> +\t\t)\n> +\t'\n> +\n>  \t# Updating a submodule to an invalid sha1 doesn't update the\n>  \t# superproject nor the submodule's work tree.\n>  \ttest_expect_success \"$command: updating to a missing submodule commit fails\" '\n> diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\n> index f3b0a8d30a..162baf101f 100755\n> --- a/t/t5526-fetch-submodules.sh\n> +++ b/t/t5526-fetch-submodules.sh\n> @@ -71,6 +71,16 @@ test_expect_success \"fetch --recurse-submodules recurses into submodules\" '\n>  \ttest_i18ncmp expect.err actual.err\n>  '\n>  \n> +test_expect_success \"submodule.recurse option triggers recursive fetch\" '\n> +\tadd_upstream_commit &&\n> +\t(\n> +\t\tcd downstream &&\n> +\t\tgit -c submodule.recurse fetch >../actual.out 2>../actual.err\n> +\t) &&\n> +\ttest_must_be_empty actual.out &&\n> +\ttest_i18ncmp expect.err actual.err\n> +'\n> +\n>  test_expect_success \"fetch --recurse-submodules -j2 has the same output behaviour\" '\n>  \tadd_upstream_commit &&\n>  \t(\n> -- \n> 2.13.0.18.g7d86cc8ba0\n> \n\n-- \nBrandon Williams\n"},{"id":"320168","messageId":"CAGZ79kZjMwoXOr_5xrU0Uy9K5bjJOd=YUBvK+Dpx+uS+5LH9Dw@mail.gmail.com","threadId":"45992","inReplyTo":"20170518153901.GB112091@google.com","subject":"Re: [PATCH 3/3] Introduce submodule.recurse option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-05-18T16:20:26Z","receivedAt":"2017-05-18T16:20:33Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, May 18, 2017 at 8:39 AM, Brandon Williams <bmwill@google.com> wrote:\n> On 05/17, Stefan Beller wrote:\n>> Any command that understands the boolean --recurse-submodule=[true/false]\n>> can have its default changed to true, by setting the submodule.recurse\n>> option.\n>>\n>> git-push takes a --recurse-submodule argument but it is not boolean,\n>> hence it is not included (yet?).\n>\n> ls-files and grep could also be added.  They don't need to be added in\n> this patch though.\n\nI agree for git-grep.\ngit-ls-files is a pure plumbing command, which usually are not very accepting\nof options. Then it is easier for script writers to rely on its expected output.\n\nI just missed those two (at least grep) and will add them in this\npatch if we think\nthis series is a good idea going forward.\n\nThanks,\nStefan\n"},{"id":"320169","messageId":"CAGZ79ka3XBu-iNbvkp9yi5ovXVaAyTH9BY+M8HOj-Q6PEMB7iw@mail.gmail.com","threadId":"45992","inReplyTo":"xmqqshk2u3kz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/3] submodule.c: add has_submodules to check if we have any submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-05-18T16:34:31Z","receivedAt":"2017-05-18T16:34:37Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, May 17, 2017 at 10:38 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> diff --git a/builtin/fetch.c b/builtin/fetch.c\n>> index 4ef7a08afc..510ef1c9de 100644\n>> --- a/builtin/fetch.c\n>> +++ b/builtin/fetch.c\n>> @@ -1344,7 +1344,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n>>                       set_config_fetch_recurse_submodules(arg);\n>>               }\n>>               gitmodules_config();\n>> -             git_config(submodule_config, NULL);\n>> +             load_submodule_config();\n>>       }\n>>\n>> ...\n>> +static enum {\n>> +     SUBMODULE_CONFIG_NOT_READ = 0,\n>> +     SUBMODULE_CONFIG_NO_CONFIG,\n>> +     SUBMODULE_CONFIG_EXISTS,\n>> +} submodule_config_reading;\n>> +\n>>  /*\n>>   * The following flag is set if the .gitmodules file is unmerged. We then\n>>   * disable recursion for all submodules where .git/config doesn't have a\n>> @@ -83,6 +89,62 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n>>       return 0;\n>>  }\n>>\n>> +static int submodule_config(const char *var, const char *value, void *cb)\n>> +{\n>> +     if (!strcmp(var, \"submodule.fetchjobs\")) {\n>> +...\n>> +     }\n>> +     return 0;\n>> +}\n>> +\n>> +void load_submodule_config(void)\n>> +{\n>> +     submodule_config_reading = SUBMODULE_CONFIG_NO_CONFIG;\n>> +     git_config(submodule_config, NULL);\n>> +}\n>\n> OK, so anybody who does the git_config(submodule_config) must\n> instead call this one, so that we can notice there is some\n> \"submodule\" stuff configured.  And that is ensured by making\n> submodule_config private to this module.\n\nExactly.\n\n> Nicely done.\n\nThanks.\n\n>\n> On a possibly related tangent, I've often found it somewhat\n> irritating that that these two calls have to go hand-in-hand.\n>\n>>               gitmodules_config();\n>> -             git_config(submodule_config, NULL);\n>\n> I wonder if it makes sense to roll that gitmodule_config() thing\n> into this function as well?\n\nThis patch has been sitting on my hard drive for quite a while\nand I just vaguely remember that I did not do that for some\ncomplication along the way. I'll re-examine the situation.\n\nThanks,\nStefan\n"},{"id":"320170","messageId":"CAGZ79kZZF7L0Wv5kPO+LxM6_7tA=DrD2RSn9SM708yvnxPnW3A@mail.gmail.com","threadId":"45992","inReplyTo":"20170518153508.GA112091@google.com","subject":"Re: [PATCH 1/3] submodule.c: add has_submodules to check if we have any submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-05-18T16:38:55Z","receivedAt":"2017-05-18T16:39:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":">> +static enum {\n>> +     SUBMODULE_CONFIG_NOT_READ = 0,\n>> +     SUBMODULE_CONFIG_NO_CONFIG,\n>> +     SUBMODULE_CONFIG_EXISTS,\n>> +} submodule_config_reading;\n>\n> Any way we can have this not be a global, but rather a parameter?  You\n> could pass in a pointer to this value via the callback data parameter in\n> the submodule_config function.\n\nAs said in the reply to Junio, this patch has been sitting on my hard drive\nfor a while and was written before you started the attempt to de-globalize\nthe state of git.\n\nIdeally this setting would be part of the repository object. For example\nthe repository object would have a \"submodule_config\" pointer, initialized\nto NULL, which can then be set to the read config or a static empty_config\nif no such config exists.\n"},{"id":"320177","messageId":"20170518190027.GD112091@google.com","threadId":"45992","inReplyTo":"CAGZ79kZZF7L0Wv5kPO+LxM6_7tA=DrD2RSn9SM708yvnxPnW3A@mail.gmail.com","subject":"Re: [PATCH 1/3] submodule.c: add has_submodules to check if we have any submodules","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-18T19:00:27Z","receivedAt":"2017-05-18T19:00:36Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 05/18, Stefan Beller wrote:\n> >> +static enum {\n> >> +     SUBMODULE_CONFIG_NOT_READ = 0,\n> >> +     SUBMODULE_CONFIG_NO_CONFIG,\n> >> +     SUBMODULE_CONFIG_EXISTS,\n> >> +} submodule_config_reading;\n> >\n> > Any way we can have this not be a global, but rather a parameter?  You\n> > could pass in a pointer to this value via the callback data parameter in\n> > the submodule_config function.\n> \n> As said in the reply to Junio, this patch has been sitting on my hard drive\n> for a while and was written before you started the attempt to de-globalize\n> the state of git.\n> \n> Ideally this setting would be part of the repository object. For example\n> the repository object would have a \"submodule_config\" pointer, initialized\n> to NULL, which can then be set to the read config or a static empty_config\n> if no such config exists.\n\nI'm not quite sure I agree, or rather we may be talking about two\ndifferent things or I'm misinterpreting the patches.  From these patches\nit seems like 'submodule_config' that you are refering to is not the\nactual submodule configuration but rather some options that are stored\nin .git/config or other various config locations (home, system, etc).\nWhat would need to be part of the repository object (and is in my WIP\nthat I'll hopefully send out so i can get some feedback) would be the\nsubmodule_cache which is the internal representation of a repository's\n.gitmodules files.\n\n-- \nBrandon Williams\n"},{"id":"320178","messageId":"20170518190541.GE112091@google.com","threadId":"45992","inReplyTo":"20170517213135.20988-1-sbeller@google.com","subject":"Re: [PATCH 0/3] Add option to recurse into submodules","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-18T19:05:41Z","receivedAt":"2017-05-18T19:05:49Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 05/17, Stefan Beller wrote:\n> In a submodule heavy workflow it becomes tedious to pass in\n> --recurse-submodules all the time, so make an option for it.\n\nI'm all for adding the config which applies to multiple commands.  I\nthink its probably necessary as we increase the number of commands which\ncan recurse.\n\nI'm not as thrilled over the first patch in the series though.  Least\nthat's my first impression.  I probably need think about it a bit more\nthough.\n\n> \n> Thanks,\n> Stefan\n> \n> Stefan Beller (3):\n>   submodule.c: add has_submodules to check if we have any submodules\n>   submodule test invocation: only pass additional arguments\n>   Introduce submodule.recurse option\n> \n>  Documentation/config.txt       |  5 +++\n>  builtin/checkout.c             |  8 ++--\n>  builtin/fetch.c                |  2 +-\n>  builtin/read-tree.c            |  2 +-\n>  builtin/reset.c                |  2 +-\n>  builtin/submodule--helper.c    |  6 +--\n>  submodule.c                    | 83 ++++++++++++++++++++++++++++++++++--------\n>  submodule.h                    |  8 +++-\n>  t/lib-submodule-update.sh      | 18 ++++++++-\n>  t/t1013-read-tree-submodule.sh |  4 +-\n>  t/t2013-checkout-submodule.sh  |  4 +-\n>  t/t5526-fetch-submodules.sh    | 10 +++++\n>  t/t7112-reset-submodule.sh     |  4 +-\n>  unpack-trees.c                 |  2 +-\n>  14 files changed, 121 insertions(+), 37 deletions(-)\n> \n> -- \n> 2.13.0.18.g7d86cc8ba0\n> \n\n-- \nBrandon Williams\n"},{"id":"320179","messageId":"CAGZ79kb+31h6f_9fuUpbfeY7xxjP-kbvqz6J5K5sbPEmdPNvpw@mail.gmail.com","threadId":"45992","inReplyTo":"20170518190027.GD112091@google.com","subject":"Re: [PATCH 1/3] submodule.c: add has_submodules to check if we have any submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-05-18T19:15:25Z","receivedAt":"2017-05-18T19:15:38Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, May 18, 2017 at 12:00 PM, Brandon Williams <bmwill@google.com> wrote:\n> On 05/18, Stefan Beller wrote:\n>> >> +static enum {\n>> >> +     SUBMODULE_CONFIG_NOT_READ = 0,\n>> >> +     SUBMODULE_CONFIG_NO_CONFIG,\n>> >> +     SUBMODULE_CONFIG_EXISTS,\n>> >> +} submodule_config_reading;\n>> >\n>> > Any way we can have this not be a global, but rather a parameter?  You\n>> > could pass in a pointer to this value via the callback data parameter in\n>> > the submodule_config function.\n>>\n>> As said in the reply to Junio, this patch has been sitting on my hard drive\n>> for a while and was written before you started the attempt to de-globalize\n>> the state of git.\n>>\n>> Ideally this setting would be part of the repository object. For example\n>> the repository object would have a \"submodule_config\" pointer, initialized\n>> to NULL, which can then be set to the read config or a static empty_config\n>> if no such config exists.\n>\n> I'm not quite sure I agree, or rather we may be talking about two\n> different things or I'm misinterpreting the patches.  From these patches\n> it seems like 'submodule_config' that you are refering to is not the\n> actual submodule configuration but rather some options that are stored\n> in .git/config or other various config locations (home, system, etc).\n\nYou are reading the patch correctly.\n\n> What would need to be part of the repository object (and is in my WIP\n> that I'll hopefully send out so i can get some feedback) would be the\n> submodule_cache which is the internal representation of a repository's\n> .gitmodules files.\n\nand in the light of this patch, we'd want to have a cache the flag if the\nregular config contains any submodule related things, so for now a static\nseems to be the best option and once we have the repo object we'd\nhave a bit from the flags section (assuming we'll have a flags\nsection down the road):\n\nstruct repo {\n    ....\n    struct regular_config *config_ptr;\n    unsigned config_has_submodule_stuff : 1;\n    unsigned config_loaded_submodule_config : 1;\n}\n\nThen the caching decision would be:\n\nstatic int submodule_config(const char *var, const char *value, void *cb)\n{\n       struct repo *r = cb;\n       if (!strcmp(var, \"submodule.fetchjobs\")) {\n               r->config_has_submodule_stuff = 1;\n               parallel_jobs = git_config_int(var, value);\n               if (parallel_jobs < 0)\n                       die(_(\"negative values not allowed for\nsubmodule.fetchJobs\"));\n               return 0;\n       } else if (starts_with(var, \"submodule.\")) {\n               r->config_has_submodule_stuff = 1;\n               return parse_submodule_config_option(var, value);\n       } else if (!strcmp(var, \"fetch.recursesubmodules\")) {\n               r->config_has_submodule_stuff = 1;\n               config_fetch_recurse_submodules =\nparse_fetch_recurse_submodules_arg(var, value);\n               return 0;\n       }\n       return 0;\n}\n\nvoid load_submodule_config(struct repo *r)\n{\n      # assume r->config_has_submodule_stuff and config_loaded_submodule_config\n      # was set to 0 on repo init\n      if (r->config_loaded_submodule_config)\n            return;\n\n      git_config(submodule_config, r);\n      r->config_loaded_submodule_config = 1;\n}\n\nThat said, I agree that the\n\n>> >> +static enum {\n>> >> +     SUBMODULE_CONFIG_NOT_READ = 0,\n>> >> +     SUBMODULE_CONFIG_NO_CONFIG,\n>> >> +     SUBMODULE_CONFIG_EXISTS,\n>> >> +} submodule_config_reading;\n\nwith its global state is counterproductive for your series, but I see\nan easy integration path. As we do not have the repo struct,\nI proposed it this way to make submodule progress.\n\nThanks,\nStefan\n"}]}