Skip to content

Commit cf7b2c6

Browse files
committed
Use Swift Concurrency in SwiftcImportScanner
This allows `Workspace.loadPluginImports` to be implemented in a non-blocking way without any uses of `temp_await`. Also a dependency of #6624.
1 parent 4714ea9 commit cf7b2c6

File tree

5 files changed

+73
-86
lines changed

5 files changed

+73
-86
lines changed

Sources/Basics/ImportScanning.swift

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,7 @@ private struct Imports: Decodable {
2323
}
2424

2525
public protocol ImportScanner {
26-
func scanImports(
27-
_ filePathToScan: AbsolutePath,
28-
callbackQueue: DispatchQueue,
29-
completion: @escaping (Result<[String], Error>) -> Void
30-
)
26+
func scanImports(_ filePathToScan: AbsolutePath) async throws -> [String]
3127
}
3228

3329
public struct SwiftcImportScanner: ImportScanner {
@@ -45,32 +41,15 @@ public struct SwiftcImportScanner: ImportScanner {
4541
self.swiftCompilerPath = swiftCompilerPath
4642
}
4743

48-
public func scanImports(
49-
_ filePathToScan: AbsolutePath,
50-
callbackQueue: DispatchQueue,
51-
completion: @escaping (Result<[String], Error>) -> Void
52-
) {
44+
public func scanImports(_ filePathToScan: AbsolutePath) async throws -> [String] {
5345
let cmd = [swiftCompilerPath.pathString,
5446
filePathToScan.pathString,
5547
"-scan-dependencies", "-Xfrontend", "-import-prescan"] + self.swiftCompilerFlags
5648

57-
TSCBasic.Process
58-
.popen(arguments: cmd, environment: self.swiftCompilerEnvironment, queue: callbackQueue) { result in
59-
dispatchPrecondition(condition: .onQueue(callbackQueue))
60-
61-
do {
62-
let stdout = try result.get().utf8Output()
63-
let imports = try JSONDecoder.makeWithDefaults().decode(Imports.self, from: stdout).imports
64-
.filter { !defaultImports.contains($0) }
49+
let result = try await TSCBasic.Process.popen(arguments: cmd, environment: self.swiftCompilerEnvironment)
6550

66-
callbackQueue.async {
67-
completion(.success(imports))
68-
}
69-
} catch {
70-
callbackQueue.async {
71-
completion(.failure(error))
72-
}
73-
}
74-
}
51+
let stdout = try result.utf8Output()
52+
return try JSONDecoder.makeWithDefaults().decode(Imports.self, from: stdout).imports
53+
.filter { !defaultImports.contains($0) }
7554
}
7655
}

Sources/PackageLoading/ManifestLoader.swift

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
import _Concurrency
1314
import Basics
1415
import Dispatch
1516
@_implementationOnly import Foundation
@@ -658,23 +659,19 @@ public final class ManifestLoader: ManifestLoaderProtocol {
658659

659660
// we must not block the calling thread (for concurrency control) so nesting this in a queue
660661
self.evaluationQueue.addOperation {
661-
do {
662-
// park the evaluation thread based on the max concurrency allowed
663-
self.concurrencySemaphore.wait()
662+
// park the evaluation thread based on the max concurrency allowed
663+
self.concurrencySemaphore.wait()
664664

665-
let importScanner = SwiftcImportScanner(swiftCompilerEnvironment: self.toolchain.swiftCompilerEnvironment,
666-
swiftCompilerFlags: self.extraManifestFlags,
667-
swiftCompilerPath: self.toolchain.swiftCompilerPathForManifests)
665+
let importScanner = SwiftcImportScanner(swiftCompilerEnvironment: self.toolchain.swiftCompilerEnvironment,
666+
swiftCompilerFlags: self.extraManifestFlags,
667+
swiftCompilerPath: self.toolchain.swiftCompilerPathForManifests)
668668

669-
importScanner.scanImports(manifestPath, callbackQueue: callbackQueue) { result in
670-
do {
671-
let imports = try result.get().filter { !allowedImports.contains($0) }
672-
guard imports.isEmpty else {
673-
throw ManifestParseError.importsRestrictedModules(imports)
674-
}
675-
} catch {
676-
completion(.failure(error))
677-
}
669+
Task {
670+
let result = try await importScanner.scanImports(manifestPath)
671+
let imports = result.filter { !allowedImports.contains($0) }
672+
guard imports.isEmpty else {
673+
completion(.failure(ManifestParseError.importsRestrictedModules(imports)))
674+
return
678675
}
679676
}
680677
}

Sources/SPMTestSupport/misc.swift

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,40 @@ import enum TSCUtility.Git
3232
@_exported import func TSCTestSupport.systemQuietly
3333
@_exported import enum TSCTestSupport.StringPattern
3434

35+
/// Test helper utility for executing a block with a temporary directory.
3536
public func testWithTemporaryDirectory(
3637
function: StaticString = #function,
3738
body: (AbsolutePath) throws -> Void
3839
) throws {
3940
let body2 = { (path: TSCAbsolutePath) in
4041
try body(AbsolutePath(path))
4142
}
43+
4244
try TSCTestSupport.testWithTemporaryDirectory(
4345
function: function,
4446
body: body2
4547
)
4648
}
4749

50+
public func testWithTemporaryDirectory(
51+
function: StaticString = #function,
52+
body: (AbsolutePath) async throws -> Void
53+
) async throws {
54+
let cleanedFunction = function.description
55+
.replacingOccurrences(of: "(", with: "")
56+
.replacingOccurrences(of: ")", with: "")
57+
.replacingOccurrences(of: ".", with: "")
58+
.replacingOccurrences(of: ":", with: "_")
59+
try await withTemporaryDirectory(prefix: "spm-tests-\(cleanedFunction)") { tmpDirPath in
60+
defer {
61+
// Unblock and remove the tmp dir on deinit.
62+
try? localFileSystem.chmod(.userWritable, path: tmpDirPath, options: [.recursive])
63+
try? localFileSystem.removeFileTree(tmpDirPath)
64+
}
65+
try await body(tmpDirPath)
66+
}
67+
}
68+
4869
/// Test-helper function that runs a block of code on a copy of a test fixture
4970
/// package. The copy is made into a temporary directory, and the block is
5071
/// given a path to that directory. The block is permitted to modify the copy.

Sources/Workspace/Workspace.swift

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1409,8 +1409,8 @@ extension Workspace {
14091409
}
14101410

14111411
public func loadPluginImports(
1412-
packageGraph: PackageGraph,
1413-
completion: @escaping(Result<[PackageIdentity: [String: [String]]], Error>) -> Void) {
1412+
packageGraph: PackageGraph
1413+
) async throws -> [PackageIdentity: [String: [String]]] {
14141414
let pluginTargets = packageGraph.allTargets.filter{$0.type == .plugin}
14151415
let scanner = SwiftcImportScanner(swiftCompilerEnvironment: hostToolchain.swiftCompilerEnvironment, swiftCompilerFlags: hostToolchain.swiftCompilerFlags + ["-I", hostToolchain.swiftPMLibrariesLocation.pluginLibraryPath.pathString], swiftCompilerPath: hostToolchain.swiftCompilerPath)
14161416
var importList = [PackageIdentity: [String: [String]]]()
@@ -1426,17 +1426,11 @@ extension Workspace {
14261426
}
14271427

14281428
for path in paths {
1429-
do {
1430-
let result = try temp_await {
1431-
scanner.scanImports(path, callbackQueue: DispatchQueue.sharedConcurrent, completion: $0)
1432-
}
1433-
importList[pkgId]?[pluginTarget.name]?.append(contentsOf: result)
1434-
} catch {
1435-
completion(.failure(error))
1436-
}
1429+
let result = try await scanner.scanImports(path)
1430+
importList[pkgId]?[pluginTarget.name]?.append(contentsOf: result)
14371431
}
14381432
}
1439-
completion(.success(importList))
1433+
return importList
14401434
}
14411435

14421436
/// Loads a single package in the context of a previously loaded graph. This can be useful for incremental loading in a longer-lived program, like an IDE.

Tests/SPMBuildCoreTests/PluginInvocationTests.swift

Lines changed: 29 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -916,11 +916,11 @@ class PluginInvocationTests: XCTestCase {
916916
}
917917
}
918918

919-
func testScanImportsInPluginTargets() throws {
919+
func testScanImportsInPluginTargets() async throws {
920920
// Only run the test if the environment in which we're running actually supports Swift concurrency (which the plugin APIs require).
921921
try XCTSkipIf(!UserToolchain.default.supportsSwiftConcurrency(), "skipping because test environment doesn't support concurrency")
922922

923-
try testWithTemporaryDirectory { tmpPath in
923+
try await testWithTemporaryDirectory { tmpPath in
924924
// Create a sample package with a library target and a plugin.
925925
let packageDir = tmpPath.appending(components: "MyPackage")
926926
try localFileSystem.createDirectory(packageDir, recursive: true)
@@ -1058,39 +1058,35 @@ class PluginInvocationTests: XCTestCase {
10581058
XCTAssert(rootManifests.count == 1, "\(rootManifests)")
10591059

10601060
let graph = try workspace.loadPackageGraph(rootInput: rootInput, observabilityScope: observability.topScope)
1061-
workspace.loadPluginImports(packageGraph: graph) { (result: Result<[PackageIdentity : [String : [String]]], Error>) in
1062-
1063-
var count = 0
1064-
if let dict = try? result.get() {
1065-
for (pkg, entry) in dict {
1066-
if pkg.description == "mypackage" {
1067-
XCTAssertNotNil(entry["XPlugin"])
1068-
let XPluginPossibleImports1 = ["PackagePlugin", "XcodeProjectPlugin"]
1069-
let XPluginPossibleImports2 = ["PackagePlugin", "XcodeProjectPlugin", "_SwiftConcurrencyShims"]
1070-
XCTAssertTrue(entry["XPlugin"] == XPluginPossibleImports1 ||
1071-
entry["XPlugin"] == XPluginPossibleImports2)
1072-
1073-
let YPluginPossibleImports1 = ["PackagePlugin", "Foundation"]
1074-
let YPluginPossibleImports2 = ["PackagePlugin", "Foundation", "_SwiftConcurrencyShims"]
1075-
XCTAssertTrue(entry["YPlugin"] == YPluginPossibleImports1 ||
1076-
entry["YPlugin"] == YPluginPossibleImports2)
1077-
count += 1
1078-
} else if pkg.description == "otherpackage" {
1079-
XCTAssertNotNil(dict[pkg]?["QPlugin"])
1080-
1081-
let possibleImports1 = ["PackagePlugin", "XcodeProjectPlugin", "ModuleFoundViaExtraSearchPaths"]
1082-
let possibleImports2 = ["PackagePlugin", "XcodeProjectPlugin", "ModuleFoundViaExtraSearchPaths", "_SwiftConcurrencyShims"]
1083-
XCTAssertTrue(entry["QPlugin"] == possibleImports1 ||
1084-
entry["QPlugin"] == possibleImports2)
1085-
count += 1
1086-
}
1087-
}
1088-
} else {
1089-
XCTFail("Scanned import list should not be empty")
1061+
let dict = try await workspace.loadPluginImports(packageGraph: graph)
1062+
1063+
var count = 0
1064+
for (pkg, entry) in dict {
1065+
if pkg.description == "mypackage" {
1066+
XCTAssertNotNil(entry["XPlugin"])
1067+
let XPluginPossibleImports1 = ["PackagePlugin", "XcodeProjectPlugin"]
1068+
let XPluginPossibleImports2 = ["PackagePlugin", "XcodeProjectPlugin", "_SwiftConcurrencyShims"]
1069+
XCTAssertTrue(entry["XPlugin"] == XPluginPossibleImports1 ||
1070+
entry["XPlugin"] == XPluginPossibleImports2)
1071+
1072+
let YPluginPossibleImports1 = ["PackagePlugin", "Foundation"]
1073+
let YPluginPossibleImports2 = ["PackagePlugin", "Foundation", "_SwiftConcurrencyShims"]
1074+
XCTAssertTrue(entry["YPlugin"] == YPluginPossibleImports1 ||
1075+
entry["YPlugin"] == YPluginPossibleImports2)
1076+
count += 1
1077+
} else if pkg.description == "otherpackage" {
1078+
XCTAssertNotNil(dict[pkg]?["QPlugin"])
1079+
1080+
let possibleImports1 = ["PackagePlugin", "XcodeProjectPlugin", "ModuleFoundViaExtraSearchPaths"]
1081+
let possibleImports2 = ["PackagePlugin", "XcodeProjectPlugin", "ModuleFoundViaExtraSearchPaths", "_SwiftConcurrencyShims"]
1082+
XCTAssertTrue(entry["QPlugin"] == possibleImports1 ||
1083+
entry["QPlugin"] == possibleImports2)
1084+
count += 1
10901085
}
1091-
1092-
XCTAssertEqual(count, 2)
10931086
}
1087+
1088+
1089+
XCTAssertEqual(count, 2)
10941090
}
10951091
}
10961092

0 commit comments

Comments
 (0)