diff mbox series

[v4,2/6] clone: make it possible to specify --tags

Message ID 20250131-toon-clone-refs-v4-2-2a4ff851498f@iotcl.com (mailing list archive)
State Superseded
Headers show
Series Enable doing a shallow clone of a specific git revision | expand

Commit Message

Toon Claes Jan. 31, 2025, 3:30 p.m. UTC
Option --no-tags was added in 0dab2468ee (clone: add a --no-tags option
to clone without tags, 2017-04-26). At the time there was no need to
support --tags as well, although there was some conversation about
it[1].

To simplify the code and to prepare for future commits, invert the flag
internally. Functionally there is no change, because the flag is
default-enabled passing `--tags` has no effect, so there's no need to
add tests for this.

[1]: https://lore.kernel.org/git/CAGZ79kbHuMpiavJ90kQLEL_AR0BEyArcZoEWAjPPhOFacN16YQ@mail.gmail.com/

Signed-off-by: Toon Claes <toon@iotcl.com>
---
 Documentation/git-clone.txt |  7 ++++---
 builtin/clone.c             | 14 +++++++-------
 2 files changed, 11 insertions(+), 10 deletions(-)

Comments

Jean-Noël AVILA Feb. 1, 2025, 4:47 p.m. UTC | #1
Hello,

I'm only reviewing the doc part.

On Friday, 31 January 2025 16:30:30 UTC+1 Toon Claes wrote:
> Option --no-tags was added in 0dab2468ee (clone: add a --no-tags option
> to clone without tags, 2017-04-26). At the time there was no need to
> support --tags as well, although there was some conversation about
> it[1].
> 
> To simplify the code and to prepare for future commits, invert the flag
> internally. Functionally there is no change, because the flag is
> default-enabled passing `--tags` has no effect, so there's no need to
> add tests for this.
> 
> [1]:
> https://lore.kernel.org/git/
CAGZ79kbHuMpiavJ90kQLEL_AR0BEyArcZoEWAjPPhOFacN16
> YQ@mail.gmail.com/
> 
> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
>  Documentation/git-clone.txt |  7 ++++---
>  builtin/clone.c             | 14 +++++++-------
>  2 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
> index
> de8d8f58930ecff305f79480b13ddce10cd96c60..99a9222e63429b3447deb3e7c08962d4ec6
> 0a295 100644 --- a/Documentation/git-clone.txt
> +++ b/Documentation/git-clone.txt
> @@ -13,7 +13,7 @@ git clone [--template=<template-directory>]
>  	  [-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]

There's an extra '-' : it should read '--[no-]tags'

>  	  [--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]
>  	  [--[no-]remote-submodules] [--jobs <n>] [--sparse] [--
[no-]reject-shallow]
> [--filter=<filter-spec>] [--also-filter-submodules]] [--] <repository> @@
> -273,8 +273,9 @@ corresponding `--mirror` and `--no-tags` options instead.
> branch when `--single-branch` clone was made, no remote-tracking
>  	branch is created.
> 
> -`--no-tags`::
> -	Don't clone any tags, and set
> +`--[no-]tags`::
> +	By default tags are cloned, and passing `--tags` doesn't change 
that.
> +	With `--no-tags`, no tags are cloned, and set

Better keep and start the description  with the imperative mood , as in the 
previous version and add the "By default,..." at the end of the description.

>  	`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,
> diff --git a/builtin/clone.c b/builtin/clone.c
> index
diff mbox series

Patch

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index de8d8f58930ecff305f79480b13ddce10cd96c60..99a9222e63429b3447deb3e7c08962d4ec60a295 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -13,7 +13,7 @@  git clone [--template=<template-directory>]
 	  [-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]
 	  [--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]
 	  [--[no-]remote-submodules] [--jobs <n>] [--sparse] [--[no-]reject-shallow]
 	  [--filter=<filter-spec>] [--also-filter-submodules]] [--] <repository>
@@ -273,8 +273,9 @@  corresponding `--mirror` and `--no-tags` options instead.
 	branch when `--single-branch` clone was made, no remote-tracking
 	branch is created.
 
-`--no-tags`::
-	Don't clone any tags, and set
+`--[no-]tags`::
+	By default tags are cloned, and passing `--tags` doesn't change that.
+	With `--no-tags`, no tags are cloned, 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,
diff --git a/builtin/clone.c b/builtin/clone.c
index 5ed0802f1d0ddebaf512aac93bf8c8b340494323..69d1ad029dfa84a2f7136fa4a3c4c8a594b179c4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -59,7 +59,7 @@ 
 
 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_tags = 1; /* default enabled */
 static int option_shallow_submodules;
 static int config_reject_shallow = -1;    /* unspecified */
 static char *remote_name = NULL;
@@ -470,7 +470,7 @@  static struct ref *wanted_peer_refs(const struct ref *refs,
 			get_fetch_map(refs, &refspec->items[i], &tail, 0);
 	}
 
-	if (!option_mirror && !option_single_branch && !option_no_tags)
+	if (!option_mirror && !option_single_branch && option_tags)
 		get_fetch_map(refs, &tag_refspec, &tail, 0);
 
 	refspec_item_clear(&tag_refspec);
@@ -562,7 +562,7 @@  static void update_remote_refs(const struct ref *refs,
 
 	if (refs) {
 		write_remote_refs(mapped_refs);
-		if (option_single_branch && !option_no_tags)
+		if (option_single_branch && option_tags)
 			write_followtags(refs, msg);
 	}
 
@@ -964,8 +964,8 @@  int cmd_clone(int argc,
 				N_("deepen history of shallow clone, excluding ref")),
 		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, "tags", &option_tags,
+			 N_("clone 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"),
@@ -1296,7 +1296,7 @@  int cmd_clone(int argc,
 	git_config_set(key.buf, repo);
 	strbuf_reset(&key);
 
-	if (option_no_tags) {
+	if (!option_tags) {
 		strbuf_addf(&key, "remote.%s.tagOpt", remote_name);
 		git_config_set(key.buf, "--no-tags");
 		strbuf_reset(&key);
@@ -1389,7 +1389,7 @@  int cmd_clone(int argc,
 	if (option_branch)
 		expand_ref_prefix(&transport_ls_refs_options.ref_prefixes,
 				  option_branch);
-	if (!option_no_tags)
+	if (option_tags)
 		strvec_push(&transport_ls_refs_options.ref_prefixes,
 			    "refs/tags/");