Skip to content

Fix some cases where AbsolutePath::relative(to: AbsolutePath) would assert on Windows #534

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.

Sign up for GitHub

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

Open
daveinglis wants to merge 1 commit into
base: main
Choose a base branch
from
+30 −11
Open

Fix some cases where AbsolutePath::relative(to: AbsolutePath) would assert on Windows #534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Sources/TSCBasic/FileSystem.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ private struct LocalFileSystem: FileSystem {
let fsr: UnsafePointer<Int8> = cwdStr.fileSystemRepresentation
defer { fsr.deallocate() }

return try? AbsolutePath(String(cString: fsr))
return try? AbsolutePath(validating: String(cString: fsr))
#endif
}

Expand Down
31 changes: 24 additions & 7 deletions Sources/TSCBasic/Path.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public struct AbsolutePath: Hashable, Sendable {
}
defer { LocalFree(pwszResult) }

self.init(String(decodingCString: pwszResult, as: UTF16.self))
try self.init(validating: String(decodingCString: pwszResult, as: UTF16.self))
#else
try self.init(basePath, RelativePath(validating: str))
#endif
Expand Down Expand Up @@ -515,12 +515,28 @@ private struct WindowsPath: Path, Sendable {
}

init(string: String) {
Copy link

@bkhouri bkhouri Jan 22, 2026

Choose a reason for hiding this comment

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

suggestion (blocking): Can we augment the automated tests to ensure the defect is fixed, and that we won't regress?

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,
Copy link

@bkhouri bkhouri Jan 22, 2026

Choose a reason for hiding this comment

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

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?

Copy link
Member

@compnerd compnerd Jan 22, 2026

Choose a reason for hiding this comment

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

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).

string.count > 1, string[string.index(string.startIndex, offsetBy: 1)] == ":"
{
self.string = "\(string.first!.uppercased())\(string.dropFirst(1))"
hasDrive = true
path = "\(string.first!.uppercased())\(string.dropFirst(1))"
} else {
self.string = string
hasDrive = false
path = string
}
var droppedTailing: Bool = false
var substring = path[path.startIndex..<path.endIndex]
while substring.count > 1 && substring.utf8.last == UInt8(ascii: "\\") {
substring = substring.dropLast()
droppedTailing = true
}
// Don't drop the trailing slash is we only have <drive>: left
if hasDrive && substring.count == 2 && droppedTailing {
self.string = Self.repr(path) + "\\"
} else {
self.string = Self.repr(String(substring))
}
}

Expand All @@ -544,7 +560,7 @@ private struct WindowsPath: Path, Sendable {
self.init(string: ".")
} else {
let realpath: String = Self.repr(path)
// Treat a relative path as an invalid relative path...
// Treat an absolute path as an invalid relative path
if Self.isAbsolutePath(realpath) || realpath.first == "\\" {
throw PathValidationError.invalidRelativePath(path)
}
Expand All @@ -568,6 +584,7 @@ private struct WindowsPath: Path, Sendable {
_ = string.withCString(encodedAs: UTF16.self) { root in
name.withCString(encodedAs: UTF16.self) { path in
PathAllocCombine(root, path, ULONG(PATHCCH_ALLOW_LONG_PATHS.rawValue), &result)
_ = PathCchStripPrefix(result, wcslen(result))
}
}
defer { LocalFree(result) }
Expand All @@ -579,6 +596,7 @@ private struct WindowsPath: Path, Sendable {
_ = string.withCString(encodedAs: UTF16.self) { root in
relativePath.string.withCString(encodedAs: UTF16.self) { path in
PathAllocCombine(root, path, ULONG(PATHCCH_ALLOW_LONG_PATHS.rawValue), &result)
_ = PathCchStripPrefix(result, wcslen(result))
}
}
defer { LocalFree(result) }
Expand Down Expand Up @@ -965,8 +983,7 @@ extension AbsolutePath {
preconditionFailure("invalid relative path computed from \(pathString)")
}
}

assert(AbsolutePath(base, result) == self)
assert(AbsolutePath(base, result) == self, "\(AbsolutePath(base, result)) != \(self)")
return result
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/TSCBasic/PathShims.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public func resolveSymlinks(_ path: AbsolutePath) throws -> AbsolutePath {
} else {
pathBaseAddress = UnsafePointer($0.baseAddress!)
}
return try AbsolutePath(String(decodingCString: pathBaseAddress, as: UTF16.self))
return try AbsolutePath(validating: String(decodingCString: pathBaseAddress, as: UTF16.self))
}
#else
let pathStr = path.pathString
Expand Down
6 changes: 4 additions & 2 deletions Sources/TSCUtility/FSWatch.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,9 @@ public final class RDCWatcher {
}

if !GetOverlappedResult(watch.hDirectory, &watch.overlapped, &dwBytesReturned, false) {
guard let path = try? AbsolutePath(validating: watch.path) else { continue }
queue.async {
delegate?.pathsDidReceiveEvent([AbsolutePath(watch.path)])
delegate?.pathsDidReceiveEvent([path])
}
return
}
Expand All @@ -272,7 +273,8 @@ public final class RDCWatcher {
String(utf16CodeUnitsNoCopy: &pNotify.pointee.FileName,
count: Int(pNotify.pointee.FileNameLength) / MemoryLayout<WCHAR>.stride,
freeWhenDone: false)
paths.append(AbsolutePath(file))
guard let path = try? AbsolutePath(validating: file) else { continue }
paths.append(path)

pNotify = (UnsafeMutableRawPointer(pNotify) + Int(pNotify.pointee.NextEntryOffset))
.assumingMemoryBound(to: FILE_NOTIFY_INFORMATION.self)
Expand Down