Re: [PATCH v4 07/10] submodule init: initialize active submodules

From: Stefan Beller <sbeller@google.com>
Date: Mar 17, 2017, 18:17 UTC
Message-ID: <CAGZ79ka-rQ3YtdduAMYf-XYitKMfX2iEVh4wfR1vD6RzqhHDzw@mail.gmail.com>
In-Reply-To: <20170316222952.53801-8-bmwill@google.com>

On Thu, Mar 16, 2017 at 3:29 PM, Brandon Williams <bmwill@google.com> wrote:

Show 14 quoted lines

> Teach `submodule init` to initialize submodules which have been
> configured to be active by setting 'submodule.active' with a pathspec.
>
> Now if no path arguments are given and 'submodule.active' is configured,
> `init` will initialize all submodules which have been configured to be
> active.  If no path arguments are given and 'submodule.active' is not
> configured, then `init` will retain the old behavior of initializing all
> submodules.
>
> This allows users to record more complex patterns as it saves retyping
> them whenever you invoke update.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---

Show 13 quoted lines

> @@ -417,7 +445,13 @@ static int module_init(int argc, const char **argv, const char *prefix)
>         argc = parse_options(argc, argv, prefix, module_init_options,
>                              git_submodule_helper_usage, 0);
>
> -       if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
> +       /*
> +        * If there are no path args and submodule.active is set then,
> +        * by default, only initialize 'active' modules.
> +        */
> +       if (!argc && git_config_get_value_multi("submodule.active"))
> +               module_list_active(&list);
> +       else if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
>                 return 1;

I would rather reuse module_list_compute and then post-process the list to filter out inactive submodules iff "submodule.active" is set as that seems cleaner and performance is not a pressing issue here?

Show 9 quoted lines

>
> +static void module_list_active(struct module_list *list)
> +{
> +       int i;
> +
> +       if (read_cache() < 0)
> +               die(_("index file corrupt"));
> +
> +       gitmodules_config();

Here we also need to have

  git_config(submodule_config, NULL);

such that is_submodule_initialized works correctly, I would assume?

Show 19 quoted lines

> +
> +       for (i = 0; i < active_nr; i++) {
> +               const struct cache_entry *ce = active_cache[i];
> +
> +               if (!S_ISGITLINK(ce->ce_mode) ||
> +                   !is_submodule_initialized(ce->name))
> +                       continue;
> +
> +               ALLOC_GROW(list->entries, list->nr + 1, list->alloc);
> +               list->entries[list->nr++] = ce;
> +               while (i + 1 < active_nr &&
> +                      !strcmp(ce->name, active_cache[i + 1]->name))
> +                       /*
> +                        * Skip entries with the same name in different stages
> +                        * to make sure an entry is returned only once.
> +                        */
> +                       i++;
> +       }
> +}