-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[windows] Fix paths in test to work for Windows. #26037
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
[windows] Fix paths in test to work for Windows. #26037
Conversation
@swift-ci please smoke test |
@swift-ci please test Windows platform |
The Windows failure here has already been addressed. |
// OSX: -L {{[^ ]*}}/Inputs/clang-importer-sdk/usr/lib/swift | ||
// OSX: -rpath {{[^ ]*}}/Inputs/clang-importer-sdk/usr/lib/swift | ||
// OSX: -L {{[^ ]*}}/Inputs/clang-importer-sdk{{/|\\\\}}usr{{/|\\\\}}lib{{/|\\\\}}swift | ||
// OSX: -rpath {{[^ ]*}}/Inputs/clang-importer-sdk{{/|\\\\}}usr{{/|\\\\}}lib{{/|\\\\}}swift |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there something we could do here that's isn't…you know, this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😢 I tried, but I cannot make it work.
In #25546 I tried to find all the things that look like Windows absolute paths in the output, and transform them to use POSIX slashes. That worked for many tests, but I had to revert in #25595 because it was also triggering false positives, and breaking other tests (check the comments in that last PR for a clear example).
In #24940 I have an approach that should be safer, but only applies to paths relative to SOURCE_DIR
or BUILD_DIR
. It would not had help in this case.
I tried to use a couple more approaches, but sadly FileCheck is not very flexible, so I don't have a lot of tools. PathSanitizingFileCheck seems like a good place to make the fix (and some fixes are there to avoid many problems in Windows), but it has to work in every case, and I haven't found the right solution yet. Ideally I want a solution that doesn't involve changing the tests from the POSIX paths, but at this point, I think I will be happy with a solution, even if it involves marking the paths somehow.
A solution that I would not like is going through all the tools and standardinzing on POSIX paths everywhere. But that's going to be finding 1000 needles in a very large haystack. This might not be possible and might mean changes in LLVM, which is unfortunate. It also means that Windows will show "foreign" paths (they still work in most cases, but they look wrong). I would actually prefer standardizing paths in Windows to not have both slashes, but give the right Windows slashes.
If you have any ideas, we are really interested in hearing them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a [[/]]
pattern to FileCheck which expands to this? It'd be better than nothing.
Even if we don't adopt that specific solution, can you at least file a bug about the need to replace this with something better? I don't want this to just quietly spread through half of our test suite.
This might not be possible and might mean changes in LLVM, which is unfortunate.
Has LLVM solved this problem? I'm guessing they don't have {{/|\\\\}}
s all over the place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: https://bugs.swift.org/browse/SR-11103
Adding that pattern to FileCheck might be possible, but sadly in some cases the test check for strings with their escaped characters, and in those cases the directory separators are escaped themselves, so some times one has to actually include a pattern like {{/|\\\\\\\\}}
instead (the POSIX separators appears unescaped, while the Windows separator has to be look for escaped, but also one has to escape those slashes for the regular expression).
In LLVM test suite, when I looked at it, I found two things: they don’t check against paths as much as the Swift test suite does, and when they do, they add the patterns (for example https://github.com/llvm/llvm-project/blob/master/llvm/test/tools/dsymutil/X86/basic-lto-linking-x86.test#L19).
No description provided.