Skip to content

[Traits] Re-enable no unused dependencies assertion in TraitTests.swift #8360

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

Merged
merged 9 commits into from
Mar 17, 2025
342 changes: 187 additions & 155 deletions Sources/CoreCommands/SwiftCommandState.swift

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions Sources/PackageGraph/GraphLoadingNode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,25 +45,25 @@ public struct GraphLoadingNode: Equatable, Hashable {
}

/// Returns the dependencies required by this node.
internal var requiredDependencies: [PackageDependency] {
var requiredDependencies: [PackageDependency] {
guard let requiredDeps = try? self.manifest.dependenciesRequired(for: self.productFilter, enabledTraits) else {
return []
}
return requiredDeps
}

internal var traitGuardedDependencies: [PackageDependency] {
return self.manifest.dependenciesGuarded(by: enabledTraits)
var traitGuardedDependencies: [PackageDependency] {
self.manifest.dependenciesTraitGuarded(withEnabledTraits: self.enabledTraits)
}
}

extension GraphLoadingNode: CustomStringConvertible {
public var description: String {
switch productFilter {
switch self.productFilter {
case .everything:
return self.identity.description
self.identity.description
case .specific(let set):
return "\(self.identity.description)[\(set.sorted().joined(separator: ", "))]"
"\(self.identity.description)[\(set.sorted().joined(separator: ", "))]"
}
}
}
409 changes: 242 additions & 167 deletions Sources/PackageGraph/ModulesGraph+Loading.swift

Large diffs are not rendered by default.

262 changes: 173 additions & 89 deletions Sources/PackageModel/Manifest/Manifest.swift

Large diffs are not rendered by default.

327 changes: 184 additions & 143 deletions Sources/Workspace/Workspace+Manifests.swift

Large diffs are not rendered by default.

10 changes: 7 additions & 3 deletions Sources/Workspace/Workspace+Registry.swift
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,8 @@ extension Workspace {
):
if let packageName,
// makes sure we use the updated package name for target based dependencies
let modifiedPackageName = targetDependencyPackageNameTransformations[packageName.lowercased()]
let modifiedPackageName =
targetDependencyPackageNameTransformations[packageName.lowercased()]
{
modifiedDependency = .product(
name: name,
Expand All @@ -276,7 +277,9 @@ extension Workspace {
)
}
case .byName(name: let packageName, condition: let condition):
if let modifiedPackageName = targetDependencyPackageNameTransformations[packageName.lowercased()] {
if let modifiedPackageName =
targetDependencyPackageNameTransformations[packageName.lowercased()]
{
modifiedDependency = .product(
name: packageName,
package: modifiedPackageName,
Expand Down Expand Up @@ -331,7 +334,8 @@ extension Workspace {
dependencies: modifiedDependencies,
products: manifest.products,
targets: modifiedTargets,
traits: manifest.traits
traits: manifest.traits,
pruneDependencies: manifest.pruneDependencies
)

return modifiedManifest
Expand Down
75 changes: 48 additions & 27 deletions Sources/Workspace/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,12 @@ public class Workspace {
public let state: WorkspaceState

// `public` visibility for testing
@available(*, deprecated, renamed: "resolvedPackagesStore", message: "Renamed for consistency with the actual name of the feature")
@available(
*,
deprecated,
renamed: "resolvedPackagesStore",
message: "Renamed for consistency with the actual name of the feature"
)
public var pinsStore: LoadableResult<PinsStore> { self.resolvedPackagesStore }

/// The `Package.resolved` store. The `Package.resolved` file will be created when first resolved package is added
Expand Down Expand Up @@ -320,7 +325,8 @@ public class Workspace {
toolchain: customHostToolchain,
cacheDir: location.sharedManifestsCacheDirectory,
importRestrictions: configuration?.manifestImportRestrictions,
delegate: delegate.map(WorkspaceManifestLoaderDelegate.init(workspaceDelegate:))
delegate: delegate.map(WorkspaceManifestLoaderDelegate.init(workspaceDelegate:)),
pruneDependencies: configuration?.pruneDependencies ?? false
)
try self.init(
fileSystem: fileSystem,
Expand Down Expand Up @@ -455,7 +461,8 @@ public class Workspace {
var manifestLoader = customManifestLoader ?? ManifestLoader(
toolchain: hostToolchain,
cacheDir: location.sharedManifestsCacheDirectory,
importRestrictions: configuration?.manifestImportRestrictions
importRestrictions: configuration?.manifestImportRestrictions,
pruneDependencies: configuration?.pruneDependencies ?? false
)
// set delegate if not set
if let manifestLoader = manifestLoader as? ManifestLoader, manifestLoader.delegate == nil {
Expand Down Expand Up @@ -552,7 +559,8 @@ public class Workspace {
authorizationProvider: authorizationProvider,
hostToolchain: hostToolchain,
checksumAlgorithm: checksumAlgorithm,
cachePath: customBinaryArtifactsManager?.useCache == false || !configuration.sharedDependenciesCacheEnabled ? .none : location.sharedBinaryArtifactsCacheDirectory,
cachePath: customBinaryArtifactsManager?.useCache == false || !configuration
.sharedDependenciesCacheEnabled ? .none : location.sharedBinaryArtifactsCacheDirectory,
customHTTPClient: customBinaryArtifactsManager?.httpClient,
customArchiver: customBinaryArtifactsManager?.archiver,
delegate: delegate.map(WorkspaceBinaryArtifactsManagerDelegate.init(workspaceDelegate:))
Expand All @@ -564,7 +572,8 @@ public class Workspace {
fileSystem: fileSystem,
authorizationProvider: authorizationProvider,
scratchPath: location.prebuiltsDirectory,
cachePath: customPrebuiltsManager?.useCache == false || !configuration.sharedDependenciesCacheEnabled ? .none : location.sharedPrebuiltsCacheDirectory,
cachePath: customPrebuiltsManager?.useCache == false || !configuration
.sharedDependenciesCacheEnabled ? .none : location.sharedPrebuiltsCacheDirectory,
customHTTPClient: customPrebuiltsManager?.httpClient,
customArchiver: customPrebuiltsManager?.archiver,
delegate: delegate.map(WorkspacePrebuiltsManagerDelegate.init(workspaceDelegate:))
Expand Down Expand Up @@ -747,19 +756,20 @@ extension Workspace {
}

// Compute the custom or extra constraint we need to impose.
let requirement: PackageRequirement
if let version {
requirement = .versionSet(.exact(version))
let requirement: PackageRequirement = if let version {
.versionSet(.exact(version))
} else if let branch {
requirement = .revision(branch)
.revision(branch)
} else if let revision {
requirement = .revision(revision)
.revision(revision)
} else {
requirement = defaultRequirement
defaultRequirement
}

var dependencyEnabledTraits: Set<String>?
if let traits = root.dependencies.first(where: { $0.nameForModuleDependencyResolutionOnly == packageName })?.traits {
if let traits = root.dependencies.first(where: { $0.nameForModuleDependencyResolutionOnly == packageName })?
.traits
{
dependencyEnabledTraits = Set(traits.map(\.name))
}

Expand Down Expand Up @@ -945,7 +955,13 @@ extension Workspace {
}

let prebuilts: [PackageIdentity: [String: PrebuiltLibrary]] = await self.state.prebuilts.reduce(into: .init()) {
let prebuilt = PrebuiltLibrary(packageRef: $1.packageRef, libraryName: $1.libraryName, path: $1.path, products: $1.products, cModules: $1.cModules)
let prebuilt = PrebuiltLibrary(
packageRef: $1.packageRef,
libraryName: $1.libraryName,
path: $1.path,
products: $1.products,
cModules: $1.cModules
)
for product in $1.products {
$0[$1.packageRef.identity, default: [:]][product] = prebuilt
}
Expand Down Expand Up @@ -1026,7 +1042,7 @@ extension Workspace {
let lock = NSLock()
let sync = DispatchGroup()
var rootManifests = [AbsolutePath: Manifest]()
Set(packages).forEach { package in
for package in Set(packages) {
sync.enter()
// TODO: this does not use the identity resolver which is probably fine since its the root packages
self.loadManifest(
Expand Down Expand Up @@ -1157,7 +1173,7 @@ extension Workspace {
fileSystem: self.fileSystem,
observabilityScope: observabilityScope,
// For now we enable all traits
enabledTraits: Set(manifest.traits.map { $0.name })
enabledTraits: Set(manifest.traits.map(\.name))
)
return try builder.construct()
}
Expand Down Expand Up @@ -1201,9 +1217,14 @@ extension Workspace {
observabilityScope: ObservabilityScope
) async throws -> Package {
try await withCheckedThrowingContinuation { continuation in
self.loadPackage(with: identity, packageGraph: packageGraph, observabilityScope: observabilityScope, completion: {
continuation.resume(with: $0)
})
self.loadPackage(
with: identity,
packageGraph: packageGraph,
observabilityScope: observabilityScope,
completion: {
continuation.resume(with: $0)
}
)
}
}

Expand Down Expand Up @@ -1242,7 +1263,7 @@ extension Workspace {
fileSystem: self.fileSystem,
observabilityScope: observabilityScope,
// For now we enable all traits
enabledTraits: Set(manifest.traits.map { $0.name })
enabledTraits: Set(manifest.traits.map(\.name))
)
return try builder.construct()
}
Expand Down Expand Up @@ -1331,9 +1352,9 @@ extension Workspace.ManagedArtifact {
fileprivate var originURL: String? {
switch self.source {
case .remote(let url, _):
return url
url
case .local:
return nil
nil
}
}
}
Expand All @@ -1356,11 +1377,11 @@ extension PackageDependency {
private var isLocal: Bool {
switch self {
case .fileSystem:
return true
true
case .sourceControl:
return false
false
case .registry:
return false
false
}
}
}
Expand Down Expand Up @@ -1525,11 +1546,11 @@ extension ContainerUpdateStrategy {
var repositoryUpdateStrategy: RepositoryUpdateStrategy {
switch self {
case .always:
return .always
.always
case .never:
return .never
.never
case .ifNeeded(let revision):
return .ifNeeded(revision: .init(identifier: revision))
.ifNeeded(revision: .init(identifier: revision))
}
}
}
Loading