Skip to content

t3070: skip ls-files tests with backslash patterns on Windows#2128

Open
spkrka wants to merge 1 commit into
gitgitgadget:masterfrom
spkrka:wildmatch-windows-fix
Open

t3070: skip ls-files tests with backslash patterns on Windows#2128
spkrka wants to merge 1 commit into
gitgitgadget:masterfrom
spkrka:wildmatch-windows-fix

Conversation

@spkrka
Copy link
Copy Markdown

@spkrka spkrka commented May 28, 2026

On Windows (MINGW), backslashes in pathspecs are silently converted to
forward slashes (directory separators), which changes the glob semantics.
This causes 36 test failures in t3070-wildmatch when the "via ls-files"
variants test patterns containing backslash escapes (e.g. \[ab],
[\-_], [A-\\]).

The wildmatch function itself handles these patterns correctly — only the
ls-files code path fails because pathspec parsing converts the
backslashes before they reach the glob matcher.

Skip these ls-files tests on platforms where BSLASHPSPEC is not set,
which is the existing prereq that captures exactly this semantic:
"backslashes in pathspec are not directory separators."

cc: @dscho
cc: Kristofer Karlsson krka@spotify.com

On Windows (MINGW), backslashes in pathspecs are silently converted to
forward slashes (directory separators), which changes the glob semantics.
This causes 36 test failures in t3070-wildmatch when the "via ls-files"
variants test patterns containing backslash escapes (e.g. '\[ab]',
'[\-_]', '[A-\\]').

The wildmatch function itself handles these patterns correctly — only the
ls-files code path fails because pathspec parsing converts the
backslashes before they reach the glob matcher.

Skip these ls-files tests on platforms where BSLASHPSPEC is not set,
which is the existing prereq that captures exactly this semantic:
"backslashes in pathspec are not directory separators."

Signed-off-by: Kristofer Karlsson <krka@spotify.com>
Copy link
Copy Markdown
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@dscho
Copy link
Copy Markdown
Member

dscho commented May 28, 2026

/allow

@spkrka
Copy link
Copy Markdown
Author

spkrka commented May 28, 2026

/submit

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget Bot commented May 28, 2026

User spkrka already allowed to use GitGitGadget.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget Bot commented May 28, 2026

Submitted as pull.2128.git.1779958849319.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2128/spkrka/wildmatch-windows-fix-v1

To fetch this version to local tag pr-2128/spkrka/wildmatch-windows-fix-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2128/spkrka/wildmatch-windows-fix-v1

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget Bot commented May 28, 2026

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

"Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Kristofer Karlsson <krka@spotify.com>
>
> On Windows (MINGW), backslashes in pathspecs are silently converted to
> forward slashes (directory separators), which changes the glob semantics.
> This causes 36 test failures in t3070-wildmatch when the "via ls-files"
> variants test patterns containing backslash escapes (e.g. '\[ab]',
> '[\-_]', '[A-\\]').
>
> The wildmatch function itself handles these patterns correctly — only the
> ls-files code path fails because pathspec parsing converts the
> backslashes before they reach the glob matcher.
>
> Skip these ls-files tests on platforms where BSLASHPSPEC is not set,
> which is the existing prereq that captures exactly this semantic:
> "backslashes in pathspec are not directory separators."
>
> Signed-off-by: Kristofer Karlsson <krka@spotify.com>
> ---

Thanks for noticing and addressing this.  I think we fairly recently
started seeing this in GitHub actions CI, which puzzles me since
neither t3070 or wildmatch.[ch] have changed for quite some time.
8a6d158a (doc: document backslash in gitignore patterns, 2025-10-29)
added a few lines to the test about matching with backslash to t3070.

Two questions.

 * Has this been broken on Windows since October, or has something
   external change on Windows recently?  I do not know.  Anybody
   knows?

 * Is this change a workaround that sweeps ugly breakage under the
   rug, or is backslash inherently unusable as an excape character
   when handling paths on Windows (which I am afraid would make
   wildmatch fairly useless there)?

Will queue.  Thanks.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget Bot commented May 29, 2026

This patch series was integrated into seen via git@07b73d6.

@gitgitgadget gitgitgadget Bot added the seen label May 29, 2026
@gitgitgadget
Copy link
Copy Markdown

gitgitgadget Bot commented May 29, 2026

Kristofer Karlsson wrote on the Git mailing list (how to reply to this email):

On Thu, 28 May 2026 at 22:26, Junio C Hamano <gitster@pobox.com> wrote:
> Two questions.
>
>  * Has this been broken on Windows since October, or has something
>    external change on Windows recently?  I do not know.  Anybody
>    knows?
>
>  * Is this change a workaround that sweeps ugly breakage under the
>    rug, or is backslash inherently unusable as an excape character
>    when handling paths on Windows (which I am afraid would make
>    wildmatch fairly useless there)?
>

I am fairly new to the git ecosystem as a developer (not as a user),
so I am not sure how long this has been broken. The backslash patterns
in the ls-files test path predate 8a6d158a - patterns like 'foo\*'
and '[\-_]' have been there since de8bada2bf (2018) - so it may
have been failing for a while before anyone noticed.

My thinking was that it would be good in general if the CI results
were green and did not include false positives for errors that we
know cannot work on this platform. The risk is that people stop
looking into CI failures in detail because they start to assume it
is the same old backslash problem.

That said, there is also a risk that the real underlying issue does
not get fixed. I am hoping it is sufficient that the BSLASHPSPEC
prereq and the case *\\* filter make it obvious to anyone reading
the test what we are skipping over and why.

> Will queue.  Thanks.

Thanks! It felt a bit heavyweight to add noise to the list for trivial CI test
changes but I suppose the process is the same even if it does not
affect the production code.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget Bot commented May 29, 2026

User Kristofer Karlsson <krka@spotify.com> has been added to the cc: list.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget Bot commented May 30, 2026

This branch is now known as kk/wildmatch-windows-ls-files-prereq.

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget Bot commented May 30, 2026

This patch series was integrated into seen via git@2945625.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants