[PATCH v2 00/12] Harden the sparse-checkout builtin
STALE2430d
From: Derrick Stolee via GitGitGadget <hidden>
Date: 2020-01-24 21:19:49
Revision v2 of 4 in this series.
Revisions (4)
- v1 [diff vs current]
- v2 current
- v3 [diff vs current]
- v4 [diff vs current]
This series is based on ds/sparse-list-in-cone-mode.
This series attempts to clean up some rough edges in the sparse-checkout
feature, especially around the cone mode.
Unfortunately, after the v2.25.0 release, we noticed an issue with the "git
clone --sparse" option when using a URL instead of a local path. This is
fixed and properly tested here.
Also, let's improve Git's response to these more complicated scenarios:
1. Running "git sparse-checkout init" in a worktree would complain because
the "info" dir doesn't exist.
2. Tracked paths that include "*" and "\" in their filenames.
3. If a user edits the sparse-checkout file to have non-cone pattern, such
as "**" anywhere or "*" in the wrong place, then we should respond
appropriately. That is: warn that the patterns are not cone-mode, then
revert to the old logic.
Updates in V2:
* Added C-style quoting to the output of "git sparse-checkout list" in cone
mode.
* Improved documentation.
* Responded to most style feedback. Hopefully I didn't miss anything.
* I was lingering on this a little to see if I could also fix the issue
raised in [1], but I have not figured that one out, yet.
[1]
https://lore.kernel.org/git/062301d5d0bc$c3e17760$4ba46620$@Frontier.com/ (local)
Thanks, -Stolee
Derrick Stolee (11):
t1091: use check_files to reduce boilerplate
t1091: improve here-docs
sparse-checkout: create leading directories
clone: fix --sparse option with URLs
sparse-checkout: cone mode does not recognize "**"
sparse-checkout: detect short patterns
sparse-checkout: warn on incorrect '*' in patterns
sparse-checkout: properly match escaped characters
sparse-checkout: write escaped patterns in cone mode
sparse-checkout: use C-style quotes in 'list' subcommand
sparse-checkout: improve docs around 'set' in cone mode
Jeff King (1):
sparse-checkout: fix documentation typo for core.sparseCheckoutCone
Documentation/git-sparse-checkout.txt | 19 +-
builtin/clone.c | 2 +-
builtin/sparse-checkout.c | 59 ++++-
dir.c | 68 +++++-
dir.h | 1 +
t/t1091-sparse-checkout-builtin.sh | 323 +++++++++++++++-----------
6 files changed, 317 insertions(+), 155 deletions(-)
base-commit: 4fd683b6a35eabd23dd5183da7f654a1e1f00325
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-513%2Fderrickstolee%2Fsparse-harden-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-513/derrickstolee/sparse-harden-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/513
Range-diff vs v1:
-: ---------- > 1: 1cc825412f t1091: use check_files to reduce boilerplate
1: 9f7791ae5e ! 2: b7a6ad145a t1091: use check_files to reduce boilerplate
@@ -1,14 +1,11 @@
Author: Derrick Stolee [off-list ref]
- t1091: use check_files to reduce boilerplate
+ t1091: improve here-docs
- When testing the sparse-checkout feature, we need to compare the
- contents of the working-directory against some expected output.
- Using here-docs was useful in the beginning, but became repetetive
- as the test script grew.
-
- Create a check_files helper to make the tests simpler and easier
- to extend. It also reduces instances of bad here-doc whitespace.
+ t1091-sparse-checkout-builtin.sh uses here-docs to populate the
+ expected contents of the sparse-checkout file. These do not use
+ shell interpolation, so use "-\EOF" instead of "-EOF". Also use
+ proper tabbing.
Signed-off-by: Derrick Stolee [off-list ref]
@@ -16,20 +13,6 @@
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh
@@
- (cd "$1" && printf '%s\n' *)
- }
-
-+check_files() {
-+ DIR=$1
-+ printf "%s\n" $2 >expect &&
-+ list_files $DIR >actual &&
-+ test_cmp expect actual
-+}
-+
- test_expect_success 'setup' '
- git init repo &&
- (
-@@
test_expect_success 'git sparse-checkout list (populated)' '
test_when_finished rm -f repo/.git/info/sparse-checkout &&
@@ -59,11 +42,7 @@
EOF
test_cmp expect repo/.git/info/sparse-checkout &&
test_cmp_config -C repo true core.sparsecheckout &&
-- list_files repo >dir &&
-- echo a >expect &&
-- test_cmp expect dir
-+ check_files repo a
- '
+@@
test_expect_success 'git sparse-checkout list after init' '
git -C repo sparse-checkout list >actual &&
@@ -90,16 +69,8 @@
+ *folder*
EOF
test_cmp expect repo/.git/info/sparse-checkout &&
-- list_files repo >dir &&
-- cat >expect <<-EOF &&
-- a
-- folder1
-- folder2
-- EOF
-- test_cmp expect dir
-+ check_files repo "a folder1 folder2"
- '
-
+ check_files repo a folder1 folder2
+@@
test_expect_success 'clone --sparse' '
git clone --sparse repo clone &&
git -C clone sparse-checkout list >actual &&
@@ -111,13 +82,7 @@
+ !/*/
EOF
test_cmp expect actual &&
-- list_files clone >dir &&
-- echo a >expect &&
-- test_cmp expect dir
-+ check_files clone a
- '
-
- test_expect_success 'set enables config' '
+ check_files clone a
@@
test_expect_success 'set sparse-checkout using builtin' '
@@ -133,15 +98,7 @@
EOF
git -C repo sparse-checkout list >actual &&
test_cmp expect actual &&
- test_cmp expect repo/.git/info/sparse-checkout &&
-- list_files repo >dir &&
-- cat >expect <<-EOF &&
-- a
-- folder1
-- folder2
-- EOF
-- test_cmp expect dir
-+ check_files repo "a folder1 folder2"
+@@
'
test_expect_success 'set sparse-checkout using --stdin' '
@@ -158,72 +115,10 @@
EOF
git -C repo sparse-checkout set --stdin <expect &&
git -C repo sparse-checkout list >actual &&
- test_cmp expect actual &&
- test_cmp expect repo/.git/info/sparse-checkout &&
-- list_files repo >dir &&
-- cat >expect <<-EOF &&
-- a
-- folder1
-- folder2
-- EOF
-- test_cmp expect dir
-+ check_files repo "a folder1 folder2"
- '
-
- test_expect_success 'cone mode: match patterns' '
-@@
- git -C repo read-tree -mu HEAD 2>err &&
- test_i18ngrep ! "disabling cone patterns" err &&
- git -C repo reset --hard &&
-- list_files repo >dir &&
-- cat >expect <<-EOF &&
-- a
-- folder1
-- folder2
-- EOF
-- test_cmp expect dir
-+ check_files repo "a folder1 folder2"
- '
-
- test_expect_success 'cone mode: warn on bad pattern' '
@@
- test_path_is_file repo/.git/info/sparse-checkout &&
- git -C repo config --list >config &&
- test_must_fail git config core.sparseCheckout &&
-- list_files repo >dir &&
-- cat >expect <<-EOF &&
-- a
-- deep
-- folder1
-- folder2
-- EOF
-- test_cmp expect dir
-+ check_files repo "a deep folder1 folder2"
- '
-
- test_expect_success 'cone mode: init and set' '
-@@
- test_cmp expect dir &&
- git -C repo sparse-checkout set deep/deeper1/deepest/ 2>err &&
- test_must_be_empty err &&
-- list_files repo >dir &&
-- cat >expect <<-EOF &&
-- a
-- deep
-- EOF
-- test_cmp expect dir &&
-- list_files repo/deep >dir &&
-- cat >expect <<-EOF &&
-- a
-- deeper1
-- EOF
-- test_cmp expect dir &&
-- list_files repo/deep/deeper1 >dir &&
-- cat >expect <<-EOF &&
-- a
-- deepest
-- EOF
-- test_cmp expect dir &&
+ check_files repo a deep &&
+ check_files repo/deep a deeper1 &&
+ check_files repo/deep/deeper1 a deepest &&
- cat >expect <<-EOF &&
- /*
- !/*/
@@ -232,9 +127,6 @@
- /deep/deeper1/
- !/deep/deeper1/*/
- /deep/deeper1/deepest/
-+ check_files repo "a deep" &&
-+ check_files repo/deep "a deeper1" &&
-+ check_files repo/deep/deeper1 "a deepest" &&
+ cat >expect <<-\EOF &&
+ /*
+ !/*/
@@ -253,14 +145,7 @@
+ folder2
EOF
test_must_be_empty err &&
-- cat >expect <<-EOF &&
-- a
-- folder1
-- folder2
-- EOF
-- list_files repo >dir &&
-- test_cmp expect dir
-+ check_files repo "a folder1 folder2"
+ check_files repo a folder1 folder2
'
test_expect_success 'cone mode: list' '
@@ -288,21 +173,6 @@
EOF
test_cmp repo/.git/info/sparse-checkout expect
'
-@@
- test_must_fail git -C repo sparse-checkout set deep/deeper1 2>err &&
- test_i18ngrep "cannot set sparse-checkout patterns" err &&
- test_cmp repo/.git/info/sparse-checkout expect &&
-- list_files repo/deep >dir &&
-- cat >expect <<-EOF &&
-- a
-- deeper1
-- deeper2
-- EOF
-- test_cmp dir expect
-+ check_files repo/deep "a deeper1 deeper2"
- '
-
- test_expect_success 'revert to old sparse-checkout on empty update' '
@@
test_expect_success 'cone mode: set with core.ignoreCase=true' '
git -C repo sparse-checkout init --cone &&
@@ -317,37 +187,4 @@
+ /folder1/
EOF
test_cmp expect repo/.git/info/sparse-checkout &&
-- list_files repo >dir &&
-- cat >expect <<-EOF &&
-- a
-- folder1
-- EOF
-- test_cmp expect dir
-+ check_files repo "a folder1"
- '
-
- test_expect_success 'interaction with submodules' '
-@@
- git sparse-checkout init --cone &&
- git sparse-checkout set folder1
- ) &&
-- list_files super >dir &&
-- cat >expect <<-\EOF &&
-- a
-- folder1
-- modules
-- EOF
-- test_cmp expect dir &&
-- list_files super/modules/child >dir &&
-- cat >expect <<-\EOF &&
-- a
-- deep
-- folder1
-- folder2
-- EOF
-- test_cmp expect dir
-+ check_files super "a folder1 modules" &&
-+ check_files super/modules/child "a deep folder1 folder2"
- '
-
- test_done
+ check_files repo a folder1
2: 53a266f9aa ! 3: 5497ad8778 sparse-checkout: create leading directories
@@ -34,7 +34,7 @@
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh
@@
- check_files super/modules/child "a deep folder1 folder2"
+ check_files super/modules/child a deep folder1 folder2
'
+test_expect_success 'different sparse-checkouts with worktrees' '
@@ -43,8 +43,8 @@
+ git -C worktree sparse-checkout init --cone &&
+ git -C repo sparse-checkout set folder1 &&
+ git -C worktree sparse-checkout set deep/deeper1 &&
-+ check_files repo "a folder1" &&
-+ check_files worktree "a deep"
++ check_files repo a folder1 &&
++ check_files worktree a deep
+'
+
test_done
3: 3ef8e021a5 ! 4: 4991a51f6d clone: fix --sparse option with URLs
@@ -22,6 +22,7 @@
Update that target directory to evaluate this correctly. I have also
manually tested that https:// URLs are handled correctly as well.
+ Acked-by: Taylor Blau [off-list ref]
Signed-off-by: Derrick Stolee [off-list ref]
diff --git a/builtin/clone.c b/builtin/clone.c
-: ---------- > 5: ae78c3069b sparse-checkout: fix documentation typo for core.sparseCheckoutCone
4: dfa7e20444 ! 6: 2ad4d3e467 sparse-checkout: cone mode does not recognize "**"
@@ -30,7 +30,6 @@
+ if (strstr(given->pattern, "**")) {
+ /* Not a cone pattern. */
-+ pl->use_cone_patterns = 0;
+ warning(_("unrecognized pattern: '%s'"), given->pattern);
+ goto clear_hashmaps;
+ }
@@ -38,12 +37,17 @@
if (given->patternlen > 2 &&
!strcmp(given->pattern + given->patternlen - 2, "/*")) {
if (!(given->flags & PATTERN_FLAG_NEGATIVE)) {
+ /* Not a cone pattern. */
+- pl->use_cone_patterns = 0;
+ warning(_("unrecognized pattern: '%s'"), given->pattern);
+ goto clear_hashmaps;
+ }
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
@@
- check_files worktree "a deep"
+ check_files worktree a deep
'
+check_read_tree_errors () {
@@ -57,7 +61,7 @@
+ else
+ test_i18ngrep "$ERRORS" err
+ fi &&
-+ check_files $REPO "$FILES"
++ check_files $REPO $FILES
+}
+
+test_expect_success 'pattern-checks: /A/**' '
5: 9be49908fd ! 7: aace064510 sparse-checkout: detect short patterns
@@ -21,8 +21,8 @@
+ if (given->patternlen <= 2 ||
+ strstr(given->pattern, "**")) {
/* Not a cone pattern. */
- pl->use_cone_patterns = 0;
warning(_("unrecognized pattern: '%s'"), given->pattern);
+ goto clear_hashmaps;
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
--- a/t/t1091-sparse-checkout-builtin.sh
6: 77a514f50b ! 8: d2a510a3bb sparse-checkout: warn on incorrect '*' in patterns
@@ -34,8 +34,7 @@
+ *given->pattern == '*' ||
strstr(given->pattern, "**")) {
/* Not a cone pattern. */
- pl->use_cone_patterns = 0;
-@@
+ warning(_("unrecognized pattern: '%s'"), given->pattern);
goto clear_hashmaps;
}
@@ -57,7 +56,6 @@
+ goto increment;
+
+ /* Not a cone pattern. */
-+ pl->use_cone_patterns = 0;
+ warning(_("unrecognized pattern: '%s'"), given->pattern);
+ goto clear_hashmaps;
+
7: 09dbe1f902 ! 9: 65c53d7526 sparse-checkout: properly match escaped characters
@@ -9,7 +9,7 @@
Create dup_and_filter_pattern() method to convert a pattern by
removing escape characters and dropping an optional "/*" at the end.
This method is available in dir.h as we will use it in
- builtin/sparse-chekcout.c in a later change.
+ builtin/sparse-checkout.c in a later change.
Signed-off-by: Derrick Stolee [off-list ref]
8: 79b6e9a565 = 10: c27a17a2fc sparse-checkout: write escaped patterns in cone mode
-: ---------- > 11: 526d5becbc sparse-checkout: use C-style quotes in 'list' subcommand
-: ---------- > 12: 1b5858adee sparse-checkout: improve docs around 'set' in cone mode
--
gitgitgadget