Skip to content

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

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.

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

Conversation

derrickstolee
Copy link

@derrickstolee derrickstolee commented Feb 20, 2020

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

In 9e6d3e6 (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.

Signed-off-by: Derrick Stolee <[email protected]>
@derrickstolee
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 20, 2020

Submitted as [email protected]

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 20, 2020

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Derrick Stolee via GitGitGadget" <[email protected]> writes:

> 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.
>
> Signed-off-by: Derrick Stolee <[email protected]>
> ---
>     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.

Indeed, and thanks for a quick fix.

>     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(-)
>
> diff --git a/dir.c b/dir.c
> index 7ac0920b713..a87900d43a2 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -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. */
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> index c35cbdef454..b4c9c32a037 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -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

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 21, 2020

This branch is now known as ds/sparse-add.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 21, 2020

This patch series was integrated into pu via git@738f147.

@gitgitgadget gitgitgadget bot added the pu label Feb 21, 2020
@gitgitgadget
Copy link

gitgitgadget bot commented Feb 22, 2020

This patch series was integrated into pu via git@cd9aef4.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 25, 2020

This patch series was integrated into pu via git@3fc2e37.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 25, 2020

This patch series was integrated into next via git@de68d14.

@gitgitgadget gitgitgadget bot added the next label Feb 25, 2020
@gitgitgadget
Copy link

gitgitgadget bot commented Feb 26, 2020

This patch series was integrated into pu via git@c463ce5.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 28, 2020

This patch series was integrated into pu via git@726665a.

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 3, 2020

This patch series was integrated into pu via git@e8104ff.

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 5, 2020

This patch series was integrated into pu via git@f4d7dfc.

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 5, 2020

This patch series was integrated into next via git@f4d7dfc.

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 5, 2020

This patch series was integrated into master via git@f4d7dfc.

@gitgitgadget gitgitgadget bot added the master label Mar 5, 2020
@gitgitgadget gitgitgadget bot closed this Mar 5, 2020
@gitgitgadget
Copy link

gitgitgadget bot commented Mar 5, 2020

Closed via f4d7dfc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant