On 05/22, Stefan Beller wrote:
> When submodules are involved, it often slows down the process, as most
> submodule related handling is either done via a child process or by
> iterating over the index finding all gitlinks.
>
> For most commands that may interact with submodules, we need have a
> quick check if we do have any submodules at all, such that we can
> be fast in the case when no submodules are in use. For this quick
> check introduce a function that checks with different heuristics if
> we do have submodules around, checking if
> * anything related to submodules is configured,
> * absorbed git dirs for submodules are present,
> * the '.gitmodules' file exists
> * gitlinks are recorded in the index.
>
> Each heuristic has advantages and disadvantages.
> For example in a later patch, when we first use this function in
> git-clone, we'll just check for the existence of the '.gitmodules'
> file, because at the time of running the clone command there will
> be no absorbed git dirs or submodule configuration around.
>
> Checking for any configuration related to submodules would be useful
> in a later stage (after cloning) to see if the submodules are actually
> in use.
>
> Checking for absorbed git directories is good to see if the user has
> actually cloned submodules already (i.e. not just initialized them by
> configuring them).
>
> The heuristic for checking the configuration requires this patch
> to have have a global state, whether the submodule config has already
> been read, and if there were any submodule related keys. Make
> 'submodule_config' private to the submodule code, and introduce
> 'load_submodule_config' that will take care of this global state.
It doesn't look like any patches actually use this helper, is this
intended?
>
> Signed-off-by: Stefan Beller <sbel...@google.com>
> ---
> builtin/checkout.c | 2 +-
> builtin/fetch.c | 3 +-
> builtin/read-tree.c | 3 +-
> builtin/reset.c | 3 +-
> builtin/submodule--helper.c | 10 ++----
> submodule.c | 80
> ++++++++++++++++++++++++++++++++++++---------
> submodule.h | 8 ++++-
> unpack-trees.c | 3 +-
> 8 files changed, 79 insertions(+), 33 deletions(-)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index bfa5419f33..2787b343b1 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -1215,7 +1215,7 @@ int cmd_checkout(int argc, const char **argv, const
> char *prefix)
> }
>
> if (recurse_submodules != RECURSE_SUBMODULES_OFF) {
> - git_config(submodule_config, NULL);
> + load_submodule_config();
> if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
>
> set_config_update_recurse_submodules(recurse_submodules);
> }
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 4ef7a08afc..4b5f172623 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1343,8 +1343,7 @@ int cmd_fetch(int argc, const char **argv, const char
> *prefix)
> int arg =
> parse_fetch_recurse_submodules_arg("--recurse-submodules-default",
> recurse_submodules_default);
> set_config_fetch_recurse_submodules(arg);
> }
> - gitmodules_config();
> - git_config(submodule_config, NULL);
> + load_submodule_config();
> }
>
> if (all) {
> diff --git a/builtin/read-tree.c b/builtin/read-tree.c
> index 23e212ee8c..2f7f085b82 100644
> --- a/builtin/read-tree.c
> +++ b/builtin/read-tree.c
> @@ -176,8 +176,7 @@ int cmd_read_tree(int argc, const char **argv, const char
> *unused_prefix)
> hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
>
> if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT) {
> - gitmodules_config();
> - git_config(submodule_config, NULL);
> + load_submodule_config();
> set_config_update_recurse_submodules(RECURSE_SUBMODULES_ON);
> }
>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 5ce27fcaed..319d8c1201 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -320,8 +320,7 @@ int cmd_reset(int argc, const char **argv, const char
> *prefix)
> parse_args(&pathspec, argv, prefix, patch_mode, &rev);
>
> if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT) {
> - gitmodules_config();
> - git_config(submodule_config, NULL);
> + load_submodule_config();
> set_config_update_recurse_submodules(RECURSE_SUBMODULES_ON);
> }
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 85aafe46a4..92e13abe2d 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1013,9 +1013,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);
> + load_submodule_config();
>
> if (max_jobs < 0)
> max_jobs = parallel_submodules();
> @@ -1057,9 +1055,8 @@ 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;
> - gitmodules_config();
> - git_config(submodule_config, NULL);
>
> + load_submodule_config();
> sub = submodule_from_path(null_sha1, path);
> if (!sub)
> return NULL;
> @@ -1129,8 +1126,7 @@ 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();
> - git_config(submodule_config, NULL);
> + load_submodule_config();
>
> if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
> return 1;
> diff --git a/submodule.c b/submodule.c
> index 20ed5b5681..dda5ed210f 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -24,6 +24,12 @@ static int initialized_fetch_ref_tips;
> static struct sha1_array ref_tips_before_fetch;
> static struct sha1_array ref_tips_after_fetch;
>
> +static enum {
> + SUBMODULE_CONFIG_NOT_READ = 0,
> + SUBMODULE_CONFIG_NO_CONFIG,
> + SUBMODULE_CONFIG_EXISTS,
> +} submodule_config_reading;
> +
> /*
> * The following flag is set if the .gitmodules file is unmerged. We then
> * disable recursion for all submodules where .git/config doesn't have a
> @@ -83,6 +89,64 @@ int update_path_in_gitmodules(const char *oldpath, const
> char *newpath)
> return 0;
> }
>
> +static int submodule_config(const char *var, const char *value, void *cb)
> +{
> + if (!strcmp(var, "submodule.fetchjobs")) {
> + submodule_config_reading = SUBMODULE_CONFIG_EXISTS;
> + parallel_jobs = git_config_int(var, value);
> + if (parallel_jobs < 0)
> + die(_("negative values not allowed for
> submodule.fetchJobs"));
> + return 0;
> + } else if (starts_with(var, "submodule.")) {
> + submodule_config_reading = SUBMODULE_CONFIG_EXISTS;
> + return parse_submodule_config_option(var, value);
> + } else if (!strcmp(var, "fetch.recursesubmodules")) {
> + submodule_config_reading = SUBMODULE_CONFIG_EXISTS;
> + config_fetch_recurse_submodules =
> parse_fetch_recurse_submodules_arg(var, value);
> + return 0;
> + }
> + return 0;
> +}
> +
> +void load_submodule_config(void)
> +{
> + submodule_config_reading = SUBMODULE_CONFIG_NO_CONFIG;
> +
> + gitmodules_config();
> + git_config(submodule_config, NULL);
> +}
> +
> +int has_submodules(unsigned what_to_check)
> +{
> + if (what_to_check & SUBMODULE_CHECK_ANY_CONFIG) {
> + if (submodule_config_reading == SUBMODULE_CONFIG_NOT_READ)
> + load_submodule_config();
> + if (submodule_config_reading == SUBMODULE_CONFIG_EXISTS)
> + return 1;
> + }
> +
> + if ((what_to_check & SUBMODULE_CHECK_ABSORBED_GIT_DIRS) &&
> + file_exists(git_path("modules")))
> + return 1;
> +
> + if ((what_to_check & SUBMODULE_CHECK_GITMODULES_IN_WT) &&
> + (!is_bare_repository() && file_exists(".gitmodules")))
> + return 1;
> +
> + if (what_to_check & SUBMODULE_CHECK_GITLINKS_IN_TREE) {
> + int i;
> +
> + if (read_cache() < 0)
> + die(_("index file corrupt"));
> +
> + for (i = 0; i < active_nr; i++)
> + if (S_ISGITLINK(active_cache[i]->ce_mode))
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> /*
> * Try to remove the "submodule.<name>" section from .gitmodules where the
> given
> * path is configured. Return 0 only if a .gitmodules file was found, a
> section
> @@ -152,22 +216,6 @@ void set_diffopt_flags_from_submodule_config(struct
> diff_options *diffopt,
> }
> }
>
> -int submodule_config(const char *var, const char *value, void *cb)
> -{
> - if (!strcmp(var, "submodule.fetchjobs")) {
> - parallel_jobs = git_config_int(var, value);
> - if (parallel_jobs < 0)
> - die(_("negative values not allowed for
> submodule.fetchJobs"));
> - return 0;
> - } else if (starts_with(var, "submodule."))
> - return parse_submodule_config_option(var, value);
> - else if (!strcmp(var, "fetch.recursesubmodules")) {
> - config_fetch_recurse_submodules =
> parse_fetch_recurse_submodules_arg(var, value);
> - return 0;
> - }
> - return 0;
> -}
> -
> void gitmodules_config(void)
> {
> const char *work_tree = get_git_work_tree();
> diff --git a/submodule.h b/submodule.h
> index 8a8bc49dc9..5ec72fbb16 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -1,6 +1,12 @@
> #ifndef SUBMODULE_H
> #define SUBMODULE_H
>
> +#define SUBMODULE_CHECK_ANY_CONFIG (1<<0)
> +#define SUBMODULE_CHECK_ABSORBED_GIT_DIRS (1<<1)
> +#define SUBMODULE_CHECK_GITMODULES_IN_WT (1<<2)
> +#define SUBMODULE_CHECK_GITLINKS_IN_TREE (1<<3)
> +int has_submodules(unsigned what_to_check);
> +
> struct diff_options;
> struct argv_array;
> struct sha1_array;
> @@ -37,7 +43,7 @@ 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 void load_submodule_config(void);
> extern void gitmodules_config(void);
> extern void gitmodules_config_sha1(const unsigned char *commit_sha1);
> extern int is_submodule_initialized(const char *path);
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 4b3f9518e5..e3174b3b66 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -291,8 +291,7 @@ static void reload_gitmodules_file(struct index_state
> *index,
> else if (r == 0) {
> submodule_free();
> checkout_entry(ce, state, NULL);
> - gitmodules_config();
> - git_config(submodule_config, NULL);
> + load_submodule_config();
> } else
> break;
> }
> --
> 2.13.0.18.g7d86cc8ba0
>
--
Brandon Williams
