Re: [PATCH 01/10] submodule: decouple url and submodule existence

From: Brandon Williams <bmwill@google.com>
Date: Mar 6, 2017, 18:50 UTC
Message-ID: <20170306185048.GC183239@google.com>
In-Reply-To: <CAGZ79kb9Fhb0z5AbygBx9wNyLr-ovb-74xeaD8p+C5Xz9Dekcw@mail.gmail.com>

On 03/01, Stefan Beller wrote:

Sorry I've been slow at rerolling this.  I'll send out a reroll today.

Show 7 quoted lines

> IIRC most of the series is actually refactoring, i.e.
> providing a central function that answers
> "is this submodule active/initialized?" and then making use of this
> function.
> 
> Maybe it would be easier to reroll these refactorings first without adding
> in the change of behavior.

I agree, when I resend out the series I can drop the first patch so that the series as a whole just moves to using a standardized function to check that a submodule is active. I can then resend out the first patch in the series on its own so that we can more easily discuss the best route to take.

Show 24 quoted lines

> 
> Off the list we talked about different models how to indicate that
> a submodule is active.
> 
> Current situation:
> submodule.<name>.URL is used as a boolean flag
> 
> Possible future A:
> 1) If submodule.active exists use that
> 2) otherwise go be individual .URL configs
> 
> submodule.active is a config that contains a pathspec,
> i.e. specifies the group of submodules instead of marking
> each submodule individually.
> 
> How to get to future A:
> * apply this patch series
> 
> Possible future B:
> 1) the boolean submodule.<name>.exists is used
>     if existent
> 2) If that boolean is missing we'd respect a grouping
>    feature such as submodule.active
> 3) fallback to the .URL as a boolean indicator.

Thinking on this, and to maintain simplicity and uniformity, we could have the per-submodule boolean flag to be "submodule.<name>.active" while the general pathspec based config option would be "submodule.active".

Show 28 quoted lines

> 
> How to get to future B:
> 1) possibly take the refactoring part from this series
> 2) implement the .exists flag (that can solve the worktree
>   problem, as then we don't have to abuse the .URL
>   as a boolean indicator any more.)
> 3) implement the grouping feature that takes precedence
>   over .URL, but yields precedence to the .exists flag.
>   (This would solve the grouping problem)
> 
> By having two different series (2) and (3) we'd solve
> just one thing at a time with each series, such that
> this seems easier to understand and review.
> 
> The question is if this future B is also easier to use for
> users and I think it would be, despite being technically more
> complex.
> 
> Most users only have a few submodules. Also the assumption
> is to have either interest in very few specific submodules
> or all of them. For both of these cases the .exists is a good idea
> as it is very explicit, but verbose.
> 
> The grouping feature would help in the case of lots of
> submodules, e.g. to select all of them you would just give
> an easy pathspec "." and that would help going forward
> as by such a submodule spec all new submodules would
> be "auto-on".

And if you wanted all but one you could have submodule.active="." while submodule.dontwant.active=false instead of having a multi value submodule.active with an exclusion pathspec. Both work but one may be easier for a user to understand.

Show 8 quoted lines

> 
> Possible future C:
> Similar to B, but with branch specific configuration.
> submodule.<branchname>.active would work as a
> grouping feature. I wonder how it would work with
> the .exists flag.
> 
> Stefan

-- 
Brandon Williams