Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert ManifestLoader.evaluateManifest to async #8104

Draft
wants to merge 34 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
3863edc
Clean up binary artifacts code
MaxDesiatov Oct 30, 2024
1c5af4a
`HTTPClient` in `BinaryArtifacts` WIP
MaxDesiatov Oct 30, 2024
a913419
Update `WorkspaceTests.swift`
MaxDesiatov Oct 30, 2024
f60977c
Minor fixups
MaxDesiatov Oct 31, 2024
4914d9a
Address remaining warnings in `Workspace+BinaryArtifacts.swift`
MaxDesiatov Oct 31, 2024
291705a
Break up long line for nicer formatting
MaxDesiatov Oct 31, 2024
8250523
async `RegistryClient` WIP
MaxDesiatov Oct 29, 2024
0db7971
`LegacyHTTPClient` removal WIP
MaxDesiatov Oct 30, 2024
dd35171
Fix one use of `HTTPClient` in `RegistryClientTests`
MaxDesiatov Oct 31, 2024
238fd15
Fix build errors
MaxDesiatov Nov 1, 2024
6731eb5
Fix recursion deadlock in `RegistryPackageContainer`, cleanup warnings
MaxDesiatov Nov 1, 2024
085bad1
Add async `withLock` to `InMemoryFileSystem`
MaxDesiatov Nov 1, 2024
7cc81af
Make sure `Workspace.loadRootManifests` doesn't throw
MaxDesiatov Nov 1, 2024
ad8b612
Don't duplicate `Diagnostics.fatalError` in diagnostics
MaxDesiatov Nov 1, 2024
4c00304
Fix `InMemoryFileSystem.withLock` deadlock
MaxDesiatov Nov 1, 2024
5890073
Fix formatting in `Workspace+Manifests.swift`
MaxDesiatov Nov 1, 2024
d91c0f8
Document new `ThreadSafeBox.mutate` overload
MaxDesiatov Nov 4, 2024
c12aff1
Fix remaining `WorkspaceRegistryTests` failures
MaxDesiatov Nov 4, 2024
4f443d0
Clean up formatting
MaxDesiatov Nov 4, 2024
c204af2
Fix up formatting
MaxDesiatov Nov 4, 2024
80f54a8
Merge branch 'main' of github.com:swiftlang/swift-package-manager int…
MaxDesiatov Nov 4, 2024
885f314
Refactor some of `RegistryDownloadsManagerTests` for Swift concurrency
MaxDesiatov Nov 4, 2024
83355bf
Fix most of `RegistryDownloadsManagerTests`
MaxDesiatov Nov 5, 2024
0e159d9
Fix `RegistryDownloadsManagerTests.testConcurrency`
MaxDesiatov Nov 5, 2024
c5ee0de
Fix remaining `PackageRegistry` tests
MaxDesiatov Nov 6, 2024
9aa2ae0
Fix `SignatureValidationTests`
MaxDesiatov Nov 6, 2024
d167aed
Fix `SigningEntityTOFU` tests
MaxDesiatov Nov 6, 2024
b8f4c15
Clean up formatting
MaxDesiatov Nov 6, 2024
0b6bbd9
Address redundant use of `try` warnings
MaxDesiatov Nov 6, 2024
683dd13
Fix `AsyncIteratorProtocol` in `MockRegistryDownloadsManagerDelegate`
MaxDesiatov Nov 6, 2024
e90022c
Avoid Swift 6.0+ features in MockRegistryDownloadsManagerDelegate
MaxDesiatov Nov 6, 2024
4840fac
Enable `RegistryDownloadsManagerTests` only with Swift 6.0+
MaxDesiatov Nov 6, 2024
22d68f4
Convert `ManifestLoader.evaluateManifest` to `async`
MaxDesiatov Nov 6, 2024
a8a995b
Fix build issues in tests
MaxDesiatov Nov 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Sources/Basics/Concurrency/ThreadSafeBox.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ public final class ThreadSafeBox<Value> {
}
}

/// Modifies value stored in the box in-place, potentially avoiding copies.
/// - Parameter body: function applied to the stored value that modifies it.
public func mutate(body: (inout Value?) throws -> ()) rethrows {
try self.lock.withLock {
try body(&self.underlying)
Expand Down
17 changes: 16 additions & 1 deletion Sources/Basics/FileSystem/FileSystem+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,25 @@ extension FileSystem {
}

/// Execute the given block while holding the lock.
public func withLock<T>(on path: AbsolutePath, type: FileLock.LockType, blocking: Bool = true, _ body: () throws -> T) throws -> T {
public func withLock<T>(
on path: AbsolutePath,
type: FileLock.LockType,
blocking: Bool = true,
_ body: () throws -> T
) throws -> T {
try self.withLock(on: path.underlying, type: type, blocking: blocking, body)
}

/// Execute the given block while holding the lock.
public func withLock<T>(
on path: AbsolutePath,
type: FileLock.LockType,
blocking: Bool = true,
_ body: () async throws -> T
) async throws -> T {
try await self.withLock(on: path.underlying, type: type, blocking: blocking, body)
}

/// Returns any known item replacement directories for a given path. These may be used by platform-specific
/// libraries to handle atomic file system operations, such as deletion.
func itemReplacementDirectories(for path: AbsolutePath) throws -> [AbsolutePath] {
Expand Down
44 changes: 38 additions & 6 deletions Sources/Basics/FileSystem/InMemoryFileSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
*/

import class Foundation.NSLock
import class Dispatch.DispatchQueue
import class Foundation.NSLock
import struct TSCBasic.AbsolutePath
import struct TSCBasic.ByteString
import class TSCBasic.FileLock
Expand All @@ -23,7 +23,7 @@ public final class InMemoryFileSystem: FileSystem {
private class Node {
/// The actual node data.
let contents: NodeContents

/// Whether the node has executable bit enabled.
var isExecutable: Bool

Expand Down Expand Up @@ -86,10 +86,12 @@ public final class InMemoryFileSystem: FileSystem {
/// tests.
private let lock = NSLock()
/// A map that keeps weak references to all locked files.
private var lockFiles = Dictionary<TSCBasic.AbsolutePath, WeakReference<DispatchQueue>>()
private var lockFiles = [TSCBasic.AbsolutePath: WeakReference<DispatchQueue>]()
/// Used to access lockFiles in a thread safe manner.
private let lockFilesLock = NSLock()

private let asyncFilesLock = [TSCBasic.AbsolutePath: NSLock]()

/// Exclusive file system lock vended to clients through `withLock()`.
/// Used to ensure that DispatchQueues are released when they are no longer in use.
private struct WeakReference<Value: AnyObject> {
Expand Down Expand Up @@ -488,10 +490,40 @@ public final class InMemoryFileSystem: FileSystem {
}
}

return try fileQueue.sync(flags: type == .exclusive ? .barrier : .init() , execute: body)
return try fileQueue.sync(flags: type == .exclusive ? .barrier : .init(), execute: body)
}

public func withLock<T>(on path: TSCBasic.AbsolutePath, type: FileLock.LockType, blocking: Bool, _ body: () throws -> T) throws -> T {

@available(*, deprecated, message: "Use of this overload can lead to deadlocks, use `AsyncFileSystem` instead.")
public func withLock<T>(
on path: TSCBasic.AbsolutePath,
type: FileLock.LockType = .exclusive,
blocking: Bool,
_ body: () async throws -> T
) async throws -> T {
let resolvedPath: TSCBasic.AbsolutePath = try lock.withLock {
if case let .symlink(destination) = try getNode(path)?.contents {
return try .init(validating: destination, relativeTo: path.parentDirectory)
} else {
return path
}
}

// FIXME: code calling this function should be migrated to `AsyncFileSystem` instead.
self.asyncFilesLock[resolvedPath, default: NSLock()].lock()

let result = try await body()

self.asyncFilesLock[resolvedPath, default: NSLock()].unlock()

return result
}

public func withLock<T>(
on path: TSCBasic.AbsolutePath,
type: FileLock.LockType,
blocking: Bool,
_ body: () throws -> T
) throws -> T {
try self.withLock(on: path, type: type, body)
}
}
Expand Down
3 changes: 1 addition & 2 deletions Sources/Basics/HTTPClient/HTTPClientConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ import Foundation

public struct HTTPClientConfiguration: Sendable {
// FIXME: this should be unified with ``AuthorizationProvider`` protocol or renamed to avoid unintended shadowing.
public typealias AuthorizationProvider = @Sendable (URL)
-> String?
public typealias AuthorizationProvider = @Sendable (URL) -> String?

public init(
requestHeaders: HTTPClientHeaders? = nil,
Expand Down
52 changes: 32 additions & 20 deletions Sources/Commands/CommandWorkspaceDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -184,30 +184,42 @@ final class CommandWorkspaceDelegate: WorkspaceDelegate {

// registry signature handlers

func onUnsignedRegistryPackage(registryURL: URL, package: PackageModel.PackageIdentity, version: TSCUtility.Version, completion: (Bool) -> Void) {
self.inputHandler("\(package) \(version) from \(registryURL) is unsigned. okay to proceed? (yes/no) ") { response in
switch response?.lowercased() {
case "yes":
completion(true) // continue
case "no":
completion(false) // stop resolution
default:
self.outputHandler("invalid response: '\(response ?? "")'", false)
completion(false)
func onUnsignedRegistryPackage(
registryURL: URL,
package: PackageModel.PackageIdentity,
version: TSCUtility.Version
) async -> Bool {
await withCheckedContinuation { continuation in
self.inputHandler("\(package) \(version) from \(registryURL) is unsigned. okay to proceed? (yes/no) ") { response in
switch response?.lowercased() {
case "yes":
continuation.resume(returning: true) // continue
case "no":
continuation.resume(returning: false) // stop resolution
default:
self.outputHandler("invalid response: '\(response ?? "")'", false)
continuation.resume(returning: false)
}
}
}
}

func onUntrustedRegistryPackage(registryURL: URL, package: PackageModel.PackageIdentity, version: TSCUtility.Version, completion: (Bool) -> Void) {
self.inputHandler("\(package) \(version) from \(registryURL) is signed with an untrusted certificate. okay to proceed? (yes/no) ") { response in
switch response?.lowercased() {
case "yes":
completion(true) // continue
case "no":
completion(false) // stop resolution
default:
self.outputHandler("invalid response: '\(response ?? "")'", false)
completion(false)
func onUntrustedRegistryPackage(
registryURL: URL,
package: PackageModel.PackageIdentity,
version: TSCUtility.Version
) async -> Bool {
await withCheckedContinuation { continuation in
self.inputHandler("\(package) \(version) from \(registryURL) is signed with an untrusted certificate. okay to proceed? (yes/no) ") { response in
switch response?.lowercased() {
case "yes":
continuation.resume(returning: true) // continue
case "no":
continuation.resume(returning: false) // stop resolution
default:
self.outputHandler("invalid response: '\(response ?? "")'", false)
continuation.resume(returning: false)
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/Commands/PackageCommands/DumpCommands.swift
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ struct DumpPackage: AsyncSwiftCommand {
let workspace = try swiftCommandState.getActiveWorkspace()
let root = try swiftCommandState.getWorkspaceRoot()

let rootManifests = try await workspace.loadRootManifests(
let rootManifests = await workspace.loadRootManifests(
packages: root.packages,
observabilityScope: swiftCommandState.observabilityScope
)
Expand Down
4 changes: 2 additions & 2 deletions Sources/Commands/SwiftTestCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ public struct SwiftTestCommand: AsyncSwiftCommand {
) async throws {
let workspace = try swiftCommandState.getActiveWorkspace()
let root = try swiftCommandState.getWorkspaceRoot()
let rootManifests = try await workspace.loadRootManifests(
let rootManifests = await workspace.loadRootManifests(
packages: root.packages,
observabilityScope: swiftCommandState.observabilityScope
)
Expand Down Expand Up @@ -674,7 +674,7 @@ extension SwiftTestCommand {
func printCodeCovPath(_ swiftCommandState: SwiftCommandState) async throws {
let workspace = try swiftCommandState.getActiveWorkspace()
let root = try swiftCommandState.getWorkspaceRoot()
let rootManifests = try await workspace.loadRootManifests(
let rootManifests = await workspace.loadRootManifests(
packages: root.packages,
observabilityScope: swiftCommandState.observabilityScope
)
Expand Down
2 changes: 1 addition & 1 deletion Sources/CoreCommands/SwiftCommandState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ public final class SwiftCommandState {
public func getRootPackageInformation() async throws -> (dependencies: [PackageIdentity: [PackageIdentity]], targets: [PackageIdentity: [String]]) {
let workspace = try self.getActiveWorkspace()
let root = try self.getWorkspaceRoot()
let rootManifests = try await workspace.loadRootManifests(
let rootManifests = await workspace.loadRootManifests(
packages: root.packages,
observabilityScope: self.observabilityScope
)
Expand Down
Loading