Re: [PATCH 1/3] clone: add `--shallow-submodules` flag

[Stefan Beller <sbeller@google.com>]

On Mon, Apr 25, 2016 at 4:50 AM, Lars Schneider
<larsxschneider@gmail.com> wrote:
>> first case (as we wouldd have already transmitted the non shallow over
> s/wouldd/would/

will be fixed in a reroll

>> --depth <depth>::
>>       Create a 'shallow' clone with a history truncated to the
>> -     specified number of revisions.
>> +     specified number of revisions. Implies `--single-branch` unless
>> +     `--no-single-branch` is given to fetch the histories near the
>> +     tips of all branches. This implies `--shallow-submodules`. If
>> +     you want to have a shallow clone, but full submodules, also pass
> Can we make this more explicit. Attention, bikeshedding:
> If you want to have a shallow parent clone, but full submodules...

Good point, but with s/parent/superproject/ ? (What is a "parent clone"?
It is not defined in the glossary, so let's not make up more confusing words
for Git here :)

>
>
>> +     `--no-shallow-submodules`.
>>
>> --[no-]single-branch::
>>       Clone only the history leading to the tip of a single branch,
>> @@ -214,6 +218,9 @@ objects from the source repository into a pack in the cloned repository.
>>       repository does not have a worktree/checkout (i.e. if any of
>>       `--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
>>
>> +--shallow-submodules::
> Should that be "--[no-]shallow-submodules" ?

will be fixed in a reroll

>
>
>> +     All submodules which are cloned, will be shallow.
> Maybe you could extend it with "... will be shallow with a depth of 1." ?

done

>
>
>> +
>> --separate-git-dir=<git dir>::
>>       Instead of placing the cloned repository where it is supposed
>>       to be, place the cloned repository at the specified directory,
>> diff --git a/builtin/clone.c b/builtin/clone.c
>> index b004fb4..ecdf308 100644
>> --- a/builtin/clone.c
>> +++ b/builtin/clone.c
>> @@ -40,6 +40,7 @@ static const char * const builtin_clone_usage[] = {
>>
>> static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;
>> static int option_local = -1, option_no_hardlinks, option_shared, option_recursive;
>> +static int option_shallow_submodules = -1;
>> static char *option_template, *option_depth;
>> static char *option_origin = NULL;
>> static char *option_branch = NULL;
>> @@ -91,6 +92,8 @@ static struct option builtin_clone_options[] = {
>>                   N_("create a shallow clone of that depth")),
>>       OPT_BOOL(0, "single-branch", &option_single_branch,
>>                   N_("clone only one branch, HEAD or --branch")),
>> +     OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
>> +                 N_("any cloned submodules will be shallow")),
> I am not familiar with this code but I assume the "no-" prefix flips the bool?

Giving the --no-option stores an explicit 0 (it is initialized as -1),
and passing --option stores a 1.

>> @@ -727,6 +730,10 @@ static int checkout(void)
>>               struct argv_array args = ARGV_ARRAY_INIT;
>>               argv_array_pushl(&args, "submodule", "update", "--init", "--recursive", NULL);
>>
>> +             if (option_shallow_submodules == 1
>> +                 || (option_shallow_submodules == -1 && option_depth))
>> +                     argv_array_push(&args, "--depth=1");
>> +

which explains this here as:

        If  --shallow-submodules was given
          || (neither --[no-]shallow-submodules was given, but --depth
was given,
            i.e. depth implies --shallow-submodule only if no explicit
choice was made
            by the user.

>>               if (max_jobs != -1)
>>                       argv_array_pushf(&args, "--jobs=%d", max_jobs);
>>
>> --
>> 2.7.0.rc0.42.g8e9204f.dirty
>>
>