diff --git a/Sources/SwiftExtensions/URLExtensions.swift b/Sources/SwiftExtensions/URLExtensions.swift index 71cdee8a9..472dd2e82 100644 --- a/Sources/SwiftExtensions/URLExtensions.swift +++ b/Sources/SwiftExtensions/URLExtensions.swift @@ -12,6 +12,10 @@ package import Foundation +#if os(Windows) +import WinSDK +#endif + enum FilePathError: Error, CustomStringConvertible { case noFileSystemRepresentation(URL) case noFileURL(URL) @@ -82,15 +86,17 @@ extension URL { } } + /// Assuming this URL is a file URL, checks if it looks like a root path. This is a string check, ie. the return + /// value for a path of `"/foo/.."` would be `false`. An error will be thrown is this is a non-file URL. package var isRoot: Bool { - #if os(Windows) - // FIXME: We should call into Windows' native check to check if this path is a root once https://github.com/swiftlang/swift-foundation/issues/976 is fixed. - return self.pathComponents.count <= 1 - #else - // On Linux, we may end up with an string for the path due to https://github.com/swiftlang/swift-foundation/issues/980 - // TODO: Remove the check for "" once https://github.com/swiftlang/swift-foundation/issues/980 is fixed. - return self.path == "/" || self.path == "" - #endif + get throws { + let checkPath = try filePath + #if os(Windows) + return checkPath.withCString(encodedAs: UTF16.self, PathCchIsRoot) + #else + return checkPath == "/" + #endif + } } /// Returns true if the path of `self` starts with the path in `other`. diff --git a/Sources/ToolchainRegistry/Toolchain.swift b/Sources/ToolchainRegistry/Toolchain.swift index 13dd0dfe3..41d697bd1 100644 --- a/Sources/ToolchainRegistry/Toolchain.swift +++ b/Sources/ToolchainRegistry/Toolchain.swift @@ -366,11 +366,24 @@ public final class Toolchain: Sendable { } /// Find a containing xctoolchain with plist, if available. -func containingXCToolchain( +private func containingXCToolchain( _ path: URL ) -> (XCToolchainPlist, URL)? { - var path = path - while !path.isRoot { + // `deletingLastPathComponent` only makes sense on resolved paths (ie. those without symlinks or `..`). Any given + // toolchain path should have already been realpathed, but since this can turn into an infinite loop otherwise, it's + // better to be safe than sorry. + let resolvedPath = orLog("Toolchain realpath") { + try path.realpath + } + guard let resolvedPath else { + return nil + } + if path != resolvedPath { + logger.fault("\(path) was not realpathed") + } + + var path = resolvedPath + while !((try? path.isRoot) ?? true) { if path.pathExtension == "xctoolchain" { if let infoPlist = orLog("Loading information from xctoolchain", { try XCToolchainPlist(fromDirectory: path) }) { return (infoPlist, path) diff --git a/Sources/ToolchainRegistry/ToolchainRegistry.swift b/Sources/ToolchainRegistry/ToolchainRegistry.swift index 514949156..bbfce64bc 100644 --- a/Sources/ToolchainRegistry/ToolchainRegistry.swift +++ b/Sources/ToolchainRegistry/ToolchainRegistry.swift @@ -60,13 +60,13 @@ package final actor ToolchainRegistry { private let toolchainsByIdentifier: [String: [Toolchain]] /// The toolchains indexed by their path. - private let toolchainsByPath: [URL: Toolchain] + private var toolchainsByPath: [URL: Toolchain] /// Map from compiler paths (`clang`, `swift`, `swiftc`) mapping to the toolchain that contained them. /// /// This allows us to find the toolchain that should be used for semantic functionality based on which compiler it is /// built with in the `compile_commands.json`. - private let toolchainsByCompiler: [URL: Toolchain] + private var toolchainsByCompiler: [URL: Toolchain] /// The currently selected toolchain identifier on Darwin. package let darwinToolchainOverride: String? @@ -96,6 +96,13 @@ package final actor ToolchainRegistry { var toolchainsByPath: [URL: Toolchain] = [:] var toolchainsByCompiler: [URL: Toolchain] = [:] for (toolchain, reason) in toolchainsAndReasonsParam { + // Toolchain should always be unique by path. It isn't particularly useful to log if we already have a toolchain + // though, as we could have just found toolchains through symlinks (this is actually quite normal - eg. OSS + // toolchains add a `swift-latest.xctoolchain` symlink on macOS). + if toolchainsByPath[toolchain.path] != nil { + continue + } + // Non-XcodeDefault toolchain: disallow all duplicates. if toolchainsByIdentifier[toolchain.identifier] != nil, toolchain.identifier != ToolchainRegistry.darwinDefaultToolchainIdentifier @@ -104,12 +111,6 @@ package final actor ToolchainRegistry { continue } - // Toolchain should always be unique by path. - if toolchainsByPath[toolchain.path] != nil { - logger.fault("Found two toolchains with the same path: \(toolchain.path)") - continue - } - toolchainsByPath[toolchain.path] = toolchain toolchainsByIdentifier[toolchain.identifier, default: []].append(toolchain) @@ -218,9 +219,14 @@ package final actor ToolchainRegistry { } } - let toolchainsAndReasons = toolchainPaths.compactMap { - if let toolchain = Toolchain($0.path) { - return (toolchain, $0.reason) + let toolchainsAndReasons = toolchainPaths.compactMap { toolchainAndReason in + let resolvedPath = orLog("Toolchain realpath") { + try toolchainAndReason.path.realpath + } + if let resolvedPath, + let toolchain = Toolchain(resolvedPath) + { + return (toolchain, toolchainAndReason.reason) } return nil } @@ -285,7 +291,43 @@ package final actor ToolchainRegistry { /// If we have a toolchain in the toolchain registry that contains the compiler with the given URL, return it. /// Otherwise, return `nil`. package func toolchain(withCompiler compiler: URL) -> Toolchain? { - return toolchainsByCompiler[compiler] + if let toolchain = toolchainsByCompiler[compiler] { + return toolchain + } + + // Only canonicalize the folder path, as we don't want to resolve symlinks to eg. `swift-driver`. + let resolvedPath = orLog("Compiler realpath") { + try compiler.deletingLastPathComponent().realpath + }?.appending(component: compiler.lastPathComponent) + guard let resolvedPath, + let toolchain = toolchainsByCompiler[resolvedPath] + else { + return nil + } + + // Cache mapping of non-realpath to the realpath toolchain for faster subsequent lookups + toolchainsByCompiler[compiler] = toolchain + return toolchain + } + + /// If we have a toolchain in the toolchain registry with the given URL, return it. Otherwise, return `nil`. + package func toolchain(withPath path: URL) -> Toolchain? { + if let toolchain = toolchainsByPath[path] { + return toolchain + } + + let resolvedPath = orLog("Toolchain realpath") { + try path.realpath + } + guard let resolvedPath, + let toolchain = toolchainsByPath[resolvedPath] + else { + return nil + } + + // Cache mapping of non-realpath to the realpath toolchain for faster subsequent lookups + toolchainsByPath[path] = toolchain + return toolchain } } @@ -294,10 +336,6 @@ extension ToolchainRegistry { package func toolchains(withIdentifier identifier: String) -> [Toolchain] { return toolchainsByIdentifier[identifier] ?? [] } - - package func toolchain(withPath path: URL) -> Toolchain? { - return toolchainsByPath[path] - } } extension ToolchainRegistry { diff --git a/Tests/ToolchainRegistryTests/ToolchainRegistryTests.swift b/Tests/ToolchainRegistryTests/ToolchainRegistryTests.swift index 929add581..60c4bf10c 100644 --- a/Tests/ToolchainRegistryTests/ToolchainRegistryTests.swift +++ b/Tests/ToolchainRegistryTests/ToolchainRegistryTests.swift @@ -325,8 +325,12 @@ final class ToolchainRegistryTests: XCTestCase { func testSearchPATH() async throws { try await withTestScratchDir { tempDir in - let binPath = tempDir.appendingPathComponent("bin", isDirectory: true) - try makeToolchain(binPath: binPath, sourcekitd: true) + let binPath = tempDir.appending(component: "bin", directoryHint: .isDirectory) + try makeToolchain(binPath: binPath, clang: true, sourcekitd: true) + + let compilerPath = binPath.appending(component: "clang" + (Platform.current?.executableExtension ?? "")) + let linkPath = tempDir.appending(component: "link") + try FileManager.default.createSymbolicLink(at: linkPath, withDestinationURL: compilerPath) #if os(Windows) let separator: String = ";" @@ -336,7 +340,12 @@ final class ToolchainRegistryTests: XCTestCase { try ProcessEnv.setVar( "SOURCEKIT_PATH_FOR_TEST", - value: ["/bogus", binPath.filePath, "/bogus2"].joined(separator: separator) + value: [ + "/bogus/../parent", + "/bogus", + binPath.appending(components: "..", "bin", directoryHint: .isDirectory).filePath, + "/bogus2", + ].joined(separator: separator) ) defer { try! ProcessEnv.setVar("SOURCEKIT_PATH_FOR_TEST", value: "") } @@ -349,15 +358,21 @@ final class ToolchainRegistryTests: XCTestCase { darwinToolchainOverride: nil ) - let tc = try unwrap(await tr.toolchains.first(where: { $0.path == binPath })) + let pathTC = try unwrap(await tr.toolchain(withPath: binPath)) - await assertEqual(tr.default?.identifier, tc.identifier) - XCTAssertEqual(tc.identifier, try binPath.filePath) - XCTAssertNil(tc.clang) - XCTAssertNil(tc.clangd) - XCTAssertNil(tc.swiftc) - XCTAssertNotNil(tc.sourcekitd) - XCTAssertNil(tc.libIndexStore) + await assertEqual(tr.default?.identifier, pathTC.identifier) + XCTAssertEqual(pathTC.identifier, try binPath.filePath) + XCTAssertNotNil(pathTC.clang) + XCTAssertNil(pathTC.clangd) + XCTAssertNil(pathTC.swiftc) + XCTAssertNotNil(pathTC.sourcekitd) + XCTAssertNil(pathTC.libIndexStore) + + let compilerTC = try unwrap(await tr.toolchain(withCompiler: compilerPath)) + XCTAssertEqual(compilerTC.identifier, try binPath.filePath) + + let linkTC = await tr.toolchain(withCompiler: linkPath) + XCTAssertNil(linkTC) } }