Skip to content

Fix file URL in test-static-stdlib #143

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
Jul 29, 2024
Merged

Conversation

jrflat
Copy link
Contributor

@jrflat jrflat commented Jul 28, 2024

URL(fileURLWithPath:) takes a URL path that doesn't include the file: scheme, so URL(fileURLWithPath: "file:///foo") treats the entire file:///foo as a relative path (since it doesn't start with a /).

In swiftlang/swift-foundation#792, we updated relative path parsing to no longer compress slashes (matching the behavior of all common RFC 3986 and WHATWG parsers), so the output of u.relativePath changed from file:/foo to file:///foo, causing this test to fail.

This PR updates the test to use the file URL API as expected and updates the check in accordance, e.g.

let u = URL(fileURLWithPath: "/foo")
print("foo bar baz: \(u.relativePath)")
// ...
CHECK: foo bar baz: /foo

and should fix the Linux builds.

@jrflat
Copy link
Contributor Author

jrflat commented Jul 28, 2024

Linux smoke test passed at swiftlang/swift#75354

@jrflat
Copy link
Contributor Author

jrflat commented Jul 28, 2024

All smoke tests passed

Copy link
Contributor

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

At least trying it on my mac, let u = URL(fileURLWithPath: "file:///foo") outputs file:/foo as the relative path. Should we continue to ensure that the outputs still match across implementations or does the root change expected to alter the behavior everywhere?

@jrflat
Copy link
Contributor Author

jrflat commented Jul 29, 2024

At least trying it on my mac, let u = URL(fileURLWithPath: "file:///foo") outputs file:/foo as the relative path. Should we continue to ensure that the outputs still match across implementations or does the root change expected to alter the behavior everywhere?

We're working on updating the behavior to be the same across all Darwin and non-Darwin platforms, where not compressing the slashes will be expected behavior

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.

2 participants