From 3a1cebc4b1e331113ce8e841b02d0a4acb5498ba Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Mon, 8 Nov 2021 16:10:13 -0500 Subject: [PATCH 1/3] sparse-checkout: disable cone mode on init with invalid patterns When reusing an existing `.git/info/sparse-checkout` file in `git sparse-checkout init --cone`, warn user and fall back on non-cone mode sparse checkout if the sparse patterns are not cone mode-compatible. This prevents users from ending up with erroneous/corrupted patterns in their cone mode sparse-checkout (assuming no manual edits to the `.git/info/sparse-checkout` file). Additionally, by allowing the `git sparse-checkout init` to continue with cone mode disabled, the user is able to correct or reset their patterns (via `git sparse-checkout set`) and retry `git sparse-checkout init --cone` without requiring manual editing of `.git/info/sparse-checkout`. Signed-off-by: Victoria Dye --- builtin/sparse-checkout.c | 27 +++++++++++++++++++++++++++ t/t1091-sparse-checkout-builtin.sh | 13 +++++++++++++ 2 files changed, 40 insertions(+) diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index e390c2e4c6cf9d..7b513f764db033 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -464,10 +464,37 @@ static int sparse_checkout_init(int argc, const char **argv) return 1; memset(&pl, 0, sizeof(pl)); + pl.use_cone_patterns = core_sparse_checkout_cone; sparse_filename = get_sparse_checkout_filename(); res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL, 0); + /* + * If res >= 0, file already exists. If in cone mode init, verify that the + * patterns are cone mode-compatible (if applicable). Otherwise, fall back + * on non-cone mode sparse checkout. + */ + if (res >= 0 && core_sparse_checkout_cone && !pl.use_cone_patterns) { + warning(_("unable to initialize from existing patterns; disabling cone mode")); + core_sparse_checkout_cone = 0; + + if (set_config(MODE_ALL_PATTERNS)) + return 1; + + /* Set sparse-index/non-sparse-index mode if specified */ + if (init_opts.sparse_index >= 0) { + if (set_sparse_index_config(the_repository, init_opts.sparse_index) < 0) + die(_("failed to modify sparse-index config")); + + /* force an index rewrite */ + repo_read_index(the_repository); + the_repository->index->updated_workdir = 1; + + if (!init_opts.sparse_index) + ensure_full_index(the_repository->index); + } + } + /* If we already have a sparse-checkout file, use it. */ if (res >= 0) { free(sparse_filename); diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh index 79148abb5c0fbf..436582ee8493ae 100755 --- a/t/t1091-sparse-checkout-builtin.sh +++ b/t/t1091-sparse-checkout-builtin.sh @@ -770,4 +770,17 @@ test_expect_success 'malformed cone-mode patterns' ' grep "warning: disabling cone pattern matching" err ' +test_expect_success 'init with cone mode verifies existing cone patterns' ' + rm -f repo/.git/info/sparse-checkout && + + # Set non-cone mode pattern + git -C repo sparse-checkout init && + git -C repo sparse-checkout set deep/deeper*/ && + git -C repo sparse-checkout disable && + + git -C repo sparse-checkout init --cone 2>err && + test_i18ngrep "disabling cone mode" err && + test_must_fail git -C repo config core.sparsecheckoutcone +' + test_done From 43f1da79cea75b16ddc1d5c7da15eeee228fd7dc Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 9 Nov 2021 14:20:16 -0500 Subject: [PATCH 2/3] sparse-checkout: fail cone mode add with existing invalid patterns Add check for `git sparse-checkout add` when existing patterns are not cone mode-compatible, exiting when invalid patterns are found. When a non-cone mode pattern (e.g., one containing glob pattern matching) exists in the on-disk `.git/info/sparse-checkout` file, `git sparse-checkout add` would warn that some patterns were incompatible with cone mode, but otherwise succeed with exit code 0. However, the original (incompatible) patterns would have been removed in the process. For example: $ git sparse-checkout init $ git sparse-checkout add folder*/ $ git config core.sparsecheckoutcone true $ git sparse-checkout add different-folder/ would result in the patterns: /* !/*/ /different-folder/ To prevent the unintentional pattern removal, `git sparse-checkout add` exits with an error if incompatible patterns were found (and therefore not included in the hashmaps used to generate the output sparse pattern file). Signed-off-by: Victoria Dye --- builtin/sparse-checkout.c | 2 +- t/t1091-sparse-checkout-builtin.sh | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index 7b513f764db033..7611d152b55077 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -639,7 +639,7 @@ static void add_patterns_cone_mode(int argc, const char **argv, add_patterns_from_input(pl, argc, argv, use_stdin); memset(&existing, 0, sizeof(existing)); - existing.use_cone_patterns = core_sparse_checkout_cone; + existing.use_cone_patterns = 1; if (add_patterns_from_file_to_list(sparse_filename, "", 0, &existing, NULL, 0)) diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh index 436582ee8493ae..32cf8f045377b2 100755 --- a/t/t1091-sparse-checkout-builtin.sh +++ b/t/t1091-sparse-checkout-builtin.sh @@ -783,4 +783,18 @@ test_expect_success 'init with cone mode verifies existing cone patterns' ' test_must_fail git -C repo config core.sparsecheckoutcone ' +test_expect_success 'add with cone mode verifies existing cone patterns' ' + rm -f repo/.git/info/sparse-checkout && + + git -C repo sparse-checkout init --cone && + cat >repo/.git/info/sparse-checkout <<-\EOF && + /* + !/*/ + /deep/deeper*/ + EOF + + test_must_fail git -C repo sparse-checkout add folder1 2>err && + test_i18ngrep "unable to use existing sparse-checkout patterns in cone mode" err +' + test_done From fed98621f601f96ad30cc9de2974a8716214eff3 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 9 Nov 2021 13:29:37 -0500 Subject: [PATCH 3/3] sparse-checkout: require starting '/' in cone mode patterns Add check for missing initial '/' when verifying cone patterns in `add_pattern_to_hashsets(...)`, disabling cone patterns if not found. Update tests in `t1091-sparse-checkout-builtin.sh` to ensure the starting '/' is present unless the test verifies behavior when it is missing. The implicit assumption of a starting '/' in cone mode patterns exists throughout the code paths of `git sparse-checkout`, including in the construction of the sparse pattern hashmaps. However, without enforcement of that assumption, the presence of a directory pattern missing the starting '/' would cause any subsequent cone mode `git sparse-checkout add` to enter an infinite recursive loop in `insert_recursive_pattern(...)` (`sparse-checkout.c`). By adding the starting '/' requirement to `add_pattern_to_hashsets(...)` (`dir.c`), cone mode is disabled on the pattern list, forcing `git sparse-checkout add` to exit early. Signed-off-by: Victoria Dye --- dir.c | 5 ++++ t/t1091-sparse-checkout-builtin.sh | 45 ++++++++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/dir.c b/dir.c index 65b7024c87966d..4e8be6c828d536 100644 --- a/dir.c +++ b/dir.c @@ -715,6 +715,11 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern if (!pl->use_cone_patterns) return; + if (*given->pattern != '/') { + warning(_("unrecognized pattern: '%s'"), given->pattern); + goto clear_hashmaps; + } + if (given->flags & PATTERN_FLAG_NEGATIVE && given->flags & PATTERN_FLAG_MUSTBEDIR && !strcmp(given->pattern, "/*")) { diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh index 32cf8f045377b2..8e9424db470eba 100755 --- a/t/t1091-sparse-checkout-builtin.sh +++ b/t/t1091-sparse-checkout-builtin.sh @@ -213,7 +213,7 @@ test_expect_success 'cone mode: match patterns' ' test_expect_success 'cone mode: warn on bad pattern' ' test_when_finished mv sparse-checkout repo/.git/info/ && cp repo/.git/info/sparse-checkout . && - echo "!/deep/deeper/*" >>repo/.git/info/sparse-checkout && + echo "!/deep/deeper/*" >repo/.git/info/sparse-checkout && git -C repo read-tree -mu HEAD 2>err && test_i18ngrep "unrecognized negative pattern" err ' @@ -610,7 +610,7 @@ test_expect_success 'pattern-checks: starting "*"' ' cat >repo/.git/info/sparse-checkout <<-\EOF && /* !/*/ - *eep/ + /*eep/ EOF check_read_tree_errors repo "a deep" "disabling cone pattern matching" ' @@ -621,12 +621,21 @@ test_expect_success 'pattern-checks: contained glob characters' ' cat >repo/.git/info/sparse-checkout <<-EOF && /* !/*/ - something$c-else/ + /something$c-else/ EOF check_read_tree_errors repo "a" "disabling cone pattern matching" || return 1 done ' +test_expect_success 'pattern-checks: starting "/"' ' + cat >repo/.git/info/sparse-checkout <<-\EOF && + /* + !/*/ + deep/ + EOF + check_read_tree_errors repo "a deep" "disabling cone pattern matching" +' + test_expect_success BSLASHPSPEC 'pattern-checks: escaped characters' ' git clone repo escaped && TREEOID=$(git -C escaped rev-parse HEAD:folder1) && @@ -797,4 +806,34 @@ test_expect_success 'add with cone mode verifies existing cone patterns' ' test_i18ngrep "unable to use existing sparse-checkout patterns in cone mode" err ' +# NEEDSWORK: in the case of directory patterns like `deep/`, it might be worth trying +# to "correct" the patterns to match a cone mode style. However, that may be more difficult +# for nested directories (like `deep/deeper1/`) in which multiple individual patterns +# would be mapped from the original (`/deep/`, `!/deep/*/`, `/deep/deeper1/`). +test_expect_success 'add cone pattern disallowed with existing non-cone directory pattern' ' + rm -f repo/.git/info/sparse-checkout && + + git -C repo sparse-checkout init --cone && + + # Manually set the sparse checkout pattern to a directory pattern + # without preceding slash + cat >repo/.git/info/sparse-checkout <<-\EOF && + deep/ + EOF + + # `add` fails because `deep/` is not a valid cone pattern. + test_must_fail git -C repo sparse-checkout add folder1/ 2>err && + test_i18ngrep "unable to use existing sparse-checkout patterns in cone mode" err && + + # `set` succeeds with same patterns set properly for cone mode. + git -C repo sparse-checkout set deep/ folder1/ && + cat >expect <<-\EOF && + /* + !/*/ + /deep/ + /folder1/ + EOF + test_cmp expect repo/.git/info/sparse-checkout +' + test_done