Don’t store the default toolchain in the ToolchainRegistry

The computation of the default toolchain was a weird mixture of manually setting it and lazy resolving that made it hard to understand where the default toolchain was coming from.

To simplify the logic, always compute the default toolchain from the raw data that we have.

Author: ahoppen

Sources/SKCore/ToolchainRegistry.swift

@@ -41,9 +41,6 @@ public final actor ToolchainRegistry {
   /// Note: Not all toolchains have a path.
   private var toolchainsByPath: [AbsolutePath: Toolchain] = [:]
 
-  /// The default toolchain.
-  private var _default: Toolchain? = nil
-
   /// The currently selected toolchain identifier on Darwin.
   public lazy var darwinToolchainOverride: String? = {
     if let id = ProcessEnv.vars["TOOLCHAINS"], !id.isEmpty, id != "default" {
@@ -90,9 +87,6 @@ public final actor ToolchainRegistry {
   ) async {
     scanForToolchains(installPath: installPath, fileSystem)
   }
-}
-
-extension ToolchainRegistry {
 
   /// The default toolchain.
   ///
@@ -103,26 +97,12 @@ extension ToolchainRegistry {
   /// The default toolchain must be only of the registered toolchains.
   public var `default`: Toolchain? {
     get {
-      if _default == nil {
-        if let tc = toolchainsByIdentifier[darwinToolchainIdentifier]?.first {
-          _default = tc
-        } else {
-          _default = toolchains.first
-        }
-      }
-      return _default
-    }
-
-    set {
-      guard let toolchain = newValue else {
-        _default = nil
-        return
+      if let tc = toolchainsByIdentifier[darwinToolchainIdentifier]?.first {
+        return tc
+      } else {
+        return toolchains.first
       }
-      precondition(
-        toolchains.contains { $0 === toolchain },
-        "default toolchain must be registered first"
-      )
-      _default = toolchain
+      return nil
     }
   }
 
@@ -206,12 +186,11 @@ extension ToolchainRegistry {
   public func registerToolchain(
     _ path: AbsolutePath,
     _ fileSystem: FileSystem = localFileSystem
-  ) throws -> Toolchain {
+  ) throws {
     guard let toolchain = Toolchain(path, fileSystem) else {
       throw Error.invalidToolchain
     }
     try registerToolchain(toolchain)
-    return toolchain
   }
 }
 
@@ -240,12 +219,9 @@ extension ToolchainRegistry {
         AbsolutePath(validating: "/Library/Developer/Toolchains"),
       ]
 
-    scanForToolchains(environmentVariables: environmentVariables, setDefault: true, fileSystem)
-    if let installPath = installPath,
-      let toolchain = try? registerToolchain(installPath, fileSystem),
-      _default == nil
-    {
-      _default = toolchain
+    scanForToolchains(environmentVariables: environmentVariables, fileSystem)
+    if let installPath = installPath {
+      try? registerToolchain(installPath, fileSystem)
     }
     for xcode in xcodes {
       scanForToolchains(xcode: xcode, fileSystem)
@@ -261,22 +237,16 @@ extension ToolchainRegistry {
   ///
   /// - parameters:
   ///   - environmentVariables: A list of environment variable names to search for toolchain paths.
-  ///   - setDefault: If true, the first toolchain found will be set as the default.
   @_spi(Testing)
   public func scanForToolchains(
     environmentVariables: [String],
-    setDefault: Bool,
     _ fileSystem: FileSystem = localFileSystem
   ) {
-    var shouldSetDefault = setDefault
     for envVar in environmentVariables {
       if let pathStr = ProcessEnv.vars[envVar],
-        let path = try? AbsolutePath(validating: pathStr),
-        let toolchain = try? registerToolchain(path, fileSystem),
-        shouldSetDefault
+        let path = try? AbsolutePath(validating: pathStr)
       {
-        shouldSetDefault = false
-        _default = toolchain
+        try? registerToolchain(path, fileSystem)
       }
     }
   }
@@ -285,7 +255,6 @@ extension ToolchainRegistry {
   ///
   /// - parameters:
   ///   - pathVariables: A list of PATH-like environment variable names to search.
-  ///   - setDefault: If true, the first toolchain found will be set as the default.
   @_spi(Testing)
   public func scanForToolchains(pathVariables: [String], _ fileSystem: FileSystem = localFileSystem) {
     pathVariables.lazy.flatMap { envVar in

Tests/SKCoreTests/ToolchainRegistryTests.swift

@@ -20,10 +20,6 @@ import XCTest
 import enum PackageLoading.Platform
 
 extension ToolchainRegistry {
-  func setDefault(_ newDefault: Toolchain?) {
-    self.default = newDefault
-  }
-
   func setDarwinToolchainOverride(_ newValue: String?) {
     self.darwinToolchainOverride = newValue
   }
@@ -38,10 +34,6 @@ final class ToolchainRegistryTests: XCTestCase {
     let b = Toolchain(identifier: "b", displayName: "b", path: nil)
     try await tr.registerToolchain(b)
     await assertEqual(tr.default?.identifier, "a")
-    await tr.setDefault(b)
-    await assertEqual(tr.default?.identifier, "b")
-    await tr.setDefault(nil)
-    await assertEqual(tr.default?.identifier, "a")
     await assertTrue(tr.default === tr.toolchain(identifier: "a"))
   }
 
@@ -59,10 +51,6 @@ final class ToolchainRegistryTests: XCTestCase {
       Toolchain(identifier: ToolchainRegistry.darwinDefaultToolchainIdentifier, displayName: "a", path: nil)
     )
     await assertEqual(tr.default?.identifier, ToolchainRegistry.darwinDefaultToolchainIdentifier)
-    await tr.setDefault(a)
-    await assertEqual(tr.default?.identifier, "a")
-    await tr.setDefault(nil)
-    await assertEqual(tr.default?.identifier, ToolchainRegistry.darwinDefaultToolchainIdentifier)
   }
 
   func testUnknownPlatform() throws {
@@ -322,7 +310,6 @@ final class ToolchainRegistryTests: XCTestCase {
 
     await tr.scanForToolchains(
       environmentVariables: ["TEST_ENV_SOURCEKIT_TOOLCHAIN_PATH_2"],
-      setDefault: false,
       fs
     )
 
@@ -388,8 +375,8 @@ final class ToolchainRegistryTests: XCTestCase {
       XCTAssertNotNil(t2.swiftc)
 
       let tr = ToolchainRegistry.empty
-      let t3 = try await tr.registerToolchain(path.parentDirectory, fs)
-      XCTAssertEqual(t3.identifier, t2.identifier)
+      try await tr.registerToolchain(path.parentDirectory, fs)
+      let t3 = try await unwrap(tr.toolchain(identifier: t2.identifier))
       XCTAssertEqual(t3.sourcekitd, t2.sourcekitd)
       XCTAssertEqual(t3.clang, t2.clang)
       XCTAssertEqual(t3.clangd, t2.clangd)