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

From: Brandon Williams <bmwill@google.com>
Date: Jul 25, 2017, 23:39 UTC
Message-ID: <20170725233903.GB71799@google.com>
In-Reply-To: <CAGZ79kacdTFVJknTx+ceT8epytXSJDRVAwZO4HyzpsmVbK5VTQ@mail.gmail.com>

On 07/25, Stefan Beller wrote:

Show 38 quoted lines

> On Tue, Jul 25, 2017 at 2:39 PM, Brandon Williams <bmwill@google.com> 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 <bmwill@google.com>
> > ---
> ...
> 
> > +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