Skip to content

Commit 1325e32

Browse files
committed
Standardize file paths when attempting to find toolchains
`containingXCToolchain` loops infinitely when given eg. `C:\foo\..\bar`. The underlying cause is that `deletingLastPathComponent` does not remove the `..` on Windows. On macOS it's potentially worse - it *adds* `..`. We don't see the infinite loop on macOS/Linux though because `AbsolutePath` already removes them (which is not the case on Windows). Resolves #2174.
1 parent 016a0b1 commit 1325e32

File tree

4 files changed

+37
-15
lines changed

4 files changed

+37
-15
lines changed

Sources/SwiftExtensions/URLExtensions.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ extension URL {
8787
}
8888
}
8989

90+
/// Assuming this URL is a file URL, checks if it looks like a root path. This is a string check, ie. the return
91+
/// value for a path of `"/foo/.."` would be `false`. An error will be thrown is this is a non-file URL.
9092
package var isRoot: Bool {
9193
get throws {
9294
let checkPath = try filePath

Sources/ToolchainRegistry/Toolchain.swift

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -366,10 +366,20 @@ public final class Toolchain: Sendable {
366366
}
367367

368368
/// Find a containing xctoolchain with plist, if available.
369-
func containingXCToolchain(
369+
private func containingXCToolchain(
370370
_ path: URL
371371
) -> (XCToolchainPlist, URL)? {
372-
var path = path
372+
// `deletingLastPathComponent` only makes sense on resolved paths (ie. those without symlinks or `..`). Any given
373+
// toolchain path should have already been realpathed, but since this can turn into an infinite loop otherwise, it's
374+
// better to be safe than sorry.
375+
guard let resolvedPath = try? path.realpath else {
376+
return nil
377+
}
378+
if path != resolvedPath {
379+
logger.fault("\(path) was not realpathed")
380+
}
381+
382+
var path = resolvedPath
373383
while !((try? path.isRoot) ?? true) {
374384
if path.pathExtension == "xctoolchain" {
375385
if let infoPlist = orLog("Loading information from xctoolchain", { try XCToolchainPlist(fromDirectory: path) }) {

Sources/ToolchainRegistry/ToolchainRegistry.swift

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,13 @@ package final actor ToolchainRegistry {
9696
var toolchainsByPath: [URL: Toolchain] = [:]
9797
var toolchainsByCompiler: [URL: Toolchain] = [:]
9898
for (toolchain, reason) in toolchainsAndReasonsParam {
99+
// Toolchain should always be unique by path. It isn't particularly useful to log if we already have a toolchain
100+
// though, as we could have just found toolchains through symlinks (this is actually quite normal - eg. OSS
101+
// toolchains add a `swift-latest.xctoolchain` symlink on macOS).
102+
if toolchainsByPath[toolchain.path] != nil {
103+
continue
104+
}
105+
99106
// Non-XcodeDefault toolchain: disallow all duplicates.
100107
if toolchainsByIdentifier[toolchain.identifier] != nil,
101108
toolchain.identifier != ToolchainRegistry.darwinDefaultToolchainIdentifier
@@ -104,12 +111,6 @@ package final actor ToolchainRegistry {
104111
continue
105112
}
106113

107-
// Toolchain should always be unique by path.
108-
if toolchainsByPath[toolchain.path] != nil {
109-
logger.fault("Found two toolchains with the same path: \(toolchain.path)")
110-
continue
111-
}
112-
113114
toolchainsByPath[toolchain.path] = toolchain
114115
toolchainsByIdentifier[toolchain.identifier, default: []].append(toolchain)
115116

@@ -219,7 +220,9 @@ package final actor ToolchainRegistry {
219220
}
220221

221222
let toolchainsAndReasons = toolchainPaths.compactMap {
222-
if let toolchain = Toolchain($0.path) {
223+
if let resolvedPath = try? $0.path.realpath,
224+
let toolchain = Toolchain(resolvedPath)
225+
{
223226
return (toolchain, $0.reason)
224227
}
225228
return nil
@@ -283,7 +286,18 @@ package final actor ToolchainRegistry {
283286
/// If we have a toolchain in the toolchain registry that contains the compiler with the given URL, return it.
284287
/// Otherwise, return `nil`.
285288
package func toolchain(withCompiler compiler: URL) -> Toolchain? {
286-
return toolchainsByCompiler[compiler]
289+
if let resolvedPath = try? compiler.realpath {
290+
return toolchainsByCompiler[resolvedPath]
291+
}
292+
return nil
293+
}
294+
295+
/// If we have a toolchain in the toolchain registry with the given URL, return it. Otherwise, return `nil`.
296+
package func toolchain(withPath path: URL) -> Toolchain? {
297+
if let resolvedPath = try? path.realpath {
298+
return toolchainsByPath[resolvedPath]
299+
}
300+
return nil
287301
}
288302
}
289303

@@ -292,10 +306,6 @@ extension ToolchainRegistry {
292306
package func toolchains(withIdentifier identifier: String) -> [Toolchain] {
293307
return toolchainsByIdentifier[identifier] ?? []
294308
}
295-
296-
package func toolchain(withPath path: URL) -> Toolchain? {
297-
return toolchainsByPath[path]
298-
}
299309
}
300310

301311
extension ToolchainRegistry {

Tests/ToolchainRegistryTests/ToolchainRegistryTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ final class ToolchainRegistryTests: XCTestCase {
336336

337337
try ProcessEnv.setVar(
338338
"SOURCEKIT_PATH_FOR_TEST",
339-
value: ["/bogus", binPath.filePath, "/bogus2"].joined(separator: separator)
339+
value: ["/bogus/../parent", "/bogus", binPath.filePath, "/bogus2"].joined(separator: separator)
340340
)
341341
defer { try! ProcessEnv.setVar("SOURCEKIT_PATH_FOR_TEST", value: "") }
342342

0 commit comments

Comments
 (0)