Re: [PATCH v3 07/10] clone: add --submodule-spec=<pathspec> switch
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 14, 2017, 19:38 UTC
- Message-ID
- <xmqqmvcnir8m.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170313214341.172676-8-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
Show 28 quoted lines
> 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.
I am also wondering "--recurse-submodules=<pathspec>" would be a better UI, instead of introducing yet another option.
Show 16 quoted lines
> @@ -217,12 +218,20 @@ objects from the source repository into a pack in the cloned repository. > > --recursive:: > --recurse-submodules:: > - After the clone is created, initialize all submodules within, > - using their default settings. This is equivalent to running > - `git submodule update --init --recursive` immediately after > - the clone is finished. This option is ignored if the cloned > - repository does not have a worktree/checkout (i.e. if any of > - `--no-checkout`/`-n`, `--bare`, or `--mirror` is given) > + After the clone is created, initialize and clone all submodules > + within, using their default settings. This is equivalent to > + running `git submodule update --recursive --init` immediately > + after the clone is finished. This option is ignored if the > + cloned repository does not have a worktree/checkout (i.e. if > + any of `--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
With reflowing it is unnecessarily harder to spot what got changed. "and clone" is inserted, "--init" and "--recursive" were swapped. Any other changes?
Show 18 quoted lines
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 3f63edbbf..c6731379b 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -56,6 +56,16 @@ static struct string_list option_required_reference = STRING_LIST_INIT_NODUP;
> static struct string_list option_optional_reference = STRING_LIST_INIT_NODUP;
> static int option_dissociate;
> static int max_jobs = -1;
> +static struct string_list submodule_spec;
> +
> +static int submodule_spec_cb(const struct option *opt, const char *arg, int unset)
> +{
> + if (unset)
> + return -1;
> +
> + string_list_append((struct string_list *)opt->value, arg);
> + return 0;
> +}Hmph, doesn't OPT_STRING_LIST work for this thing?
Show 10 quoted lines
> + if (submodule_spec.nr > 0) {
> + struct string_list_item *item;
> + struct strbuf sb = STRBUF_INIT;
> + for_each_string_list_item(item, &submodule_spec) {
> + strbuf_addf(&sb, "submodule.active=%s",
> + item->string);
> + string_list_append(&option_config,
> + strbuf_detach(&sb, NULL));
> + }
> + }OK. Each pathspec becomes submodule.active in the newly created repository.