Merge pull request microsoft#465 from vdye/bugfix/sparse-checkout-fixes

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

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);
@@ -612,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))

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, "/*")) {

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) &&
@@ -770,4 +779,61 @@ 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_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/`).
+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