[PATCH v3 00/18] Sparse checkout improvements -- improved sparsity updating
- From
Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Mar 27, 2020, 00:48 UTC
- Message-ID
- <pull.726.v3.git.git.1585270142.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.726.v2.git.git.1584813609.gitgitgadget@gmail.com>
This series provides a replacement for the sparsity updating in sparse-checkout that is based on the logic from git read-tree -mu HEAD. The most important bit is patch 9 and its lengthy commit message explaining the current state and rationale for most the series, though patches 16 and 17 have additional related directions and rationale for the series. Those three patches are the most important to review.
Changes since v2:
* addressed Stolee's cleanups and added his Reviewed-by to the series (hopefully the next two changes don't invalidate that) * added a test for the new 'reapply' subcommand (noticed it was missing with Stolee's test coverage report) * fix a minor issue with two of the other tests I modified -- when I'm grepping through stderr for a command, I need to make sure to actually record stderr from that command (otherwise my grep is looking through the stderr of a previous command that was recorded). Also, since I messed this up, I added one or two more sanity checks while I was at it.
Elijah Newren (18):
unpack-trees: fix minor typo in comment
unpack-trees: remove unused error type
unpack-trees: simplify verify_absent_sparse()
unpack-trees: simplify pattern_list freeing
t1091: make some tests a little more defensive against failures
unpack-trees: allow check_updates() to work on a different index
unpack-trees: do not mark a dirty path with SKIP_WORKTREE
unpack-trees: pull sparse-checkout pattern reading into a new function
unpack-trees: add a new update_sparsity() function
sparse-checkout: use new update_sparsity() function
sparse-checkout: use improved unpack_trees porcelain messages
unpack-trees: move ERROR_WOULD_LOSE_SUBMODULE earlier
unpack-trees: rename ERROR_* fields meant for warnings to WARNING_*
unpack-trees: split display_error_msgs() into two
unpack-trees: make sparse path messages sound like warnings
unpack-trees: provide warnings on sparse updates for unmerged paths
too
unpack-trees: failure to set SKIP_WORKTREE bits always just a warning
sparse-checkout: provide a new reapply subcommandDocumentation/git-sparse-checkout.txt | 10 + builtin/sparse-checkout.c | 55 +++--- t/t1011-read-tree-sparse-checkout.sh | 11 +- t/t1091-sparse-checkout-builtin.sh | 105 ++++++++++- t/t2018-checkout-branch.sh | 22 +++ unpack-trees.c | 255 ++++++++++++++++++++------ unpack-trees.h | 28 ++- 7 files changed, 375 insertions(+), 111 deletions(-)
base-commit: be8661a3286c67a5d4088f4226cbd7f8b76544b0 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-726%2Fnewren%2Fsparse-checkout-improvements-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-726/newren/sparse-checkout-improvements-v3 Pull-Request: https://github.com/git/git/pull/726
Range-diff vs v2:
1: 3c183727c98 ! 1: bb41a2a52f4 unpack-trees: fix minor typo in comment
@@ -2,6 +2,7 @@
unpack-trees: fix minor typo in comment
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/unpack-trees.c b/unpack-trees.c
2: e6beb1c5810 ! 2: 1dc36eb33f7 unpack-trees: remove unused error type
@@ -19,6 +19,7 @@
introduced and nothing else has used it in the intervening decade
either. Remove it.
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/unpack-trees.c b/unpack-trees.c
3: 739b52393fe ! 3: 97a95e91d8f unpack-trees: simplify verify_absent_sparse()
@@ -11,6 +11,7 @@
and no other caller has been created, and no other value has ever been
passed, so just pass the expected value to begin with.
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/unpack-trees.c b/unpack-trees.c
4: 65772dd46df ! 4: 1ce5dbe9c68 unpack-trees: simplify pattern_list freeing
@@ -13,6 +13,7 @@
additional flag in unpack_trees_options. Put the logic back in the
relevant function and discard the now unnecessary flag.
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
5: 5cbab2a7a56 ! 5: da4b74093cd t1091: make some tests a little more defensive against failures
@@ -2,6 +2,7 @@
t1091: make some tests a little more defensive against failures
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
6: 5fea19f0136 ! 6: 555eedf5380 unpack-trees: allow check_updates() to work on a different index
@@ -6,6 +6,7 @@
to use this function in combination with a different index_state, so
take the intended index_state as a parameter.
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/unpack-trees.c b/unpack-trees.c
7: 3d2531ca818 ! 7: 10ec70fa697 unpack-trees: do not mark a dirty path with SKIP_WORKTREE
@@ -15,6 +15,7 @@
paths. Leaving dirty working files in the working tree is thus a
natural extension of what we have already been doing.
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/unpack-trees.c b/unpack-trees.c
8: 27d6304d33f ! 8: 8ed2f4e49ed unpack-trees: pull sparse-checkout pattern reading into a new function
@@ -6,6 +6,7 @@
path_patterns from $GIT_DIR/info/sparse-checkout so that we can re-use
it elsewhere.
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/unpack-trees.c b/unpack-trees.c
9: a46439c8536 ! 9: 3a1f084641e unpack-trees: add a new update_sparsity() function
@@ -71,6 +71,7 @@
are marked SKIP_WORKTREE and thus doesn't delete them, and `commit -a`
is similar), it seems logical to me.
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/unpack-trees.c b/unpack-trees.c
@@ -130,7 +131,6 @@
+
+ if (!ce_skip_worktree(ce))
+ empty_worktree = 0;
-+
+ }
+
+ /*
@@ -166,8 +166,8 @@
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@
- NB_UNPACK_TREES_ERROR_TYPES
- };
+ int unpack_trees(unsigned n, struct tree_desc *t,
+ struct unpack_trees_options *options);
+enum update_sparsity_result {
+ UPDATE_SPARSITY_SUCCESS = 0,
@@ -176,14 +176,7 @@
+ UPDATE_SPARSITY_WORKTREE_UPDATE_FAILURES = -2
+};
+
- /*
- * Sets the list of user-friendly error messages to be used by the
- * command "cmd" (either merge or checkout), and show_all_errors to 1.
-@@
- int unpack_trees(unsigned n, struct tree_desc *t,
- struct unpack_trees_options *options);
-
-+int update_sparsity(struct unpack_trees_options *options);
++enum update_sparsity_result update_sparsity(struct unpack_trees_options *options);
+
int verify_uptodate(const struct cache_entry *ce,
struct unpack_trees_options *o);
10: 27ed4a3f7a2 ! 10: 9040ac97175 sparse-checkout: use new update_sparsity() function
@@ -6,6 +6,12 @@
codepaths for setting the SKIP_WORKTREE bits and instead use the new
update_sparsity() function.
+ Note that when an issue is hit, the error message splits 'error' and
+ 'Cannot update sparse checkout' on separate lines. For now, we use two
+ greps to find both pieces of the error message but subsequent commits
+ will clean up the messages reported to the user.
+
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
@@ -147,14 +153,21 @@
+ test_i18ngrep "error" err &&
+ test_i18ngrep "Cannot update sparse checkout" err &&
+
-+ git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* &&
++ git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* 2>err &&
+ test_i18ngrep "error" err &&
+ test_i18ngrep "Cannot update sparse checkout" err &&
++ test_path_is_file dirty/folder1/a &&
+
-+ git -C dirty sparse-checkout disable &&
-+ test_i18ngrep "error" err &&
-+ test_i18ngrep "Cannot update sparse checkout" err &&
++ git -C dirty sparse-checkout disable 2>err &&
++ test_must_be_empty err &&
+
git -C dirty reset --hard &&
git -C dirty sparse-checkout init &&
git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* &&
+- git -C dirty sparse-checkout disable
++ test_path_is_missing dirty/folder1/a &&
++ git -C dirty sparse-checkout disable &&
++ test_path_is_file dirty/folder1/a
+ '
+
+ test_expect_success 'cone mode: set with core.ignoreCase=true' '
11: 79b9cc1df55 ! 11: e9c7e8ec46f sparse-checkout: use improved unpack_trees porcelain messages
@@ -23,6 +23,7 @@
we even called unpack_trees(). The previous commit changed that,
though, so let's use the better error messages.
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
@@ -50,15 +51,10 @@
- test_i18ngrep "Cannot update sparse checkout" err &&
+ test_i18ngrep "error.*Cannot update sparse checkout" err &&
- git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* &&
+ git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* 2>err &&
- test_i18ngrep "error" err &&
- test_i18ngrep "Cannot update sparse checkout" err &&
+ test_i18ngrep "error.*Cannot update sparse checkout" err &&
+ test_path_is_file dirty/folder1/a &&
- git -C dirty sparse-checkout disable &&
-- test_i18ngrep "error" err &&
-- test_i18ngrep "Cannot update sparse checkout" err &&
-+ test_i18ngrep "error.*Cannot update sparse checkout" err &&
-
- git -C dirty reset --hard &&
- git -C dirty sparse-checkout init &&
+ git -C dirty sparse-checkout disable 2>err &&
12: 2ba7bbdaee3 ! 12: c3741107a9d unpack-trees: move ERROR_WOULD_LOSE_SUBMODULE earlier
@@ -5,6 +5,7 @@
A minor change, but we want to convert the sparse messages to warnings
and this allows us to group warnings and errors.
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/unpack-trees.c b/unpack-trees.c
13: 203ece0ec2f ! 13: 90c514e220b unpack-trees: rename ERROR_* fields meant for warnings to WARNING_*
@@ -6,6 +6,7 @@
rather than an error; rename the enum values to reflect this intent as
a simple step towards that goal.
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/unpack-trees.c b/unpack-trees.c
@@ -72,4 +73,4 @@
+ NB_UNPACK_TREES_ERROR_TYPES,
};
- enum update_sparsity_result {
+ /*
14: 57679c8e292 ! 14: 5676e890f65 unpack-trees: split display_error_msgs() into two
@@ -10,6 +10,7 @@
A subsequent commit will further modify unpack_trees() and how it calls
the new display_warning_msgs().
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
@@ -22,16 +23,12 @@
- test_i18ngrep "error.*Cannot update sparse checkout" err &&
+ test_i18ngrep "warning.*Cannot update sparse checkout" err &&
- git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* &&
+ git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* 2>err &&
- test_i18ngrep "error.*Cannot update sparse checkout" err &&
+ test_i18ngrep "warning.*Cannot update sparse checkout" err &&
+ test_path_is_file dirty/folder1/a &&
- git -C dirty sparse-checkout disable &&
-- test_i18ngrep "error.*Cannot update sparse checkout" err &&
-+ test_i18ngrep "warning.*Cannot update sparse checkout" err &&
-
- git -C dirty reset --hard &&
- git -C dirty sparse-checkout init &&
+ git -C dirty sparse-checkout disable 2>err &&
diff --git a/unpack-trees.c b/unpack-trees.c
--- a/unpack-trees.c
@@ -100,7 +97,7 @@
+{
+ int e;
+ unsigned warning_displayed = 0;
-+ for (e = NB_UNPACK_TREES_ERROR_TYPES+1;
++ for (e = NB_UNPACK_TREES_ERROR_TYPES + 1;
+ e < NB_UNPACK_TREES_WARNING_TYPES; e++) {
+ struct string_list *rejects = &o->unpack_rejects[e];
+
@@ -160,7 +157,7 @@
+ NB_UNPACK_TREES_WARNING_TYPES,
};
- enum update_sparsity_result {
+ /*
@@
struct dir_struct *dir;
struct pathspec *pathspec;
15: 02a8e01f35b ! 15: 74e13ccee40 unpack-trees: make sparse path messages sound like warnings
@@ -7,6 +7,7 @@
operation abort. Reword the messages to make sense in their new
context.
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
@@ -28,16 +29,12 @@
- test_i18ngrep "warning.*Cannot update sparse checkout" err &&
+ test_i18ngrep "warning.*The following paths are not up to date" err &&
- git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* &&
+ git -C dirty sparse-checkout set /folder2/* /deep/deeper1/* 2>err &&
- test_i18ngrep "warning.*Cannot update sparse checkout" err &&
+ test_i18ngrep "warning.*The following paths are not up to date" err &&
+ test_path_is_file dirty/folder1/a &&
- git -C dirty sparse-checkout disable &&
-- test_i18ngrep "warning.*Cannot update sparse checkout" err &&
-+ test_i18ngrep "warning.*The following paths are not up to date" err &&
-
- git -C dirty reset --hard &&
- git -C dirty sparse-checkout init &&
+ git -C dirty sparse-checkout disable 2>err &&
diff --git a/unpack-trees.c b/unpack-trees.c
--- a/unpack-trees.c
16: f664a6dcbb3 ! 16: 5d9e8a9d27e unpack-trees: provide warnings on sparse updates for unmerged paths too
@@ -7,13 +7,14 @@
those files have dirty changes. Add a similar warning for unmerged
paths as well.
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh
@@
- git -C dirty sparse-checkout disable
+ test_path_is_file dirty/folder1/a
'
+test_expect_success 'sparse-checkout (init|set|disable) warns with unmerged status' '
@@ -28,10 +29,11 @@
+ git -C unmerged sparse-checkout init 2>err &&
+ test_i18ngrep "warning.*The following paths are unmerged" err &&
+
-+ git -C unmerged sparse-checkout set /folder2/* /deep/deeper1/* &&
++ git -C unmerged sparse-checkout set /folder2/* /deep/deeper1/* 2>err &&
+ test_i18ngrep "warning.*The following paths are unmerged" err &&
++ test_path_is_file dirty/folder1/a &&
+
-+ git -C unmerged sparse-checkout disable &&
++ git -C unmerged sparse-checkout disable 2>err &&
+ test_i18ngrep "warning.*The following paths are unmerged" err &&
+
+ git -C unmerged reset --hard &&
17: fa997a1c72f ! 17: 844306c3e86 unpack-trees: failure to set SKIP_WORKTREE bits always just a warning
@@ -37,6 +37,7 @@
Downgrade any errors from inability to toggle the SKIP_WORKTREE bit to a
warning and allow the operations to continue.
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh
18: 5730f7d250a ! 18: 93dbdd78acf sparse-checkout: provide a new reapply subcommand
@@ -7,6 +7,7 @@
due to dirty changes, users may want a command to simply 'reapply' the
sparsity rules. Provide one.
+ Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
@@ -64,3 +65,55 @@
if (!strcmp(argv[0], "disable"))
return sparse_checkout_disable(argc, argv);
}
+
+ diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
+ --- a/t/t1091-sparse-checkout-builtin.sh
+ +++ b/t/t1091-sparse-checkout-builtin.sh
+@@
+ git -C unmerged sparse-checkout disable
+ '
+
++test_expect_success 'sparse-checkout reapply' '
++ git clone repo tweak &&
++
++ echo dirty >tweak/deep/deeper2/a &&
++
++ cat >input <<-EOF &&
++ 0 0000000000000000000000000000000000000000 folder1/a
++ 100644 $(git -C tweak rev-parse HEAD:folder1/a) 1 folder1/a
++ EOF
++ git -C tweak update-index --index-info <input &&
++
++ git -C tweak sparse-checkout init --cone 2>err &&
++ test_i18ngrep "warning.*The following paths are not up to date" err &&
++ test_i18ngrep "warning.*The following paths are unmerged" err &&
++
++ git -C tweak sparse-checkout set folder2 deep/deeper1 2>err &&
++ test_i18ngrep "warning.*The following paths are not up to date" err &&
++ test_i18ngrep "warning.*The following paths are unmerged" err &&
++
++ git -C tweak sparse-checkout reapply 2>err &&
++ test_i18ngrep "warning.*The following paths are not up to date" err &&
++ test_path_is_file tweak/deep/deeper2/a &&
++ test_i18ngrep "warning.*The following paths are unmerged" err &&
++ test_path_is_file tweak/folder1/a &&
++
++ git -C tweak checkout HEAD deep/deeper2/a &&
++ git -C tweak sparse-checkout reapply 2>err &&
++ test_i18ngrep ! "warning.*The following paths are not up to date" err &&
++ test_path_is_missing tweak/deep/deeper2/a &&
++ test_i18ngrep "warning.*The following paths are unmerged" err &&
++ test_path_is_file tweak/folder1/a &&
++
++ git -C tweak add folder1/a &&
++ git -C tweak sparse-checkout reapply 2>err &&
++ test_must_be_empty err &&
++ test_path_is_missing tweak/deep/deeper2/a &&
++ test_path_is_missing tweak/folder1/a &&
++
++ git -C tweak sparse-checkout disable
++'
++
+ test_expect_success 'cone mode: set with core.ignoreCase=true' '
+ rm repo/.git/info/sparse-checkout &&
+ git -C repo sparse-checkout init --cone &&-- gitgitgadget