Elijah Newren via GitGitGadget wrote: > == Updates Log == > > Changes since v1: > > * rebased on master, to remove dependence on en/present-despite-skipped > (which has since merged to master). Earlier version of series wasn't > picked up anyway, so rebasing should be safe. > * Wording and code style tweaks suggested by Stolee in his review > > == 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 > After looking over the patches (and Stolee's prior feedback), I don't have any additional comments - it all looks great! I also ran through some scenarios going from "non-cone" to "cone mode" that have been buggy in the past (e.g., non-cone directory patterns missing a leading '/', then 'git sparse-checkout init --cone'), but it looks like the checks added back in a3eca58445 (sparse-checkout: refuse to add to bad patterns, 2021-12-16) prevent any unexplained behavior for a user that might not be aware of the new default. In other words, changing the default looks to be safe and overall pushes users towards a better experience using sparse-checkout. > == 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): > > 1. git-sparse-checkout.txt has the following huge warning early in the > manual: > > """ THIS COMMAND IS EXPERIMENTAL. ITS BEHAVIOR, AND THE BEHAVIOR OF OTHER > COMMANDS IN THE PRESENCE OF SPARSE-CHECKOUTS, WILL LIKELY CHANGE IN THE > FUTURE. """ > > 2. 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. > > 3. 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.) > Agreed with your comments here and Stolee's earlier that changing the default is a better approach. > == CCs == > > Elijah Newren (9): > tests: stop assuming --no-cone is the default mode for sparse-checkout > sparse-checkout: make --cone the default > git-sparse-checkout.txt: wording updates for the cone mode default > git-sparse-checkout.txt: update docs for deprecation of 'init' > git-sparse-checkout.txt: shuffle some sections and mark as internal > git-sparse-checkout.txt: add a new EXAMPLES section > git-sparse-checkout.txt: flesh out non-cone mode pattern discussion a > bit > git-sparse-checkout.txt: mark non-cone mode as deprecated > Documentation: some sparsity wording clarifications > > Documentation/git-read-tree.txt | 9 +- > Documentation/git-sparse-checkout.txt | 249 +++++++++++++++++++------- > builtin/sparse-checkout.c | 2 +- > t/t1091-sparse-checkout-builtin.sh | 15 +- > t/t3602-rm-sparse-checkout.sh | 6 +- > t/t3705-add-sparse-checkout.sh | 4 +- > t/t6428-merge-conflicts-sparse.sh | 4 +- > t/t7002-mv-sparse-checkout.sh | 2 +- > t/t7012-skip-worktree-writing.sh | 2 +- > 9 files changed, 207 insertions(+), 86 deletions(-) > > > base-commit: 1a4874565fa3b6668042216189551b98b4dc0b1b > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1148%2Fnewren%2Fsparse-checkout-default-v2 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1148/newren/sparse-checkout-default-v2 > Pull-Request: https://github.com/gitgitgadget/git/pull/1148 > > Range-diff vs v1: > > 1: 8c3e730e86b = 1: 05dba7069c5 tests: stop assuming --no-cone is the default mode for sparse-checkout > 2: b174b42ed82 ! 2: a53179764bc sparse-checkout: make --cone the default > @@ builtin/sparse-checkout.c: static int update_modes(int *cone_mode, int *sparse_i > /* Set cone/non-cone mode appropriately */ > core_apply_sparse_checkout = 1; > - if (*cone_mode == 1) { > -+ if (*cone_mode == 1 || *cone_mode == -1) { > ++ if (*cone_mode) { /* also handles "not specified" (value of -1) */ > mode = MODE_CONE_PATTERNS; > core_sparse_checkout_cone = 1; > } else { > 3: f98b3fac78a = 3: 8eab21996c7 git-sparse-checkout.txt: wording updates for the cone mode default > 4: cd871966c06 = 4: eb3b318b39e git-sparse-checkout.txt: update docs for deprecation of 'init' > 5: fe37a966699 = 5: 7333198b778 git-sparse-checkout.txt: shuffle some sections and mark as internal > 6: 3265f41bcab ! 6: a814ea9e8d2 git-sparse-checkout.txt: add a new EXAMPLES section > @@ Documentation/git-sparse-checkout.txt: paths to pass to a subsequent 'set' or 'a > + > +`git sparse-checkout reapply`:: > + > -+ It is possible for commands to update the working tree in a way > -+ that does not respect the selected sparsity directories, either > -+ because of special cases (such as hitting conflicts when > -+ merging/rebasing), or because some commands didn't fully support > -+ sparse checkouts (e.g. the old `recursive` merge backend had > -+ only limited support). This command reapplies the existing > -+ sparse directory specifications to make the working directory > -+ match. > ++ It is possible for commands to update the working tree in a > ++ way that does not respect the selected sparsity directories. > ++ This can come from tools external to Git writing files, or > ++ even affect Git commands because of either special cases (such > ++ as hitting conflicts when merging/rebasing), or because some > ++ commands didn't fully support sparse checkouts (e.g. the old > ++ `recursive` merge backend had only limited support). This > ++ command reapplies the existing sparse directory specifications > ++ to make the working directory match. > + > INTERNALS -- SPARSE CHECKOUT > ---------------------------- > 7: bdbf61ee6a0 = 7: 78028ecaa58 git-sparse-checkout.txt: flesh out non-cone mode pattern discussion a bit > 8: 3e35d62c5ee = 8: 2d2b81986a5 git-sparse-checkout.txt: mark non-cone mode as deprecated > 9: e7d3ff70914 = 9: 4b89a3392b0 Documentation: some sparsity wording clarifications >
