scalar: add --no-tags option #1780
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
derrickstolee
force-pushed
the
scalar-no-tags
branch
from
6d268ad to
f0e8217
Compare
|
/submit |
|
Submitted as pull.1780.git.1725545614416.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 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).
> 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"). |
|
This patch series was integrated into seen via git@e800186. |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "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? |
derrickstolee
force-pushed
the
scalar-no-tags
branch
from
f0e8217 to
35ee84d
Compare
|
This patch series was integrated into seen via git@d0f4b21. |
|
On the Git mailing list, Johannes Schindelin wrote (reply to this): 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/
> 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.
> + `--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",
> + 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
> +'
> +
> 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 |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): 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. |
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 0dab246 (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 4527db8 (scalar: add --[no-]src option, 2023-08-28). Signed-off-by: Derrick Stolee <stolee@gmail.com>
derrickstolee
force-pushed
the
scalar-no-tags
branch
from
35ee84d to
3378b4f
Compare
|
On the Git mailing list, Derrick Stolee wrote (reply to this): On 9/6/24 11:13 AM, Junio C Hamano wrote:
> 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
|
|
/submit |
|
Submitted as pull.1780.v2.git.1725654102035.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
This patch series was integrated into seen via git@9ec5ba0. |
|
This branch is now known as |
|
This patch series was integrated into seen via git@3602cea. |
|
This patch series was integrated into next via git@fc06d19. |
|
There was a status update in the "Cooking" section about the branch The "scalar clone" command learned the "--no-tags" option. Will merge to 'master'. source: <pull.1780.v2.git.1725654102035.gitgitgadget@gmail.com> |
|
There was a status update in the "Cooking" section about the branch The "scalar clone" command learned the "--no-tags" option. Will merge to 'master'. source: <pull.1780.v2.git.1725654102035.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@e012d31. |
|
This patch series was integrated into seen via git@b642c98. |
|
This patch series was integrated into seen via git@1f4ea90. |
|
There was a status update in the "Cooking" section about the branch The "scalar clone" command learned the "--no-tags" option. Will merge to 'master'. source: <pull.1780.v2.git.1725654102035.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@c2ed197. |
|
This patch series was integrated into seen via git@0299251. |
|
This patch series was integrated into master via git@0299251. |
|
This patch series was integrated into next via git@0299251. |
|
Closed via 0299251. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
cc: gitster@pobox.com
cc: johannes.schindelin@gmx.de