Skip to content

[Windows] Added a test for _getFilesystemRepresentation #2367

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
Apr 25, 2020

Conversation

gmittert
Copy link
Contributor

@gmittert gmittert commented Jun 19, 2019

A small test to ensure that Windows FSRs don't regress

@gmittert gmittert changed the title Use win fsr [Windows] Replace uses of withCString with _getFilesystemRepresentation Jun 20, 2019
@gmittert gmittert force-pushed the UseWinFSR branch 2 times, most recently from fdb10d4 to 75dda63 Compare June 25, 2019 22:35
@compnerd
Copy link
Member

@gmittert would you mind rebasing this change? I think most of the issues are taken care of, but I think it would get good ensure that everything is merged before closing this out.

@gmittert
Copy link
Contributor Author

gmittert commented Apr 19, 2020

@compnerd looks like you got all the ones I found. The only thing left is the test that I added. I've rebased the change to just be the added test now.

This ensures that paths are properly normalized when handed off to win32
functions.
@gmittert
Copy link
Contributor Author

@swift-ci please test

@gmittert gmittert changed the title [Windows] Replace uses of withCString with _getFilesystemRepresentation [Windows] Added a test for _getFilesystemRepresentation Apr 19, 2020
@compnerd
Copy link
Member

@swift-ci please test Linux platform

@compnerd
Copy link
Member

@compnerd compnerd merged commit e73b85b into swiftlang:master Apr 25, 2020
"\(tmpPath)\\testfile", // C:/Users/...\testfile
"\(tmpPath)/testfile", // C:/Users.../testfile
"\(tmpPath)/testfile", // /Users/user/.../testfile
"\(tmpPath.first!):testfile", // C:testfile
Copy link
Contributor

Choose a reason for hiding this comment

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

Btw, this fails when runner has no Administrator privileges. CI agents are usually elevated, but manual run as user would report failure.

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