-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sparse-checkout: make cone mode the default #1148
Conversation
27486cd
to
af002cc
Compare
9c596d3
to
e7d3ff7
Compare
/submit |
Submitted as pull.1148.git.1646725188.gitgitgadget@gmail.com To fetch this version into
To fetch this version to local tag
|
On the Git mailing list, Derrick Stolee wrote (reply to this):
|
User |
Add an explicit --no-cone to several sparse-checkout invocations in preparation for changing the default to cone mode. Signed-off-by: Elijah Newren <newren@gmail.com>
e7d3ff7
to
4b89a33
Compare
/submit |
Submitted as pull.1148.v2.git.1647054681.gitgitgadget@gmail.com To fetch this version into
To fetch this version to local tag
|
On the Git mailing list, Derrick Stolee wrote (reply to this):
|
On the Git mailing list, Victoria Dye wrote (reply to this):
|
On the Git mailing list, Junio C Hamano wrote (reply to this):
|
@@ -22,8 +22,8 @@ present, or undo and go back to having all tracked files present in the | |||
working copy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> -When `--no-cone` is passed or `core.sparseCheckoutCone` is false,
> -the input list is considered a list of patterns. This mode is harder
> -to use and less performant, and is thus not recommended. See the
> -"Sparse Checkout" section of linkgit:git-read-tree[1] and the "Pattern
> -Set" sections below for more details.
> +When `--no-cone` is passed, the input list is considered a list of
> +patterns. This mode is harder to use and less performant, and is thus
"less perfromant" can be quantified, but "harder to use" is probably
harder to defend. Those on a project with need for more flexible
way to specify than "these are the directories I care about" would
not find it convincing.
> +not recommended. See the "Sparse Checkout" section of
> +linkgit:git-read-tree[1]
The referenced section (I am reading "git read-tree --help" from
seen with these patches) may need updating. It shows an example
of including everything except for unwanted, without mentioning
if that is for cone or non-cone.
> and the "Pattern Set" sections below for more
> +details.
Are we referring to "Internals - cone/full pattern set" sections?
This may be a topic of another step in this series, but the "core"
section starts by mentioning what characteristics the full pattern
set has and uses it to steer readers away from it, which I find it
less than ideal. As we present "core" first (because it is the
default), we should present "core" by itself, without requiring to
know what the other thing is. Perhaps replace the entire first
paragraph so that the section begins more like so:
The "cone mode", which is the default, lets you specify only
what directories to include and what directories to exclude.
The accepted patterns in the cone pattern set are:...
Especially, the last sentence in the paragraph to be struck still
lives in the old world in which you needed to opt into the cone
mode.
Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the Git mailing list, Elijah Newren wrote (reply to this):
On Mon, Mar 14, 2022 at 1:53 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > -When `--no-cone` is passed or `core.sparseCheckoutCone` is false,
> > -the input list is considered a list of patterns. This mode is harder
> > -to use and less performant, and is thus not recommended. See the
> > -"Sparse Checkout" section of linkgit:git-read-tree[1] and the "Pattern
> > -Set" sections below for more details.
> > +When `--no-cone` is passed, the input list is considered a list of
> > +patterns. This mode is harder to use and less performant, and is thus
>
> "less perfromant" can be quantified, but "harder to use" is probably
> harder to defend. Those on a project with need for more flexible
> way to specify than "these are the directories I care about" would
> not find it convincing.
That text wasn't added by this patch (or even by any patch in this
series); it was in the pre-image. I can still change it, of course,
but I don't think it belongs in this particular patch. How about I
replace this text in 8/9 with a link to the new section that was added
(the one that explains the problems we see with non-cone mode).
> > +not recommended. See the "Sparse Checkout" section of
> > +linkgit:git-read-tree[1]
>
> The referenced section (I am reading "git read-tree --help" from
> seen with these patches) may need updating. It shows an example
> of including everything except for unwanted, without mentioning
> if that is for cone or non-cone.
I wouldn't really say that the example is for either mode (the point
of the sparse-checkout command is so users don't need to directly edit
the $GIT_DIR/info/sparse-checkout file), but the examples might be
useful for people trying to understand the patterns used in non-cone
mode. Perhaps this change would be helpful?:
diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt
index 99bb387134..0eae9e01fd 100644
--- a/Documentation/git-read-tree.txt
+++ b/Documentation/git-read-tree.txt
@@ -378,7 +378,9 @@ SPARSE CHECKOUT
Note: The `update-index` and `read-tree` primitives for supporting the
skip-worktree bit predated the introduction of
linkgit:git-sparse-checkout[1]. Users are encouraged to use
-`sparse-checkout` in preference to these low-level primitives.
+`sparse-checkout` in preference to these low-level primitives. However,
+the information below might be useful to users trying to understand the
+pattern style used in non-cone mode of the `sparse-checkout` command.
However, I don't think that belongs in this patch (since you were
commenting on text that only appeared in the diff due to reflowing
paragraphs), but I can add it to 9/9.
> > and the "Pattern Set" sections below for more
> > +details.
>
> Are we referring to "Internals - cone/full pattern set" sections?
Yeah, as you noted in a later patch, you got confused by looking at
the end result rather than the state of the tree as of this patch.
The "Internals -" prefix was added in a later patch.
> This may be a topic of another step in this series, but the "core"
> section starts by mentioning what characteristics the full pattern
> set has and uses it to steer readers away from it, which I find it
> less than ideal. As we present "core" first (because it is the
> default), we should present "core" by itself, without requiring to
> know what the other thing is.
By '"core" section' do you mean the "cone pattern set" section?
That's my best guess after several comparisons. I agree that it's
less than ideal, though I thought that perhaps adding the "Internals"
prefix would allow folks to just ignore it. But you may be right that
I should also overhaul it a bit, maybe splitting it into two sections,
one about some implementation details about the mode, and another
later section about how the patterns in cone mode are a strictly
controlled subset of the possible full pattern set. However, I think
that'd go better in patch 7 rather than here.
> Perhaps replace the entire first
> paragraph so that the section begins more like so:
>
> The "cone mode", which is the default, lets you specify only
> what directories to include and what directories to exclude.
There's no provision to specify individual directories to exclude,
only which to include. Everything not listed is excluded.
> The accepted patterns in the cone pattern set are:...
>
> Especially, the last sentence in the paragraph to be struck still
> lives in the old world in which you needed to opt into the cone
> mode.
This patch didn't strike any paragraph, so I'm not sure which sentence
you're referring to. I tried looking around, and didn't readily find
it either. Perhaps my big reshufflings in my new patch 7 addressed
what you're talking about, though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Elijah Newren <newren@gmail.com> writes:
>> less than ideal. As we present "core" first (because it is the
>> default), we should present "core" by itself, without requiring to
>> know what the other thing is.
>
> By '"core" section' do you mean the "cone pattern set" section?
s/core/cone mode/g, I guess (because it is so long ago I do not
offhand recall what I had in mind exactly back when I wrote it).
This branch is now known as |
This patch series was integrated into seen via git@4392dc6. |
There was a status update in the "New Topics" section about the branch source: <pull.1148.v2.git.1647054681.gitgitgadget@gmail.com> |
On the Git mailing list, Junio C Hamano wrote (reply to this):
|
This patch series was integrated into seen via git@00de32f. |
This patch series was integrated into seen via git@acef92b. |
This patch series was integrated into seen via git@8c8b62d. |
This patch series was integrated into next via git@c168eb5. |
This patch series was integrated into seen via git@71ec893. |
There was a status update in the "Cooking" section about the branch Deprecate non-cone mode of the sparse-checkout feature. Will cook in 'next' til 06-03 and then merge to 'master'. source: <pull.1148.v3.git.1650594746.gitgitgadget@gmail.com> |
This patch series was integrated into seen via git@5b9271e. |
This patch series was integrated into seen via git@db5d79f. |
This patch series was integrated into seen via git@34c3e6e. |
This patch series was integrated into seen via git@b2c5a18. |
This patch series was integrated into seen via git@e34c123. |
There was a status update in the "Cooking" section about the branch Deprecate non-cone mode of the sparse-checkout feature. Will cook in 'next' til 06-03 and then merge to 'master'. source: <pull.1148.v3.git.1650594746.gitgitgadget@gmail.com> |
This patch series was integrated into seen via git@3ed8320. |
This patch series was integrated into seen via git@42d4cfc. |
This patch series was integrated into seen via git@c91d153. |
There was a status update in the "Cooking" section about the branch Deprecate non-cone mode of the sparse-checkout feature. Will cook in 'next' til 06-03 and then merge to 'master'. source: <pull.1148.v3.git.1650594746.gitgitgadget@gmail.com> |
This patch series was integrated into seen via git@2770fc3. |
This patch series was integrated into seen via git@414bad1. |
This patch series was integrated into seen via git@102e418. |
This patch series was integrated into seen via git@14b6581. |
This patch series was integrated into seen via git@21100b1. |
There was a status update in the "Cooking" section about the branch Deprecate non-cone mode of the sparse-checkout feature. Will cook in 'next' til 06-03 and then merge to 'master'. source: <pull.1148.v3.git.1650594746.gitgitgadget@gmail.com> |
This patch series was integrated into seen via git@6f80ca8. |
This patch series was integrated into seen via git@c870bca. |
This patch series was integrated into seen via git@377d347. |
This patch series was integrated into master via git@377d347. |
This patch series was integrated into next via git@377d347. |
Closed via 377d347. |
Sorry for the long delay since v1. On the plus side, perhaps there will be a longer feedback period for this series since it's now really early in the 2.37 series?
== Updates Log ==
Changes since v2:
Changes since v1:
== Overview ==
This patch changes the default mode for sparse-checkout from non-cone mode to cone-mode, and marks non-cone mode as deprecated. There is no plan to remove non-cone mode, we are merely recommending against its use.
The code change is pretty small, and most of this series is about documentation updates -- to focus on directories rather than patterns, to explain the new default, to explain why we are deprecating non-cone mode (the final patch), and to make other related cleanups to simplify the manual.
Patch 1: Update tests to not assume cone-mode is the default
Patch 2: Make cone-mode the default
Patches 3-9: Various updates to git-sparse-checkout.txt, divided up for ease of review
== Alternative ==
There is one primary alternative to this series: make sparse-checkout error when neither --cone nor --no-cone are specified (so that there is no default), and wait until a future date to make --cone the default. That'd be reasonable, but I had three reason to avoid going this route (note that item 2 means there's little practical difference between cone-mode-as-default and no-mode-is-default):
"""
THIS COMMAND IS EXPERIMENTAL. ITS BEHAVIOR, AND THE BEHAVIOR OF OTHER
COMMANDS IN THE PRESENCE OF SPARSE-CHECKOUTS, WILL LIKELY CHANGE IN
THE FUTURE.
"""
If users are unaware of the default change and attempt to provide patterns instead of directories, then they will get error messages added from en/sparse-checkout-fixes. They can learn at that time to get around the error messages by providing --no-cone.
If users are unaware of the default change and provide directories, then that's where non-cone mode and cone mode overlap and things happen to work. (There is a slight difference in that cone mode will include files from parent directories of any specified directory, but that means the user gets a few more files in their sparse-checkout with cone mode than they would with non-cone mode.)
== CCs ==
cc: Victoria Dye vdye@github.com
cc: Lessley Dennington lessleydennington@gmail.com
cc: Derrick Stolee derrickstolee@github.com
cc: Elijah Newren newren@gmail.com