Skip to content

Commit 8021081

Browse files
authored
Use async HTTPClient in BinaryArtifactsManager (#8087)
### Motivation: To adopt Swift concurrency in `ManifestLoader`, we need to convert `RegistryClient` first, which then would require use of `HTTPClient` instead of `LegacyHTTPClient`. That would also cascade to `BinaryArtifactsManager`, so I'm starting there first to make PRs smaller. ### Modifications: Converted (and as a consequence, simplified) code in `Workspace+BinaryArtifacts.swift`, also adopted `HTTPClient` in `WorkspaceTests` to fix tests after the change. ### Result: One major previous adopter of `LegacyHTTPClient` is now compatible with Swift concurrency. Since `BinaryArtifactsManager` was blocking Swift concurrency thread pool and also used `.sharedConcurrent` dispatch queue, this potentially fixes thread explosion/deadlocks that may have been caused by previously broken code.
1 parent f3e3fb6 commit 8021081

File tree

7 files changed

+819
-941
lines changed

7 files changed

+819
-941
lines changed

Sources/Basics/Concurrency/ThreadSafeBox.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ public final class ThreadSafeBox<Value> {
3030
self.underlying = value
3131
}
3232
}
33+
34+
public func mutate(body: (inout Value?) throws -> ()) rethrows {
35+
try self.lock.withLock {
36+
try body(&self.underlying)
37+
}
38+
}
3339

3440
@discardableResult
3541
public func memoize(body: () throws -> Value) rethrows -> Value {

Sources/Workspace/Diagnostics.swift

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -123,59 +123,67 @@ extension Basics.Diagnostic {
123123
static func customDependencyMissing(packageName: String) -> Self {
124124
.warning("dependency '\(packageName)' is missing; retrieving again")
125125
}
126+
}
127+
128+
struct BinaryArtifactsManagerError: Error, CustomStringConvertible {
129+
let description: String
130+
131+
private init(description: String) {
132+
self.description = description
133+
}
126134

127135
static func artifactInvalidArchive(artifactURL: URL, targetName: String) -> Self {
128-
.error(
129-
"invalid archive returned from '\(artifactURL.absoluteString)' which is required by binary target '\(targetName)'"
136+
.init(
137+
description: "invalid archive returned from '\(artifactURL.absoluteString)' which is required by binary target '\(targetName)'"
130138
)
131139
}
132140

133141
static func artifactChecksumChanged(targetName: String) -> Self {
134-
.error(
135-
"artifact of binary target '\(targetName)' has changed checksum; this is a potential security risk so the new artifact won't be downloaded"
142+
.init(
143+
description: "artifact of binary target '\(targetName)' has changed checksum; this is a potential security risk so the new artifact won't be downloaded"
136144
)
137145
}
138146

139147
static func artifactInvalidChecksum(targetName: String, expectedChecksum: String, actualChecksum: String?) -> Self {
140-
.error(
141-
"checksum of downloaded artifact of binary target '\(targetName)' (\(actualChecksum ?? "none")) does not match checksum specified by the manifest (\(expectedChecksum))"
148+
.init(
149+
description: "checksum of downloaded artifact of binary target '\(targetName)' (\(actualChecksum ?? "none")) does not match checksum specified by the manifest (\(expectedChecksum))"
142150
)
143151
}
144152

145153
static func artifactFailedDownload(artifactURL: URL, targetName: String, reason: String) -> Self {
146-
.error(
147-
"failed downloading '\(artifactURL.absoluteString)' which is required by binary target '\(targetName)': \(reason)"
154+
.init(
155+
description: "failed downloading '\(artifactURL.absoluteString)' which is required by binary target '\(targetName)': \(reason)"
148156
)
149157
}
150158

151159
static func artifactFailedValidation(artifactURL: URL, targetName: String, reason: String) -> Self {
152-
.error(
153-
"failed validating archive from '\(artifactURL.absoluteString)' which is required by binary target '\(targetName)': \(reason)"
160+
.init(
161+
description: "failed validating archive from '\(artifactURL.absoluteString)' which is required by binary target '\(targetName)': \(reason)"
154162
)
155163
}
156164

157165
static func remoteArtifactFailedExtraction(artifactURL: URL, targetName: String, reason: String) -> Self {
158-
.error(
159-
"failed extracting '\(artifactURL.absoluteString)' which is required by binary target '\(targetName)': \(reason)"
166+
.init(
167+
description: "failed extracting '\(artifactURL.absoluteString)' which is required by binary target '\(targetName)': \(reason)"
160168
)
161169
}
162170

163171
static func localArtifactFailedExtraction(artifactPath: AbsolutePath, targetName: String, reason: String) -> Self {
164-
.error("failed extracting '\(artifactPath)' which is required by binary target '\(targetName)': \(reason)")
172+
.init(description: "failed extracting '\(artifactPath)' which is required by binary target '\(targetName)': \(reason)")
165173
}
166174

167175
static func remoteArtifactNotFound(artifactURL: URL, targetName: String) -> Self {
168-
.error(
169-
"downloaded archive of binary target '\(targetName)' from '\(artifactURL.absoluteString)' does not contain a binary artifact."
176+
.init(
177+
description: "downloaded archive of binary target '\(targetName)' from '\(artifactURL.absoluteString)' does not contain a binary artifact."
170178
)
171179
}
172180

173181
static func localArchivedArtifactNotFound(archivePath: AbsolutePath, targetName: String) -> Self {
174-
.error("local archive of binary target '\(targetName)' at '\(archivePath)' does not contain a binary artifact.")
182+
.init(description: "local archive of binary target '\(targetName)' at '\(archivePath)' does not contain a binary artifact.")
175183
}
176184

177185
static func localArtifactNotFound(artifactPath: AbsolutePath, targetName: String) -> Self {
178-
.error("local binary target '\(targetName)' at '\(artifactPath)' does not contain a binary artifact.")
186+
.init(description: "local binary target '\(targetName)' at '\(artifactPath)' does not contain a binary artifact.")
179187
}
180188

181189
static func exhaustedAttempts(missing: [PackageReference]) -> Self {
@@ -189,8 +197,8 @@ extension Basics.Diagnostic {
189197
return "'\($0.identity)' at \(path)"
190198
}
191199
}
192-
return .error(
193-
"exhausted attempts to resolve the dependencies graph, with the following dependencies unresolved:\n* \(missing.joined(separator: "\n* "))"
200+
return .init(
201+
description: "exhausted attempts to resolve the dependencies graph, with the following dependencies unresolved:\n* \(missing.joined(separator: "\n* "))"
194202
)
195203
}
196204
}

0 commit comments

Comments
 (0)