Skip to contentSkip to conversation

sparse-checkout: allow one-character directories in cone mode

PATCH In review Derrick Stolee via GitGitGadget 2 participants 2 messages Feb 20, 2020

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 sparse-checkout: allow one-character directories in cone mode

From: Derrick Stolee <[email protected]>

In 9e6d3e64 (sparse-checkout: detect short patterns, 2020-01-24), a condition on the minimum length of a cone-mode pattern was introduced. However, this condition was off-by-one.

If we have a directory with a single character, say "b", then the command

git sparse-checkout set b

will correctly add the pattern "/b/" to the sparse-checkout file. When this is interpeted in dir.c, the pattern is "/b" with the PATTERN_FLAG_MUSTBEDIR flag. This string has length two, which satisfies our inclusive inequality (<= 2).

The reason for this inequality is that we will start to read the pattern string character-by-character using three char pointers: prev, cur, next. In particular, next is set to the current pattern plus two. The mistake was that next will still be a valid pointer when the pattern length is two, since the string is null-terminated.

Make this inequality strict so these patterns work.

---
sparse-checkout: allow one-character directories in cone mode

This is based on ds/sparse-add.

I discovered this while taking v2.25.1 and ds/sparse-add into our fork of Git and testing it with Scalar.

Off-by-one errors are tricky, sometimes.

Thanks, -Stolee

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-558%2Fderrickstolee%2Fsparse-short-pattern-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-558/derrickstolee/sparse-short-pattern-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/558

 dir.c                              |  2 +-
 t/t1091-sparse-checkout-builtin.sh | 12 +++++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)
dir.c +1 -1
index 7ac0920b713..a87900d43a2 100644
@@ -682,7 +682,7 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern
 		return;
 	}
 
-	if (given->patternlen <= 2 ||
+	if (given->patternlen < 2 ||
 	    *given->pattern == '*' ||
 	    strstr(given->pattern, "**")) {
 		/* Not a cone pattern. */
t/t1091-sparse-checkout-builtin.sh +11 -1
index c35cbdef454..b4c9c32a037 100755
@@ -417,10 +417,20 @@ test_expect_success 'pattern-checks: too short' '
 	cat >repo/.git/info/sparse-checkout <<-\EOF &&
 	/*
 	!/*/
-	/a
+	/
 	EOF
 	check_read_tree_errors repo "a" "disabling cone pattern matching"
 '
+test_expect_success 'pattern-checks: not too short' '
+	cat >repo/.git/info/sparse-checkout <<-\EOF &&
+	/*
+	!/*/
+	/b/
+	EOF
+	git -C repo read-tree -mu HEAD 2>err &&
+	test_must_be_empty err &&
+	check_files repo a
+'
 
 test_expect_success 'pattern-checks: trailing "*"' '
 	cat >repo/.git/info/sparse-checkout <<-\EOF &&

base-commit: ef07659926f64d70e8cb41025c3d7456eecb962e

Signature · 1 line
gitgitgadget
#1 lore raw
Quoted 28 lines · Derrick Stolee via GitGitGadget

From: Derrick Stolee <[email protected]>

In 9e6d3e64 (sparse-checkout: detect short patterns, 2020-01-24), a condition on the minimum length of a cone-mode pattern was introduced. However, this condition was off-by-one.

If we have a directory with a single character, say "b", then the command

git sparse-checkout set b

will correctly add the pattern "/b/" to the sparse-checkout file. When this is interpeted in dir.c, the pattern is "/b" with the PATTERN_FLAG_MUSTBEDIR flag. This string has length two, which satisfies our inclusive inequality (<= 2).

The reason for this inequality is that we will start to read the pattern string character-by-character using three char pointers: prev, cur, next. In particular, next is set to the current pattern plus two. The mistake was that next will still be a valid pointer when the pattern length is two, since the string is null-terminated.

Make this inequality strict so these patterns work.

---
sparse-checkout: allow one-character directories in cone mode

Quoted 6 lines

This is based on ds/sparse-add.

I discovered this while taking v2.25.1 and ds/sparse-add into our fork of Git and testing it with Scalar.

Off-by-one errors are tricky, sometimes.

Indeed, and thanks for a quick fix.

Quoted 51 lines

Thanks, -Stolee

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-558%2Fderrickstolee%2Fsparse-short-pattern-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-558/derrickstolee/sparse-short-pattern-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/558

 dir.c                              |  2 +-
 t/t1091-sparse-checkout-builtin.sh | 12 +++++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)
dir.c +1 -1
index 7ac0920b713..a87900d43a2 100644
@@ -682,7 +682,7 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern
 		return;
 	}
 
-	if (given->patternlen <= 2 ||
+	if (given->patternlen < 2 ||
 	    *given->pattern == '*' ||
 	    strstr(given->pattern, "**")) {
 		/* Not a cone pattern. */
t/t1091-sparse-checkout-builtin.sh +11 -1
index c35cbdef454..b4c9c32a037 100755
@@ -417,10 +417,20 @@ test_expect_success 'pattern-checks: too short' '
 	cat >repo/.git/info/sparse-checkout <<-\EOF &&
 	/*
 	!/*/
-	/a
+	/
 	EOF
 	check_read_tree_errors repo "a" "disabling cone pattern matching"
 '
+test_expect_success 'pattern-checks: not too short' '
+	cat >repo/.git/info/sparse-checkout <<-\EOF &&
+	/*
+	!/*/
+	/b/
+	EOF
+	git -C repo read-tree -mu HEAD 2>err &&
+	test_must_be_empty err &&
+	check_files repo a
+'
 
 test_expect_success 'pattern-checks: trailing "*"' '
 	cat >repo/.git/info/sparse-checkout <<-\EOF &&

base-commit: ef07659926f64d70e8cb41025c3d7456eecb962e