{"thread":{"id":"52852","subject":"[PATCH] sparse-checkout: allow one-character directories in cone mode","startedAt":"2020-02-20T20:07:11Z","lastAt":"2020-02-20T22:46:10Z","messageCount":2,"participants":["Derrick Stolee via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"392191","messageId":"pull.558.git.1582229226637.gitgitgadget@gmail.com","threadId":"52852","inReplyTo":null,"subject":"[PATCH] sparse-checkout: allow one-character directories in cone mode","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-02-20T20:07:06Z","receivedAt":"2020-02-20T20:07:11Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <dstolee@microsoft.com>\n\nIn 9e6d3e64 (sparse-checkout: detect short patterns, 2020-01-24), a\ncondition on the minimum length of a cone-mode pattern was introduced.\nHowever, this condition was off-by-one.\n\nIf we have a directory with a single character, say \"b\", then the\ncommand\n\n\tgit sparse-checkout set b\n\nwill correctly add the pattern \"/b/\" to the sparse-checkout file. When\nthis is interpeted in dir.c, the pattern is \"/b\" with the\nPATTERN_FLAG_MUSTBEDIR flag. This string has length two, which satisfies\nour inclusive inequality (<= 2).\n\nThe reason for this inequality is that we will start to read the pattern\nstring character-by-character using three char pointers: prev, cur,\nnext. In particular, next is set to the current pattern plus two. The\nmistake was that next will still be a valid pointer when the pattern\nlength is two, since the string is null-terminated.\n\nMake this inequality strict so these patterns work.\n\nSigned-off-by: Derrick Stolee <dstolee@microsoft.com>\n---\n    sparse-checkout: allow one-character directories in cone mode\n    \n    This is based on ds/sparse-add.\n    \n    I discovered this while taking v2.25.1 and ds/sparse-add into our fork\n    of Git and testing it with Scalar.\n    \n    Off-by-one errors are tricky, sometimes.\n    \n    Thanks, -Stolee\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-558%2Fderrickstolee%2Fsparse-short-pattern-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-558/derrickstolee/sparse-short-pattern-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/558\n\n dir.c                              |  2 +-\n t/t1091-sparse-checkout-builtin.sh | 12 +++++++++++-\n 2 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 7ac0920b713..a87900d43a2 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -682,7 +682,7 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern\n \t\treturn;\n \t}\n \n-\tif (given->patternlen <= 2 ||\n+\tif (given->patternlen < 2 ||\n \t    *given->pattern == '*' ||\n \t    strstr(given->pattern, \"**\")) {\n \t\t/* Not a cone pattern. */\ndiff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\nindex c35cbdef454..b4c9c32a037 100755\n--- a/t/t1091-sparse-checkout-builtin.sh\n+++ b/t/t1091-sparse-checkout-builtin.sh\n@@ -417,10 +417,20 @@ test_expect_success 'pattern-checks: too short' '\n \tcat >repo/.git/info/sparse-checkout <<-\\EOF &&\n \t/*\n \t!/*/\n-\t/a\n+\t/\n \tEOF\n \tcheck_read_tree_errors repo \"a\" \"disabling cone pattern matching\"\n '\n+test_expect_success 'pattern-checks: not too short' '\n+\tcat >repo/.git/info/sparse-checkout <<-\\EOF &&\n+\t/*\n+\t!/*/\n+\t/b/\n+\tEOF\n+\tgit -C repo read-tree -mu HEAD 2>err &&\n+\ttest_must_be_empty err &&\n+\tcheck_files repo a\n+'\n \n test_expect_success 'pattern-checks: trailing \"*\"' '\n \tcat >repo/.git/info/sparse-checkout <<-\\EOF &&\n\nbase-commit: ef07659926f64d70e8cb41025c3d7456eecb962e\n-- \ngitgitgadget\n"},{"id":"392214","messageId":"xmqqk14g98t3.fsf@gitster-ct.c.googlers.com","threadId":"52852","inReplyTo":"pull.558.git.1582229226637.gitgitgadget@gmail.com","subject":"Re: [PATCH] sparse-checkout: allow one-character directories in cone mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-20T22:46:00Z","receivedAt":"2020-02-20T22:46:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Derrick Stolee <dstolee@microsoft.com>\n>\n> In 9e6d3e64 (sparse-checkout: detect short patterns, 2020-01-24), a\n> condition on the minimum length of a cone-mode pattern was introduced.\n> However, this condition was off-by-one.\n>\n> If we have a directory with a single character, say \"b\", then the\n> command\n>\n> \tgit sparse-checkout set b\n>\n> will correctly add the pattern \"/b/\" to the sparse-checkout file. When\n> this is interpeted in dir.c, the pattern is \"/b\" with the\n> PATTERN_FLAG_MUSTBEDIR flag. This string has length two, which satisfies\n> our inclusive inequality (<= 2).\n>\n> The reason for this inequality is that we will start to read the pattern\n> string character-by-character using three char pointers: prev, cur,\n> next. In particular, next is set to the current pattern plus two. The\n> mistake was that next will still be a valid pointer when the pattern\n> length is two, since the string is null-terminated.\n>\n> Make this inequality strict so these patterns work.\n>\n> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>\n> ---\n>     sparse-checkout: allow one-character directories in cone mode\n>     \n>     This is based on ds/sparse-add.\n>     \n>     I discovered this while taking v2.25.1 and ds/sparse-add into our fork\n>     of Git and testing it with Scalar.\n>     \n>     Off-by-one errors are tricky, sometimes.\n\nIndeed, and thanks for a quick fix.\n\n>     Thanks, -Stolee\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-558%2Fderrickstolee%2Fsparse-short-pattern-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-558/derrickstolee/sparse-short-pattern-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/558\n>\n>  dir.c                              |  2 +-\n>  t/t1091-sparse-checkout-builtin.sh | 12 +++++++++++-\n>  2 files changed, 12 insertions(+), 2 deletions(-)\n>\n> diff --git a/dir.c b/dir.c\n> index 7ac0920b713..a87900d43a2 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -682,7 +682,7 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern\n>  \t\treturn;\n>  \t}\n>  \n> -\tif (given->patternlen <= 2 ||\n> +\tif (given->patternlen < 2 ||\n>  \t    *given->pattern == '*' ||\n>  \t    strstr(given->pattern, \"**\")) {\n>  \t\t/* Not a cone pattern. */\n> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\n> index c35cbdef454..b4c9c32a037 100755\n> --- a/t/t1091-sparse-checkout-builtin.sh\n> +++ b/t/t1091-sparse-checkout-builtin.sh\n> @@ -417,10 +417,20 @@ test_expect_success 'pattern-checks: too short' '\n>  \tcat >repo/.git/info/sparse-checkout <<-\\EOF &&\n>  \t/*\n>  \t!/*/\n> -\t/a\n> +\t/\n>  \tEOF\n>  \tcheck_read_tree_errors repo \"a\" \"disabling cone pattern matching\"\n>  '\n> +test_expect_success 'pattern-checks: not too short' '\n> +\tcat >repo/.git/info/sparse-checkout <<-\\EOF &&\n> +\t/*\n> +\t!/*/\n> +\t/b/\n> +\tEOF\n> +\tgit -C repo read-tree -mu HEAD 2>err &&\n> +\ttest_must_be_empty err &&\n> +\tcheck_files repo a\n> +'\n>  \n>  test_expect_success 'pattern-checks: trailing \"*\"' '\n>  \tcat >repo/.git/info/sparse-checkout <<-\\EOF &&\n>\n> base-commit: ef07659926f64d70e8cb41025c3d7456eecb962e\n"}]}