Skip to content

Commit 6f342e2

Browse files
ahoppenahoppen
committed
Remove static shared toolchain registry
Global state is never a good thing and we needed to modify it in tests. The design becomes a lot cleaner if we explicitly pass the toolchain registry around.
1 parent 3e73f11 commit 6f342e2

File tree

11 files changed

+52
-62
lines changed

11 files changed

+52
-62
lines changed

‎Sources/SKCore/ToolchainRegistry.swift

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -53,32 +53,21 @@ public final actor ToolchainRegistry {
5353
}()
5454

5555
/// Creates an empty toolchain registry.
56-
public init() {}
56+
private init() {}
5757

58-
/// A task that produces the shared toolchain registry.
59-
private static var sharedRegistryTask: Task<ToolchainRegistry, Never> = Task {
60-
await ToolchainRegistry(localFileSystem)
61-
}
62-
63-
/// The global toolchain registry, initially populated by scanning for toolchains.
64-
///
65-
/// Scans for toolchains in:
66-
/// * env SOURCEKIT_TOOLCHAIN_PATH <-- will override default toolchain
67-
/// * (Darwin) The currently selected Xcode
68-
/// * (Darwin) [~]/Library/Developer/Toolchains
69-
/// * env SOURCEKIT_PATH, PATH
70-
public static var shared: ToolchainRegistry {
58+
/// A toolchain registry used for testing that scans for toolchains based on environment variables and Xcode
59+
/// installations but not next to the `sourcekit-lsp` binary because there is no `sourcekit-lsp` binary during
60+
/// testing.
61+
@_spi(Testing)
62+
public static var forTesting: ToolchainRegistry {
7163
get async {
72-
return await sharedRegistryTask.value
64+
await ToolchainRegistry(localFileSystem)
7365
}
7466
}
7567

76-
/// Set the globally shared toolchain registry.
77-
public static func setSharedToolchainRegistry(_ toolchainRegistry: ToolchainRegistry) {
78-
sharedRegistryTask = Task {
79-
toolchainRegistry
80-
}
81-
}
68+
/// A toolchain registry that doesn't contain any toolchains.
69+
@_spi(Testing)
70+
public static var empty: ToolchainRegistry { ToolchainRegistry() }
8271

8372
/// Creates a toolchain registry populated by scanning for toolchains according to the given paths
8473
/// and variables.
@@ -99,7 +88,6 @@ public final actor ToolchainRegistry {
9988
installPath: AbsolutePath? = nil,
10089
_ fileSystem: FileSystem
10190
) async {
102-
self.init()
10391
scanForToolchains(installPath: installPath, fileSystem)
10492
}
10593
}

‎Sources/SKTestSupport/IndexedSingleSwiftFileWorkspace.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import Foundation
1414
import ISDBTibs
1515
import LanguageServerProtocol
16-
import SKCore
16+
@_spi(Testing) import SKCore
1717
import SourceKitLSP
1818
import TSCBasic
1919

@@ -46,7 +46,7 @@ public struct IndexedSingleSwiftFileWorkspace {
4646
let testFileURL = testWorkspaceDirectory.appendingPathComponent("test.swift")
4747
let indexURL = testWorkspaceDirectory.appendingPathComponent("index")
4848
self.indexDBURL = testWorkspaceDirectory.appendingPathComponent("index-db")
49-
guard let swiftc = await ToolchainRegistry.shared.default?.swiftc?.asURL else {
49+
guard let swiftc = await ToolchainRegistry.forTesting.default?.swiftc?.asURL else {
5050
throw Error.swiftcNotFound
5151
}
5252

‎Sources/SKTestSupport/SwiftPMTestWorkspace.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
import Foundation
1414
import LanguageServerProtocol
15-
import SKCore
15+
@_spi(Testing) import SKCore
1616
import TSCBasic
1717

1818
public class SwiftPMTestWorkspace: MultiFileTestWorkspace {
@@ -72,7 +72,7 @@ public class SwiftPMTestWorkspace: MultiFileTestWorkspace {
7272

7373
/// Build a SwiftPM package package manifest is located in the directory at `path`.
7474
public static func build(at path: URL) async throws {
75-
guard let swift = await ToolchainRegistry.shared.default?.swift?.asURL else {
75+
guard let swift = await ToolchainRegistry.forTesting.default?.swift?.asURL else {
7676
throw Error.swiftNotFound
7777
}
7878
let arguments = [

‎Sources/SKTestSupport/TestSourceKitLSPClient.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import Foundation
1414
import LSPTestSupport
1515
import LanguageServerProtocol
1616
import LanguageServerProtocolJSONRPC
17-
import SKCore
17+
@_spi(Testing) import SKCore
1818
import SKSupport
1919
import SourceKitLSP
2020
import SwiftSyntax
@@ -107,6 +107,7 @@ public final class TestSourceKitLSPClient: MessageHandler {
107107
self.serverToClientConnection = clientConnection
108108
server = await SourceKitServer(
109109
client: clientConnection,
110+
toolchainRegistry: ToolchainRegistry.forTesting,
110111
options: serverOptions,
111112
onExit: {
112113
clientConnection.close()

‎Sources/SourceKitLSP/SourceKitServer.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -439,11 +439,12 @@ public actor SourceKitServer {
439439
public init(
440440
client: Connection,
441441
fileSystem: FileSystem = localFileSystem,
442+
toolchainRegistry: ToolchainRegistry,
442443
options: Options,
443444
onExit: @escaping () -> Void = {}
444-
) async {
445+
) {
445446
self.fs = fileSystem
446-
self.toolchainRegistry = await ToolchainRegistry.shared
447+
self.toolchainRegistry = toolchainRegistry
447448
self.options = options
448449
self.onExit = onExit
449450

‎Sources/sourcekit-lsp/SourceKitLSP.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,10 +241,10 @@ struct SourceKitLSP: ParsableCommand {
241241
)
242242

243243
let installPath = try AbsolutePath(validating: Bundle.main.bundlePath)
244-
await ToolchainRegistry.setSharedToolchainRegistry(ToolchainRegistry(installPath: installPath, localFileSystem))
245244

246245
let server = await SourceKitServer(
247246
client: clientConnection,
247+
toolchainRegistry: ToolchainRegistry(installPath: installPath, localFileSystem),
248248
options: mapOptions(),
249249
onExit: {
250250
clientConnection.close()

‎Tests/SKCoreTests/ToolchainRegistryTests.swift

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ extension ToolchainRegistry {
3030

3131
final class ToolchainRegistryTests: XCTestCase {
3232
func testDefaultBasic() async throws {
33-
let tr = ToolchainRegistry()
33+
let tr = ToolchainRegistry.empty
3434
await assertNil(tr.default)
3535
try await tr.registerToolchain(Toolchain(identifier: "a", displayName: "a", path: nil))
3636
await assertEqual(tr.default?.identifier, "a")
@@ -49,7 +49,7 @@ final class ToolchainRegistryTests: XCTestCase {
4949
defer { Platform.current = prevPlatform }
5050
Platform.current = .darwin
5151

52-
let tr = ToolchainRegistry()
52+
let tr = ToolchainRegistry.empty
5353
await tr.setDarwinToolchainOverride(nil)
5454
await assertNil(tr.default)
5555
let a = Toolchain(identifier: "a", displayName: "a", path: nil)
@@ -204,7 +204,7 @@ final class ToolchainRegistryTests: XCTestCase {
204204
XCTAssertEqual(tc?.path, tcBin?.path)
205205
XCTAssertEqual(tc?.displayName, tcBin?.displayName)
206206

207-
let trInstall = ToolchainRegistry()
207+
let trInstall = ToolchainRegistry.empty
208208
await trInstall.scanForToolchains(
209209
installPath: path.appending(components: "usr", "bin"),
210210
environmentVariables: [],
@@ -221,7 +221,7 @@ final class ToolchainRegistryTests: XCTestCase {
221221
await assertEqual(overrideReg.darwinToolchainIdentifier, "org.fake.global.B")
222222
await assertEqual(overrideReg.default?.identifier, "org.fake.global.B")
223223

224-
let checkByDir = ToolchainRegistry()
224+
let checkByDir = ToolchainRegistry.empty
225225
await checkByDir.scanForToolchains(xctoolchainSearchPath: toolchains, fs)
226226
await assertEqual(checkByDir.toolchains.count, 4)
227227
#endif
@@ -386,7 +386,7 @@ final class ToolchainRegistryTests: XCTestCase {
386386
XCTAssertNotNil(t2.clangd)
387387
XCTAssertNotNil(t2.swiftc)
388388

389-
let tr = ToolchainRegistry()
389+
let tr = ToolchainRegistry.empty
390390
let t3 = try await tr.registerToolchain(path.parentDirectory, fs)
391391
XCTAssertEqual(t3.identifier, t2.identifier)
392392
XCTAssertEqual(t3.sourcekitd, t2.sourcekitd)
@@ -426,7 +426,7 @@ final class ToolchainRegistryTests: XCTestCase {
426426
}
427427

428428
func testDuplicateError() async throws {
429-
let tr = ToolchainRegistry()
429+
let tr = ToolchainRegistry.empty
430430
let toolchain = Toolchain(identifier: "a", displayName: "a", path: nil)
431431
try await tr.registerToolchain(toolchain)
432432
await assertThrowsError(
@@ -441,7 +441,7 @@ final class ToolchainRegistryTests: XCTestCase {
441441
}
442442

443443
func testDuplicatePathError() async throws {
444-
let tr = ToolchainRegistry()
444+
let tr = ToolchainRegistry.empty
445445
let path = try AbsolutePath(validating: "/foo/bar")
446446
let first = Toolchain(identifier: "a", displayName: "a", path: path)
447447
let second = Toolchain(identifier: "b", displayName: "b", path: path)
@@ -458,7 +458,7 @@ final class ToolchainRegistryTests: XCTestCase {
458458
}
459459

460460
func testDuplicateXcodeError() async throws {
461-
let tr = ToolchainRegistry()
461+
let tr = ToolchainRegistry.empty
462462
let xcodeToolchain = Toolchain(
463463
identifier: ToolchainRegistry.darwinDefaultToolchainIdentifier,
464464
displayName: "a",
@@ -477,7 +477,7 @@ final class ToolchainRegistryTests: XCTestCase {
477477
}
478478

479479
func testMultipleXcodes() async throws {
480-
let tr = ToolchainRegistry()
480+
let tr = ToolchainRegistry.empty
481481
let pathA = try AbsolutePath(validating: "/versionA")
482482
let xcodeA = Toolchain(
483483
identifier: ToolchainRegistry.darwinDefaultToolchainIdentifier,
@@ -523,7 +523,7 @@ final class ToolchainRegistryTests: XCTestCase {
523523

524524
try ProcessEnv.setVar("TEST_SOURCEKIT_TOOLCHAIN_PATH_1", value: "/t2/bin")
525525

526-
let tr = ToolchainRegistry()
526+
let tr = ToolchainRegistry.empty
527527
await tr.scanForToolchains(
528528
installPath: try AbsolutePath(validating: "/t1/bin"),
529529
environmentVariables: ["TEST_SOURCEKIT_TOOLCHAIN_PATH_1"],

‎Tests/SKSwiftPMWorkspaceTests/SwiftPMWorkspaceTests.swift

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import Build
1414
import LSPTestSupport
1515
import LanguageServerProtocol
1616
import PackageModel
17-
import SKCore
17+
@_spi(Testing) import SKCore
1818
import SKSwiftPMWorkspace
1919
import SKTestSupport
2020
import SourceKitLSP
@@ -39,7 +39,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
3939
]
4040
)
4141
let packageRoot = tempDir.appending(component: "pkg")
42-
let tr = await ToolchainRegistry.shared
42+
let tr = await ToolchainRegistry.forTesting
4343
await assertThrowsError(
4444
try await SwiftPMWorkspace(
4545
workspacePath: packageRoot,
@@ -66,7 +66,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
6666
]
6767
)
6868
let packageRoot = tempDir.appending(component: "pkg")
69-
let tr = await ToolchainRegistry.shared
69+
let tr = await ToolchainRegistry.forTesting
7070
await assertThrowsError(
7171
try await SwiftPMWorkspace(
7272
workspacePath: packageRoot,
@@ -97,7 +97,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
9797
await assertThrowsError(
9898
try await SwiftPMWorkspace(
9999
workspacePath: packageRoot,
100-
toolchainRegistry: ToolchainRegistry(),
100+
toolchainRegistry: ToolchainRegistry.empty,
101101
fileSystem: fs,
102102
buildSetup: SourceKitServer.Options.testDefault.buildSetup
103103
)
@@ -122,7 +122,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
122122
]
123123
)
124124
let packageRoot = try resolveSymlinks(tempDir.appending(component: "pkg"))
125-
let tr = await ToolchainRegistry.shared
125+
let tr = await ToolchainRegistry.forTesting
126126
let ws = try await SwiftPMWorkspace(
127127
workspacePath: packageRoot,
128128
toolchainRegistry: tr,
@@ -182,7 +182,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
182182
]
183183
)
184184
let packageRoot = tempDir.appending(component: "pkg")
185-
let tr = await ToolchainRegistry.shared
185+
let tr = await ToolchainRegistry.forTesting
186186

187187
let config = BuildSetup(
188188
configuration: .release,
@@ -228,7 +228,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
228228
]
229229
)
230230
let packageRoot = tempDir.appending(component: "pkg")
231-
let tr = await ToolchainRegistry.shared
231+
let tr = await ToolchainRegistry.forTesting
232232
let ws = try await SwiftPMWorkspace(
233233
workspacePath: packageRoot,
234234
toolchainRegistry: tr,
@@ -262,7 +262,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
262262
]
263263
)
264264
let packageRoot = try resolveSymlinks(tempDir.appending(component: "pkg"))
265-
let tr = await ToolchainRegistry.shared
265+
let tr = await ToolchainRegistry.forTesting
266266
let ws = try await SwiftPMWorkspace(
267267
workspacePath: packageRoot,
268268
toolchainRegistry: tr,
@@ -306,7 +306,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
306306
]
307307
)
308308
let packageRoot = try resolveSymlinks(tempDir.appending(component: "pkg"))
309-
let tr = await ToolchainRegistry.shared
309+
let tr = await ToolchainRegistry.forTesting
310310
let ws = try await SwiftPMWorkspace(
311311
workspacePath: packageRoot,
312312
toolchainRegistry: tr,
@@ -368,7 +368,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
368368
]
369369
)
370370
let packageRoot = tempDir.appending(component: "pkg")
371-
let tr = await ToolchainRegistry.shared
371+
let tr = await ToolchainRegistry.forTesting
372372
let ws = try await SwiftPMWorkspace(
373373
workspacePath: packageRoot,
374374
toolchainRegistry: tr,
@@ -404,7 +404,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
404404
]
405405
)
406406
let packageRoot = try resolveSymlinks(tempDir.appending(component: "pkg"))
407-
let tr = await ToolchainRegistry.shared
407+
let tr = await ToolchainRegistry.forTesting
408408
let ws = try await SwiftPMWorkspace(
409409
workspacePath: packageRoot,
410410
toolchainRegistry: tr,
@@ -499,7 +499,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
499499
let packageRoot = tempDir.appending(component: "pkg")
500500
let ws = try await SwiftPMWorkspace(
501501
workspacePath: packageRoot,
502-
toolchainRegistry: ToolchainRegistry.shared,
502+
toolchainRegistry: ToolchainRegistry.forTesting,
503503
fileSystem: fs,
504504
buildSetup: SourceKitServer.Options.testDefault.buildSetup
505505
)
@@ -544,7 +544,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
544544
withDestinationURL: URL(fileURLWithPath: tempDir.appending(component: "pkg_real").pathString)
545545
)
546546

547-
let tr = await ToolchainRegistry.shared
547+
let tr = await ToolchainRegistry.forTesting
548548
let ws = try await SwiftPMWorkspace(
549549
workspacePath: packageRoot,
550550
toolchainRegistry: tr,
@@ -603,7 +603,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
603603
withDestinationURL: URL(fileURLWithPath: tempDir.appending(component: "pkg_real").pathString)
604604
)
605605

606-
let tr = await ToolchainRegistry.shared
606+
let tr = await ToolchainRegistry.forTesting
607607
let ws = try await SwiftPMWorkspace(
608608
workspacePath: packageRoot,
609609
toolchainRegistry: tr,
@@ -648,7 +648,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
648648
]
649649
)
650650
let packageRoot = try resolveSymlinks(tempDir.appending(component: "pkg"))
651-
let tr = await ToolchainRegistry.shared
651+
let tr = await ToolchainRegistry.forTesting
652652
let ws = try await SwiftPMWorkspace(
653653
workspacePath: packageRoot,
654654
toolchainRegistry: tr,
@@ -684,7 +684,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
684684
]
685685
)
686686
let packageRoot = try resolveSymlinks(tempDir.appending(components: "pkg", "Sources", "lib"))
687-
let tr = await ToolchainRegistry.shared
687+
let tr = await ToolchainRegistry.forTesting
688688
let ws = try await SwiftPMWorkspace(
689689
workspacePath: packageRoot,
690690
toolchainRegistry: tr,

‎Tests/SourceKitDTests/SourceKitDTests.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import ISDBTestSupport
1515
import ISDBTibs
1616
import LSPTestSupport
1717
import LanguageServerProtocol
18-
import SKCore
18+
@_spi(Testing) import SKCore
1919
import SKSupport
2020
import SKTestSupport
2121
import SourceKitD
@@ -27,7 +27,7 @@ import class TSCBasic.Process
2727

2828
final class SourceKitDTests: XCTestCase {
2929
func testMultipleNotificationHandlers() async throws {
30-
let sourcekitdPath = await ToolchainRegistry.shared.default!.sourcekitd!
30+
let sourcekitdPath = await ToolchainRegistry.forTesting.default!.sourcekitd!
3131
let sourcekitd = try SourceKitDImpl.getOrCreate(dylibPath: sourcekitdPath)
3232
let keys = sourcekitd.keys
3333
let path = DocumentURI.for(.swift).pseudoPath

0 commit comments

Comments
 (0)