Skip to content

Commit d2a0167

Browse files
AndrewHoosAndrewHoos
authored
Replace withCheckedThrowingContinuation with native async/await (#7860)
### Motivation: Replace callback and continuation usage with `async`/`await`. ### Modifications: Replaced calls to `withCheckedThrowingContinuation` on APIs that have `async` alternatives. Replaced 3 completion handler APIs in swift-bootstrap making it fully `async`/`await`. ### Result: 11 fewer continuation usages. 1 fewer unstructured `Task`s. 3 fewer completion handler APIs.
1 parent 476ec77 commit d2a0167

File tree

9 files changed

+130
-174
lines changed

9 files changed

+130
-174
lines changed

‎Sources/Build/BuildOperation.swift

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -556,16 +556,14 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
556556
preparationStepName: "Compiling plugin \(plugin.moduleName)",
557557
progressTracker: self.current?.tracker
558558
)
559-
let result = try await withCheckedThrowingContinuation {
560-
pluginConfiguration.scriptRunner.compilePluginScript(
561-
sourceFiles: plugin.sources.paths,
562-
pluginName: plugin.moduleName,
563-
toolsVersion: plugin.toolsVersion,
564-
observabilityScope: self.observabilityScope,
565-
callbackQueue: DispatchQueue.sharedConcurrent,
566-
delegate: delegate,
567-
completion: $0.resume(with:))
568-
}
559+
let result = try await pluginConfiguration.scriptRunner.compilePluginScript(
560+
sourceFiles: plugin.sources.paths,
561+
pluginName: plugin.moduleName,
562+
toolsVersion: plugin.toolsVersion,
563+
observabilityScope: self.observabilityScope,
564+
callbackQueue: DispatchQueue.sharedConcurrent,
565+
delegate: delegate
566+
)
569567

570568
// Throw an error on failure; we will already have emitted the compiler's output in this case.
571569
if !result.succeeded {

‎Sources/Build/BuildPlan/BuildPlan.swift

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -800,32 +800,30 @@ extension BuildPlan {
800800
pluginDerivedResources = []
801801
}
802802

803-
let result = try await withCheckedThrowingContinuation {
804-
pluginModule.invoke(
805-
module: plugin,
806-
action: .createBuildToolCommands(
807-
package: package,
808-
target: module,
809-
pluginGeneratedSources: pluginDerivedSources.paths,
810-
pluginGeneratedResources: pluginDerivedResources.map(\.path)
811-
),
812-
buildEnvironment: buildParameters.buildEnvironment,
813-
scriptRunner: configuration.scriptRunner,
814-
workingDirectory: package.path,
815-
outputDirectory: pluginOutputDir,
816-
toolSearchDirectories: [buildParameters.toolchain.swiftCompilerPath.parentDirectory],
817-
accessibleTools: accessibleTools,
818-
writableDirectories: writableDirectories,
819-
readOnlyDirectories: readOnlyDirectories,
820-
allowNetworkConnections: [],
821-
pkgConfigDirectories: pkgConfigDirectories,
822-
sdkRootPath: buildParameters.toolchain.sdkRootPath,
823-
fileSystem: fileSystem,
824-
modulesGraph: modulesGraph,
825-
observabilityScope: observabilityScope,
826-
completion: $0.resume(with:)
827-
)
828-
}
803+
let result = try await pluginModule.invoke(
804+
module: plugin,
805+
action: .createBuildToolCommands(
806+
package: package,
807+
target: module,
808+
pluginGeneratedSources: pluginDerivedSources.paths,
809+
pluginGeneratedResources: pluginDerivedResources.map(\.path)
810+
),
811+
buildEnvironment: buildParameters.buildEnvironment,
812+
scriptRunner: configuration.scriptRunner,
813+
workingDirectory: package.path,
814+
outputDirectory: pluginOutputDir,
815+
toolSearchDirectories: [buildParameters.toolchain.swiftCompilerPath.parentDirectory],
816+
accessibleTools: accessibleTools,
817+
writableDirectories: writableDirectories,
818+
readOnlyDirectories: readOnlyDirectories,
819+
allowNetworkConnections: [],
820+
pkgConfigDirectories: pkgConfigDirectories,
821+
sdkRootPath: buildParameters.toolchain.sdkRootPath,
822+
fileSystem: fileSystem,
823+
modulesGraph: modulesGraph,
824+
observabilityScope: observabilityScope
825+
)
826+
829827

830828
if surfaceDiagnostics {
831829
let diagnosticsEmitter = observabilityScope.makeDiagnosticsEmitter {

‎Sources/Commands/PackageCommands/PluginCommand.swift

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ struct PluginCommand: AsyncSwiftCommand {
361361
let allowNetworkConnectionsCopy = allowNetworkConnections
362362

363363
let buildEnvironment = buildParameters.buildEnvironment
364-
let _ = try await withCheckedThrowingContinuation { pluginTarget.invoke(
364+
let _ = try await pluginTarget.invoke(
365365
action: .performCommand(package: package, arguments: arguments),
366366
buildEnvironment: buildEnvironment,
367367
scriptRunner: pluginScriptRunner,
@@ -378,9 +378,8 @@ struct PluginCommand: AsyncSwiftCommand {
378378
modulesGraph: packageGraph,
379379
observabilityScope: swiftCommandState.observabilityScope,
380380
callbackQueue: delegateQueue,
381-
delegate: pluginDelegate,
382-
completion: $0.resume(with:)
383-
) }
381+
delegate: pluginDelegate
382+
)
384383

385384
// TODO: We should also emit a final line of output regarding the result.
386385
}

‎Sources/Commands/SwiftTestCommand.swift

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -673,13 +673,10 @@ extension SwiftTestCommand {
673673
func printCodeCovPath(_ swiftCommandState: SwiftCommandState) async throws {
674674
let workspace = try swiftCommandState.getActiveWorkspace()
675675
let root = try swiftCommandState.getWorkspaceRoot()
676-
let rootManifests = try await withCheckedThrowingContinuation {
677-
workspace.loadRootManifests(
678-
packages: root.packages,
679-
observabilityScope: swiftCommandState.observabilityScope,
680-
completion: $0.resume(with: )
681-
)
682-
}
676+
let rootManifests = try await workspace.loadRootManifests(
677+
packages: root.packages,
678+
observabilityScope: swiftCommandState.observabilityScope
679+
)
683680
guard let rootManifest = rootManifests.values.first else {
684681
throw StringError("invalid manifests at \(root.packages)")
685682
}

‎Sources/CoreCommands/SwiftCommandState.swift

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -486,13 +486,10 @@ public final class SwiftCommandState {
486486
public func getRootPackageInformation() async throws -> (dependencies: [PackageIdentity: [PackageIdentity]], targets: [PackageIdentity: [String]]) {
487487
let workspace = try self.getActiveWorkspace()
488488
let root = try self.getWorkspaceRoot()
489-
let rootManifests = try await withCheckedThrowingContinuation {
490-
workspace.loadRootManifests(
491-
packages: root.packages,
492-
observabilityScope: self.observabilityScope,
493-
completion: $0.resume(with:)
494-
)
495-
}
489+
let rootManifests = try await workspace.loadRootManifests(
490+
packages: root.packages,
491+
observabilityScope: self.observabilityScope
492+
)
496493

497494
var identities = [PackageIdentity: [PackageIdentity]]()
498495
var targets = [PackageIdentity: [String]]()

‎Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift

Lines changed: 53 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -490,9 +490,7 @@ public struct PubGrubDependencyResolver {
490490

491491
// If decision making determines that no more decisions are to be
492492
// made, it returns nil to signal that version solving is done.
493-
next = try await withCheckedThrowingContinuation {
494-
self.makeDecision(state: state, completion: $0.resume(with:))
495-
}
493+
next = try await self.makeDecision(state: state)
496494
}
497495
}
498496

@@ -691,77 +689,70 @@ public struct PubGrubDependencyResolver {
691689
}
692690

693691
internal func makeDecision(
694-
state: State,
695-
completion: @escaping (Result<DependencyResolutionNode?, Error>) -> Void
696-
) {
692+
state: State
693+
) async throws -> DependencyResolutionNode? {
697694
// If there are no more undecided terms, version solving is complete.
698695
let undecided = state.solution.undecided
699696
guard !undecided.isEmpty else {
700-
return completion(.success(nil))
697+
return nil
701698
}
702699

703700
// Prefer packages with least number of versions that fit the current requirements so we
704701
// get conflicts (if any) sooner.
705-
Task {
706-
do {
707-
let start = DispatchTime.now()
708-
let counts = try await self.computeCounts(for: undecided)
709-
// forced unwraps safe since we are testing for count and errors above
710-
let pkgTerm = undecided.min {
711-
// Prefer packages that don't allow pre-release versions
712-
// to allow propagation logic to find dependencies that
713-
// limit the range before making any decisions. This means
714-
// that we'd always prefer release versions.
715-
if $0.supportsPrereleases != $1.supportsPrereleases {
716-
return !$0.supportsPrereleases
717-
}
718-
719-
return counts[$0]! < counts[$1]!
720-
}!
721-
self.delegate?.willResolve(term: pkgTerm)
722-
// at this point the container is cached
723-
let container = try self.provider.getCachedContainer(for: pkgTerm.node.package)
724-
725-
// Get the best available version for this package.
726-
guard let version = try await container.getBestAvailableVersion(for: pkgTerm) else {
727-
state.addIncompatibility(try Incompatibility(pkgTerm, root: state.root, cause: .noAvailableVersion), at: .decisionMaking)
728-
return completion(.success(pkgTerm.node))
729-
}
730-
731-
// Add all of this version's dependencies as incompatibilities.
732-
let depIncompatibilities = try await container.incompatibilites(
733-
at: version,
734-
node: pkgTerm.node,
735-
overriddenPackages: state.overriddenPackages,
736-
root: state.root
737-
)
702+
let start = DispatchTime.now()
703+
let counts = try await self.computeCounts(for: undecided)
704+
// forced unwraps safe since we are testing for count and errors above
705+
let pkgTerm = undecided.min {
706+
// Prefer packages that don't allow pre-release versions
707+
// to allow propagation logic to find dependencies that
708+
// limit the range before making any decisions. This means
709+
// that we'd always prefer release versions.
710+
if $0.supportsPrereleases != $1.supportsPrereleases {
711+
return !$0.supportsPrereleases
712+
}
713+
714+
return counts[$0]! < counts[$1]!
715+
}!
716+
self.delegate?.willResolve(term: pkgTerm)
717+
// at this point the container is cached
718+
let container = try self.provider.getCachedContainer(for: pkgTerm.node.package)
719+
720+
// Get the best available version for this package.
721+
guard let version = try await container.getBestAvailableVersion(for: pkgTerm) else {
722+
state.addIncompatibility(try Incompatibility(pkgTerm, root: state.root, cause: .noAvailableVersion), at: .decisionMaking)
723+
return pkgTerm.node
724+
}
738725

739-
var haveConflict = false
740-
for incompatibility in depIncompatibilities {
741-
// Add the incompatibility to our partial solution.
742-
state.addIncompatibility(incompatibility, at: .decisionMaking)
743-
744-
// Check if this incompatibility will satisfy the solution.
745-
haveConflict = haveConflict || incompatibility.terms.allSatisfy {
746-
// We only need to check if the terms other than this package
747-
// are satisfied because we _know_ that the terms matching
748-
// this package will be satisfied if we make this version
749-
// as a decision.
750-
$0.node == pkgTerm.node || state.solution.satisfies($0)
751-
}
752-
}
726+
// Add all of this version's dependencies as incompatibilities.
727+
let depIncompatibilities = try await container.incompatibilites(
728+
at: version,
729+
node: pkgTerm.node,
730+
overriddenPackages: state.overriddenPackages,
731+
root: state.root
732+
)
753733

754-
// Decide this version if there was no conflict with its dependencies.
755-
if !haveConflict {
756-
self.delegate?.didResolve(term: pkgTerm, version: version, duration: start.distance(to: .now()))
757-
state.decide(pkgTerm.node, at: version)
758-
}
734+
var haveConflict = false
735+
for incompatibility in depIncompatibilities {
736+
// Add the incompatibility to our partial solution.
737+
state.addIncompatibility(incompatibility, at: .decisionMaking)
759738

760-
completion(.success(pkgTerm.node))
761-
} catch {
762-
completion(.failure(error))
739+
// Check if this incompatibility will satisfy the solution.
740+
haveConflict = haveConflict || incompatibility.terms.allSatisfy {
741+
// We only need to check if the terms other than this package
742+
// are satisfied because we _know_ that the terms matching
743+
// this package will be satisfied if we make this version
744+
// as a decision.
745+
$0.node == pkgTerm.node || state.solution.satisfies($0)
763746
}
764747
}
748+
749+
// Decide this version if there was no conflict with its dependencies.
750+
if !haveConflict {
751+
self.delegate?.didResolve(term: pkgTerm, version: version, duration: start.distance(to: .now()))
752+
state.decide(pkgTerm.node, at: version)
753+
}
754+
755+
return pkgTerm.node
765756
}
766757
}
767758

‎Sources/_InternalTestSupport/MockWorkspace.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -779,9 +779,7 @@ public final class MockWorkspace {
779779
let rootInput = PackageGraphRootInput(
780780
packages: try rootPaths(for: roots), dependencies: dependencies
781781
)
782-
let rootManifests = try await withCheckedThrowingContinuation {
783-
workspace.loadRootManifests(packages: rootInput.packages, observabilityScope: observability.topScope, completion: $0.resume(with:))
784-
}
782+
let rootManifests = try await workspace.loadRootManifests(packages: rootInput.packages, observabilityScope: observability.topScope)
785783
let graphRoot = PackageGraphRoot(input: rootInput, manifests: rootManifests, observabilityScope: observability.topScope)
786784
let manifests = try await workspace.loadDependencyManifests(root: graphRoot, observabilityScope: observability.topScope)
787785
result(manifests, observability.diagnostics)

0 commit comments

Comments
 (0)