Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sparse-checkout: avoid crash when switching between cone and non-cone #465

Merged
merged 3 commits into from
Nov 10, 2021
+98 −7
Merged

sparse-checkout: avoid crash when switching between cone and non-cone #465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4aaf1a3
sparse-checkout: disable cone mode on init with invalid patterns
vdye Nov 8, 2021
ffc7893
sparse-checkout: fail cone mode add with existing invalid patterns
vdye Nov 9, 2021
95fe29c
sparse-checkout: require starting '/' in cone mode patterns
vdye Nov 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions builtin/sparse-checkout.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,14 +428,26 @@ static int sparse_checkout_init(int argc, const char **argv)
} else
mode = MODE_ALL_PATTERNS;

if (set_config(mode))
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"));
mode = MODE_ALL_PATTERNS;
core_sparse_checkout_cone = 0;
}

if (set_config(mode))
return 1;

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"));
Expand Down Expand Up @@ -601,13 +613,21 @@ static void add_patterns_cone_mode(int argc, const char **argv,
add_patterns_from_input(pl, argc, argv);

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))
die(_("unable to load existing sparse-checkout patterns"));
free(sparse_filename);

/*
* If use_cone_patterns has been disabled, at least one of the existing
* patterns is invalid for cone mode. In that case, the hashmap does not
* correctly reflect patterns, so we must exit early.
*/
if (existing.use_cone_patterns == 0)
die(_("unable to use existing sparse-checkout patterns in cone mode"));

hashmap_for_each_entry(&existing.recursive_hashmap, &iter, pe, ent) {
if (!hashmap_contains_parent(&pl->recursive_hashmap,
pe->pattern, &buffer) ||
Expand Down
5 changes: 5 additions & 0 deletions dir.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, "/*")) {
Expand Down
72 changes: 69 additions & 3 deletions t/t1091-sparse-checkout-builtin.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,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
'
Expand Down Expand Up @@ -573,7 +573,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"
'
Expand All @@ -584,12 +584,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"
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) &&
Expand Down Expand Up @@ -712,4 +721,61 @@ test_expect_success 'cone mode clears ignored subdirectories' '
test_cmp expect out
'

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_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
'

# 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/`).
Comment on lines +751 to +754
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting idea, but I agree that the complexity might not be worth it.

I think "deep/" also means "include anything that's in a deep/ directory, including subdirectories such as /bin/deep/", so we can't really convert in this way without changing the definition. Even scanning the index for these deep/ directories is only enough for HEAD and not enough as HEAD moves throughout history.

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