Skip to content

Deleting last path component on Linux can result in URL with empty path #980

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

Closed
ahoppen opened this issue Oct 11, 2024 · 3 comments
Closed
Assignees

Comments

@ahoppen
Copy link
Member

ahoppen commented Oct 11, 2024

var url = URL(fileURLWithPath: "/whatever/test.swift")
print("'\(url.path)'") // prints '/whatever/test.swift'
url.deleteLastPathComponent()
print("'\(url.path)'") // prints '/whatever'
url.deleteLastPathComponent()
print("'\(url.path)'") // prints ''

url = URL(fileURLWithPath: "/test.swift")
print("'\(url.path)'") // prints '/test.swift'
url.deleteLastPathComponent()
print("'\(url.path)'") // prints '/'

Where it prints '' at the end of the first section, I think it should print /.

This is in a Swift 6.0 Linux docker image.

ahoppen added a commit to ahoppen/swift-format that referenced this issue Oct 11, 2024
Due to swiftlang/swift-foundation#980, the we may end up with a `path == ""` instead of `/`.

We didn’t catch this in the test because we only end up with an empty path when searching for a `.swift-format` file from a `.swift-file` that was not at the root of a directory.
ahoppen added a commit to ahoppen/swift-format that referenced this issue Oct 11, 2024
Due to swiftlang/swift-foundation#980, the we may end up with a `path == ""` instead of `/`.

We didn’t catch this in the test because we only end up with an empty path when searching for a `.swift-format` file from a `.swift-file` that was not at the root of a directory.
@jrflat
Copy link
Contributor

jrflat commented Oct 25, 2024

This String.deletingLastPathComponent() bug was fixed in main by #897. The implementation was updated in main (still fixes the bug) in #927. Neither of those were cherry-picked to release/6.0, though.

@parkera Can we cherry-pick #927 (String+Path parsing performance improvements) to release/6.0, or should we just take the minimal fix from #897?

@jrflat jrflat self-assigned this Oct 25, 2024
@parkera
Copy link
Contributor

parkera commented Oct 25, 2024

I think I'd rather take the full patch than attempt to cut it down.

@jrflat
Copy link
Contributor

jrflat commented Nov 6, 2024

Resolved by #1013 in release/6.0, please verify with a recent 6.0 toolchain, thanks!

@jrflat jrflat closed this as completed Nov 6, 2024
macshome added a commit to macshome/swift-format that referenced this issue Dec 6, 2024
Now that swiftlang/swift-foundation#976 and swiftlang/swift-foundation#980 are fixed we can clean this code up a bit by removing the empty path check on Linux and by using the native `PathCchIsRoot` on Windows.
macshome added a commit to macshome/swift-format that referenced this issue Dec 9, 2024
Now that swiftlang/swift-foundation#976 and swiftlang/swift-foundation#980 are fixed we can clean this code up a bit by removing the empty path check on Linux and by using the native `PathCchIsRoot` on Windows.

Existing code is retained for toolchains <6.1
macshome added a commit to macshome/swift-format that referenced this issue Dec 10, 2024
Now that swiftlang/swift-foundation#976 and swiftlang/swift-foundation#980 are fixed we can clean this code up a bit by removing the empty path check on Linux and by using the native `PathCchIsRoot` on Windows.

Existing code is retained for toolchains <6.1
ahoppen pushed a commit to macshome/swift-format that referenced this issue Dec 10, 2024
Now that swiftlang/swift-foundation#976 and swiftlang/swift-foundation#980 are fixed we can clean this code up a bit by removing the empty path check on Linux and by using the native `PathCchIsRoot` on Windows.

Existing code is retained for toolchains <6.1
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

No branches or pull requests

3 participants