[PATCH v3 0/5] clone: --no-tags option

[PATCH v3 0/5] clone: --no-tags option

[Ævar Arnfjörð Bjarmason]

This is an expansion of the previously solo 02/05 "clone: add a
--no-tags option to clone without tags" patch (see
<20170418191553.15464-1-avarab@gmail.com>).

This addresses the comments by Junio & Jonathan Nieder on v2 (thanks a
lot), and in addition implements a --no-tags-submodules option. That
code was implemented by Brandon & sent to me privately after I'd
failed to come up with it, but I added tests, a commit message & bash
completion to it.

The WIP 5/5 patch implements a submodule.NAME.tags config facility for
the option, but is broken currently & floats along in this submission
as an RFC patch. AFAICT it *should* work and it goes through all the
motions the similar existing *.shallow config does, but for some
reason the tags=false option isn't picked up & propagated in a freshly
cloned submodule.

I'm probably missing something trivial, but I can't see what it is,
I'm hoping thath either Stefan or Brandon will see what that is.

Brandon Williams (1):
  clone: add a --no-tags-submodules to pass --no-tags to submodules

Ævar Arnfjörð Bjarmason (4):
  tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"
  clone: add a --no-tags option to clone without tags
  tests: rename a test having to do with shallow submodules
  WIP clone: add a --[no-]recommend-tags & submodule.NAME.tags config

 Documentation/git-clone.txt                        |  21 ++++
 Documentation/git-submodule.txt                    |   8 +-
 builtin/clone.c                                    |  19 +++-
 builtin/submodule--helper.c                        |  21 +++-
 contrib/completion/git-completion.bash             |   3 +
 git-submodule.sh                                   |  13 ++-
 submodule-config.c                                 |   8 ++
 submodule-config.h                                 |   1 +
 t/t5612-clone-refspec.sh                           | 103 +++++++++++++++++---
 ...odules.sh => t5614-clone-submodules-shallow.sh} |   0
 t/t5616-clone-submodules-tags.sh                   | 106 +++++++++++++++++++++
 11 files changed, 284 insertions(+), 19 deletions(-)
 rename t/{t5614-clone-submodules.sh => t5614-clone-submodules-shallow.sh} (100%)
 create mode 100755 t/t5616-clone-submodules-tags.sh

-- 
2.11.0

[PATCH v3 1/5] tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"

[Ævar Arnfjörð Bjarmason]

Change occurrences "cd" followed by "fetch" on a single line to be on
two lines.

This is purely a stylistic change pointed out in code review for an
unrelated patch. Change the these tests use so new tests added later
using the more common style don't look out of place.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t5612-clone-refspec.sh | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/t/t5612-clone-refspec.sh b/t/t5612-clone-refspec.sh
index 7ace2535c8..97c847fab6 100755
--- a/t/t5612-clone-refspec.sh
+++ b/t/t5612-clone-refspec.sh
@@ -59,7 +59,8 @@ test_expect_success 'setup' '
 
 test_expect_success 'by default all branches will be kept updated' '
 	(
-		cd dir_all && git fetch &&
+		cd dir_all &&
+		git fetch &&
 		git for-each-ref refs/remotes/origin |
 		sed -e "/HEAD$/d" \
 		    -e "s|/remotes/origin/|/heads/|" >../actual
@@ -71,7 +72,8 @@ test_expect_success 'by default all branches will be kept updated' '
 
 test_expect_success 'by default no tags will be kept updated' '
 	(
-		cd dir_all && git fetch &&
+		cd dir_all &&
+		git fetch &&
 		git for-each-ref refs/tags >../actual
 	) &&
 	git for-each-ref refs/tags >expect &&
@@ -80,7 +82,8 @@ test_expect_success 'by default no tags will be kept updated' '
 
 test_expect_success '--single-branch while HEAD pointing at master' '
 	(
-		cd dir_master && git fetch &&
+		cd dir_master &&
+		git fetch &&
 		git for-each-ref refs/remotes/origin |
 		sed -e "/HEAD$/d" \
 		    -e "s|/remotes/origin/|/heads/|" >../actual
@@ -92,7 +95,8 @@ test_expect_success '--single-branch while HEAD pointing at master' '
 
 test_expect_success '--single-branch while HEAD pointing at side' '
 	(
-		cd dir_side && git fetch &&
+		cd dir_side &&
+		git fetch &&
 		git for-each-ref refs/remotes/origin |
 		sed -e "/HEAD$/d" \
 		    -e "s|/remotes/origin/|/heads/|" >../actual
@@ -104,7 +108,8 @@ test_expect_success '--single-branch while HEAD pointing at side' '
 
 test_expect_success '--single-branch with explicit --branch side' '
 	(
-		cd dir_side2 && git fetch &&
+		cd dir_side2 &&
+		git fetch &&
 		git for-each-ref refs/remotes/origin |
 		sed -e "/HEAD$/d" \
 		    -e "s|/remotes/origin/|/heads/|" >../actual
@@ -116,7 +121,8 @@ test_expect_success '--single-branch with explicit --branch side' '
 
 test_expect_success '--single-branch with explicit --branch with tag fetches updated tag' '
 	(
-		cd dir_tag && git fetch &&
+		cd dir_tag &&
+		git fetch &&
 		git for-each-ref refs/tags >../actual
 	) &&
 	git for-each-ref refs/tags >expect &&
@@ -125,7 +131,8 @@ test_expect_success '--single-branch with explicit --branch with tag fetches upd
 
 test_expect_success '--single-branch with --mirror' '
 	(
-		cd dir_mirror && git fetch &&
+		cd dir_mirror &&
+		git fetch &&
 		git for-each-ref refs > ../actual
 	) &&
 	git for-each-ref refs >expect &&
@@ -134,7 +141,8 @@ test_expect_success '--single-branch with --mirror' '
 
 test_expect_success '--single-branch with explicit --branch and --mirror' '
 	(
-		cd dir_mirror_side && git fetch &&
+		cd dir_mirror_side &&
+		git fetch &&
 		git for-each-ref refs > ../actual
 	) &&
 	git for-each-ref refs >expect &&
@@ -143,7 +151,8 @@ test_expect_success '--single-branch with explicit --branch and --mirror' '
 
 test_expect_success '--single-branch with detached' '
 	(
-		cd dir_detached && git fetch &&
+		cd dir_detached &&
+		git fetch &&
 		git for-each-ref refs/remotes/origin |
 		sed -e "/HEAD$/d" \
 		    -e "s|/remotes/origin/|/heads/|" >../actual
-- 
2.11.0

[PATCH v3 2/5] clone: add a --no-tags option to clone without tags

[Ævar Arnfjörð Bjarmason]

Add a --no-tags option to clone without fetching any tags.

Without this change there's no easy way to clone a repository without
also fetching its tags.

When supplying --single-branch the primary remote branch will be
cloned, but in addition tags will be followed & retrieved. Now
--no-tags can be added --single-branch to clone a repository without
tags, and which only tracks a single upstream branch.

This option works without --single-branch as well, and will do a
normal clone but not fetch any tags.

Many git commands pay some fixed overhead as a function of the number
of references. E.g. creating ~40k tags in linux.git will cause a
command like `git log -1 >/dev/null` to run in over a second instead
of in a matter of milliseconds, in addition numerous other things will
slow down, e.g. "git log <TAB>" with the bash completion will slowly
show ~40k references instead of 1.

The user might want to avoid all of that overhead to simply use a
repository like that to browse the "master" branch, or something like
a CI tool might want to keep that one branch up-to-date without caring
about any other references.

Without this change the only way of accomplishing this was either by
manually tweaking the config in a fresh repository:

    git init git &&
    cat >git/.git/config <<EOF &&
    [remote "origin"]
        url = git@github.com:git/git.git
        tagOpt = --no-tags
        fetch = +refs/heads/master:refs/remotes/origin/master
    [branch "master"]
        remote = origin
        merge = refs/heads/master
    EOF
    cd git &&
    git pull

Which requires hardcoding the "master" name, which may not be the main
--single-branch would have retrieved, or alternatively by setting
tagOpt=--no-tags right after cloning & deleting any existing tags:

    git clone --single-branch git@github.com:git/git.git &&
    cd git &&
    git config remote.origin.tagOpt --no-tags &&
    git tag -l | xargs git tag -d

Which of course was also subtly buggy if --branch was pointed at a
tag, leaving the user in a detached head:

    git clone --single-branch --branch v2.12.0 git@github.com:git/git.git &&
    cd git &&
    git config remote.origin.tagOpt --no-tags &&
    git tag -l | xargs git tag -d

Now all this complexity becomes the much simpler:

    git clone --single-branch --no-tags git@github.com:git/git.git

Or in the case of cloning a single tag "branch":

    git clone --single-branch --branch v2.12.0 --no-tags git@github.com:git/git.git

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-clone.txt            | 14 ++++++-
 builtin/clone.c                        | 13 +++++-
 contrib/completion/git-completion.bash |  1 +
 t/t5612-clone-refspec.sh               | 76 +++++++++++++++++++++++++++++++++-
 4 files changed, 99 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 30052cce49..83c8e9b394 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 	  [-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror]
 	  [-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
 	  [--dissociate] [--separate-git-dir <git dir>]
-	  [--depth <depth>] [--[no-]single-branch]
+	  [--depth <depth>] [--[no-]single-branch] [--no-tags]
 	  [--recurse-submodules] [--[no-]shallow-submodules]
 	  [--jobs <n>] [--] <repository> [<directory>]
 
@@ -215,6 +215,18 @@ objects from the source repository into a pack in the cloned repository.
 	branch when `--single-branch` clone was made, no remote-tracking
 	branch is created.
 
+--no-tags::
+	Don't clone any tags, and set
+	`remote.<remote>.tagOpt=--no-tags` in the config, ensuring
+	that future `git pull` and `git fetch` operations won't follow
+	any tags. Subsequent explicit tag fetches will still work,
+	(see linkgit:git-fetch[1]).
++
+Can be used in conjunction with `--single-branch` to clone and
+maintain a branch with no references other than a single cloned
+branch. This is useful e.g. to maintain minimal clones of the default
+branch of some repository for search indexing.
+
 --recurse-submodules[=<pathspec]::
 	After the clone is created, initialize and clone submodules
 	within based on the provided pathspec.  If no pathspec is
diff --git a/builtin/clone.c b/builtin/clone.c
index de85b85254..05f52d6f2b 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;
+static int option_no_tags;
 static int option_shallow_submodules;
 static int deepen;
 static char *option_template, *option_depth, *option_since;
@@ -120,6 +121,8 @@ static struct option builtin_clone_options[] = {
 			N_("deepen history of shallow clone, excluding rev")),
 	OPT_BOOL(0, "single-branch", &option_single_branch,
 		    N_("clone only one branch, HEAD or --branch")),
+	OPT_BOOL(0, "no-tags", &option_no_tags,
+		 N_("don't clone any tags, and make later fetches not to follow them")),
 	OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
 		    N_("any cloned submodules will be shallow")),
 	OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
@@ -563,7 +566,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
 	} else
 		get_fetch_map(refs, refspec, &tail, 0);
 
-	if (!option_mirror && !option_single_branch)
+	if (!option_mirror && !option_single_branch && !option_no_tags)
 		get_fetch_map(refs, tag_refspec, &tail, 0);
 
 	return local_refs;
@@ -652,7 +655,7 @@ static void update_remote_refs(const struct ref *refs,
 
 	if (refs) {
 		write_remote_refs(mapped_refs);
-		if (option_single_branch)
+		if (option_single_branch && !option_no_tags)
 			write_followtags(refs, msg);
 	}
 
@@ -1035,6 +1038,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	git_config_set(key.buf, repo);
 	strbuf_reset(&key);
 
+	if (option_no_tags) {
+		strbuf_addf(&key, "remote.%s.tagOpt", option_origin);
+		git_config_set(key.buf, "--no-tags");
+		strbuf_reset(&key);
+	}
+
 	if (option_required_reference.nr || option_optional_reference.nr)
 		setup_reference();
 
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1150164d5c..6da997cf42 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1309,6 +1309,7 @@ _git_clone ()
 			--template=
 			--depth
 			--single-branch
+			--no-tags
 			--branch
 			--recurse-submodules
 			--no-single-branch
diff --git a/t/t5612-clone-refspec.sh b/t/t5612-clone-refspec.sh
index 97c847fab6..fac5a73851 100755
--- a/t/t5612-clone-refspec.sh
+++ b/t/t5612-clone-refspec.sh
@@ -17,13 +17,20 @@ test_expect_success 'setup' '
 	echo four >file &&
 	git commit -a -m four &&
 	git checkout master &&
+	git tag five &&
 
 	# default clone
 	git clone . dir_all &&
 
+	# default clone --no-tags
+	git clone --no-tags . dir_all_no_tags &&
+
 	# default --single that follows HEAD=master
 	git clone --single-branch . dir_master &&
 
+	# default --single that follows HEAD=master with no tags
+	git clone --single-branch --no-tags . dir_master_no_tags &&
+
 	# default --single that follows HEAD=side
 	git checkout side &&
 	git clone --single-branch . dir_side &&
@@ -45,6 +52,9 @@ test_expect_success 'setup' '
 	# explicit --single with tag
 	git clone --single-branch --branch two . dir_tag &&
 
+	# explicit --single with tag and --no-tags
+	git clone --single-branch --no-tags --branch two . dir_tag_no_tags &&
+
 	# advance both "master" and "side" branches
 	git checkout side &&
 	echo five >file &&
@@ -77,7 +87,18 @@ test_expect_success 'by default no tags will be kept updated' '
 		git for-each-ref refs/tags >../actual
 	) &&
 	git for-each-ref refs/tags >expect &&
-	test_must_fail test_cmp expect actual
+	test_must_fail test_cmp expect actual &&
+	test_line_count = 2 actual
+'
+
+test_expect_success 'clone with --no-tags' '
+	(
+		cd dir_all_no_tags &&
+		git fetch &&
+		git for-each-ref refs/tags >../actual
+	) &&
+	>expect &&
+	test_cmp expect actual
 '
 
 test_expect_success '--single-branch while HEAD pointing at master' '
@@ -90,7 +111,47 @@ test_expect_success '--single-branch while HEAD pointing at master' '
 	) &&
 	# only follow master
 	git for-each-ref refs/heads/master >expect &&
-	test_cmp expect actual
+	# get & check latest tags
+	test_cmp expect actual &&
+	(
+		cd dir_master &&
+		git fetch --tags &&
+		git for-each-ref refs/tags >../actual
+	) &&
+	git for-each-ref refs/tags >expect &&
+	test_cmp expect actual &&
+	test_line_count = 2 actual
+'
+
+test_expect_success '--single-branch while HEAD pointing at master and --no-tags' '
+	(
+		cd dir_master_no_tags &&
+		git fetch &&
+		git for-each-ref refs/remotes/origin |
+		sed -e "/HEAD$/d" \
+		    -e "s|/remotes/origin/|/heads/|" >../actual
+	) &&
+	# only follow master
+	git for-each-ref refs/heads/master >expect &&
+	test_cmp expect actual &&
+	# get tags (noop)
+	(
+		cd dir_master_no_tags &&
+		git fetch &&
+		git for-each-ref refs/tags >../actual
+	) &&
+	>expect &&
+	test_cmp expect actual &&
+	test_line_count = 0 actual &&
+	# get tags with --tags overrides tagOpt
+	(
+		cd dir_master_no_tags &&
+		git fetch --tags &&
+		git for-each-ref refs/tags >../actual
+	) &&
+	git for-each-ref refs/tags >expect &&
+	test_cmp expect actual &&
+	test_line_count = 2 actual
 '
 
 test_expect_success '--single-branch while HEAD pointing at side' '
@@ -129,6 +190,17 @@ test_expect_success '--single-branch with explicit --branch with tag fetches upd
 	test_cmp expect actual
 '
 
+test_expect_success '--single-branch with explicit --branch with tag fetches updated tag despite --no-tags' '
+	(
+		cd dir_tag_no_tags &&
+		git fetch &&
+		git for-each-ref refs/tags >../actual
+	) &&
+	git for-each-ref refs/tags/two >expect &&
+	test_cmp expect actual &&
+	test_line_count = 1 actual
+'
+
 test_expect_success '--single-branch with --mirror' '
 	(
 		cd dir_mirror &&
-- 
2.11.0

Re: [PATCH v3 2/5] clone: add a --no-tags option to clone without tags

[Stefan Beller]

On Wed, Apr 26, 2017 at 4:12 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> Add a --no-tags option to clone without fetching any tags.
>
> Without this change there's no easy way to clone a repository without
> also fetching its tags.
>
> When supplying --single-branch the primary remote branch will be
> cloned, but in addition tags will be followed & retrieved. Now
> --no-tags can be added --single-branch to clone a repository without
> tags, and which only tracks a single upstream branch.
>
> This option works without --single-branch as well, and will do a
> normal clone but not fetch any tags.
>
> Many git commands pay some fixed overhead as a function of the number
> of references. E.g. creating ~40k tags in linux.git will cause a
> command like `git log -1 >/dev/null` to run in over a second instead
> of in a matter of milliseconds, in addition numerous other things will
> slow down, e.g. "git log <TAB>" with the bash completion will slowly
> show ~40k references instead of 1.
>
> The user might want to avoid all of that overhead to simply use a
> repository like that to browse the "master" branch, or something like
> a CI tool might want to keep that one branch up-to-date without caring
> about any other references.
>
> Without this change the only way of accomplishing this was either by
> manually tweaking the config in a fresh repository:
>
>     git init git &&
>     cat >git/.git/config <<EOF &&
>     [remote "origin"]
>         url = git@github.com:git/git.git
>         tagOpt = --no-tags
>         fetch = +refs/heads/master:refs/remotes/origin/master
>     [branch "master"]
>         remote = origin
>         merge = refs/heads/master
>     EOF
>     cd git &&
>     git pull
>
> Which requires hardcoding the "master" name, which may not be the main
> --single-branch would have retrieved, or alternatively by setting
> tagOpt=--no-tags right after cloning & deleting any existing tags:
>
>     git clone --single-branch git@github.com:git/git.git &&
>     cd git &&
>     git config remote.origin.tagOpt --no-tags &&
>     git tag -l | xargs git tag -d
>
> Which of course was also subtly buggy if --branch was pointed at a
> tag, leaving the user in a detached head:
>
>     git clone --single-branch --branch v2.12.0 git@github.com:git/git.git &&
>     cd git &&
>     git config remote.origin.tagOpt --no-tags &&
>     git tag -l | xargs git tag -d
>
> Now all this complexity becomes the much simpler:
>
>     git clone --single-branch --no-tags git@github.com:git/git.git
>
> Or in the case of cloning a single tag "branch":
>
>     git clone --single-branch --branch v2.12.0 --no-tags git@github.com:git/git.git
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

I like the option, though I dislike the implementation, specifically as you
brought up e.g. "[PATCH] various: disallow --no-no-OPT for --no-opt options".

Can we have an option "--tags" instead, which is on by default
and then you can negate it to --no-tags, without having to worry
about the no-no case.

The problem with tags is that they are in a shared name-space
and not part of the remote refspec. If they were, the documentation
would be way easier, too going this way.

> +--no-tags::
> +       Don't clone any tags, and set
> +       `remote.<remote>.tagOpt=--no-tags` in the config, ensuring
> +       that future `git pull` and `git fetch` operations won't follow
> +       any tags. Subsequent explicit tag fetches will still work,
> +       (see linkgit:git-fetch[1]).
> ++
> +Can be used in conjunction with `--single-branch` to clone and
> +maintain a branch with no references other than a single cloned
> +branch. This is useful e.g. to maintain minimal clones of the default
> +branch of some repository for search indexing.

In the future we may want to have '--depth' also imply --no-tags,
just as it implies --single-branch.

> @@ -652,7 +655,7 @@ static void update_remote_refs(const struct ref *refs,
>
>         if (refs) {
>                 write_remote_refs(mapped_refs);
> -               if (option_single_branch)
> +               if (option_single_branch && !option_no_tags)

I am debating if this needs to be an || instead of &&, as you would not
want to write the tags with "--no-single-branch --no-tags" ?

Thanks,
Stefan

Re: [PATCH v3 2/5] clone: add a --no-tags option to clone without tags

[Brandon Williams]

On 04/27, Stefan Beller wrote:
> I like the option, though I dislike the implementation, specifically as you
> brought up e.g. "[PATCH] various: disallow --no-no-OPT for --no-opt options".
> 
> Can we have an option "--tags" instead, which is on by default
> and then you can negate it to --no-tags, without having to worry
> about the no-no case.

Yeah I agree that a "--tags" option would make the ui cleaner.

-- 
Brandon Williams

Re: [PATCH v3 2/5] clone: add a --no-tags option to clone without tags

[Ævar Arnfjörð Bjarmason]

On Thu, Apr 27, 2017 at 7:54 PM, Stefan Beller <sbeller@google.com> wrote:
> On Wed, Apr 26, 2017 at 4:12 PM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> Add a --no-tags option to clone without fetching any tags.
>>
>> Without this change there's no easy way to clone a repository without
>> also fetching its tags.
>>
>> When supplying --single-branch the primary remote branch will be
>> cloned, but in addition tags will be followed & retrieved. Now
>> --no-tags can be added --single-branch to clone a repository without
>> tags, and which only tracks a single upstream branch.
>>
>> This option works without --single-branch as well, and will do a
>> normal clone but not fetch any tags.
>>
>> Many git commands pay some fixed overhead as a function of the number
>> of references. E.g. creating ~40k tags in linux.git will cause a
>> command like `git log -1 >/dev/null` to run in over a second instead
>> of in a matter of milliseconds, in addition numerous other things will
>> slow down, e.g. "git log <TAB>" with the bash completion will slowly
>> show ~40k references instead of 1.
>>
>> The user might want to avoid all of that overhead to simply use a
>> repository like that to browse the "master" branch, or something like
>> a CI tool might want to keep that one branch up-to-date without caring
>> about any other references.
>>
>> Without this change the only way of accomplishing this was either by
>> manually tweaking the config in a fresh repository:
>>
>>     git init git &&
>>     cat >git/.git/config <<EOF &&
>>     [remote "origin"]
>>         url = git@github.com:git/git.git
>>         tagOpt = --no-tags
>>         fetch = +refs/heads/master:refs/remotes/origin/master
>>     [branch "master"]
>>         remote = origin
>>         merge = refs/heads/master
>>     EOF
>>     cd git &&
>>     git pull
>>
>> Which requires hardcoding the "master" name, which may not be the main
>> --single-branch would have retrieved, or alternatively by setting
>> tagOpt=--no-tags right after cloning & deleting any existing tags:
>>
>>     git clone --single-branch git@github.com:git/git.git &&
>>     cd git &&
>>     git config remote.origin.tagOpt --no-tags &&
>>     git tag -l | xargs git tag -d
>>
>> Which of course was also subtly buggy if --branch was pointed at a
>> tag, leaving the user in a detached head:
>>
>>     git clone --single-branch --branch v2.12.0 git@github.com:git/git.git &&
>>     cd git &&
>>     git config remote.origin.tagOpt --no-tags &&
>>     git tag -l | xargs git tag -d
>>
>> Now all this complexity becomes the much simpler:
>>
>>     git clone --single-branch --no-tags git@github.com:git/git.git
>>
>> Or in the case of cloning a single tag "branch":
>>
>>     git clone --single-branch --branch v2.12.0 --no-tags git@github.com:git/git.git
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>
> I like the option, though I dislike the implementation, specifically as you
> brought up e.g. "[PATCH] various: disallow --no-no-OPT for --no-opt options".
>
> Can we have an option "--tags" instead, which is on by default
> and then you can negate it to --no-tags, without having to worry
> about the no-no case.
>
> The problem with tags is that they are in a shared name-space
> and not part of the remote refspec. If they were, the documentation
> would be way easier, too going this way.

I don't mind doing it this way, but this really should be part of an
unrelated topic to eliminate --no-OPT in favor of just --OPT which is
supplied by default, if we want to go that route.

Right now it makes more sense to have a --no-tags, because this along
with e.g. --no-hardlinks and numerous other options[1] is for an
obscure use-case most users won't care about, and then we have &
document --no-BEHAVIOR.

Perhaps we shouldn't have that at all, go through all these --no-OPT
and replace them with --OPT in both the docs & code & get rid of the
--no-no-OPT & some confusion that way.

1. git grep -- '\[--no-' Documentation/git-*txt

>> +--no-tags::
>> +       Don't clone any tags, and set
>> +       `remote.<remote>.tagOpt=--no-tags` in the config, ensuring
>> +       that future `git pull` and `git fetch` operations won't follow
>> +       any tags. Subsequent explicit tag fetches will still work,
>> +       (see linkgit:git-fetch[1]).
>> ++
>> +Can be used in conjunction with `--single-branch` to clone and
>> +maintain a branch with no references other than a single cloned
>> +branch. This is useful e.g. to maintain minimal clones of the default
>> +branch of some repository for search indexing.
>
> In the future we may want to have '--depth' also imply --no-tags,
> just as it implies --single-branch.

Are there some cases where we'd like to also somehow import & rewrite
tags covering the given --depth?

>> @@ -652,7 +655,7 @@ static void update_remote_refs(const struct ref *refs,
>>
>>         if (refs) {
>>                 write_remote_refs(mapped_refs);
>> -               if (option_single_branch)
>> +               if (option_single_branch && !option_no_tags)
>
> I am debating if this needs to be an || instead of &&, as you would not
> want to write the tags with "--no-single-branch --no-tags" ?

This hunk is confusing out of context, but I believe it's doing the
right thing now. This write_followtags() codepath is only taken (and
is only meaningful) with --single-branch, there's an earlier change
just before that:

    -       if (!option_mirror && !option_single_branch)
    +       if (!option_mirror && !option_single_branch && !option_no_tags)
                    get_fetch_map(refs, tag_refspec, &tail, 0);

Which ensures that with --no-single-branch --no-tags that tags aren't
written, and there's tests for that.

[PATCH v3 3/5] tests: rename a test having to do with shallow submodules

[Ævar Arnfjörð Bjarmason]

Rename the t5614-clone-submodules.sh test to
t5614-clone-submodules-shallow.sh. It's not a general test of
submodules, but of shallow cloning in relation to submodules. Move it
to create another similar t56*-clone-submodules-*.sh test.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/{t5614-clone-submodules.sh => t5614-clone-submodules-shallow.sh} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename t/{t5614-clone-submodules.sh => t5614-clone-submodules-shallow.sh} (100%)

diff --git a/t/t5614-clone-submodules.sh b/t/t5614-clone-submodules-shallow.sh
similarity index 100%
rename from t/t5614-clone-submodules.sh
rename to t/t5614-clone-submodules-shallow.sh
-- 
2.11.0

Re: [PATCH v3 3/5] tests: rename a test having to do with shallow submodules

[Stefan Beller]

On Wed, Apr 26, 2017 at 4:12 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> Rename the t5614-clone-submodules.sh test to
> t5614-clone-submodules-shallow.sh. It's not a general test of
> submodules, but of shallow cloning in relation to submodules. Move it
> to create another similar t56*-clone-submodules-*.sh test.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  t/{t5614-clone-submodules.sh => t5614-clone-submodules-shallow.sh} | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  rename t/{t5614-clone-submodules.sh => t5614-clone-submodules-shallow.sh} (100%)
>
> diff --git a/t/t5614-clone-submodules.sh b/t/t5614-clone-submodules-shallow.sh
> similarity index 100%
> rename from t/t5614-clone-submodules.sh
> rename to t/t5614-clone-submodules-shallow.sh

Thanks for formatting the patches with rename detection. :)
The rename looks good.

Thanks,
Stefan

Re: [PATCH v3 3/5] tests: rename a test having to do with shallow submodules

[Brandon Williams]

On 04/27, Stefan Beller wrote:
> On Wed, Apr 26, 2017 at 4:12 PM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> > Rename the t5614-clone-submodules.sh test to
> > t5614-clone-submodules-shallow.sh. It's not a general test of
> > submodules, but of shallow cloning in relation to submodules. Move it
> > to create another similar t56*-clone-submodules-*.sh test.
> >
> > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> > ---
> >  t/{t5614-clone-submodules.sh => t5614-clone-submodules-shallow.sh} | 0
> >  1 file changed, 0 insertions(+), 0 deletions(-)
> >  rename t/{t5614-clone-submodules.sh => t5614-clone-submodules-shallow.sh} (100%)
> >
> > diff --git a/t/t5614-clone-submodules.sh b/t/t5614-clone-submodules-shallow.sh
> > similarity index 100%
> > rename from t/t5614-clone-submodules.sh
> > rename to t/t5614-clone-submodules-shallow.sh
> 
> Thanks for formatting the patches with rename detection. :)
> The rename looks good.

Do you have to turn that on or is that on by default?

-- 
Brandon Williams

Re: [PATCH v3 3/5] tests: rename a test having to do with shallow submodules

[Ævar Arnfjörð Bjarmason]

On Fri, Apr 28, 2017 at 12:13 AM, Brandon Williams <bmwill@google.com> wrote:
> On 04/27, Stefan Beller wrote:
>> On Wed, Apr 26, 2017 at 4:12 PM, Ævar Arnfjörð Bjarmason
>> <avarab@gmail.com> wrote:
>> > Rename the t5614-clone-submodules.sh test to
>> > t5614-clone-submodules-shallow.sh. It's not a general test of
>> > submodules, but of shallow cloning in relation to submodules. Move it
>> > to create another similar t56*-clone-submodules-*.sh test.
>> >
>> > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> > ---
>> >  t/{t5614-clone-submodules.sh => t5614-clone-submodules-shallow.sh} | 0
>> >  1 file changed, 0 insertions(+), 0 deletions(-)
>> >  rename t/{t5614-clone-submodules.sh => t5614-clone-submodules-shallow.sh} (100%)
>> >
>> > diff --git a/t/t5614-clone-submodules.sh b/t/t5614-clone-submodules-shallow.sh
>> > similarity index 100%
>> > rename from t/t5614-clone-submodules.sh
>> > rename to t/t5614-clone-submodules-shallow.sh
>>
>> Thanks for formatting the patches with rename detection. :)
>> The rename looks good.
>
> Do you have to turn that on or is that on by default?

Looks like it just uses the diff.renames setting which I don't tweak,
I didn't do anything special, but maybe it picked up some part of my
.gitconfig that doesn't look like it has anything to do with
renames...

Re: [PATCH v3 3/5] tests: rename a test having to do with shallow submodules

[Stefan Beller]

On Fri, Apr 28, 2017 at 1:59 AM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Fri, Apr 28, 2017 at 12:13 AM, Brandon Williams <bmwill@google.com> wrote:
>> On 04/27, Stefan Beller wrote:
>>> On Wed, Apr 26, 2017 at 4:12 PM, Ævar Arnfjörð Bjarmason
>>> <avarab@gmail.com> wrote:
>>> > Rename the t5614-clone-submodules.sh test to
>>> > t5614-clone-submodules-shallow.sh. It's not a general test of
>>> > submodules, but of shallow cloning in relation to submodules. Move it
>>> > to create another similar t56*-clone-submodules-*.sh test.
>>> >
>>> > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>>> > ---
>>> >  t/{t5614-clone-submodules.sh => t5614-clone-submodules-shallow.sh} | 0
>>> >  1 file changed, 0 insertions(+), 0 deletions(-)
>>> >  rename t/{t5614-clone-submodules.sh => t5614-clone-submodules-shallow.sh} (100%)
>>> >
>>> > diff --git a/t/t5614-clone-submodules.sh b/t/t5614-clone-submodules-shallow.sh
>>> > similarity index 100%
>>> > rename from t/t5614-clone-submodules.sh
>>> > rename to t/t5614-clone-submodules-shallow.sh
>>>
>>> Thanks for formatting the patches with rename detection. :)
>>> The rename looks good.
>>
>> Do you have to turn that on or is that on by default?
>
> Looks like it just uses the diff.renames setting which I don't tweak,
> I didn't do anything special, but maybe it picked up some part of my
> .gitconfig that doesn't look like it has anything to do with
> renames...

After reading the man page of format-patch, I do not think you'd need
to configure anything special as the default settings easily pickup
a rename with 100% similarity.  It is only hairy when changes are in
there as well, then you may want to play around with -B/-M.

Thanks,
Stefan

[PATCH v3 4/5] clone: add a --no-tags-submodules to pass --no-tags to submodules

[Ævar Arnfjörð Bjarmason]

From: Brandon Williams <bmwill@google.com>

Add a --no-tags-submodules which does for --no-tags what the existing
--shallow-submodules does for --depth, i.e. doing:

    git clone --recurse-submodules --no-tags --no-tags-submodules <url>

Will clone the superproject and all submodules with --no-tags
semantics.

This change does not implement a submodule.*.tags config .gitmodules
configuration option corresponding to the existing submodule.*.shallow
facility, which would make --no-tags have full feature parity with
--shallow-submodules.

Signed-off-by: Brandon Williams <bmwill@google.com>
Code-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Commit-message-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Git-Completion-Code-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Docs-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Tests-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-clone.txt            | 11 +++++-
 builtin/clone.c                        |  6 +++
 builtin/submodule--helper.c            | 16 ++++++--
 contrib/completion/git-completion.bash |  1 +
 git-submodule.sh                       |  4 ++
 t/t5616-clone-submodules-tags.sh       | 72 ++++++++++++++++++++++++++++++++++
 6 files changed, 106 insertions(+), 4 deletions(-)
 create mode 100755 t/t5616-clone-submodules-tags.sh

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 83c8e9b394..4a0e4aa4f4 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -13,7 +13,8 @@ SYNOPSIS
 	  [-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror]
 	  [-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
 	  [--dissociate] [--separate-git-dir <git dir>]
-	  [--depth <depth>] [--[no-]single-branch] [--no-tags]
+	  [--depth <depth>] [--[no-]single-branch]
+	  [--no-tags] [--no-tags-submodules]
 	  [--recurse-submodules] [--[no-]shallow-submodules]
 	  [--jobs <n>] [--] <repository> [<directory>]
 
@@ -226,6 +227,10 @@ Can be used in conjunction with `--single-branch` to clone and
 maintain a branch with no references other than a single cloned
 branch. This is useful e.g. to maintain minimal clones of the default
 branch of some repository for search indexing.
++
+
+If you want to clone submodules without tags provide the
+`--no-tags-submodules` option.
 
 --recurse-submodules[=<pathspec]::
 	After the clone is created, initialize and clone submodules
@@ -243,6 +248,10 @@ branch of some repository for search indexing.
 --[no-]shallow-submodules::
 	All submodules which are cloned will be shallow with a depth of 1.
 
+--no-tags-submodules::
+	All submodules which are cloned will be cloned with the
+	`--no-tags` option.
+
 --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 05f52d6f2b..5a945777a8 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -42,6 +42,7 @@ static int option_no_checkout, option_bare, option_mirror, option_single_branch
 static int option_local = -1, option_no_hardlinks, option_shared;
 static int option_no_tags;
 static int option_shallow_submodules;
+static int option_no_tags_submodules;
 static int deepen;
 static char *option_template, *option_depth, *option_since;
 static char *option_origin = NULL;
@@ -125,6 +126,8 @@ static struct option builtin_clone_options[] = {
 		 N_("don't clone any tags, and make later fetches not to follow them")),
 	OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
 		    N_("any cloned submodules will be shallow")),
+	OPT_BOOL(0, "no-tags-submodules", &option_no_tags_submodules,
+		     N_("any cloned submodules will have no tags")),
 	OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
 		   N_("separate git dir from working tree")),
 	OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
@@ -761,6 +764,9 @@ static int checkout(int submodule_progress)
 		if (option_shallow_submodules == 1)
 			argv_array_push(&args, "--depth=1");
 
+		if (option_no_tags_submodules)
+			argv_array_push(&args, "--no-tags");
+
 		if (max_jobs != -1)
 			argv_array_pushf(&args, "--jobs=%d", max_jobs);
 
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 36e4231821..caa4d252ee 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -489,7 +489,7 @@ static int module_name(int argc, const char **argv, const char *prefix)
 
 static int clone_submodule(const char *path, const char *gitdir, const char *url,
 			   const char *depth, struct string_list *reference,
-			   int quiet, int progress)
+			   int quiet, int progress, int no_tags)
 {
 	struct child_process cp = CHILD_PROCESS_INIT;
 
@@ -499,6 +499,8 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
 		argv_array_push(&cp.args, "--quiet");
 	if (progress)
 		argv_array_push(&cp.args, "--progress");
+	if (no_tags)
+		argv_array_push(&cp.args, "--no-tags");
 	if (depth && *depth)
 		argv_array_pushl(&cp.args, "--depth", depth, NULL);
 	if (reference->nr) {
@@ -623,6 +625,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 	struct strbuf sb = STRBUF_INIT;
 	struct string_list reference = STRING_LIST_INIT_NODUP;
 	char *sm_alternate = NULL, *error_strategy = NULL;
+	int no_tags = 0;
 
 	struct option module_clone_options[] = {
 		OPT_STRING(0, "prefix", &prefix,
@@ -643,6 +646,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "depth", &depth,
 			   N_("string"),
 			   N_("depth for shallow clones")),
+		OPT_BOOL(0, "no-tags", &no_tags,
+			 N_("don't clone any tags, and make later fetches not to follow them")),
 		OPT__QUIET(&quiet, "Suppress output for cloning a submodule"),
 		OPT_BOOL(0, "progress", &progress,
 			   N_("force cloning progress")),
@@ -680,7 +685,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 		prepare_possible_alternates(name, &reference);
 
 		if (clone_submodule(path, sm_gitdir, url, depth, &reference,
-				    quiet, progress))
+				    quiet, progress, no_tags))
 			die(_("clone of '%s' into submodule path '%s' failed"),
 			    url, path);
 	} else {
@@ -745,11 +750,12 @@ struct submodule_update_clone {
 	/* failed clones to be retried again */
 	const struct cache_entry **failed_clones;
 	int failed_clones_nr, failed_clones_alloc;
+	int no_tags;
 };
 #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
 	SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, \
 	NULL, NULL, NULL, \
-	STRING_LIST_INIT_DUP, 0, NULL, 0, 0}
+	STRING_LIST_INIT_DUP, 0, NULL, 0, 0, 0}
 
 
 static void next_submodule_warn_missing(struct submodule_update_clone *suc,
@@ -849,6 +855,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 		argv_array_pushl(&child->args, "--prefix", suc->prefix, NULL);
 	if (suc->recommend_shallow && sub->recommend_shallow == 1)
 		argv_array_push(&child->args, "--depth=1");
+	if (suc->no_tags)
+		argv_array_push(&child->args, "--no-tags");
 	argv_array_pushl(&child->args, "--path", sub->path, NULL);
 	argv_array_pushl(&child->args, "--name", sub->name, NULL);
 	argv_array_pushl(&child->args, "--url", sub->url, NULL);
@@ -988,6 +996,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)
 			    N_("parallel jobs")),
 		OPT_BOOL(0, "recommend-shallow", &suc.recommend_shallow,
 			    N_("whether the initial clone should follow the shallow recommendation")),
+		OPT_BOOL(0, "no-tags", &suc.no_tags,
+			 N_("don't clone any tags, and make later fetches not to follow them")),
 		OPT__QUIET(&suc.quiet, N_("don't print cloning progress")),
 		OPT_BOOL(0, "progress", &suc.progress,
 			    N_("force cloning progress")),
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6da997cf42..08fb1319c3 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1314,6 +1314,7 @@ _git_clone ()
 			--recurse-submodules
 			--no-single-branch
 			--shallow-submodules
+			--no-tags-submodules
 			"
 		return
 		;;
diff --git a/git-submodule.sh b/git-submodule.sh
index c0d0e9a4c6..3371775b0b 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -565,6 +565,9 @@ cmd_update()
 		--depth=*)
 			depth=$1
 			;;
+		--no-tags)
+			no_tags=1
+			;;
 		-j|--jobs)
 			case "$2" in '') usage ;; esac
 			jobs="--jobs=$2"
@@ -601,6 +604,7 @@ cmd_update()
 		${reference:+"$reference"} \
 		${depth:+--depth "$depth"} \
 		${recommend_shallow:+"$recommend_shallow"} \
+		${no_tags:+--no-tags} \
 		${jobs:+$jobs} \
 		"$@" || echo "#unmatched" $?
 	} | {
diff --git a/t/t5616-clone-submodules-tags.sh b/t/t5616-clone-submodules-tags.sh
new file mode 100755
index 0000000000..3c88265352
--- /dev/null
+++ b/t/t5616-clone-submodules-tags.sh
@@ -0,0 +1,72 @@
+#!/bin/sh
+
+test_description='Test cloning of repos with submodules & the --[no-]tags option'
+
+. ./test-lib.sh
+
+pwd=$(pwd)
+
+test_expect_success 'setup' '
+	git checkout -b master &&
+	test_commit commit1 &&
+	test_commit commit2 &&
+	mkdir sub &&
+	(
+		cd sub &&
+		git init &&
+		test_commit subcommit1 &&
+		test_commit subcommit2 &&
+		test_commit subcommit3
+	) &&
+	git submodule add "file://$pwd/sub" sub &&
+	git commit -m "add submodule" &&
+	git tag addsubcommit1
+'
+
+test_expect_success 'tags clone implies tags submodule' '
+	test_when_finished "rm -rf super_clone" &&
+	git clone --recurse-submodules "file://$pwd/." super_clone &&
+	git -C super_clone for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 3 tags &&
+	git -C super_clone/sub for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 3 tags
+'
+
+test_expect_success 'no-tags clone with no-tags submodule' '
+	test_when_finished "rm -rf super_clone" &&
+	git clone --recurse-submodules --no-tags --no-tags-submodules "file://$pwd/." super_clone &&
+	git -C super_clone for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 0 tags &&
+	git -C super_clone/sub for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 0 tags
+
+'
+
+test_expect_success 'no-tags clone does not imply no-tags submodule' '
+	test_when_finished "rm -rf super_clone" &&
+	git clone --recurse-submodules --no-tags "file://$pwd/." super_clone &&
+	git -C super_clone for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 0 tags &&
+	git -C super_clone/sub for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 3 tags
+'
+
+test_expect_success 'no-tags clone with tags submodule' '
+	test_when_finished "rm -rf super_clone" &&
+	git clone --recurse-submodules --no-tags --tags-submodules "file://$pwd/." super_clone &&
+	git -C super_clone for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 0 tags &&
+	git -C super_clone/sub for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 3 tags
+'
+
+test_expect_success 'tags clone with no-tags submodule' '
+	test_when_finished "rm -rf super_clone" &&
+	git clone --recurse-submodules --tags --no-tags-submodules "file://$pwd/." super_clone &&
+	git -C super_clone for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 3 tags &&
+	git -C super_clone/sub for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 0 tags
+'
+
+test_done
-- 
2.11.0

Re: [PATCH v3 4/5] clone: add a --no-tags-submodules to pass --no-tags to submodules

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> From: Brandon Williams <bmwill@google.com>
>
> Add a --no-tags-submodules which does for --no-tags what the existing
> --shallow-submodules does for --depth, i.e. doing:
>
>     git clone --recurse-submodules --no-tags --no-tags-submodules <url>
>
> Will clone the superproject and all submodules with --no-tags
> semantics.
>
> This change does not implement a submodule.*.tags config .gitmodules
> configuration option corresponding to the existing submodule.*.shallow
> facility, which would make --no-tags have full feature parity with
> --shallow-submodules.

Shouldn't --no-tags automatically propagate down to submodules in
--recurse-submodules mode instead?  I know --shallow-submodules
exists but it looks to me more a misdesigned interface, rather than
something we want to mimic in a new option.

The design of the traditional "git submodule" support strived to
keep the superproject and its submodules distinct and separately
managed as much as possible, and I view the motivation behind the
recent push of "--recurse-submodules" as an attempt to make them
appear as seamless as possible, so I have a feeling that a new
option (like "--no-tags") that is applied to the superproject, as
long as it also makes sense to use it in submodules, should
propagate down in that mode by default (and most likely
unconditionally).  Those who want finer granularity of control can
use the traditional "work with superproject, and then initialize and
update the submodules you are interested in in any way you want
individually" mode anyway.

> Signed-off-by: Brandon Williams <bmwill@google.com>
> Code-by: Brandon Williams <bmwill@google.com>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Commit-message-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Git-Completion-Code-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Docs-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Tests-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Is it just me who finds this way of attribution unwieldy?  I would
have expected to see something like this at the end

	The main code is by Brandon, Ævar added the docs, tests and
	completion and wrapped them up into a commit with a log
	message.

before two s-o-b.

> ---
>  Documentation/git-clone.txt            | 11 +++++-
>  builtin/clone.c                        |  6 +++
>  builtin/submodule--helper.c            | 16 ++++++--
>  contrib/completion/git-completion.bash |  1 +
>  git-submodule.sh                       |  4 ++
>  t/t5616-clone-submodules-tags.sh       | 72 ++++++++++++++++++++++++++++++++++
>  6 files changed, 106 insertions(+), 4 deletions(-)
>  create mode 100755 t/t5616-clone-submodules-tags.sh
>
> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
> index 83c8e9b394..4a0e4aa4f4 100644
> --- a/Documentation/git-clone.txt
> +++ b/Documentation/git-clone.txt
> @@ -13,7 +13,8 @@ SYNOPSIS
>  	  [-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror]
>  	  [-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
>  	  [--dissociate] [--separate-git-dir <git dir>]
> -	  [--depth <depth>] [--[no-]single-branch] [--no-tags]
> +	  [--depth <depth>] [--[no-]single-branch]
> +	  [--no-tags] [--no-tags-submodules]
>  	  [--recurse-submodules] [--[no-]shallow-submodules]
>  	  [--jobs <n>] [--] <repository> [<directory>]
>  
> @@ -226,6 +227,10 @@ Can be used in conjunction with `--single-branch` to clone and
>  maintain a branch with no references other than a single cloned
>  branch. This is useful e.g. to maintain minimal clones of the default
>  branch of some repository for search indexing.
> ++
> +
> +If you want to clone submodules without tags provide the
> +`--no-tags-submodules` option.
>  
>  --recurse-submodules[=<pathspec]::
>  	After the clone is created, initialize and clone submodules
> @@ -243,6 +248,10 @@ branch of some repository for search indexing.
>  --[no-]shallow-submodules::
>  	All submodules which are cloned will be shallow with a depth of 1.
>  
> +--no-tags-submodules::
> +	All submodules which are cloned will be cloned with the
> +	`--no-tags` option.
> +
>  --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 05f52d6f2b..5a945777a8 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -42,6 +42,7 @@ static int option_no_checkout, option_bare, option_mirror, option_single_branch
>  static int option_local = -1, option_no_hardlinks, option_shared;
>  static int option_no_tags;
>  static int option_shallow_submodules;
> +static int option_no_tags_submodules;
>  static int deepen;
>  static char *option_template, *option_depth, *option_since;
>  static char *option_origin = NULL;
> @@ -125,6 +126,8 @@ static struct option builtin_clone_options[] = {
>  		 N_("don't clone any tags, and make later fetches not to follow them")),
>  	OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
>  		    N_("any cloned submodules will be shallow")),
> +	OPT_BOOL(0, "no-tags-submodules", &option_no_tags_submodules,
> +		     N_("any cloned submodules will have no tags")),
>  	OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
>  		   N_("separate git dir from working tree")),
>  	OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
> @@ -761,6 +764,9 @@ static int checkout(int submodule_progress)
>  		if (option_shallow_submodules == 1)
>  			argv_array_push(&args, "--depth=1");
>  
> +		if (option_no_tags_submodules)
> +			argv_array_push(&args, "--no-tags");
> +
>  		if (max_jobs != -1)
>  			argv_array_pushf(&args, "--jobs=%d", max_jobs);
>  
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 36e4231821..caa4d252ee 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -489,7 +489,7 @@ static int module_name(int argc, const char **argv, const char *prefix)
>  
>  static int clone_submodule(const char *path, const char *gitdir, const char *url,
>  			   const char *depth, struct string_list *reference,
> -			   int quiet, int progress)
> +			   int quiet, int progress, int no_tags)
>  {
>  	struct child_process cp = CHILD_PROCESS_INIT;
>  
> @@ -499,6 +499,8 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
>  		argv_array_push(&cp.args, "--quiet");
>  	if (progress)
>  		argv_array_push(&cp.args, "--progress");
> +	if (no_tags)
> +		argv_array_push(&cp.args, "--no-tags");
>  	if (depth && *depth)
>  		argv_array_pushl(&cp.args, "--depth", depth, NULL);
>  	if (reference->nr) {
> @@ -623,6 +625,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
>  	struct strbuf sb = STRBUF_INIT;
>  	struct string_list reference = STRING_LIST_INIT_NODUP;
>  	char *sm_alternate = NULL, *error_strategy = NULL;
> +	int no_tags = 0;
>  
>  	struct option module_clone_options[] = {
>  		OPT_STRING(0, "prefix", &prefix,
> @@ -643,6 +646,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
>  		OPT_STRING(0, "depth", &depth,
>  			   N_("string"),
>  			   N_("depth for shallow clones")),
> +		OPT_BOOL(0, "no-tags", &no_tags,
> +			 N_("don't clone any tags, and make later fetches not to follow them")),
>  		OPT__QUIET(&quiet, "Suppress output for cloning a submodule"),
>  		OPT_BOOL(0, "progress", &progress,
>  			   N_("force cloning progress")),
> @@ -680,7 +685,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
>  		prepare_possible_alternates(name, &reference);
>  
>  		if (clone_submodule(path, sm_gitdir, url, depth, &reference,
> -				    quiet, progress))
> +				    quiet, progress, no_tags))
>  			die(_("clone of '%s' into submodule path '%s' failed"),
>  			    url, path);
>  	} else {
> @@ -745,11 +750,12 @@ struct submodule_update_clone {
>  	/* failed clones to be retried again */
>  	const struct cache_entry **failed_clones;
>  	int failed_clones_nr, failed_clones_alloc;
> +	int no_tags;
>  };
>  #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
>  	SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, \
>  	NULL, NULL, NULL, \
> -	STRING_LIST_INIT_DUP, 0, NULL, 0, 0}
> +	STRING_LIST_INIT_DUP, 0, NULL, 0, 0, 0}
>  
>  
>  static void next_submodule_warn_missing(struct submodule_update_clone *suc,
> @@ -849,6 +855,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
>  		argv_array_pushl(&child->args, "--prefix", suc->prefix, NULL);
>  	if (suc->recommend_shallow && sub->recommend_shallow == 1)
>  		argv_array_push(&child->args, "--depth=1");
> +	if (suc->no_tags)
> +		argv_array_push(&child->args, "--no-tags");
>  	argv_array_pushl(&child->args, "--path", sub->path, NULL);
>  	argv_array_pushl(&child->args, "--name", sub->name, NULL);
>  	argv_array_pushl(&child->args, "--url", sub->url, NULL);
> @@ -988,6 +996,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)
>  			    N_("parallel jobs")),
>  		OPT_BOOL(0, "recommend-shallow", &suc.recommend_shallow,
>  			    N_("whether the initial clone should follow the shallow recommendation")),
> +		OPT_BOOL(0, "no-tags", &suc.no_tags,
> +			 N_("don't clone any tags, and make later fetches not to follow them")),
>  		OPT__QUIET(&suc.quiet, N_("don't print cloning progress")),
>  		OPT_BOOL(0, "progress", &suc.progress,
>  			    N_("force cloning progress")),
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 6da997cf42..08fb1319c3 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1314,6 +1314,7 @@ _git_clone ()
>  			--recurse-submodules
>  			--no-single-branch
>  			--shallow-submodules
> +			--no-tags-submodules
>  			"
>  		return
>  		;;
> diff --git a/git-submodule.sh b/git-submodule.sh
> index c0d0e9a4c6..3371775b0b 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -565,6 +565,9 @@ cmd_update()
>  		--depth=*)
>  			depth=$1
>  			;;
> +		--no-tags)
> +			no_tags=1
> +			;;
>  		-j|--jobs)
>  			case "$2" in '') usage ;; esac
>  			jobs="--jobs=$2"
> @@ -601,6 +604,7 @@ cmd_update()
>  		${reference:+"$reference"} \
>  		${depth:+--depth "$depth"} \
>  		${recommend_shallow:+"$recommend_shallow"} \
> +		${no_tags:+--no-tags} \
>  		${jobs:+$jobs} \
>  		"$@" || echo "#unmatched" $?
>  	} | {
> diff --git a/t/t5616-clone-submodules-tags.sh b/t/t5616-clone-submodules-tags.sh
> new file mode 100755
> index 0000000000..3c88265352
> --- /dev/null
> +++ b/t/t5616-clone-submodules-tags.sh
> @@ -0,0 +1,72 @@
> +#!/bin/sh
> +
> +test_description='Test cloning of repos with submodules & the --[no-]tags option'
> +
> +. ./test-lib.sh
> +
> +pwd=$(pwd)
> +
> +test_expect_success 'setup' '
> +	git checkout -b master &&
> +	test_commit commit1 &&
> +	test_commit commit2 &&
> +	mkdir sub &&
> +	(
> +		cd sub &&
> +		git init &&
> +		test_commit subcommit1 &&
> +		test_commit subcommit2 &&
> +		test_commit subcommit3
> +	) &&
> +	git submodule add "file://$pwd/sub" sub &&
> +	git commit -m "add submodule" &&
> +	git tag addsubcommit1
> +'
> +
> +test_expect_success 'tags clone implies tags submodule' '
> +	test_when_finished "rm -rf super_clone" &&
> +	git clone --recurse-submodules "file://$pwd/." super_clone &&
> +	git -C super_clone for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
> +	test_line_count = 3 tags &&
> +	git -C super_clone/sub for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
> +	test_line_count = 3 tags
> +'
> +
> +test_expect_success 'no-tags clone with no-tags submodule' '
> +	test_when_finished "rm -rf super_clone" &&
> +	git clone --recurse-submodules --no-tags --no-tags-submodules "file://$pwd/." super_clone &&
> +	git -C super_clone for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
> +	test_line_count = 0 tags &&
> +	git -C super_clone/sub for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
> +	test_line_count = 0 tags
> +
> +'
> +
> +test_expect_success 'no-tags clone does not imply no-tags submodule' '
> +	test_when_finished "rm -rf super_clone" &&
> +	git clone --recurse-submodules --no-tags "file://$pwd/." super_clone &&
> +	git -C super_clone for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
> +	test_line_count = 0 tags &&
> +	git -C super_clone/sub for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
> +	test_line_count = 3 tags
> +'
> +
> +test_expect_success 'no-tags clone with tags submodule' '
> +	test_when_finished "rm -rf super_clone" &&
> +	git clone --recurse-submodules --no-tags --tags-submodules "file://$pwd/." super_clone &&
> +	git -C super_clone for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
> +	test_line_count = 0 tags &&
> +	git -C super_clone/sub for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
> +	test_line_count = 3 tags
> +'
> +
> +test_expect_success 'tags clone with no-tags submodule' '
> +	test_when_finished "rm -rf super_clone" &&
> +	git clone --recurse-submodules --tags --no-tags-submodules "file://$pwd/." super_clone &&
> +	git -C super_clone for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
> +	test_line_count = 3 tags &&
> +	git -C super_clone/sub for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
> +	test_line_count = 0 tags
> +'
> +
> +test_done

Re: [PATCH v3 4/5] clone: add a --no-tags-submodules to pass --no-tags to submodules

[Brandon Williams]

On 04/26, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
> 
> > From: Brandon Williams <bmwill@google.com>
> >
> > Add a --no-tags-submodules which does for --no-tags what the existing
> > --shallow-submodules does for --depth, i.e. doing:
> >
> >     git clone --recurse-submodules --no-tags --no-tags-submodules <url>
> >
> > Will clone the superproject and all submodules with --no-tags
> > semantics.
> >
> > This change does not implement a submodule.*.tags config .gitmodules
> > configuration option corresponding to the existing submodule.*.shallow
> > facility, which would make --no-tags have full feature parity with
> > --shallow-submodules.
> 
> Shouldn't --no-tags automatically propagate down to submodules in
> --recurse-submodules mode instead?  I know --shallow-submodules
> exists but it looks to me more a misdesigned interface, rather than
> something we want to mimic in a new option.

I agree, I would think that the --no-tags option should be propagated
down when --recurse-submodules is provided, without needed to provide
another option for that.  Thinking about the recursive case here
(submodules in side of submodules) you would then need to propagate down
two options, --no-tags and --no-tags-submodules, which feels a bit
awkward.

> The design of the traditional "git submodule" support strived to
> keep the superproject and its submodules distinct and separately
> managed as much as possible, and I view the motivation behind the
> recent push of "--recurse-submodules" as an attempt to make them
> appear as seamless as possible, so I have a feeling that a new
> option (like "--no-tags") that is applied to the superproject, as
> long as it also makes sense to use it in submodules, should
> propagate down in that mode by default (and most likely
> unconditionally).  Those who want finer granularity of control can
> use the traditional "work with superproject, and then initialize and
> update the submodules you are interested in in any way you want
> individually" mode anyway.
> 
> > Signed-off-by: Brandon Williams <bmwill@google.com>
> > Code-by: Brandon Williams <bmwill@google.com>
> > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> > Commit-message-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> > Git-Completion-Code-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> > Docs-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> > Tests-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> 
> Is it just me who finds this way of attribution unwieldy?  I would
> have expected to see something like this at the end
> 
> 	The main code is by Brandon, Ævar added the docs, tests and
> 	completion and wrapped them up into a commit with a log
> 	message.
> 
> before two s-o-b.

Yeah this looks a bit messy.  Ævar, you've spent much longer on this
topic than I have and should probably take authorship.  I mean I only
spent a little bit cobbling together a patch that I never tested. :)
You're more than welcome to include my 'Signed-off-by' and/or a
'Helped-by' line for this patch though.

-- 
Brandon Williams

Re: [PATCH v3 4/5] clone: add a --no-tags-submodules to pass --no-tags to submodules

[Stefan Beller]

On Wed, Apr 26, 2017 at 4:12 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> From: Brandon Williams <bmwill@google.com>
>
> Add a --no-tags-submodules which does for --no-tags what the existing
> --shallow-submodules does for --depth, i.e. doing:
>
>     git clone --recurse-submodules --no-tags --no-tags-submodules <url>
>
> Will clone the superproject and all submodules with --no-tags
> semantics.

My gut reaction to this is that it is ugly and maybe we should rather have
a --no-tags[=<repo-spec>].

We have had the discussion for e.g. git-push, for which we would like to
have a way to specify to push the superproject, or some submodules or
all of them, or just the superproject and changed submodules,
such that there we have a "git push --recurse-submodules= \
[check|on-demand|only|no]" which comes close to what I mean by
"repo-spec".

> This change does not implement a submodule.*.tags config .gitmodules
> configuration option corresponding to the existing submodule.*.shallow
> facility, which would make --no-tags have full feature parity with
> --shallow-submodules.

Okay, that can be added later if desired.

> Signed-off-by: Brandon Williams <bmwill@google.com>
> Code-by: Brandon Williams <bmwill@google.com>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Commit-message-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Git-Completion-Code-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Docs-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Tests-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Quite a lot of collaboration. ;)

> @@ -499,6 +499,8 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
>                 argv_array_push(&cp.args, "--quiet");
>         if (progress)
>                 argv_array_push(&cp.args, "--progress");
> +       if (no_tags)
> +               argv_array_push(&cp.args, "--no-tags");

Here you would need to also push --no-tags-submodules to keep it recursive?

> diff --git a/t/t5616-clone-submodules-tags.sh b/t/t5616-clone-submodules-tags.sh
> new file mode 100755
> index 0000000000..3c88265352
> --- /dev/null
> +++ b/t/t5616-clone-submodules-tags.sh

Name is good as it describes the niche we're looking at.
(previous commit sounded as if you wanted to introduce
a plain clone-submodules.sh)

> @@ -0,0 +1,72 @@
> +#!/bin/sh
> +
> +test_description='Test cloning of repos with submodules & the --[no-]tags option'
> +
> +. ./test-lib.sh
> +
> +pwd=$(pwd)
> +
> +test_expect_success 'setup' '
> +       git checkout -b master &&
> +       test_commit commit1 &&
> +       test_commit commit2 &&
> +       mkdir sub &&
> +       (
> +               cd sub &&
> +               git init &&
> +               test_commit subcommit1 &&
> +               test_commit subcommit2 &&
> +               test_commit subcommit3
> +       ) &&

This can be written easier with

    test_create_repo sub &&
    test_commit -C sub subcommit1 &&
    test_commit -C sub subcommit2 &&
    test_commit -C sub subcommit3 &&

Most submodule code thought naming a submodule
"sub" was a good idea. I also wrote such code. But please
let's stop with this tradition. Name the submodule after your
favorite dish (that hopefully doesn't have "sub" in its name),
as it is much easier to read the test code when there are fewer
strings "sub".


> +       git submodule add "file://$pwd/sub" sub &&
> +       git commit -m "add submodule" &&
> +       git tag addsubcommit1
> +'
> +
> +test_expect_success 'tags clone implies tags submodule' '
> +       test_when_finished "rm -rf super_clone" &&
> +       git clone --recurse-submodules "file://$pwd/." super_clone &&
> +       git -C super_clone for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&

Why do we need to strip the refnames here? Full ref names ought to be fine?
Specifically when testing in combination of single-branch (does that imply
any tags setting), I'd rather expect an rstrip and then counting refs/heads
and refs/tags.

Thanks,
Stefan

Re: [PATCH v3 4/5] clone: add a --no-tags-submodules to pass --no-tags to submodules

[Brandon Williams]

On 04/27, Stefan Beller wrote:
> On Wed, Apr 26, 2017 at 4:12 PM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> > From: Brandon Williams <bmwill@google.com>
> >
> > Add a --no-tags-submodules which does for --no-tags what the existing
> > --shallow-submodules does for --depth, i.e. doing:
> >
> >     git clone --recurse-submodules --no-tags --no-tags-submodules <url>
> >
> > Will clone the superproject and all submodules with --no-tags
> > semantics.
> 
> My gut reaction to this is that it is ugly and maybe we should rather have
> a --no-tags[=<repo-spec>].

See my gut reaction to that is: that's ugly :).  The --no-tags option
should just be propagated down (like I mentioned in another thread).

> We have had the discussion for e.g. git-push, for which we would like to
> have a way to specify to push the superproject, or some submodules or
> all of them, or just the superproject and changed submodules,
> such that there we have a "git push --recurse-submodules= \
> [check|on-demand|only|no]" which comes close to what I mean by
> "repo-spec".

This makes more sense to me in terms of pushing...its still not my
favorite ui design but its what we have.

-- 
Brandon Williams

[RFC/PATCH v3 5/5] WIP clone: add a --[no-]recommend-tags & submodule.NAME.tags config

[Ævar Arnfjörð Bjarmason]

Add a --no-recommend-tags option & support for
submodule.NAME.tags=[true|false] config facility. This does for
--no-tags what --no-recommend-shallow & submodule.NAME.shallow does
for --shallow-submodules.

This is almost exactly the same code change as in Stefan Beller's
commit abed000aca ("submodule update: learn `--[no-]recommend-shallow`
option", 2016-05-26) & 37f52e9344 ("submodule-config: keep shallow
recommendation around", 2016-05-26).

The difference in the configuration facility is that setting
submodule.NAME.tags=false will turn on --no-tags, i.e. "false" means
"don't give me tags", with "true" being the default.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-submodule.txt        |  8 +++++++-
 builtin/submodule--helper.c            |  7 +++++--
 contrib/completion/git-completion.bash |  1 +
 git-submodule.sh                       |  9 ++++++++-
 submodule-config.c                     |  8 ++++++++
 submodule-config.h                     |  1 +
 t/t5616-clone-submodules-tags.sh       | 34 ++++++++++++++++++++++++++++++++++
 7 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 74bc6200d5..9a431a833d 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -159,7 +159,7 @@ instead of deinit-ing everything, to prevent mistakes.
 If `--force` is specified, the submodule's working tree will
 be removed even if it contains local modifications.
 
-update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference <repository>] [--depth <depth>] [--recursive] [--jobs <n>] [--] [<path>...]::
+update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [--[no-]recommend-tags] [-f|--force] [--checkout|--rebase|--merge] [--reference <repository>] [--depth <depth>] [--recursive] [--jobs <n>] [--] [<path>...]::
 +
 --
 Update the registered submodules to match what the superproject
@@ -416,6 +416,12 @@ for linkgit:git-clone[1]'s `--reference` and `--shared` options carefully.
 	`submodule.<name>.shallow` as provided by the .gitmodules file
 	by default. To ignore the suggestions use `--no-recommend-shallow`.
 
+--[no-]recommend-tags::
+	This option is only valid for the update command.
+	The initial clone of a submodule will use the recommended
+	`submodule.<name>.tags` as provided by the .gitmodules file
+	by default. To ignore the suggestions use `--no-recommend-tags`.
+
 -j <n>::
 --jobs <n>::
 	This option is only valid for the update command.
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index caa4d252ee..424ea7a680 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -736,6 +736,7 @@ struct submodule_update_clone {
 	int progress;
 	int quiet;
 	int recommend_shallow;
+	int recommend_tags;
 	struct string_list references;
 	const char *depth;
 	const char *recursive_prefix;
@@ -753,7 +754,7 @@ struct submodule_update_clone {
 	int no_tags;
 };
 #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
-	SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, \
+	SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, -1, STRING_LIST_INIT_DUP, \
 	NULL, NULL, NULL, \
 	STRING_LIST_INIT_DUP, 0, NULL, 0, 0, 0}
 
@@ -855,7 +856,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 		argv_array_pushl(&child->args, "--prefix", suc->prefix, NULL);
 	if (suc->recommend_shallow && sub->recommend_shallow == 1)
 		argv_array_push(&child->args, "--depth=1");
-	if (suc->no_tags)
+	if (suc->no_tags || suc->recommend_tags == 0)
 		argv_array_push(&child->args, "--no-tags");
 	argv_array_pushl(&child->args, "--path", sub->path, NULL);
 	argv_array_pushl(&child->args, "--name", sub->name, NULL);
@@ -996,6 +997,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)
 			    N_("parallel jobs")),
 		OPT_BOOL(0, "recommend-shallow", &suc.recommend_shallow,
 			    N_("whether the initial clone should follow the shallow recommendation")),
+		OPT_BOOL(0, "recommend-tags", &suc.recommend_tags,
+			    N_("whether the initial clone should follow the tags recommendation")),
 		OPT_BOOL(0, "no-tags", &suc.no_tags,
 			 N_("don't clone any tags, and make later fetches not to follow them")),
 		OPT__QUIET(&suc.quiet, N_("don't print cloning progress")),
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 08fb1319c3..8eb32032a8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2878,6 +2878,7 @@ _git_submodule ()
 		__gitcomp "
 			--init --remote --no-fetch
 			--recommend-shallow --no-recommend-shallow
+			--recommend-tags --no-recommend-tags
 			--force --rebase --merge --reference --depth --recursive --jobs
 		"
 		;;
diff --git a/git-submodule.sh b/git-submodule.sh
index 3371775b0b..5b204a9679 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -9,7 +9,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <re
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
    or: $dashless [--quiet] deinit [-f|--force] (--all| [--] <path>...)
-   or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--] [<path>...]
+   or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--[no-]recommend-tags] [--reference <repository>] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
    or: $dashless [--quiet] sync [--recursive] [--] [<path>...]
@@ -557,6 +557,12 @@ cmd_update()
 		--no-recommend-shallow)
 			recommend_shallow="--no-recommend-shallow"
 			;;
+		--recommend-tags)
+			recommend_tags="--recommend-tags"
+			;;
+		--no-recommend-tags)
+			recommend_tags="--no-recommend-tags"
+			;;
 		--depth)
 			case "$2" in '') usage ;; esac
 			depth="--depth=$2"
@@ -604,6 +610,7 @@ cmd_update()
 		${reference:+"$reference"} \
 		${depth:+--depth "$depth"} \
 		${recommend_shallow:+"$recommend_shallow"} \
+		${recommend_tags:+"$recommend_tags"} \
 		${no_tags:+--no-tags} \
 		${jobs:+$jobs} \
 		"$@" || echo "#unmatched" $?
diff --git a/submodule-config.c b/submodule-config.c
index 4f58491ddb..68fbd9d1f0 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -202,6 +202,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,
 	submodule->ignore = NULL;
 	submodule->branch = NULL;
 	submodule->recommend_shallow = -1;
+	submodule->recommend_tags = -1;
 
 	hashcpy(submodule->gitmodules_sha1, gitmodules_sha1);
 
@@ -385,6 +386,13 @@ static int parse_config(const char *var, const char *value, void *data)
 		else
 			submodule->recommend_shallow =
 				git_config_bool(var, value);
+	} else if (!strcmp(item.buf, "tags")) {
+		if (!me->overwrite && submodule->recommend_tags != -1)
+			warn_multiple_config(me->treeish_name, submodule->name,
+					     "tags");
+		else
+			submodule->recommend_tags =
+				git_config_bool(var, value);
 	} else if (!strcmp(item.buf, "branch")) {
 		if (!me->overwrite && submodule->branch)
 			warn_multiple_config(me->treeish_name, submodule->name,
diff --git a/submodule-config.h b/submodule-config.h
index d434ecdb45..be3c567803 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -20,6 +20,7 @@ struct submodule {
 	/* the sha1 blob id of the responsible .gitmodules file */
 	unsigned char gitmodules_sha1[20];
 	int recommend_shallow;
+	int recommend_tags;
 };
 
 extern int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
diff --git a/t/t5616-clone-submodules-tags.sh b/t/t5616-clone-submodules-tags.sh
index 3c88265352..caded2fb24 100755
--- a/t/t5616-clone-submodules-tags.sh
+++ b/t/t5616-clone-submodules-tags.sh
@@ -69,4 +69,38 @@ test_expect_success 'tags clone with no-tags submodule' '
 	test_line_count = 0 tags
 '
 
+test_expect_success 'clone follows tags=false recommendation' '
+	test_when_finished "rm -rf super_clone" &&
+	git config -f .gitmodules submodule.sub.tags false &&
+	git add .gitmodules &&
+	git commit -m "recommed no-tags for sub" &&
+	git clone --recurse-submodules --no-local "file://$pwd/." super_clone &&
+	git -C super_clone for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 3 tags &&
+	git -C super_clone/sub for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 0 tags
+'
+
+test_expect_success 'get tags recommended no-tags submodule' '
+	test_when_finished "rm -rf super_clone" &&
+	git clone --no-local "file://$pwd/." super_clone &&
+	git -C super_clone submodule update --init --no-recommend-tags &&
+	git -C super_clone for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 3 tags &&
+	git -C super_clone/sub for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 3 tags
+'
+
+test_expect_success 'clone follows tags=true recommendation' '
+	test_when_finished "rm -rf super_clone" &&
+	git config -f .gitmodules submodule.sub.tags true &&
+	git add .gitmodules &&
+	git commit -m "recommed tags for sub" &&
+	git clone --recurse-submodules --no-local "file://$pwd/." super_clone &&
+	git -C super_clone for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 3 tags &&
+	git -C super_clone/sub for-each-ref --format="%(refname:strip=2)" refs/tags/ >tags &&
+	test_line_count = 3 tags
+'
+
 test_done
-- 
2.11.0

Re: [RFC/PATCH v3 5/5] WIP clone: add a --[no-]recommend-tags & submodule.NAME.tags config

[Stefan Beller]

On Wed, Apr 26, 2017 at 4:12 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:

> @@ -855,7 +856,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
>                 argv_array_pushl(&child->args, "--prefix", suc->prefix, NULL);
>         if (suc->recommend_shallow && sub->recommend_shallow == 1)
>                 argv_array_push(&child->args, "--depth=1");
> -       if (suc->no_tags)
> +       if (suc->no_tags || suc->recommend_tags == 0)
>                 argv_array_push(&child->args, "--no-tags");

Here you would also need to pay attention to sub->recommend_tags?

Re: [PATCH v3 0/5] clone: --no-tags option

[Brandon Williams]

On 04/26, Ævar Arnfjörð Bjarmason wrote:
> This is an expansion of the previously solo 02/05 "clone: add a
> --no-tags option to clone without tags" patch (see
> <20170418191553.15464-1-avarab@gmail.com>).
> 
> This addresses the comments by Junio & Jonathan Nieder on v2 (thanks a
> lot), and in addition implements a --no-tags-submodules option. That
> code was implemented by Brandon & sent to me privately after I'd
> failed to come up with it, but I added tests, a commit message & bash
> completion to it.

Na you would have come up with it, I've just lived in submodule land a
little too long (though not as long as Stephan has!) :D

> The WIP 5/5 patch implements a submodule.NAME.tags config facility for
> the option, but is broken currently & floats along in this submission
> as an RFC patch. AFAICT it *should* work and it goes through all the
> motions the similar existing *.shallow config does, but for some
> reason the tags=false option isn't picked up & propagated in a freshly
> cloned submodule.
> 
> I'm probably missing something trivial, but I can't see what it is,
> I'm hoping thath either Stefan or Brandon will see what that is.

Overall the series looks good.  I've mentioned in the other threads that
it probably makes more sense to have --recurse-submodules simply pass
through known good commands to its children (e.g. --no-tags) simply
because it makes the UX a little bit easier to work with (I don't have
to remember all the fancy --OPT-submodules stuff, only
--recurse-submodules).  That is unless you have some good rational that
I'm not considering (completely possible :D).

> 
> Brandon Williams (1):
>   clone: add a --no-tags-submodules to pass --no-tags to submodules
> 
> Ævar Arnfjörð Bjarmason (4):
>   tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"
>   clone: add a --no-tags option to clone without tags
>   tests: rename a test having to do with shallow submodules
>   WIP clone: add a --[no-]recommend-tags & submodule.NAME.tags config
> 
>  Documentation/git-clone.txt                        |  21 ++++
>  Documentation/git-submodule.txt                    |   8 +-
>  builtin/clone.c                                    |  19 +++-
>  builtin/submodule--helper.c                        |  21 +++-
>  contrib/completion/git-completion.bash             |   3 +
>  git-submodule.sh                                   |  13 ++-
>  submodule-config.c                                 |   8 ++
>  submodule-config.h                                 |   1 +
>  t/t5612-clone-refspec.sh                           | 103 +++++++++++++++++---
>  ...odules.sh => t5614-clone-submodules-shallow.sh} |   0
>  t/t5616-clone-submodules-tags.sh                   | 106 +++++++++++++++++++++
>  11 files changed, 284 insertions(+), 19 deletions(-)
>  rename t/{t5614-clone-submodules.sh => t5614-clone-submodules-shallow.sh} (100%)
>  create mode 100755 t/t5616-clone-submodules-tags.sh
> 
> -- 
> 2.11.0
> 

-- 
Brandon Williams

Re: [PATCH v3 0/5] clone: --no-tags option

[Ævar Arnfjörð Bjarmason]

On Fri, Apr 28, 2017 at 12:27 AM, Brandon Williams <bmwill@google.com> wrote:
> On 04/26, Ævar Arnfjörð Bjarmason wrote:
>> This is an expansion of the previously solo 02/05 "clone: add a
>> --no-tags option to clone without tags" patch (see
>> <20170418191553.15464-1-avarab@gmail.com>).
>>
>> This addresses the comments by Junio & Jonathan Nieder on v2 (thanks a
>> lot), and in addition implements a --no-tags-submodules option. That
>> code was implemented by Brandon & sent to me privately after I'd
>> failed to come up with it, but I added tests, a commit message & bash
>> completion to it.
>
> Na you would have come up with it, I've just lived in submodule land a
> little too long (though not as long as Stephan has!) :D
>
>> The WIP 5/5 patch implements a submodule.NAME.tags config facility for
>> the option, but is broken currently & floats along in this submission
>> as an RFC patch. AFAICT it *should* work and it goes through all the
>> motions the similar existing *.shallow config does, but for some
>> reason the tags=false option isn't picked up & propagated in a freshly
>> cloned submodule.
>>
>> I'm probably missing something trivial, but I can't see what it is,
>> I'm hoping thath either Stefan or Brandon will see what that is.
>
> Overall the series looks good.  I've mentioned in the other threads that
> it probably makes more sense to have --recurse-submodules simply pass
> through known good commands to its children (e.g. --no-tags) simply
> because it makes the UX a little bit easier to work with (I don't have
> to remember all the fancy --OPT-submodules stuff, only
> --recurse-submodules).  That is unless you have some good rational that
> I'm not considering (completely possible :D).

I have no good (or bad) reason for that other than just wanting to add
--no-tags to submodules while I was at it, and then I was just
following the pattern the option to pass along --depth was
establishing.

But if that's some anti-pattern and the consensus is that this
submodule feature should instead work as you describe (which looks
like the case) I'll change it to work like that.

>>
>> Brandon Williams (1):
>>   clone: add a --no-tags-submodules to pass --no-tags to submodules
>>
>> Ævar Arnfjörð Bjarmason (4):
>>   tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"
>>   clone: add a --no-tags option to clone without tags
>>   tests: rename a test having to do with shallow submodules
>>   WIP clone: add a --[no-]recommend-tags & submodule.NAME.tags config
>>
>>  Documentation/git-clone.txt                        |  21 ++++
>>  Documentation/git-submodule.txt                    |   8 +-
>>  builtin/clone.c                                    |  19 +++-
>>  builtin/submodule--helper.c                        |  21 +++-
>>  contrib/completion/git-completion.bash             |   3 +
>>  git-submodule.sh                                   |  13 ++-
>>  submodule-config.c                                 |   8 ++
>>  submodule-config.h                                 |   1 +
>>  t/t5612-clone-refspec.sh                           | 103 +++++++++++++++++---
>>  ...odules.sh => t5614-clone-submodules-shallow.sh} |   0
>>  t/t5616-clone-submodules-tags.sh                   | 106 +++++++++++++++++++++
>>  11 files changed, 284 insertions(+), 19 deletions(-)
>>  rename t/{t5614-clone-submodules.sh => t5614-clone-submodules-shallow.sh} (100%)
>>  create mode 100755 t/t5616-clone-submodules-tags.sh
>>
>> --
>> 2.11.0
>>
>
> --
> Brandon Williams

Re: [PATCH v3 0/5] clone: --no-tags option

[Ævar Arnfjörð Bjarmason]

On Thu, Apr 27, 2017 at 1:12 AM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> This is an expansion of the previously solo 02/05 "clone: add a
> --no-tags option to clone without tags" patch (see
> <20170418191553.15464-1-avarab@gmail.com>).
>
> This addresses the comments by Junio & Jonathan Nieder on v2 (thanks a
> lot), and in addition implements a --no-tags-submodules option. That
> code was implemented by Brandon & sent to me privately after I'd
> failed to come up with it, but I added tests, a commit message & bash
> completion to it.
>
> The WIP 5/5 patch implements a submodule.NAME.tags config facility for
> the option, but is broken currently & floats along in this submission
> as an RFC patch. AFAICT it *should* work and it goes through all the
> motions the similar existing *.shallow config does, but for some
> reason the tags=false option isn't picked up & propagated in a freshly
> cloned submodule.
>
> I'm probably missing something trivial, but I can't see what it is,
> I'm hoping thath either Stefan or Brandon will see what that is.

Junio, can you please just take patch 1-3 in this series, i.e.:

DROP:

> Brandon Williams (1):
>   clone: add a --no-tags-submodules to pass --no-tags to submodules
> [...]
>   WIP clone: add a --[no-]recommend-tags & submodule.NAME.tags config

KEEP:

> Ævar Arnfjörð Bjarmason (4):
>   tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"
>   clone: add a --no-tags option to clone without tags
>   tests: rename a test having to do with shallow submodules

I think a fair summary of the discussion so far is that the submodule
interaction I copy/pasted from --shallow-submodules isn't how we want
to do things at all, I defer to Stefan & Brandon on that.

The only other objection to the patches marked as KEEP is from Stefan
saying it should be --tags on by default not --no-tags off by default.
As noted in <CACBZZX5dxaJDN18J5fAhjKcaP8cZSTtRw5-eScr2oq8OYyhKuQ@mail.gmail.com>
I don't disagree, but a lot of flags in git now use that pattern, and
this change is consistent with those. Makes sense to discuss changing
that as another series.

Re: [PATCH v3 0/5] clone: --no-tags option

[Stefan Beller]

On Fri, Apr 28, 2017 at 12:11 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Thu, Apr 27, 2017 at 1:12 AM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> This is an expansion of the previously solo 02/05 "clone: add a
>> --no-tags option to clone without tags" patch (see
>> <20170418191553.15464-1-avarab@gmail.com>).
>>
>> This addresses the comments by Junio & Jonathan Nieder on v2 (thanks a
>> lot), and in addition implements a --no-tags-submodules option. That
>> code was implemented by Brandon & sent to me privately after I'd
>> failed to come up with it, but I added tests, a commit message & bash
>> completion to it.
>>
>> The WIP 5/5 patch implements a submodule.NAME.tags config facility for
>> the option, but is broken currently & floats along in this submission
>> as an RFC patch. AFAICT it *should* work and it goes through all the
>> motions the similar existing *.shallow config does, but for some
>> reason the tags=false option isn't picked up & propagated in a freshly
>> cloned submodule.
>>
>> I'm probably missing something trivial, but I can't see what it is,
>> I'm hoping thath either Stefan or Brandon will see what that is.
>
> Junio, can you please just take patch 1-3 in this series, i.e.:
>
> DROP:
>
>> Brandon Williams (1):
>>   clone: add a --no-tags-submodules to pass --no-tags to submodules
>> [...]
>>   WIP clone: add a --[no-]recommend-tags & submodule.NAME.tags config
>
> KEEP:
>
>> Ævar Arnfjörð Bjarmason (4):
>>   tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"
>>   clone: add a --no-tags option to clone without tags
>>   tests: rename a test having to do with shallow submodules
>
> I think a fair summary of the discussion so far is that the submodule
> interaction I copy/pasted from --shallow-submodules isn't how we want
> to do things at all, I defer to Stefan & Brandon on that.

ok. In that case we'd want to discuss what we actually want with no-tags
in submodules.

>
> The only other objection to the patches marked as KEEP is from Stefan
> saying it should be --tags on by default not --no-tags off by default.
> As noted in <CACBZZX5dxaJDN18J5fAhjKcaP8cZSTtRw5-eScr2oq8OYyhKuQ@mail.gmail.com>
> I don't disagree, but a lot of flags in git now use that pattern, and
> this change is consistent with those. Makes sense to discuss changing
> that as another series.

Ok. I assumed with that next series on the radar, we'd not want to intentionally
add more of the no-OPTIONs as that would produce more work for that series.

Re: [PATCH v3 0/5] clone: --no-tags option

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> Junio, can you please just take patch 1-3 in this series, i.e.:

Given that the old one has been already sitting in 'next' since
April 23rd, I'd have to revert it and requeue them anew.  Which is
possible but cumbersome,...

Let me take a look at these three patches and then decide what to
do.  Thanks.