Skip to content

Commit 6c95780

Browse files
dschaefer2dschaefer2
authored
Clean up warnings, fix handling of dep version changes (#8232)
I found a bug where if you change swift syntax version in your manifest, we were not changing the use of prebuilts to match. I have added a scan to do so. Also requires that we add the version of the prebuilt to the workspace state. Assume dependent products from prebuilts are being used to remove the warning for swift-syntax.
1 parent 66ba054 commit 6c95780

File tree

7 files changed

+192
-38
lines changed

7 files changed

+192
-38
lines changed

‎Sources/PackageGraph/ModulesGraph+Loading.swift

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,12 @@ extension ModulesGraph {
260260
)
261261

262262
let rootPackages = resolvedPackages.filter { root.manifests.values.contains($0.manifest) }
263-
checkAllDependenciesAreUsed(packages: resolvedPackages, rootPackages, observabilityScope: observabilityScope)
263+
checkAllDependenciesAreUsed(
264+
packages: resolvedPackages,
265+
rootPackages,
266+
prebuilts: prebuilts,
267+
observabilityScope: observabilityScope
268+
)
264269

265270
return try ModulesGraph(
266271
rootPackages: rootPackages,
@@ -275,6 +280,7 @@ extension ModulesGraph {
275280
private func checkAllDependenciesAreUsed(
276281
packages: IdentifiableSet<ResolvedPackage>,
277282
_ rootPackages: [ResolvedPackage],
283+
prebuilts: [PackageIdentity: [String: PrebuiltLibrary]],
278284
observabilityScope: ObservabilityScope
279285
) {
280286
for package in rootPackages {
@@ -352,9 +358,10 @@ private func checkAllDependenciesAreUsed(
352358
let usedByPackage = productDependencies.contains { $0.name == product.name }
353359
// We check if any of the products of this dependency is guarded by a trait.
354360
let traitGuarded = traitGuardedProductDependencies.contains(product.name)
361+
// Consider prebuilts as used
362+
let prebuilt = prebuilts[dependency.identity]?.keys.contains(product.name) ?? false
355363

356-
// If the product is either used directly or guarded by a trait we consider it as used
357-
return usedByPackage || traitGuarded
364+
return usedByPackage || traitGuarded || prebuilt
358365
}
359366

360367
if !dependencyIsUsed && !observabilityScope.errorsReportedInAnyScope {

‎Sources/Workspace/ManagedPrebuilt.swift

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
import struct TSCUtility.Version
14+
1315
import Basics
1416
import PackageModel
1517

@@ -19,6 +21,9 @@ extension Workspace {
1921
/// The package identity
2022
public let identity: PackageIdentity
2123

24+
/// The package version
25+
public let version: Version
26+
2227
/// The name of the binary target the artifact corresponds to.
2328
public let libraryName: String
2429

@@ -45,60 +50,60 @@ extension Workspace {
4550
/// A collection of managed artifacts which have been downloaded.
4651
public final class ManagedPrebuilts {
4752
/// A mapping from package identity, to target name, to ManagedArtifact.
48-
private var artifactMap: [PackageIdentity: [String: ManagedPrebuilt]]
53+
private var prebuiltMap: [PackageIdentity: [String: ManagedPrebuilt]]
4954

50-
internal var artifacts: AnyCollection<ManagedPrebuilt> {
51-
AnyCollection(self.artifactMap.values.lazy.flatMap{ $0.values })
55+
internal var prebuilts: AnyCollection<ManagedPrebuilt> {
56+
AnyCollection(self.prebuiltMap.values.lazy.flatMap{ $0.values })
5257
}
5358

5459
init() {
55-
self.artifactMap = [:]
60+
self.prebuiltMap = [:]
5661
}
5762

58-
init(_ artifacts: [ManagedPrebuilt]) throws {
59-
let artifactsByPackagePath = Dictionary(grouping: artifacts, by: { $0.identity })
60-
self.artifactMap = try artifactsByPackagePath.mapValues{ artifacts in
61-
try Dictionary(artifacts.map { ($0.libraryName, $0) }, uniquingKeysWith: { _, _ in
63+
init(_ prebuilts: [ManagedPrebuilt]) throws {
64+
let prebuiltsByPackagePath = Dictionary(grouping: prebuilts, by: { $0.identity })
65+
self.prebuiltMap = try prebuiltsByPackagePath.mapValues{ prebuilt in
66+
try Dictionary(prebuilt.map { ($0.libraryName, $0) }, uniquingKeysWith: { _, _ in
6267
// should be unique
63-
throw StringError("binary artifact already exists in managed artifacts")
68+
throw StringError("prebuilt already exists in managed prebuilts")
6469
})
6570
}
6671
}
6772

6873
public subscript(packageIdentity packageIdentity: PackageIdentity, targetName targetName: String) -> ManagedPrebuilt? {
69-
self.artifactMap[packageIdentity]?[targetName]
74+
self.prebuiltMap[packageIdentity]?[targetName]
7075
}
7176

72-
public func add(_ artifact: ManagedPrebuilt) {
73-
self.artifactMap[artifact.identity, default: [:]][artifact.libraryName] = artifact
77+
public func add(_ prebuilt: ManagedPrebuilt) {
78+
self.prebuiltMap[prebuilt.identity, default: [:]][prebuilt.libraryName] = prebuilt
7479
}
7580

7681
public func remove(packageIdentity: PackageIdentity, targetName: String) {
77-
self.artifactMap[packageIdentity]?[targetName] = nil
82+
self.prebuiltMap[packageIdentity]?[targetName] = nil
7883
}
7984
}
8085
}
8186

8287
extension Workspace.ManagedPrebuilts: Collection {
8388
public var startIndex: AnyIndex {
84-
self.artifacts.startIndex
89+
self.prebuilts.startIndex
8590
}
8691

8792
public var endIndex: AnyIndex {
88-
self.artifacts.endIndex
93+
self.prebuilts.endIndex
8994
}
9095

9196
public subscript(index: AnyIndex) -> Workspace.ManagedPrebuilt {
92-
self.artifacts[index]
97+
self.prebuilts[index]
9398
}
9499

95100
public func index(after index: AnyIndex) -> AnyIndex {
96-
self.artifacts.index(after: index)
101+
self.prebuilts.index(after: index)
97102
}
98103
}
99104

100105
extension Workspace.ManagedPrebuilts: CustomStringConvertible {
101106
public var description: String {
102-
"<ManagedArtifacts: \(Array(self.artifacts))>"
107+
"<ManagedArtifacts: \(Array(self.prebuilts))>"
103108
}
104109
}

‎Sources/Workspace/Workspace+Prebuilts.swift

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,9 @@ extension Workspace {
269269
if fileSystem.exists(destination) {
270270
do {
271271
return try JSONDecoder().decode(
272-
PrebuiltsManifest.self,
273-
from: try Data(contentsOf: destination.asURL)
272+
path: destination,
273+
fileSystem: fileSystem,
274+
as: PrebuiltsManifest.self
274275
)
275276
} catch {
276277
// redownload it
@@ -325,10 +326,10 @@ extension Workspace {
325326
}
326327

327328
do {
328-
let data = try fileSystem.readFileContents(destination)
329329
return try JSONDecoder().decode(
330-
PrebuiltsManifest.self,
331-
from: Data(data.contents)
330+
path: destination,
331+
fileSystem: fileSystem,
332+
as: PrebuiltsManifest.self
332333
)
333334
} catch {
334335
observabilityScope.emit(
@@ -489,13 +490,11 @@ extension Workspace {
489490
return
490491
}
491492

492-
for prebuilt in prebuiltsManager.findPrebuilts(
493-
packages: try manifests.requiredPackages
494-
) {
493+
let addedPrebuilts = ManagedPrebuilts()
494+
495+
for prebuilt in prebuiltsManager.findPrebuilts(packages: try manifests.requiredPackages) {
495496
guard
496-
let manifest = manifests.allDependencyManifests[
497-
prebuilt.identity
498-
],
497+
let manifest = manifests.allDependencyManifests[prebuilt.identity],
499498
let packageVersion = manifest.manifest.version,
500499
let prebuiltManifest = try await prebuiltsManager
501500
.downloadManifest(
@@ -523,21 +522,30 @@ extension Workspace {
523522
artifact: artifact,
524523
observabilityScope: observabilityScope
525524
)
526-
{
525+
{
527526
// Add to workspace state
528527
let managedPrebuilt = ManagedPrebuilt(
529528
identity: prebuilt.identity,
529+
version: packageVersion,
530530
libraryName: library.name,
531531
path: path,
532532
products: library.products,
533533
cModules: library.cModules
534534
)
535+
addedPrebuilts.add(managedPrebuilt)
535536
self.state.prebuilts.add(managedPrebuilt)
536-
try self.state.save()
537537
}
538538
}
539539
}
540540
}
541+
542+
for prebuilt in self.state.prebuilts.prebuilts {
543+
if !addedPrebuilts.contains(where: { $0.identity == prebuilt.identity && $0.version == prebuilt.version }) {
544+
self.state.prebuilts.remove(packageIdentity: prebuilt.identity, targetName: prebuilt.libraryName)
545+
}
546+
}
547+
548+
try self.state.save()
541549
}
542550

543551
var hostPrebuiltsPlatform: PrebuiltsManifest.Platform? {

‎Sources/Workspace/Workspace+State.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
import struct TSCUtility.Version
14+
1315
import Basics
1416
import Foundation
1517
import PackageGraph
@@ -447,13 +449,15 @@ extension WorkspaceStateStorage {
447449

448450
struct Prebuilt: Codable {
449451
let identity: PackageIdentity
452+
let version: TSCUtility.Version
450453
let libraryName: String
451454
let path: AbsolutePath
452455
let products: [String]
453456
let cModules: [String]
454457

455458
init(_ managedPrebuilt: Workspace.ManagedPrebuilt) {
456459
self.identity = managedPrebuilt.identity
460+
self.version = managedPrebuilt.version
457461
self.libraryName = managedPrebuilt.libraryName
458462
self.path = managedPrebuilt.path
459463
self.products = managedPrebuilt.products
@@ -527,6 +531,7 @@ extension Workspace.ManagedPrebuilt {
527531
fileprivate init(_ prebuilt: WorkspaceStateStorage.V7.Prebuilt) throws {
528532
self.init(
529533
identity: prebuilt.identity,
534+
version: prebuilt.version,
530535
libraryName: prebuilt.libraryName,
531536
path: prebuilt.path,
532537
products: prebuilt.products,

‎Sources/_InternalTestSupport/ManifestExtensions.swift

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,4 +266,27 @@ extension Manifest {
266266
traits: self.traits
267267
)
268268
}
269+
270+
public func with(dependencies: [PackageDependency]) -> Manifest {
271+
Manifest(
272+
displayName: self.displayName,
273+
path: self.path,
274+
packageKind: self.packageKind,
275+
packageLocation: self.packageLocation,
276+
defaultLocalization: self.defaultLocalization,
277+
platforms: self.platforms,
278+
version: self.version,
279+
revision: self.revision,
280+
toolsVersion: self.toolsVersion,
281+
pkgConfig: self.pkgConfig,
282+
providers: self.providers,
283+
cLanguageStandard: self.cLanguageStandard,
284+
cxxLanguageStandard: self.cxxLanguageStandard,
285+
swiftLanguageVersions: self.swiftLanguageVersions,
286+
dependencies: dependencies,
287+
products: self.products,
288+
targets: self.targets,
289+
traits: self.traits
290+
)
291+
}
269292
}

‎Sources/_InternalTestSupport/MockWorkspace.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ public final class MockWorkspace {
507507
public func checkPackageGraph(
508508
roots: [String] = [],
509509
deps: [MockDependency],
510-
_ result: (ModulesGraph, [Basics.Diagnostic]) -> Void
510+
_ result: (ModulesGraph, [Basics.Diagnostic]) throws -> Void
511511
) async throws {
512512
let dependencies = try deps.map { try $0.convert(baseURL: packagesDir, identityResolver: self.identityResolver) }
513513
try await self.checkPackageGraph(roots: roots, dependencies: dependencies, result)

0 commit comments

Comments
 (0)