threads / patch / 62065

patchscalar: add --no-tags option

Subject: [PATCH] scalar: add --no-tags option

## tl;dr

7 messages between Sep 5, 2024 and Sep 6, 2024. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Derrick Stolee via GitGitGadget· Sep 5, 2024, 14:13 UTC · lore
From: Derrick Stolee <stolee@gmail.com>

Some large repositories use tags to track a huge list of release versions. While this is choice is costly on the ref advertisement, it is further wasteful for clients who do not need those tags. Allow clients to optionally skip the tag advertisement.

This behavior is similar to that of 'git clone --no-tags' implemented in 0dab2468ee5 (clone: add a --no-tags option to clone without tags, 2017-04-26), including the modification of the remote.origin.tagOpt config value to include "--no-tags".

One thing that is opposite of the 'git clone' implementation is that this allows '--tags' as an assumed option, which can be naturally negated with '--no-tags'. The clone command does not accept '--tags' but allows "--no-no-tags" as the negation of its '--no-tags' option.

While testing this option, combine the test with the previously untested '--no-src' option introduced in 4527db8ff8c (scalar: add --[no-]src option, 2023-08-28).

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
    scalar: add --no-tags option
    
    The need for this was discovered due to the release behavior of an
    internal monorepo. The use of Azure DevOps' limited refs feature does
    not apply to tags, so the number of tags was causing some pain for
    users.
    
    Thanks, -Stolee
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1780%2Fderrickstolee%2Fscalar-no-tags-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1780/derrickstolee/scalar-no-tags-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1780
 Documentation/scalar.txt |  6 ++++++
 scalar.c                 | 15 ++++++++++++---
 t/t9210-scalar.sh        | 18 ++++++++++++++++++
 3 files changed, 36 insertions(+), 3 deletions(-)
Show changes to 3 files +36 −3

Documentation/scalar.txt, scalar.c, t/t9210-scalar.sh

diff --git a/Documentation/scalar.txt b/Documentation/scalar.txt
index 361f51a6473..507ed2ae669 100644
--- a/Documentation/scalar.txt
+++ b/Documentation/scalar.txt
@@ -86,6 +86,12 @@ cloning. If the HEAD at the remote did not point at any branch when
 	`<entlistment>/src` directory. Use `--no-src` to place the cloned
 	repository directly in the `<enlistment>` directory.
 
+--[no-]tags::
+	By default, `scalar clone` will fetch the tag objects advertised by
+	the remote and future `git fetch` commands will do the same. Use
+	`--no-tags` to avoid fetching tags in `scalar clone` and to configure
+	the repository to avoid fetching tags in the future.
+
 --[no-]full-clone::
 	A sparse-checkout is initialized by default. This behavior can be
 	turned off via `--full-clone`.
diff --git a/scalar.c b/scalar.c
index 6166a8dd4c8..c6dd746b5b2 100644
--- a/scalar.c
+++ b/scalar.c
@@ -410,7 +410,7 @@ static int cmd_clone(int argc, const char **argv)
 {
 	const char *branch = NULL;
 	int full_clone = 0, single_branch = 0, show_progress = isatty(2);
-	int src = 1;
+	int src = 1, tags = 1;
 	struct option clone_options[] = {
 		OPT_STRING('b', "branch", &branch, N_("<branch>"),
 			   N_("branch to checkout after clone")),
@@ -421,11 +421,13 @@ static int cmd_clone(int argc, const char **argv)
 			    "be checked out")),
 		OPT_BOOL(0, "src", &src,
 			 N_("create repository within 'src' directory")),
+		OPT_BOOL(0, "tags", &tags,
+			 N_("specify if tags should be fetched during clone")),
 		OPT_END(),
 	};
 	const char * const clone_usage[] = {
 		N_("scalar clone [--single-branch] [--branch <main-branch>] [--full-clone]\n"
-		   "\t[--[no-]src] <url> [<enlistment>]"),
+		   "\t[--[no-]src] [--[no-]tags] <url> [<enlistment>]"),
 		NULL
 	};
 	const char *url;
@@ -504,6 +506,11 @@ static int cmd_clone(int argc, const char **argv)
 		goto cleanup;
 	}
 
+	if (!tags && set_config("remote.origin.tagOpt=--no-tags")) {
+		res = error(_("could not disable tags in '%s'"), dir);
+		goto cleanup;
+	}
+
 	if (!full_clone &&
 	    (res = run_git("sparse-checkout", "init", "--cone", NULL)))
 		goto cleanup;
@@ -513,7 +520,9 @@ static int cmd_clone(int argc, const char **argv)
 
 	if ((res = run_git("fetch", "--quiet",
 				show_progress ? "--progress" : "--no-progress",
-				"origin", NULL))) {
+				"origin",
+				(!tags ? "--no-tags" : NULL),
+				NULL))) {
 		warning(_("partial clone failed; attempting full clone"));
 
 		if (set_config("remote.origin.promisor") ||
diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
index a41b4fcc085..e8613990e13 100755
--- a/t/t9210-scalar.sh
+++ b/t/t9210-scalar.sh
@@ -169,6 +169,24 @@ test_expect_success 'scalar clone' '
 	)
 '
 
+test_expect_success 'scalar clone --no-... opts' '
+	# Note: redirect stderr always to avoid having a verbose test
+	# run result in a difference in the --[no-]progress option.
+	GIT_TRACE2_EVENT="$(pwd)/no-opt-trace" scalar clone \
+		--no-tags --no-src \
+		"file://$(pwd)" no-opts --single-branch 2>/dev/null &&
+
+	test_subcommand git fetch --quiet --no-progress \
+			origin --no-tags <no-opt-trace &&
+	(
+		cd no-opts &&
+
+		test_cmp_config --no-tags remote.origin.tagopt &&
+		git for-each-ref --format="%(refname)" refs/tags/ >tags &&
+		test_line_count = 0 tags
+	)
+'
+
 test_expect_success 'scalar reconfigure' '
 	git init one/src &&
 	scalar register one &&

base-commit: 2e7b89e038c0c888acf61f1b4ee5a43d4dd5e94c
-- 
gitgitgadget
Junio C Hamano· Sep 5, 2024, 16:12 UTC · re: Derrick Stolee via GitGitGadget · lore

Re: [PATCH] scalar: add --no-tags option

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 16 quoted lines
> From: Derrick Stolee <stolee@gmail.com>
>
> Some large repositories use tags to track a huge list of release
> versions. While this is choice is costly on the ref advertisement, it is
> further wasteful for clients who do not need those tags. Allow clients
> to optionally skip the tag advertisement.
>
> This behavior is similar to that of 'git clone --no-tags' implemented in
> 0dab2468ee5 (clone: add a --no-tags option to clone without tags,
> 2017-04-26), including the modification of the remote.origin.tagOpt
> config value to include "--no-tags".
>
> One thing that is opposite of the 'git clone' implementation is that
> this allows '--tags' as an assumed option, which can be naturally negated
> with '--no-tags'. The clone command does not accept '--tags' but allows
> "--no-no-tags" as the negation of its '--no-tags' option.

Yuck. The loophole may be something we may want to close later, though (and replaced with a proper "--tags" support).

Show 6 quoted lines
> While testing this option, combine the test with the previously untested
> '--no-src' option introduced in 4527db8ff8c (scalar: add --[no-]src
> option, 2023-08-28).
>
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
Makes sense.

This is a tangent, but the "--[no-]tags" option in "git fetch" is misdesigned; the default is to auto-follow tags that would annotate objects that are being fetched, and "--no-tags" is a way to decline the auto-following. But "--tags" is to say that all tags must be fetched. There is no obvious way to say "I want the auto-following behaviour" (e.g., to override an earlier "--no-tags" or "--tags").

Junio C Hamano· Sep 5, 2024, 18:25 UTC · re: Derrick Stolee via GitGitGadget · lore

Re: [PATCH] scalar: add --no-tags option

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> Subject: Re: [PATCH] scalar: add --no-tags option

Micronit. Shouldn't we make it clear that this is about the clone subcommand? E.g.

    Subject: scalar: add --no-tags option to "scalar clone"
or something?
Johannes Schindelin· Sep 6, 2024, 07:23 UTC · re: Derrick Stolee via GitGitGadget · lore

Re: [PATCH] scalar: add --no-tags option

Hi Stolee,

the patch makes sense to me. The commit message is delightfully thorough. I'll just offer minor suggestions below:

On Thu, 5 Sep 2024, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
>
> Some large repositories use tags to track a huge list of release
> versions. While this is choice is costly on the ref advertisement, it is
	s/is \(choice is\)/\1/
Show 15 quoted lines
> further wasteful for clients who do not need those tags. Allow clients
> to optionally skip the tag advertisement.
>
> [...]
> diff --git a/Documentation/scalar.txt b/Documentation/scalar.txt
> index 361f51a6473..507ed2ae669 100644
> --- a/Documentation/scalar.txt
> +++ b/Documentation/scalar.txt
> @@ -86,6 +86,12 @@ cloning. If the HEAD at the remote did not point at any branch when
>  	`<entlistment>/src` directory. Use `--no-src` to place the cloned
>  	repository directly in the `<enlistment>` directory.
>
> +--[no-]tags::
> +	By default, `scalar clone` will fetch the tag objects advertised by
> +	the remote and future `git fetch` commands will do the same. Use

It might be helpful to mention that `git fetch --tags` can be used to override this when the need arises.

Show 18 quoted lines
> +	`--no-tags` to avoid fetching tags in `scalar clone` and to configure
> +	the repository to avoid fetching tags in the future.
> +
>  --[no-]full-clone::
>  	A sparse-checkout is initialized by default. This behavior can be
>  	turned off via `--full-clone`.
> diff --git a/scalar.c b/scalar.c
> index 6166a8dd4c8..c6dd746b5b2 100644
> --- a/scalar.c
> +++ b/scalar.c
> [...]
> @@ -513,7 +520,9 @@ static int cmd_clone(int argc, const char **argv)
>
>  	if ((res = run_git("fetch", "--quiet",
>  				show_progress ? "--progress" : "--no-progress",
> -				"origin", NULL))) {
> +				"origin",
> +				(!tags ? "--no-tags" : NULL),
I find double negatives challenging, and would prefer this instead:
				tags ? NULL : "--no-tags",
Show 28 quoted lines
> +				NULL))) {
>  		warning(_("partial clone failed; attempting full clone"));
>
>  		if (set_config("remote.origin.promisor") ||
> diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
> index a41b4fcc085..e8613990e13 100755
> --- a/t/t9210-scalar.sh
> +++ b/t/t9210-scalar.sh
> @@ -169,6 +169,24 @@ test_expect_success 'scalar clone' '
>  	)
>  '
>
> +test_expect_success 'scalar clone --no-... opts' '
> +	# Note: redirect stderr always to avoid having a verbose test
> +	# run result in a difference in the --[no-]progress option.
> +	GIT_TRACE2_EVENT="$(pwd)/no-opt-trace" scalar clone \
> +		--no-tags --no-src \
> +		"file://$(pwd)" no-opts --single-branch 2>/dev/null &&
> +
> +	test_subcommand git fetch --quiet --no-progress \
> +			origin --no-tags <no-opt-trace &&
> +	(
> +		cd no-opts &&
> +
> +		test_cmp_config --no-tags remote.origin.tagopt &&
> +		git for-each-ref --format="%(refname)" refs/tags/ >tags &&
> +		test_line_count = 0 tags
> +	)

For readability (and to avoid an unnecessary subshell), this could be written as:

	test_cmp_config -C no-opts --no-tags remote.origin.tagopt &&
	git -C no-opts for-each-ref --format="%(refname)" refs/tags/ >tags &&
	test_line_count = 0 tags
Show 5 quoted lines
> +'
> +
>  test_expect_success 'scalar reconfigure' '
>  	git init one/src &&
>  	scalar register one &&

None of these suggestions, in isolation or in conjunction, seem big enough to me to warrant a new iteration; I just offer them here in case you want to iterate on the patch. If you want to keep the patch as-is, I am totally on board with that.

Ciao, Johannes

Junio C Hamano· Sep 6, 2024, 15:13 UTC · re: Johannes Schindelin · lore

Re: [PATCH] scalar: add --no-tags option

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> None of these suggestions, in isolation or in conjunction, seem big enough
> to me to warrant a new iteration; I just offer them here in case you want
> to iterate on the patch. If you want to keep the patch as-is, I am totally
> on board with that.

Everything other than "prefer repeated use of -C instead of a single subshell in there" (which I think worsens readability, even though it may save one process), I agree with your suggestions.

Thanks.
Derrick Stolee· Sep 6, 2024, 19:46 UTC · re: Junio C Hamano · lore

Re: [PATCH] scalar: add --no-tags option

On 9/6/24 11:13 AM, Junio C Hamano wrote:
Show 10 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
>> None of these suggestions, in isolation or in conjunction, seem big enough
>> to me to warrant a new iteration; I just offer them here in case you want
>> to iterate on the patch. If you want to keep the patch as-is, I am totally
>> on board with that.
> 
> Everything other than "prefer repeated use of -C instead of a single
> subshell in there" (which I think worsens readability, even though
> it may save one process), I agree with your suggestions.

Thanks, both, for your review comments. v2 is on the way with these suggestions.

-Stolee
Derrick Stolee via GitGitGadget· Sep 6, 2024, 20:21 UTC · re: Derrick Stolee via GitGitGadget · lore

[PATCH v2] scalar: add --no-tags option to 'scalar clone'

From: Derrick Stolee <stolee@gmail.com>

Some large repositories use tags to track a huge list of release versions. While this choice is costly on the ref advertisement, it is further wasteful for clients who do not need those tags. Allow clients to optionally skip the tag advertisement.

This behavior is similar to that of 'git clone --no-tags' implemented in 0dab2468ee5 (clone: add a --no-tags option to clone without tags, 2017-04-26), including the modification of the remote.origin.tagOpt config value to include "--no-tags".

One thing that is opposite of the 'git clone' implementation is that this allows '--tags' as an assumed option, which can be naturally negated with '--no-tags'. The clone command does not accept '--tags' but allows "--no-no-tags" as the negation of its '--no-tags' option.

While testing this option, combine the test with the previously untested '--no-src' option introduced in 4527db8ff8c (scalar: add --[no-]src option, 2023-08-28).

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
    scalar: add --no-tags option
    
    The need for this was discovered due to the release behavior of an
    internal monorepo. The use of Azure DevOps' limited refs feature does
    not apply to tags, so the number of tags was causing some pain for
    users.
    
    Thanks, -Stolee
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1780%2Fderrickstolee%2Fscalar-no-tags-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1780/derrickstolee/scalar-no-tags-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1780
Range-diff vs v1:
 1:  f0e8217869a ! 1:  3378b4f728a scalar: add --no-tags option
     @@ Metadata
      Author: Derrick Stolee <dstolee@microsoft.com>
      
       ## Commit message ##
     -    scalar: add --no-tags option
     +    scalar: add --no-tags option to 'scalar clone'
      
          Some large repositories use tags to track a huge list of release
     -    versions. While this is choice is costly on the ref advertisement, it is
     +    versions. While this choice is costly on the ref advertisement, it is
          further wasteful for clients who do not need those tags. Allow clients
          to optionally skip the tag advertisement.
      
     @@ Documentation/scalar.txt: cloning. If the HEAD at the remote did not point at an
      +	By default, `scalar clone` will fetch the tag objects advertised by
      +	the remote and future `git fetch` commands will do the same. Use
      +	`--no-tags` to avoid fetching tags in `scalar clone` and to configure
     -+	the repository to avoid fetching tags in the future.
     ++	the repository to avoid fetching tags in the future. To fetch tags after
     ++	cloning with `--no-tags`, run `git fetch --tags`.
      +
       --[no-]full-clone::
       	A sparse-checkout is initialized by default. This behavior can be
     @@ scalar.c: static int cmd_clone(int argc, const char **argv)
       				show_progress ? "--progress" : "--no-progress",
      -				"origin", NULL))) {
      +				"origin",
     -+				(!tags ? "--no-tags" : NULL),
     ++				(tags ? NULL : "--no-tags"),
      +				NULL))) {
       		warning(_("partial clone failed; attempting full clone"));
       
 Documentation/scalar.txt |  7 +++++++
 scalar.c                 | 15 ++++++++++++---
 t/t9210-scalar.sh        | 18 ++++++++++++++++++
 3 files changed, 37 insertions(+), 3 deletions(-)
Show changes to 3 files +37 −3

Documentation/scalar.txt, scalar.c, t/t9210-scalar.sh

diff --git a/Documentation/scalar.txt b/Documentation/scalar.txt
index 361f51a6473..7e4259c6743 100644
--- a/Documentation/scalar.txt
+++ b/Documentation/scalar.txt
@@ -86,6 +86,13 @@ cloning. If the HEAD at the remote did not point at any branch when
 	`<entlistment>/src` directory. Use `--no-src` to place the cloned
 	repository directly in the `<enlistment>` directory.
 
+--[no-]tags::
+	By default, `scalar clone` will fetch the tag objects advertised by
+	the remote and future `git fetch` commands will do the same. Use
+	`--no-tags` to avoid fetching tags in `scalar clone` and to configure
+	the repository to avoid fetching tags in the future. To fetch tags after
+	cloning with `--no-tags`, run `git fetch --tags`.
+
 --[no-]full-clone::
 	A sparse-checkout is initialized by default. This behavior can be
 	turned off via `--full-clone`.
diff --git a/scalar.c b/scalar.c
index 6166a8dd4c8..09560aeab54 100644
--- a/scalar.c
+++ b/scalar.c
@@ -410,7 +410,7 @@ static int cmd_clone(int argc, const char **argv)
 {
 	const char *branch = NULL;
 	int full_clone = 0, single_branch = 0, show_progress = isatty(2);
-	int src = 1;
+	int src = 1, tags = 1;
 	struct option clone_options[] = {
 		OPT_STRING('b', "branch", &branch, N_("<branch>"),
 			   N_("branch to checkout after clone")),
@@ -421,11 +421,13 @@ static int cmd_clone(int argc, const char **argv)
 			    "be checked out")),
 		OPT_BOOL(0, "src", &src,
 			 N_("create repository within 'src' directory")),
+		OPT_BOOL(0, "tags", &tags,
+			 N_("specify if tags should be fetched during clone")),
 		OPT_END(),
 	};
 	const char * const clone_usage[] = {
 		N_("scalar clone [--single-branch] [--branch <main-branch>] [--full-clone]\n"
-		   "\t[--[no-]src] <url> [<enlistment>]"),
+		   "\t[--[no-]src] [--[no-]tags] <url> [<enlistment>]"),
 		NULL
 	};
 	const char *url;
@@ -504,6 +506,11 @@ static int cmd_clone(int argc, const char **argv)
 		goto cleanup;
 	}
 
+	if (!tags && set_config("remote.origin.tagOpt=--no-tags")) {
+		res = error(_("could not disable tags in '%s'"), dir);
+		goto cleanup;
+	}
+
 	if (!full_clone &&
 	    (res = run_git("sparse-checkout", "init", "--cone", NULL)))
 		goto cleanup;
@@ -513,7 +520,9 @@ static int cmd_clone(int argc, const char **argv)
 
 	if ((res = run_git("fetch", "--quiet",
 				show_progress ? "--progress" : "--no-progress",
-				"origin", NULL))) {
+				"origin",
+				(tags ? NULL : "--no-tags"),
+				NULL))) {
 		warning(_("partial clone failed; attempting full clone"));
 
 		if (set_config("remote.origin.promisor") ||
diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
index a41b4fcc085..e8613990e13 100755
--- a/t/t9210-scalar.sh
+++ b/t/t9210-scalar.sh
@@ -169,6 +169,24 @@ test_expect_success 'scalar clone' '
 	)
 '
 
+test_expect_success 'scalar clone --no-... opts' '
+	# Note: redirect stderr always to avoid having a verbose test
+	# run result in a difference in the --[no-]progress option.
+	GIT_TRACE2_EVENT="$(pwd)/no-opt-trace" scalar clone \
+		--no-tags --no-src \
+		"file://$(pwd)" no-opts --single-branch 2>/dev/null &&
+
+	test_subcommand git fetch --quiet --no-progress \
+			origin --no-tags <no-opt-trace &&
+	(
+		cd no-opts &&
+
+		test_cmp_config --no-tags remote.origin.tagopt &&
+		git for-each-ref --format="%(refname)" refs/tags/ >tags &&
+		test_line_count = 0 tags
+	)
+'
+
 test_expect_success 'scalar reconfigure' '
 	git init one/src &&
 	scalar register one &&

base-commit: 2e7b89e038c0c888acf61f1b4ee5a43d4dd5e94c
-- 
gitgitgadget

← back to recent threads