Skip to content

refactor RelativePath to allow late stage canonicalization in support of windows #369

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 7 commits into from
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
113 changes: 48 additions & 65 deletions Sources/TSCBasic/Path.swift
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public struct AbsolutePath: Hashable, Sendable {

self.init(String(decodingCString: pwszResult, as: UTF16.self))
#else
self.init(basePath, RelativePath(str))
try self.init(basePath, RelativePath(validating: str))
#endif
}
}
Expand All @@ -102,8 +102,8 @@ public struct AbsolutePath: Hashable, Sendable {
}

/// Convenience initializer that appends a string to a relative path.
public init(_ absPath: AbsolutePath, _ relStr: String) {
self.init(absPath, RelativePath(relStr))
public init(_ absPath: AbsolutePath, validating relStr: String) throws {
try self.init(absPath, RelativePath(validating: relStr))
}

/// Initializes the AbsolutePath from `absStr`, which must be an absolute
Expand Down Expand Up @@ -240,23 +240,6 @@ public struct RelativePath: Hashable, Sendable {
_impl = impl
}

/// Private initializer for constructing a relative path without performing
/// normalization or canonicalization. This will construct a path without
/// an anchor and thus may be invalid.
fileprivate init(unsafeUncheckedPath string: String) {
self.init(PathImpl(string: string))
}

/// Initializes the RelativePath from `str`, which must be a relative path
/// (which means that it must not begin with a path separator or a tilde).
/// An empty input path is allowed, but will be normalized to a single `.`
/// character. The input string will be normalized if needed, as described
/// in the documentation for RelativePath.
public init(_ string: String) {
// Normalize the relative string and store it as our Path.
self.init(PathImpl(normalizingRelativePath: string))
}

/// Convenience initializer that verifies that the path is relative.
public init(validating path: String) throws {
try self.init(PathImpl(validatingRelativePath: path))
Expand Down Expand Up @@ -429,12 +412,6 @@ protocol Path: Hashable {
/// Creates a path from its normalized string representation.
init(string: String)

/// Creates a path from an absolute string representation and normalizes it.
init(normalizingAbsolutePath: String)

/// Creates a path from an relative string representation and normalizes it.
init(normalizingRelativePath: String)

/// Creates a path from a string representation, validates that it is a valid absolute path and normalizes it.
init(validatingAbsolutePath: String) throws

Expand Down Expand Up @@ -534,34 +511,12 @@ private struct WindowsPath: Path, Sendable {
return String(cString: representation)
}

init(normalizingAbsolutePath path: String) {
self.init(string: Self.repr(path).withCString(encodedAs: UTF16.self) { pwszPath in
var canonical: PWSTR!
_ = PathAllocCanonicalize(pwszPath,
ULONG(PATHCCH_ALLOW_LONG_PATHS.rawValue),
&canonical)
return String(decodingCString: canonical, as: UTF16.self)
})
}

init(validatingAbsolutePath path: String) throws {
let realpath = Self.repr(path)
if !Self.isAbsolutePath(realpath) {
throw PathValidationError.invalidAbsolutePath(path)
}
self.init(normalizingAbsolutePath: path)
}

init(normalizingRelativePath path: String) {
if path.isEmpty || path == "." {
self.init(string: ".")
} else {
var buffer: [WCHAR] = Array<WCHAR>(repeating: 0, count: Int(MAX_PATH + 1))
_ = path.replacingOccurrences(of: "/", with: "\\").withCString(encodedAs: UTF16.self) {
PathCanonicalizeW(&buffer, $0)
}
self.init(string: String(decodingCString: buffer, as: UTF16.self))
}
self.init(string: realpath)
}

init(validatingRelativePath path: String) throws {
Expand All @@ -570,11 +525,10 @@ private struct WindowsPath: Path, Sendable {
} else {
let realpath: String = Self.repr(path)
// Treat a relative path as an invalid relative path...
if Self.isAbsolutePath(realpath) ||
realpath.first == "~" || realpath.first == "\\" {
if Self.isAbsolutePath(realpath) || realpath.first == "\\" {
throw PathValidationError.invalidRelativePath(path)
}
self.init(normalizingRelativePath: path)
self.init(string: realpath)
}
}

Expand All @@ -597,7 +551,7 @@ private struct WindowsPath: Path, Sendable {
}
}
defer { LocalFree(result) }
return PathImpl(string: String(decodingCString: result!, as: UTF16.self))
return Self(string: String(decodingCString: result!, as: UTF16.self))
}

func appending(relativePath: Self) -> Self {
Expand All @@ -608,7 +562,7 @@ private struct WindowsPath: Path, Sendable {
}
}
defer { LocalFree(result) }
return PathImpl(string: String(decodingCString: result!, as: UTF16.self))
return Self(string: String(decodingCString: result!, as: UTF16.self))
}
}
#else
Expand Down Expand Up @@ -830,7 +784,7 @@ private struct UNIXPath: Path, Sendable {

init(validatingRelativePath path: String) throws {
switch path.first {
case "/", "~":
case "/":
throw PathValidationError.invalidRelativePath(path)
default:
self.init(normalizingRelativePath: path)
Expand Down Expand Up @@ -875,9 +829,9 @@ private struct UNIXPath: Path, Sendable {
}

if self == Self.root {
return PathImpl(string: "/" + name)
return Self(string: "/" + name)
} else {
return PathImpl(string: string + "/" + name)
return Self(string: string + "/" + name)
}
}

Expand All @@ -900,12 +854,12 @@ private struct UNIXPath: Path, Sendable {
// the beginning of the path only.
if relativePathString.hasPrefix(".") {
if newPathString.hasPrefix("/") {
return PathImpl(normalizingAbsolutePath: newPathString)
return Self(normalizingAbsolutePath: newPathString)
} else {
return PathImpl(normalizingRelativePath: newPathString)
return Self(normalizingRelativePath: newPathString)
}
} else {
return PathImpl(string: newPathString)
return Self(string: newPathString)
}
}
}
Expand All @@ -926,7 +880,7 @@ extension PathValidationError: CustomStringConvertible {
case .invalidAbsolutePath(let path):
return "invalid absolute path '\(path)'"
case .invalidRelativePath(let path):
return "invalid relative path '\(path)'; relative path should not begin with '\(AbsolutePath.root.pathString)' or '~'"
return "invalid relative path '\(path)'; relative path should not begin with '\(AbsolutePath.root.pathString)'"
}
}
}
Expand Down Expand Up @@ -956,10 +910,16 @@ extension AbsolutePath {
// might be an empty path (when self and the base are equal).
let relComps = pathComps.dropFirst(baseComps.count)
#if os(Windows)
result = RelativePath(unsafeUncheckedPath: relComps.joined(separator: "\\"))
let pathString = relComps.joined(separator: "\\")
#else
result = RelativePath(relComps.joined(separator: "/"))
let pathString = relComps.joined(separator: "/")
#endif
do {
result = try RelativePath(validating: pathString)
} catch {
preconditionFailure("invalid relative path computed from \(pathString)")
}

} else {
// General case, in which we might well need `..` components to go
// "up" before we can go "down" the directory tree.
Expand All @@ -975,10 +935,15 @@ extension AbsolutePath {
var relComps = Array(repeating: "..", count: newBaseComps.count)
relComps.append(contentsOf: newPathComps)
#if os(Windows)
result = RelativePath(unsafeUncheckedPath: relComps.joined(separator: "\\"))
let pathString = relComps.joined(separator: "\\")
#else
result = RelativePath(relComps.joined(separator: "/"))
let pathString = relComps.joined(separator: "/")
#endif
do {
result = try RelativePath(validating: pathString)
} catch {
preconditionFailure("invalid relative path computed from \(pathString)")
}
}

assert(AbsolutePath(base, result) == self)
Expand Down Expand Up @@ -1065,13 +1030,31 @@ private func mayNeedNormalization(absolute string: String) -> Bool {
// MARK: - `AbsolutePath` backwards compatibility, delete after deprecation period.

extension AbsolutePath {
@_disfavoredOverload
@available(*, deprecated, message: "use throwing `init(validating:)` variant instead")
public init(_ absStr: String) {
try! self.init(validating: absStr)
}

@_disfavoredOverload
@available(*, deprecated, message: "use throwing `init(validating:relativeTo:)` variant instead")
public init(_ str: String, relativeTo basePath: AbsolutePath) {
try! self.init(validating: str, relativeTo: basePath)
}

@_disfavoredOverload
@available(*, deprecated, message: "use throwing variant instead")
public init(_ absPath: AbsolutePath, _ relStr: String) {
try! self.init(absPath, validating: relStr)
}
}

// MARK: - `AbsolutePath` backwards compatibility, delete after deprecation period.

extension RelativePath {
@_disfavoredOverload
@available(*, deprecated, message: "use throwing variant instead")
public init(_ string: String) {
try! self.init(validating: string)
}
}
57 changes: 54 additions & 3 deletions Sources/TSCTestSupport/FileSystemExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,69 @@ extension FileSystem {
}
}

public extension AbsolutePath {
init(path: StaticString) {
extension AbsolutePath {
@available(*, deprecated, message: "use direct string instead")
public init(path: StaticString) {
let pathString = path.withUTF8Buffer {
String(decoding: $0, as: UTF8.self)
}
try! self.init(validating: pathString)
}

init(path: StaticString, relativeTo basePath: AbsolutePath) {
@available(*, deprecated, message: "use init(: relativeTo:) instead")
public init(path: StaticString, relativeTo basePath: AbsolutePath) {
let pathString = path.withUTF8Buffer {
String(decoding: $0, as: UTF8.self)
}
try! self.init(validating: pathString, relativeTo: basePath)
}


@available(*, deprecated, message: "use direct string instead")
public init(base: AbsolutePath, _ relStr: StaticString) {
let pathString = relStr.withUTF8Buffer {
String(decoding: $0, as: UTF8.self)
}
self.init(base, RelativePath(stringLiteral: pathString))
}
}

extension AbsolutePath: ExpressibleByStringLiteral {
public init(_ value: StringLiteralType) {
try! self.init(validating: value)
}
}

extension AbsolutePath: ExpressibleByStringInterpolation {
public init(stringLiteral value: String) {
try! self.init(validating: value)
}
}

extension AbsolutePath {
public init(_ path: StringLiteralType, relativeTo basePath: AbsolutePath) {
try! self.init(validating: path, relativeTo: basePath)
}
}

extension RelativePath {
@available(*, deprecated, message: "use direct string instead")
public init(static path: StaticString) {
let pathString = path.withUTF8Buffer {
String(decoding: $0, as: UTF8.self)
}
try! self.init(validating: pathString)
}
}

extension RelativePath: ExpressibleByStringLiteral {
public init(_ value: StringLiteralType) {
try! self.init(validating: value)
}
}

extension RelativePath: ExpressibleByStringInterpolation {
public init(stringLiteral value: String) {
try! self.init(validating: value)
}
}
2 changes: 1 addition & 1 deletion Tests/TSCBasicPerformanceTests/PathPerfTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class PathPerfTests: XCTestCasePerf {
@available(*, deprecated)
func testJoinPerf_X100000() {
#if canImport(Darwin)
let absPath = AbsolutePath(path: "/hello/little")
let absPath = AbsolutePath("/hello/little")
let relPath = RelativePath("world")
let N = 100000
self.measure {
Expand Down
Loading