Fix some cases where AbsolutePath::relative(to: AbsolutePath) would assert on Windows #534
Conversation
daveinglis
commented
Dec 9, 2025
daveinglis
commented
- there where a couple case where relative(to:) would assert on windows when a path was long (>260) and when tailing slashes where not removed in some cases.
|
@swift-ci test |
|
@swift-ci test linux |
Sources/TSCBasic/Path.swift
Outdated
| while !substring.isEmpty && substring.utf8.last == UInt8(ascii: "\\") { | ||
| substring = substring.dropLast() | ||
| } | ||
| if !substring.isEmpty && substring.last != ":" { |
There was a problem hiding this comment.
This is kinda sketchy. The last character being : could be a malformed path, e.g. C:\file.txt:.
Sources/TSCBasic/Path.swift
Outdated
| if !substring.isEmpty && substring.last != ":" { | ||
| // Drop the trailing '\', unless the string path only | ||
| // has '\', and unless the slashes are right after the drive letter. | ||
| path = String(substring) |
There was a problem hiding this comment.
Just to confirm - C: and C:\ have completely different meanings. Is this logic preserving the trailing slash in the case where the path is pointing at a drive root?
daveinglis
force-pushed
the
fix_asserts
branch
2 times, most recently
from
975683d to
df81ada
Compare
|
@swift-ci test |
|
@swift-ci test windows |
|
This needs some more work/investigation, seeing some strange crashes in swiftPM that this is causing |
…ssert - there where a couple case where relative(to:) would assert on windows when a path was long (>206) and when tailing slashes where not removed in some cases.
daveinglis
force-pushed
the
fix_asserts
branch
from
df81ada to
f520ce0
Compare
|
@swift-ci test |
|
@swift-ci test windows |
1 similar comment
|
@swift-ci test windows |
|
@swift-ci test |
| if string.first?.isASCII ?? false, string.first?.isLetter ?? false, string.first?.isLowercase ?? false, | ||
| let path: String | ||
| let hasDrive: Bool | ||
| if string.first?.isASCII ?? false, string.first?.isLetter ?? false, |
There was a problem hiding this comment.
issue (possibly-blocking): Does not account for long path.
This does not take into account long path \\?\D:\<very long path>. Can we instead use Regex to determine if the passed string matches the desired conditions?
There was a problem hiding this comment.
I think that it not supporting that is fine. Foundation does go to some lengths to ensure that long paths are transparent to the user (it converts on the way in and out). Users generally would not type the NT path (and you cannot copy it from explorer as that transacts in Win32 paths and cmd would rely on DOS paths).
| return self == .root ? self : Self(string: dirname) | ||
| } | ||
|
|
||
| init(string: String) { |
There was a problem hiding this comment.
suggestion (blocking): Can we augment the automated tests to ensure the defect is fixed, and that we won't regress?
4 participants
