Re: [PATCH v3 07/10] clone: add --submodule-spec=<pathspec> switch

From: Stefan Beller <sbeller@google.com>
To: Brandon Williams <bmwill@google.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [PATCH v3 07/10] clone: add --submodule-spec=<pathspec> switch
Date: Wed, 15 Mar 2017 16:25:54 -0700	[thread overview]
Message-ID: <CAGZ79kbqcGe30-FO4jAzx1wmJBF1dg5O5zYAK5mOFOfQk3PULQ@mail.gmail.com> (raw)
In-Reply-To: <20170315230834.GH159137@google.com>

On Wed, Mar 15, 2017 at 4:08 PM, Brandon Williams <bmwill@google.com> wrote:
> On 03/14, Junio C Hamano wrote:
>> Brandon Williams <bmwill@google.com> writes:
>>
>> > The new switch passes the pathspec to `git submodule update
>> > --init-active` which is called after the actual clone is done.
>> >
>> > Additionally this configures the submodule.active option to
>> > be the given pathspec, such that any future invocation of
>> > `git submodule update --init-active` will keep up
>> > with the pathspec.
>> >
>> > Based on a patch by Stefan Beller <sbeller@google.com>
>> >
>> > Signed-off-by: Brandon Williams <bmwill@google.com>
>> > ---
>> >  Documentation/git-clone.txt | 23 ++++++++++-----
>> >  builtin/clone.c             | 36 +++++++++++++++++++++--
>> >  t/t7400-submodule-basic.sh  | 70 +++++++++++++++++++++++++++++++++++++++++++++
>> >  3 files changed, 120 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
>> > index 35cc34b2f..9692eab30 100644
>> > --- a/Documentation/git-clone.txt
>> > +++ b/Documentation/git-clone.txt
>> > @@ -15,7 +15,8 @@ SYNOPSIS
>> >       [--dissociate] [--separate-git-dir <git dir>]
>> >       [--depth <depth>] [--[no-]single-branch]
>> >       [--recursive | --recurse-submodules] [--[no-]shallow-submodules]
>> > -     [--jobs <n>] [--] <repository> [<directory>]
>> > +     [--submodule-spec <pathspec>] [--jobs <n>] [--]
>> > +     <repository> [<directory>]
>>
>> Hmph.  Can we then make "--recurse-submodules" an obsolete way to
>> spell "--submodule-spec ."?  I am not actively suggesting to
>> deprecate it; I am trying to see if there are semantic differences
>> between the two.
>
> We can if you think that would be better.  That way if at clone time you
> say "I want the submodules too" that your default config tracks all
> submodules even new ones yet to be added.
>
>>
>> I am also wondering "--recurse-submodules=<pathspec>" would be a
>> better UI, instead of introducing yet another option.
>
> Yeah we could do that, have --recurse-submodules be a repeated option
> and if you don't specify a value it defaults to "."
>
> Any thoughts on this Stefan?

I think when I first tried to tackle this problem, I realized that the recursing
beyond the first level is orthogonal to the selection of submodules in the
first level.

Consider the following submodules

    /sub1
    /sub1/nestedA
    /sub2
    /sub2/nestedB

To select sub1
  git clone --submodule-spec=sub1

To select sub1 +nestedA
  git clone --submodule-spec=sub1 --recurse-submodules

To select sub1+sub2
  git clone --submodule-spec=. --no-recurse-submodules

How to select sub1+nestedA+sub2, but not nestedB
  not possible in one command

I wonder if we want to be able to differentiate these cases at all,
I guess if you want sub1, you also care about nestedB ?

(Or is there any way to recurse pathspecs,
which could be gitattributes)

That said, I optimized for the complex case, which we might not have
and a combination into one switch makes sense, such that
  git clone --recurse-submodules=<pathspec>
is sufficient and turns on recursion beyond the first level.

Thanks,
Stefan