Skip to content

[windows] Use both directory separators in test patterns. #25595

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

Merged
merged 1 commit into from
Jun 20, 2019

Conversation

drodriguez
Copy link
Contributor

Admit defeat and realize that one simply cannot win against Windows. Use
the pattern with both separators in the tests and remove the hack in
PathSanitizingFileCheck because it created false positives, some of them
could not have been easily fixed.

Sadly lit substitutions or other tricks in lit.cfg will not work,
because CHECK lines are not processed by lit, but by FileCheck.

Admit defeat and realize that one simply cannot win against Windows. Use
the pattern with both separators in the tests and remove the hack in
PathSanitizingFileCheck because it created false positives, some of them
could not have been easily fixed.

Sadly lit substitutions or other tricks in lit.cfg will not work,
because CHECK lines are not processed by lit, but by FileCheck.
@drodriguez drodriguez requested a review from compnerd June 19, 2019 01:55
@drodriguez
Copy link
Contributor Author

@swift-ci please smoke test

@gottesmm
Copy link
Contributor

@drodriguez question. Maybe add support for this upstream to FileCheck?

@drodriguez
Copy link
Contributor Author

@gottesmm: it doesn’t really matter. There's some moments that you want to check that \ is matched (for example https://github.com/apple/swift/blob/379d88cb122447bdd507b835e2f7b651ec09c6a0/test/Parse/raw_string.swift#L111). In those cases, one should not do the replacement, but it completely looks like a Windows path. It is sad, and I cringe every time I see the {{\\|/}}, but I think the only way of removing them is ensuring every piece emits Unix paths, even on Windows.

@swift-ci please smoke test Linux platform

@compnerd compnerd merged commit 40a49ce into swiftlang:master Jun 20, 2019
@gottesmm
Copy link
Contributor

This really sucks. We need to find a better way to do this.

@drodriguez drodriguez deleted the windows-i-give-up branch July 16, 2019 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants