Subject: Re: [PATCH 05/15] submodule--helper: don't overlay config in update-clone

From: Brandon Williams <[email-masked]>
Date: Tue, 25 Jul 2017 16:39:03 -0700

On 07/25, Stefan Beller wrote:
> On Tue, Jul 25, 2017 at 2:39 PM, Brandon Williams <[email-masked]> wrote:
> > Don't rely on overlaying the repository's config on top of the
> > submodule-config, instead query the repository's config directly for the
> > url and the update strategy configuration.
> >
> > Signed-off-by: Brandon Williams <[email-masked]>
> > ---
> ...
> 
> > +struct submodule_update_strategy submodule_strategy_with_config_overlayed(struct repository *repo,
> > +                                                                         const struct submodule *sub)
> > +{
> > +       struct submodule_update_strategy strat = sub->update_strategy;
> > +       const char *update;
> > +       char *key;
> > +
> > +       key = xstrfmt("submodule.%s.update", sub->name);
> > +       if (!repo_config_get_string_const(repo, key, &update)) {
> > +               strat.command = NULL;
> > +               if (!strcmp(update, "none")) {
> > +                       strat.type = SM_UPDATE_NONE;
> > +               } else if (!strcmp(update, "checkout")) {
> > +                       strat.type = SM_UPDATE_CHECKOUT;
> > +               } else if (!strcmp(update, "rebase")) {
> > +                       strat.type = SM_UPDATE_REBASE;
> > +               } else if (!strcmp(update, "merge")) {
> > +                       strat.type = SM_UPDATE_MERGE;
> > +               } else if (skip_prefix(update, "!", &update)) {
> > +                       strat.type = SM_UPDATE_COMMAND;
> > +                       strat.command = update;
> > +               } else {
> > +                       die("invalid submodule update strategy '%s'", update);
> > +               }
> > +       }
> 
> Can this be simplified by reusing
>     parse_submodule_update_strategy(value, dest)
> ?

It would result in a memory leak if we did.  Really I'd like to just
remove this entirely. The only reason this needs to be done is for
checkout, which if we don't have respect the update config it can be
removed.

-- 
Brandon Williams