Re: [PATCH v3 09/10] submodule--helper init: set submodule.<name>.active

From: Brandon Williams <bmwill@google.com>
Date: Mar 15, 2017, 22:46 UTC
Message-ID: <20170315224604.GG159137@google.com>
In-Reply-To: <xmqqinnbiqzs.fsf@gitster.mtv.corp.google.com>

On 03/14, Junio C Hamano wrote:

Show 25 quoted lines

> Brandon Williams <bmwill@google.com> writes:
> 
> > When initializing a submodule set the submodule.<name>.active config to
> > true to indicate that the submodule is active.
> >
> > Signed-off-by: Brandon Williams <bmwill@google.com>
> > ---
> 
> Hmph.  When you do
> 
> 	git clone --submodule-spec=lib/
> 
> and resulting repository says "submodule.active = lib/", you are
> saying that you are interested in anything in "lib/", now or in the
> future--that is the point of 08/10.
> 
> It is unclear what your desire would be for a submodule that was
> bound to somewhere in "lib/" in the superproject when you cloned
> when it later gets moved outside "lib/".  With changes up to 08/10,
> that submodule will no longer be interesting, but with this 09/10,
> when recursing clone will activate it upon the initial clone, the
> submodule is explicitly marked as active by its name, so no matter
> where it gets moved later, it will remain to be interesting.
> 
> I am not sure if that is a desired behaviour.

I'm going to blame my poor commit msg for this.

Essentially it would be nice if when a user explicitly says "I want to init submodule b" that we obey and do just that. What this patch does is allow for a user to do that and the "submodule.<name>.active" config option will only be set to 'true' in the event that that the submodule isn't already marked as being active/interesting by some other means. So if we did

  git clone --submodule-spec=lib/

then each module under 'lib/' won't have their submodule.<name>.active config set since submodule.active=lib/ already encompasses them.

But if no submodule-spec was given then it would default to all submodules and set every submodule's submodule.<name>.active config to true.

I'll add a few tests to illustrate this.

Show 22 quoted lines

> 
> >  builtin/submodule--helper.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> > index a3acc9e4c..b669ed031 100644
> > --- a/builtin/submodule--helper.c
> > +++ b/builtin/submodule--helper.c
> > @@ -361,6 +361,13 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
> >  		die(_("No url found for submodule path '%s' in .gitmodules"),
> >  			displaypath);
> >  
> > +	/* Set active flag for the submodule being initialized */
> > +	if (!is_submodule_initialized(path)) {
> > +		strbuf_reset(&sb);
> > +		strbuf_addf(&sb, "submodule.%s.active", sub->name);
> > +		git_config_set_gently(sb.buf, "true");
> > +	}
> > +
> >  	/*
> >  	 * Copy url setting when it is not set yet.
> >  	 * To look up the url in .git/config, we must not fall back to

-- 
Brandon Williams