Skip to content

Commit bafe38b

Browse files
authored
[Traits] Re-enable no unused dependencies assertion in TraitTests.swift (#8360)
Fixes #8131 Previously this check was disabled since it introduced failures when testing with the 6.1 toolchain - due to the introduction of traits in resolution (#8205) and additional behaviour from the experimental prune dependencies flag, this check should now pass as expected. ### Modifications: Re-enabled the `XCTAssertFalse(stderr.contains("warning:"))` check in the `TraitTests` where necessary and propagated the `pruneDependencies` flag to the `ManifestLoader` init in areas it was previously missing to assure no warnings would be generated upon omitting them. Also re-added the enabled traits parameter done during a computation for required dependencies when creating the package builders in `ModulesGraph+Loading.swift`. ### Result: This check shouldn't fail anymore.
1 parent 735ddd9 commit bafe38b

File tree

12 files changed

+3035
-1624
lines changed

12 files changed

+3035
-1624
lines changed

Sources/CoreCommands/SwiftCommandState.swift

+187-155
Large diffs are not rendered by default.

Sources/PackageGraph/GraphLoadingNode.swift

+6-6
Original file line numberDiff line numberDiff line change
@@ -45,25 +45,25 @@ public struct GraphLoadingNode: Equatable, Hashable {
4545
}
4646

4747
/// Returns the dependencies required by this node.
48-
internal var requiredDependencies: [PackageDependency] {
48+
var requiredDependencies: [PackageDependency] {
4949
guard let requiredDeps = try? self.manifest.dependenciesRequired(for: self.productFilter, enabledTraits) else {
5050
return []
5151
}
5252
return requiredDeps
5353
}
5454

55-
internal var traitGuardedDependencies: [PackageDependency] {
56-
return self.manifest.dependenciesGuarded(by: enabledTraits)
55+
var traitGuardedDependencies: [PackageDependency] {
56+
self.manifest.dependenciesTraitGuarded(withEnabledTraits: self.enabledTraits)
5757
}
5858
}
5959

6060
extension GraphLoadingNode: CustomStringConvertible {
6161
public var description: String {
62-
switch productFilter {
62+
switch self.productFilter {
6363
case .everything:
64-
return self.identity.description
64+
self.identity.description
6565
case .specific(let set):
66-
return "\(self.identity.description)[\(set.sorted().joined(separator: ", "))]"
66+
"\(self.identity.description)[\(set.sorted().joined(separator: ", "))]"
6767
}
6868
}
6969
}

Sources/PackageGraph/ModulesGraph+Loading.swift

+242-167
Large diffs are not rendered by default.

Sources/PackageModel/Manifest/Manifest.swift

+173-89
Large diffs are not rendered by default.

Sources/Workspace/Workspace+Manifests.swift

+184-143
Large diffs are not rendered by default.

Sources/Workspace/Workspace+Registry.swift

+7-3
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,8 @@ extension Workspace {
266266
):
267267
if let packageName,
268268
// makes sure we use the updated package name for target based dependencies
269-
let modifiedPackageName = targetDependencyPackageNameTransformations[packageName.lowercased()]
269+
let modifiedPackageName =
270+
targetDependencyPackageNameTransformations[packageName.lowercased()]
270271
{
271272
modifiedDependency = .product(
272273
name: name,
@@ -276,7 +277,9 @@ extension Workspace {
276277
)
277278
}
278279
case .byName(name: let packageName, condition: let condition):
279-
if let modifiedPackageName = targetDependencyPackageNameTransformations[packageName.lowercased()] {
280+
if let modifiedPackageName =
281+
targetDependencyPackageNameTransformations[packageName.lowercased()]
282+
{
280283
modifiedDependency = .product(
281284
name: packageName,
282285
package: modifiedPackageName,
@@ -331,7 +334,8 @@ extension Workspace {
331334
dependencies: modifiedDependencies,
332335
products: manifest.products,
333336
targets: modifiedTargets,
334-
traits: manifest.traits
337+
traits: manifest.traits,
338+
pruneDependencies: manifest.pruneDependencies
335339
)
336340

337341
return modifiedManifest

Sources/Workspace/Workspace.swift

+48-27
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,12 @@ public class Workspace {
9191
public let state: WorkspaceState
9292

9393
// `public` visibility for testing
94-
@available(*, deprecated, renamed: "resolvedPackagesStore", message: "Renamed for consistency with the actual name of the feature")
94+
@available(
95+
*,
96+
deprecated,
97+
renamed: "resolvedPackagesStore",
98+
message: "Renamed for consistency with the actual name of the feature"
99+
)
95100
public var pinsStore: LoadableResult<PinsStore> { self.resolvedPackagesStore }
96101

97102
/// The `Package.resolved` store. The `Package.resolved` file will be created when first resolved package is added
@@ -320,7 +325,8 @@ public class Workspace {
320325
toolchain: customHostToolchain,
321326
cacheDir: location.sharedManifestsCacheDirectory,
322327
importRestrictions: configuration?.manifestImportRestrictions,
323-
delegate: delegate.map(WorkspaceManifestLoaderDelegate.init(workspaceDelegate:))
328+
delegate: delegate.map(WorkspaceManifestLoaderDelegate.init(workspaceDelegate:)),
329+
pruneDependencies: configuration?.pruneDependencies ?? false
324330
)
325331
try self.init(
326332
fileSystem: fileSystem,
@@ -455,7 +461,8 @@ public class Workspace {
455461
var manifestLoader = customManifestLoader ?? ManifestLoader(
456462
toolchain: hostToolchain,
457463
cacheDir: location.sharedManifestsCacheDirectory,
458-
importRestrictions: configuration?.manifestImportRestrictions
464+
importRestrictions: configuration?.manifestImportRestrictions,
465+
pruneDependencies: configuration?.pruneDependencies ?? false
459466
)
460467
// set delegate if not set
461468
if let manifestLoader = manifestLoader as? ManifestLoader, manifestLoader.delegate == nil {
@@ -552,7 +559,8 @@ public class Workspace {
552559
authorizationProvider: authorizationProvider,
553560
hostToolchain: hostToolchain,
554561
checksumAlgorithm: checksumAlgorithm,
555-
cachePath: customBinaryArtifactsManager?.useCache == false || !configuration.sharedDependenciesCacheEnabled ? .none : location.sharedBinaryArtifactsCacheDirectory,
562+
cachePath: customBinaryArtifactsManager?.useCache == false || !configuration
563+
.sharedDependenciesCacheEnabled ? .none : location.sharedBinaryArtifactsCacheDirectory,
556564
customHTTPClient: customBinaryArtifactsManager?.httpClient,
557565
customArchiver: customBinaryArtifactsManager?.archiver,
558566
delegate: delegate.map(WorkspaceBinaryArtifactsManagerDelegate.init(workspaceDelegate:))
@@ -564,7 +572,8 @@ public class Workspace {
564572
fileSystem: fileSystem,
565573
authorizationProvider: authorizationProvider,
566574
scratchPath: location.prebuiltsDirectory,
567-
cachePath: customPrebuiltsManager?.useCache == false || !configuration.sharedDependenciesCacheEnabled ? .none : location.sharedPrebuiltsCacheDirectory,
575+
cachePath: customPrebuiltsManager?.useCache == false || !configuration
576+
.sharedDependenciesCacheEnabled ? .none : location.sharedPrebuiltsCacheDirectory,
568577
customHTTPClient: customPrebuiltsManager?.httpClient,
569578
customArchiver: customPrebuiltsManager?.archiver,
570579
delegate: delegate.map(WorkspacePrebuiltsManagerDelegate.init(workspaceDelegate:))
@@ -747,19 +756,20 @@ extension Workspace {
747756
}
748757

749758
// Compute the custom or extra constraint we need to impose.
750-
let requirement: PackageRequirement
751-
if let version {
752-
requirement = .versionSet(.exact(version))
759+
let requirement: PackageRequirement = if let version {
760+
.versionSet(.exact(version))
753761
} else if let branch {
754-
requirement = .revision(branch)
762+
.revision(branch)
755763
} else if let revision {
756-
requirement = .revision(revision)
764+
.revision(revision)
757765
} else {
758-
requirement = defaultRequirement
766+
defaultRequirement
759767
}
760768

761769
var dependencyEnabledTraits: Set<String>?
762-
if let traits = root.dependencies.first(where: { $0.nameForModuleDependencyResolutionOnly == packageName })?.traits {
770+
if let traits = root.dependencies.first(where: { $0.nameForModuleDependencyResolutionOnly == packageName })?
771+
.traits
772+
{
763773
dependencyEnabledTraits = Set(traits.map(\.name))
764774
}
765775

@@ -945,7 +955,13 @@ extension Workspace {
945955
}
946956

947957
let prebuilts: [PackageIdentity: [String: PrebuiltLibrary]] = await self.state.prebuilts.reduce(into: .init()) {
948-
let prebuilt = PrebuiltLibrary(packageRef: $1.packageRef, libraryName: $1.libraryName, path: $1.path, products: $1.products, cModules: $1.cModules)
958+
let prebuilt = PrebuiltLibrary(
959+
packageRef: $1.packageRef,
960+
libraryName: $1.libraryName,
961+
path: $1.path,
962+
products: $1.products,
963+
cModules: $1.cModules
964+
)
949965
for product in $1.products {
950966
$0[$1.packageRef.identity, default: [:]][product] = prebuilt
951967
}
@@ -1026,7 +1042,7 @@ extension Workspace {
10261042
let lock = NSLock()
10271043
let sync = DispatchGroup()
10281044
var rootManifests = [AbsolutePath: Manifest]()
1029-
Set(packages).forEach { package in
1045+
for package in Set(packages) {
10301046
sync.enter()
10311047
// TODO: this does not use the identity resolver which is probably fine since its the root packages
10321048
self.loadManifest(
@@ -1157,7 +1173,7 @@ extension Workspace {
11571173
fileSystem: self.fileSystem,
11581174
observabilityScope: observabilityScope,
11591175
// For now we enable all traits
1160-
enabledTraits: Set(manifest.traits.map { $0.name })
1176+
enabledTraits: Set(manifest.traits.map(\.name))
11611177
)
11621178
return try builder.construct()
11631179
}
@@ -1201,9 +1217,14 @@ extension Workspace {
12011217
observabilityScope: ObservabilityScope
12021218
) async throws -> Package {
12031219
try await withCheckedThrowingContinuation { continuation in
1204-
self.loadPackage(with: identity, packageGraph: packageGraph, observabilityScope: observabilityScope, completion: {
1205-
continuation.resume(with: $0)
1206-
})
1220+
self.loadPackage(
1221+
with: identity,
1222+
packageGraph: packageGraph,
1223+
observabilityScope: observabilityScope,
1224+
completion: {
1225+
continuation.resume(with: $0)
1226+
}
1227+
)
12071228
}
12081229
}
12091230

@@ -1242,7 +1263,7 @@ extension Workspace {
12421263
fileSystem: self.fileSystem,
12431264
observabilityScope: observabilityScope,
12441265
// For now we enable all traits
1245-
enabledTraits: Set(manifest.traits.map { $0.name })
1266+
enabledTraits: Set(manifest.traits.map(\.name))
12461267
)
12471268
return try builder.construct()
12481269
}
@@ -1331,9 +1352,9 @@ extension Workspace.ManagedArtifact {
13311352
fileprivate var originURL: String? {
13321353
switch self.source {
13331354
case .remote(let url, _):
1334-
return url
1355+
url
13351356
case .local:
1336-
return nil
1357+
nil
13371358
}
13381359
}
13391360
}
@@ -1356,11 +1377,11 @@ extension PackageDependency {
13561377
private var isLocal: Bool {
13571378
switch self {
13581379
case .fileSystem:
1359-
return true
1380+
true
13601381
case .sourceControl:
1361-
return false
1382+
false
13621383
case .registry:
1363-
return false
1384+
false
13641385
}
13651386
}
13661387
}
@@ -1525,11 +1546,11 @@ extension ContainerUpdateStrategy {
15251546
var repositoryUpdateStrategy: RepositoryUpdateStrategy {
15261547
switch self {
15271548
case .always:
1528-
return .always
1549+
.always
15291550
case .never:
1530-
return .never
1551+
.never
15311552
case .ifNeeded(let revision):
1532-
return .ifNeeded(revision: .init(identifier: revision))
1553+
.ifNeeded(revision: .init(identifier: revision))
15331554
}
15341555
}
15351556
}

0 commit comments

Comments
 (0)