Re: [PATCH v3 06/10] submodule update: add `--init-active` switch

From: Brandon Williams <bmwill@google.com>
Date: Mar 15, 2017, 21:52 UTC
Message-ID: <20170315215205.GE159137@google.com>
In-Reply-To: <xmqqvarbis4j.fsf@gitster.mtv.corp.google.com>

On 03/14, Junio C Hamano wrote:

Show 8 quoted lines

> Brandon Williams <bmwill@google.com> writes:
> 
> > +--init-active::
> > +	This option is only valid for the update command.
> > +	Initialize all submodules configured in "`submodule.active`"
> > +	that have not been updated before.
> 
> You mean s/not been updated/not been initialized/ here?

Yep you're right, the documentation is a little stale too so I'll update it.

Show 22 quoted lines

> 
> > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> > index f38e332c5..a3acc9e4c 100644
> > --- a/builtin/submodule--helper.c
> > +++ b/builtin/submodule--helper.c
> > @@ -270,6 +270,34 @@ static int module_list_compute(int argc, const char **argv,
> >  	return result;
> >  }
> >  
> > +static void module_list_active(struct module_list *list)
> > +{
> > +	int i;
> > +
> > +	if (read_cache() < 0)
> > +		die(_("index file corrupt"));
> > +
> > +	gitmodules_config();
> 
> To this codepath, it probably would not make any difference, but in
> general, calling gitmodules_config() after you did git_config()
> would be a bug, right?  Otherwise, gitmodules_config() would
> overwrite submodule.* configuration you read with git_config().

gitmodules_config() doesn't interact with the git config, unless you do git_config(submodule_confi, NULL) to overlay the two. But yes it doesn't effect this codepath.

> I have a feeling that we need to make this function hard to misuse;
> it is not a fault of _this_ patch, but still any and all additions
> to any of the submodule*.c files needs careful vetting.

-- 
Brandon Williams