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/ 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 <dstolee@xxxxxxxxxxxxx> - 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 <dstolee@xxxxxxxxxxxxx> @@ -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 <me@xxxxxxxxxxxx> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx> 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 <dstolee@xxxxxxxxxxxxx> 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