diff --git a/Sources/PackageGraph/GraphLoadingNode.swift b/Sources/PackageGraph/GraphLoadingNode.swift index e0c4bb7a173..6b78b1d26ab 100644 --- a/Sources/PackageGraph/GraphLoadingNode.swift +++ b/Sources/PackageGraph/GraphLoadingNode.swift @@ -30,13 +30,13 @@ public struct GraphLoadingNode: Equatable, Hashable { public let productFilter: ProductFilter /// The enabled traits for this package. - package var enabledTraits: Set + package var enabledTraits: Set? public init( identity: PackageIdentity, manifest: Manifest, productFilter: ProductFilter, - enabledTraits: Set + enabledTraits: Set? ) throws { self.identity = identity self.manifest = manifest diff --git a/Sources/PackageGraph/ModulesGraph+Loading.swift b/Sources/PackageGraph/ModulesGraph+Loading.swift index 3e7b4632129..3e728645057 100644 --- a/Sources/PackageGraph/ModulesGraph+Loading.swift +++ b/Sources/PackageGraph/ModulesGraph+Loading.swift @@ -78,7 +78,7 @@ extension ModulesGraph { identity: dependency.identity, manifest: $0.manifest, productFilter: dependency.productFilter, - enabledTraits: [] + enabledTraits: root.enabledTraits[dependency.identity] ) } } @@ -104,15 +104,10 @@ extension ModulesGraph { guard let conditionTraits = $0.condition?.traits else { return true } - return !conditionTraits.intersection(node.item.enabledTraits).isEmpty + return !conditionTraits.intersection(node.item.enabledTraits ?? []).isEmpty }.map(\.name) - let calculatedTraits = try calculateEnabledTraits( - parentPackage: node.item.identity, - identity: dependency.identity, - manifest: manifest, - explictlyEnabledTraits: explictlyEnabledTraits.flatMap { Set($0) } - ) + let calculatedTraits = try manifest.enabledTraits(using: explictlyEnabledTraits.flatMap { Set($0) }, node.item.identity.description) return try KeyedPair( GraphLoadingNode( @@ -153,7 +148,10 @@ extension ModulesGraph { allNodes[$0.key] = $0.item } onDuplicate: { first, second in // We are unifying the enabled traits on duplicate - allNodes[first.key]?.enabledTraits.formUnion(second.item.enabledTraits) + // TODO bp: this may not be necessary anymore since we pre-compute all enabled and transitively enabled traits...? + if let enabledTraits = second.item.enabledTraits { + allNodes[first.key]?.enabledTraits?.formUnion(enabledTraits) + } } // Create the packages. @@ -184,7 +182,7 @@ extension ModulesGraph { createREPLProduct: manifest.packageKind.isRoot ? createREPLProduct : false, fileSystem: fileSystem, observabilityScope: nodeObservabilityScope, - enabledTraits: node.enabledTraits + enabledTraits: node.enabledTraits ?? ["default"] ) let package = try builder.construct() manifestToPackage[manifest] = package @@ -410,7 +408,7 @@ private func createResolvedPackages( return ResolvedPackageBuilder( package, productFilter: node.productFilter, - enabledTraits: node.enabledTraits, + enabledTraits: node.enabledTraits ?? ["default"], isAllowedToVendUnsafeProducts: isAllowedToVendUnsafeProducts, allowedToOverride: allowedToOverride, platformVersionProvider: platformVersionProvider @@ -1449,7 +1447,7 @@ private final class ResolvedPackageBuilder: ResolvedBuilder { var products: [ResolvedProductBuilder] = [] /// The enabled traits of this package. - var enabledTraits: Set = [] + var enabledTraits: Set = ["default"] /// The dependencies of this package. var dependencies: [ResolvedPackageBuilder] = [] diff --git a/Sources/PackageGraph/PackageGraphRoot.swift b/Sources/PackageGraph/PackageGraphRoot.swift index 8091ba65656..c4dc52ed8db 100644 --- a/Sources/PackageGraph/PackageGraphRoot.swift +++ b/Sources/PackageGraph/PackageGraphRoot.swift @@ -137,8 +137,8 @@ public struct PackageGraphRoot { // Check that the dependency is used in at least one of the manifests. // If not, then we can omit this dependency if pruning unused dependencies // is enabled. + // TODO bp: assure that trait-guarded deps are pruned regardless return manifests.values.reduce(false) { result, manifest in - guard manifest.pruneDependencies else { return true } let enabledTraits: Set? = enableTraitsMap[manifest.packageIdentity] if let isUsed = try? manifest.isPackageDependencyUsed(dep, enabledTraits: enabledTraits) { return result || isUsed diff --git a/Sources/PackageModel/Manifest/Manifest+Traits.swift b/Sources/PackageModel/Manifest/Manifest+Traits.swift index 5f9260cc858..8531b858663 100644 --- a/Sources/PackageModel/Manifest/Manifest+Traits.swift +++ b/Sources/PackageModel/Manifest/Manifest+Traits.swift @@ -336,6 +336,10 @@ extension Manifest { let traitGuardedDeps = try target.dependencies.filter { dep in let traits = dep.condition?.traits ?? [] + // If traits is empty, then we must manually validate the explicitly enabled traits. + if traits.isEmpty { + try validateEnabledTraits(enabledTraits) + } // For each trait that is a condition on this target dependency, assure that // each one is enabled in the manifest. return try traits.allSatisfy({ try isTraitEnabled(.init(stringLiteral: $0), enabledTraits) }) @@ -433,14 +437,34 @@ extension Manifest { } /// Determines whether a given package dependency is used by this manifest given a set of enabled traits. public func isPackageDependencyUsed(_ dependency: PackageDependency, enabledTraits: Set?) throws -> Bool { - let usedDependencies = try self.usedDependencies(withTraits: enabledTraits) - let foundKnownPackage = usedDependencies.knownPackage.contains(where: { - $0.caseInsensitiveCompare(dependency.identity.description) == .orderedSame - }) - - // if there is a target dependency referenced by name and the package it originates from is unknown, default to - // tentatively marking the package dependency as used. to be resolved later on. - return foundKnownPackage || (!foundKnownPackage && !usedDependencies.unknownPackage.isEmpty) + if self.pruneDependencies { + let usedDependencies = try self.usedDependencies(withTraits: enabledTraits) + let foundKnownPackage = usedDependencies.knownPackage.contains(where: { + $0.caseInsensitiveCompare(dependency.identity.description) == .orderedSame + }) + + // if there is a target dependency referenced by name and the package it originates from is unknown, default to + // tentatively marking the package dependency as used. to be resolved later on. + return foundKnownPackage || (!foundKnownPackage && !usedDependencies.unknownPackage.isEmpty) + } else { + // alternate path to compute trait-guarded package dependencies if the prune deps feature is not enabled + try validateEnabledTraits(enabledTraits) + + let targetDependenciesForPackageDependency = self.targets.flatMap({ $0.dependencies }) + .filter({ + $0.package?.caseInsensitiveCompare(dependency.identity.description) == .orderedSame + }) + + // if target deps is empty, default to returning true here. + let isTraitGuarded = targetDependenciesForPackageDependency.isEmpty ? false : targetDependenciesForPackageDependency.compactMap({ $0.condition?.traits }).allSatisfy({ + let condition = $0.subtracting(enabledTraits ?? []) + return !condition.isEmpty + }) + + let isUsedWithoutTraitGuarding = !targetDependenciesForPackageDependency.filter({ $0.condition?.traits == nil }).isEmpty + + return isUsedWithoutTraitGuarding || !isTraitGuarded + } } } diff --git a/Sources/PackageModel/Manifest/Manifest.swift b/Sources/PackageModel/Manifest/Manifest.swift index 798a4f6543b..9301b285c23 100644 --- a/Sources/PackageModel/Manifest/Manifest.swift +++ b/Sources/PackageModel/Manifest/Manifest.swift @@ -287,21 +287,18 @@ public final class Manifest: Sendable { guard self.toolsVersion >= .v5_2 && !self.packageKind.isRoot else { var dependencies = self.dependencies - if pruneDependencies { dependencies = try dependencies.filter({ - return try self.isPackageDependencyUsed($0, enabledTraits: enabledTraits) + let isUsed = try self.isPackageDependencyUsed($0, enabledTraits: enabledTraits) + return isUsed }) - } return dependencies } // using .nothing as cache key while ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION is false if var dependencies = self._requiredDependencies[.nothing] { - if self.pruneDependencies { dependencies = try dependencies.filter({ return try self.isPackageDependencyUsed($0, enabledTraits: enabledTraits) }) - } return dependencies } else { var requiredDependencies: Set = [] diff --git a/Sources/Workspace/Workspace+Manifests.swift b/Sources/Workspace/Workspace+Manifests.swift index 6a23983e8a5..5cc82c0b55c 100644 --- a/Sources/Workspace/Workspace+Manifests.swift +++ b/Sources/Workspace/Workspace+Manifests.swift @@ -216,9 +216,11 @@ extension Workspace { return true } return !conditionTraits.intersection(allRootEnabledTraits).isEmpty - }.map(\.name) ?? [] + }.map(\.name) - traitMap[dependency.identity] = Set(explicitlyEnabledTraits) + if let explicitlyEnabledTraits { + traitMap[dependency.identity] = Set(explicitlyEnabledTraits) + } } var unusedIdentities: OrderedCollections.OrderedSet = [] @@ -226,26 +228,26 @@ extension Workspace { let inputNodes: [GraphLoadingNode] = try root.packages.map { identity, package in inputIdentities.append(package.reference) - let traits: Set? = rootEnabledTraitsMap[package.reference.identity] ?? [] + let traits: Set? = rootEnabledTraitsMap[package.reference.identity] let node = try GraphLoadingNode( identity: identity, manifest: package.manifest, productFilter: .everything, - enabledTraits: traits ?? [] + enabledTraits: traits //?? [] ) return node } + root.dependencies.compactMap { dependency in let package = dependency.packageRef inputIdentities.append(package) return try manifestsMap[dependency.identity].map { manifest in - let traits: Set? = rootDependenciesEnabledTraitsMap[dependency.identity] ?? [] + let traits: Set? = rootDependenciesEnabledTraitsMap[dependency.identity] return try GraphLoadingNode( identity: dependency.identity, manifest: manifest, productFilter: dependency.productFilter, - enabledTraits: traits ?? [] + enabledTraits: traits //?? [] ) } } @@ -265,11 +267,12 @@ extension Workspace { var requiredIdentities: OrderedCollections.OrderedSet = [] _ = try transitiveClosure(inputNodes) { node in - - try node.manifest.dependenciesRequired(for: node.productFilter, node.enabledTraits) +// print("checking \(node.manifest.displayName) with traits \(node.enabledTraits)") + return try node.manifest.dependenciesRequired(for: node.productFilter, node.enabledTraits) .compactMap { dependency in let package = dependency.packageRef +// print("checking if dep \(dependency.identity) is used in \(node.manifest.displayName) with traits \(node.enabledTraits)") // Check if traits are guarding the dependency from being enabled. // Also check whether we've enabled pruning unused dependencies. let isDepUsed = try node.manifest.isPackageDependencyUsed( @@ -280,7 +283,7 @@ extension Workspace { observabilityScope.emit(debug: """ '\(package.identity)' from '\(package.locationString)' was omitted \ from required dependencies because it is being guarded by the following traits:' \ - \(node.enabledTraits.joined(separator: ", ")) + \(node.enabledTraits?.joined(separator: ", ")) """) } else { unusedDepsPerPackage[node.identity, default: []] = unusedDepsPerPackage[ @@ -333,13 +336,13 @@ extension Workspace { guard let conditionTraits = $0.condition?.traits else { return true } - return !conditionTraits.intersection(node.enabledTraits).isEmpty + return !conditionTraits.intersection(node.enabledTraits ?? []).isEmpty }.map(\.name) return try manifestsMap[dependency.identity].map { manifest in // Calculate all transitively enabled traits for this manifest. - var allEnabledTraits: Set = [] + var allEnabledTraits: Set? if let explicitlyEnabledTraits { allEnabledTraits = Set(explicitlyEnabledTraits) @@ -425,9 +428,11 @@ extension Workspace { } return !conditionTraits .intersection(workspace.configuration.traitConfiguration.enabledTraits ?? []).isEmpty - }.map(\.name) ?? [] + }.map(\.name) - traitMap[dependency.identity] = Set(explicitlyEnabledTraits) + if let explicitlyEnabledTraits { + traitMap[dependency.identity] = Set(explicitlyEnabledTraits) + } } for (externalManifest, managedDependency, productFilter, _) in dependencies { @@ -591,7 +596,7 @@ extension Workspace { let rootManifests = try root.manifests.mapValues { manifest in let deps = try manifest.dependencies.filter { dep in - guard configuration.pruneDependencies else { return true } +// guard configuration.pruneDependencies else { return true } let enabledTraits = root.enabledTraits[manifest.packageIdentity] let isDepUsed = try manifest.isPackageDependencyUsed(dep, enabledTraits: enabledTraits) return isDepUsed @@ -628,7 +633,7 @@ extension Workspace { // optimization: preload first level dependencies manifest (in parallel) let firstLevelDependencies = try topLevelManifests.values.map { manifest in try manifest.dependencies.filter { dep in - guard configuration.pruneDependencies else { return true } +// guard configuration.pruneDependencies else { return true } let enabledTraits: Set? = root.enabledTraits[manifest.packageIdentity] let isDepUsed = try manifest.isPackageDependencyUsed(dep, enabledTraits: enabledTraits) return isDepUsed @@ -647,6 +652,7 @@ extension Workspace { GraphLoadingNode, PackageIdentity >] = { node in +// observabilityScope.emit(warning: "entering with \(node.item.manifest.displayName) and enabled traits: \(node.item.enabledTraits)") // optimization: preload manifest we know about in parallel let dependenciesRequired = try node.item.manifest.dependenciesRequired( for: node.item.productFilter, @@ -662,8 +668,10 @@ extension Workspace { observabilityScope: observabilityScope ) dependenciesManifests.forEach { loadedManifests[$0.key] = $0.value } +// observabilityScope.emit(warning: "deps: \(dependenciesGuarded.count + dependenciesRequired.count)") return try (dependenciesRequired + dependenciesGuarded).compactMap { dependency in - try loadedManifests[dependency.identity].flatMap { manifest in + return try loadedManifests[dependency.identity].flatMap { manifest in + // we also compare the location as this function may attempt to load // dependencies that have the same identity but from a different location // which is an error case we diagnose an report about in the GraphLoading part which @@ -672,9 +680,11 @@ extension Workspace { guard let conditionTraits = $0.condition?.traits else { return true } - return !conditionTraits.intersection(node.item.enabledTraits).isEmpty + return !conditionTraits.intersection(node.item.enabledTraits ?? []).isEmpty }.map(\.name) + let usingTraits = explicitlyEnabledTraits.flatMap { Set($0) } + let calculatedTraits = try manifest.enabledTraits( using: explicitlyEnabledTraits.flatMap { Set($0) }, node.item.identity.description @@ -686,7 +696,7 @@ extension Workspace { identity: dependency.identity, manifest: manifest, productFilter: dependency.productFilter, - enabledTraits: calculatedTraits ?? [] + enabledTraits: calculatedTraits ), key: dependency.identity ) : @@ -699,14 +709,15 @@ extension Workspace { do { let manifestGraphRoots = try topLevelManifests.map { identity, manifest in - let isRoot = manifest.packageKind.isRoot - let enabledTraits = isRoot ? root.enabledTraits[identity] : [] + // TODO bp: not sure if this is correct +// let isRoot = manifest.packageKind.isRoot +// let enabledTraits = isRoot ? root.enabledTraits[identity] : [] return try KeyedPair( GraphLoadingNode( identity: identity, manifest: manifest, productFilter: .everything, - enabledTraits: enabledTraits ?? [] + enabledTraits: root.enabledTraits[identity] ), key: identity ) @@ -718,7 +729,10 @@ extension Workspace { ) { allNodes[$0.key] = $0.item } onDuplicate: { old, new in - allNodes[old.key]?.enabledTraits.formUnion(new.item.enabledTraits) + if let enabledTraits = new.item.enabledTraits { + // TODO bp: this may not be necessary anymore since we pre-compute all enabled and transitively enabled traits...? +// allNodes[old.key]?.enabledTraits?.formUnion(enabledTraits) + } } } diff --git a/Tests/FunctionalTests/TraitTests.swift b/Tests/FunctionalTests/TraitTests.swift index 160ddd6c81b..a7af4641efa 100644 --- a/Tests/FunctionalTests/TraitTests.swift +++ b/Tests/FunctionalTests/TraitTests.swift @@ -43,7 +43,7 @@ struct TraitTests { let (stdout, stderr) = try await executeSwiftRun( fixturePath.appending("Example"), "Example", - extraArgs: ["--experimental-prune-unused-dependencies"], +// extraArgs: ["--experimental-prune-unused-dependencies"], buildSystem: buildSystem, ) // We expect no warnings to be produced. Specifically no unused dependency warnings. @@ -81,7 +81,7 @@ struct TraitTests { let (stdout, stderr) = try await executeSwiftRun( fixturePath.appending("Example"), "Example", - extraArgs: ["--traits", "default,Package9,Package10", "--experimental-prune-unused-dependencies"], + extraArgs: ["--traits", "default,Package9,Package10"/*, "--experimental-prune-unused-dependencies"*/], buildSystem: buildSystem, ) // We expect no warnings to be produced. Specifically no unused dependency warnings. @@ -122,7 +122,7 @@ struct TraitTests { let (stdout, stderr) = try await executeSwiftRun( fixturePath.appending("Example"), "Example", - extraArgs: ["--traits", "default,Package9", "--experimental-prune-unused-dependencies"], + extraArgs: ["--traits", "default,Package9"/*, "--experimental-prune-unused-dependencies"*/], buildSystem: buildSystem, ) // We expect no warnings to be produced. Specifically no unused dependency warnings. @@ -164,7 +164,7 @@ struct TraitTests { extraArgs: [ "--traits", "default,Package5,Package7,BuildCondition3", - "--experimental-prune-unused-dependencies", + //"--experimental-prune-unused-dependencies", ], buildSystem: buildSystem, ) @@ -205,7 +205,7 @@ struct TraitTests { let (stdout, stderr) = try await executeSwiftRun( fixturePath.appending("Example"), "Example", - extraArgs: ["--disable-default-traits", "--experimental-prune-unused-dependencies"], + extraArgs: ["--disable-default-traits"/*"--experimental-prune-unused-dependencies"*/], buildSystem: buildSystem, ) // We expect no warnings to be produced. Specifically no unused dependency warnings. @@ -238,7 +238,7 @@ struct TraitTests { let (stdout, stderr) = try await executeSwiftRun( fixturePath.appending("Example"), "Example", - extraArgs: ["--traits", "Package5,Package7", "--experimental-prune-unused-dependencies"], + extraArgs: ["--traits", "Package5,Package7"/*, "--experimental-prune-unused-dependencies"*/], buildSystem: buildSystem, ) // We expect no warnings to be produced. Specifically no unused dependency warnings. @@ -274,7 +274,7 @@ struct TraitTests { let (stdout, stderr) = try await executeSwiftRun( fixturePath.appending("Example"), "Example", - extraArgs: ["--enable-all-traits", "--experimental-prune-unused-dependencies"], + extraArgs: ["--enable-all-traits"/*, "--experimental-prune-unused-dependencies"*/], buildSystem: buildSystem, ) // We expect no warnings to be produced. Specifically no unused dependency warnings. @@ -321,7 +321,7 @@ struct TraitTests { extraArgs: [ "--enable-all-traits", "--disable-default-traits", - "--experimental-prune-unused-dependencies", +// "--experimental-prune-unused-dependencies", ], buildSystem: buildSystem, ) @@ -384,7 +384,7 @@ struct TraitTests { try await fixture(name: "Traits") { fixturePath in let (stdout, _) = try await executeSwiftTest( fixturePath.appending("Example"), - extraArgs: ["--experimental-prune-unused-dependencies"], +// extraArgs: ["--experimental-prune-unused-dependencies"], buildSystem: buildSystem, ) let expectedOut = """ @@ -421,7 +421,7 @@ struct TraitTests { extraArgs: [ "--enable-all-traits", "--disable-default-traits", - "--experimental-prune-unused-dependencies", + // "--experimental-prune-unused-dependencies", ], buildSystem: buildSystem, ) @@ -461,7 +461,7 @@ struct TraitTests { try await fixture(name: "Traits") { fixturePath in let (stdout, _) = try await executeSwiftPackage( fixturePath.appending("Package10"), - extraArgs: ["dump-symbol-graph", "--experimental-prune-unused-dependencies"], + extraArgs: ["dump-symbol-graph"/*, "--experimental-prune-unused-dependencies"*/], buildSystem: buildSystem, ) let optionalPath = stdout @@ -489,7 +489,7 @@ struct TraitTests { try await fixture(name: "Traits") { fixturePath in let (stdout, _) = try await executeSwiftPackage( fixturePath.appending("Package10"), - extraArgs: ["plugin", "extract", "--experimental-prune-unused-dependencies"], + extraArgs: ["plugin", "extract"/*, "--experimental-prune-unused-dependencies"*/], buildSystem: buildSystem, ) let path = String(stdout.split(whereSeparator: \.isNewline).first!)