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

[Brandon Williams <bmwill@google.com>]

On 03/17, Stefan Beller wrote:
> On Thu, Mar 16, 2017 at 3:29 PM, Brandon Williams <bmwill@google.com> wrote:
> > 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>
> > ---
> 
> 
> > @@ -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?

Ok, Can do.  Shouldn't be very hard to do that.

> 
> >
> > +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?

No I don't think so.  is_submodule_initialized doesn't need them to be
overlayed, it just needs the .gitmodules mappings.

> 
> > +
> > +       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++;
> > +       }
> > +}

-- 
Brandon Williams