[6.2.1] Cherry-pick: Fix traits .when(traits:) condition; pre-computation when loading dependency manifests

Fixtures/Traits/Example/Package.swift

@@ -25,6 +25,7 @@ let package = Package(
         "BuildCondition1",
         "BuildCondition2",
         "BuildCondition3",
+        "ExtraTrait",
     ],
     dependencies: [
         .package(
@@ -101,6 +102,11 @@ let package = Package(
                     package: "Package10",
                     condition: .when(traits: ["Package10"])
                 ),
+                .product(
+                    name: "Package10Library2",
+                    package: "Package10",
+                    condition: .when(traits: ["Package10", "ExtraTrait"])
+                )
             ],
             swiftSettings: [
                 .define("DEFINE1", .when(traits: ["BuildCondition1"])),

Fixtures/Traits/Example/Sources/Example/Example.swift

@@ -21,6 +21,10 @@ import Package9Library1
 #endif
 #if Package10
 import Package10Library1
+import Package10Library2
+#endif
+#if ExtraTrait
+import Package10Library2
 #endif
 
 @main
@@ -49,6 +53,10 @@ struct Example {
         #endif
         #if Package10
         Package10Library1.hello()
+        Package10Library2.hello()
+        #endif
+        #if ExtraTrait
+        Package10Library2.hello()
         #endif
         #if DEFINE1
         print("DEFINE1 enabled")

Fixtures/Traits/Package10/Package.swift

@@ -9,6 +9,10 @@ let package = Package(
             name: "Package10Library1",
             targets: ["Package10Library1"]
         ),
+        .library(
+            name: "Package10Library2",
+            targets: ["Package10Library2"]
+        ),
     ],
     traits: [
         "Package10Trait1",
@@ -18,6 +22,9 @@ let package = Package(
         .target(
             name: "Package10Library1"
         ),
+        .target(
+            name: "Package10Library2"
+        ),
         .plugin(
             name: "SymbolGraphExtract",
             capability: .command(

Fixtures/Traits/Package10/Sources/Package10Library2/Library.swift

@@ -0,0 +1,3 @@
+public func hello() {
+    print("Package10Library2 has been included.")
+}

Fixtures/Traits/PackageConditionalDeps/Package.swift

@@ -0,0 +1,39 @@
+// swift-tools-version: 6.1
+
+import PackageDescription
+
+let package = Package(
+    name: "PackageConditionalDeps",
+    products: [
+        .library(
+            name: "PackageConditionalDeps",
+            targets: ["PackageConditionalDeps"]
+        ),
+    ],
+    traits: [
+        .default(enabledTraits: ["EnablePackage1Dep"]),
+        "EnablePackage1Dep",
+        "EnablePackage2Dep"
+    ],
+    dependencies: [
+        .package(path: "../Package1"),
+        .package(path: "../Package2"),
+    ],
+    targets: [
+        .target(
+            name: "PackageConditionalDeps",
+            dependencies: [ 
+                .product(
+                    name: "Package1Library1",
+                    package: "Package1",
+                    condition: .when(traits: ["EnablePackage1Dep"])
+                ),
+                .product(
+                    name: "Package2Library1",
+                    package: "Package2",
+                    condition: .when(traits: ["EnablePackage2Dep"])
+                )
+            ]
+        ),
+    ]
+)

Fixtures/Traits/PackageConditionalDeps/Sources/PackageConditionalDeps/PackageConditionalDeps.swift

@@ -0,0 +1,3 @@
+public func nothingHappens() {
+    // Do nothing.
+}

Sources/PackageGraph/ModulesGraph.swift

@@ -497,26 +497,14 @@ public func loadModulesGraph(
                         return condition.isSatisfied(by: parentTraits)
                     }.map(\.name)
 
-                    var enabledTraitsSet = explicitlyEnabledTraits.flatMap { Set($0) }
-                    let precomputedTraits = enabledTraitsMap[dependency.identity]
-
-                    if precomputedTraits == ["default"],
-                       let enabledTraitsSet {
-                        enabledTraitsMap[dependency.identity] = enabledTraitsSet
-                    } else {
-                        // unify traits
-                        enabledTraitsSet?.formUnion(precomputedTraits)
-                        if let enabledTraitsSet {
-                            enabledTraitsMap[dependency.identity] = enabledTraitsSet
-                        }
+                    if let enabledTraitsSet = explicitlyEnabledTraits.flatMap({ Set($0) }) {
+                        let calculatedTraits = try manifest.enabledTraits(
+                            using: enabledTraitsSet,
+                            .init(parent)
+                        )
+                        enabledTraitsMap[dependency.identity] = calculatedTraits
                     }
 
-                    let calculatedTraits = try manifest.enabledTraits(
-                        using: enabledTraitsSet ?? ["default"],
-                        .init(parent)
-                    )
-
-                    enabledTraitsMap[dependency.identity] = calculatedTraits
                     let result = visited.insert(dependency.identity)
                     if result.inserted {
                         try dependencies(of: manifest, dependency.productFilter)

Sources/PackageGraph/PackageGraphRoot.swift

@@ -164,8 +164,8 @@ public struct PackageGraphRoot {
                     return condition.isSatisfied(by: rootEnabledTraits)
                 }.map(\.name)
 
-                var enabledTraitsSet = enabledTraits.flatMap { Set($0) } ?? ["default"]
-                enabledTraitsSet.formUnion(enabledTraitsMap[dep.identity])
+                var enabledTraitsSet = enabledTraitsMap[dep.identity]
+                enabledTraitsSet.formUnion(enabledTraits.flatMap({ Set($0) }) ?? [])
 
                 return PackageContainerConstraint(
                     package: dep.packageRef,

Sources/PackageGraph/PackageModel+Extensions.swift

@@ -42,7 +42,7 @@ extension Manifest {
                 return condition.isSatisfied(by: enabledTraits)
             }.map(\.name)
 
-            var enabledTraitsSet = explicitlyEnabledTraits.flatMap({ Set($0) }) ?? ["default"]
+            let enabledTraitsSet = explicitlyEnabledTraits.flatMap({ Set($0) }) ?? ["default"]
 
             return PackageContainerConstraint(
                 package: $0.packageRef,

Sources/PackageModel/EnabledTraitsMap.swift

@@ -31,7 +31,22 @@ public struct EnabledTraitsMap: ExpressibleByDictionaryLiteral {
 
     public subscript(key: PackageIdentity) -> Set<String> {
         get { storage[key] ?? ["default"] }
-        set { storage[key] = newValue }
+        set {
+            // Omit adding "default" explicitly, since the map returns "default"
+            // if there is no explicit traits declared. This will allow us to check
+            // for nil entries in the stored dictionary, which tells us whether
+            // traits have been explicitly declared.
+            guard newValue != ["default"] else { return }
+            if storage[key] == nil {
+                storage[key] = newValue
+            } else {
+                storage[key]?.formUnion(newValue)
+            }
+        }
+    }
+
+    public subscript(explicitlyEnabledTraitsFor key: PackageIdentity) -> Set<String>? {
+        get { storage[key] }
     }
 
     public var dictionaryLiteral: [PackageIdentity: Set<String>] {

Sources/PackageModel/Manifest/Manifest+Traits.swift

@@ -95,7 +95,7 @@ extension Manifest {
         _ parentPackage: PackageIdentifier? = nil
     ) throws {
         guard supportsTraits else {
-            if explicitlyEnabledTraits != ["default"] /*!explicitlyEnabledTraits.contains("default")*/ {
+            if explicitlyEnabledTraits != ["default"] {
                 throw TraitError.traitsNotSupported(
                     parent: parentPackage,
                     package: .init(self),
@@ -116,7 +116,7 @@ extension Manifest {
         let areDefaultsEnabled = enabledTraits.contains("default")
 
         // Ensure that disabling default traits is disallowed for packages that don't define any traits.
-        if !(explicitlyEnabledTraits == nil || areDefaultsEnabled) && !self.supportsTraits {
+        if !areDefaultsEnabled && !self.supportsTraits {
             // We throw an error when default traits are disabled for a package without any traits
             // This allows packages to initially move new API behind traits once.
             throw TraitError.traitsNotSupported(
@@ -313,7 +313,7 @@ extension Manifest {
         let areDefaultsEnabled = enabledTraits.remove("default") != nil
 
         // We have to enable all default traits if no traits are enabled or the defaults are explicitly enabled
-        if /*explictlyEnabledTraits == nil*//*enabledTraits.isEmpty && */explictlyEnabledTraits == ["default"] || areDefaultsEnabled {
+        if explictlyEnabledTraits == ["default"] || areDefaultsEnabled {
             if let defaultTraits {
                 enabledTraits.formUnion(defaultTraits.flatMap(\.enabledTraits))
             }
@@ -449,15 +449,18 @@ extension Manifest {
 
         let traitsToEnable = self.traitGuardedTargetDependencies(for: target)[dependency] ?? []
 
-        let isEnabled = try traitsToEnable.allSatisfy { try self.isTraitEnabled(
+        // Check if any of the traits guarding this dependency is enabled;
+        // if so, the condition is met and the target dependency is considered
+        // to be in an enabled state.
+        let isEnabled = try traitsToEnable.contains(where: { try self.isTraitEnabled(
             .init(stringLiteral: $0),
             enabledTraits,
-        ) }
+        ) })
 
         return traitsToEnable.isEmpty || isEnabled
     }
     /// Determines whether a given package dependency is used by this manifest given a set of enabled traits.
-    public func isPackageDependencyUsed(_ dependency: PackageDependency, enabledTraits: Set<String>/* = ["default"]*/) throws -> Bool {
+    public func isPackageDependencyUsed(_ dependency: PackageDependency, enabledTraits: Set<String>) throws -> Bool {
         if self.pruneDependencies {
             let usedDependencies = try self.usedDependencies(withTraits: enabledTraits)
             let foundKnownPackage = usedDependencies.knownPackage.contains(where: {
@@ -478,8 +481,8 @@ extension Manifest {
 
             // 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 isGuarded = $0.intersection(enabledTraits).isEmpty
+                return isGuarded
             })
 
             let isUsedWithoutTraitGuarding = !targetDependenciesForPackageDependency.filter({ $0.condition?.traits == nil }).isEmpty

Sources/Workspace/Workspace+Manifests.swift

@@ -313,26 +313,15 @@ extension Workspace {
                             }
                         }
 
-                        // should calculate enabled traits here.
-                        let explicitlyEnabledTraits = dependency.traits?.filter {
-                            guard let condition = $0.condition else { return true }
-                            return condition.isSatisfied(by: node.enabledTraits)
-                        }.map(\.name)
+                        let enabledTraitsSet = workspace.enabledTraitsMap[dependency.identity]
 
                         return try manifestsMap[dependency.identity].map { manifest in
-                            // Calculate all transitively enabled traits for this manifest.
-
-                            var allEnabledTraits: Set<String> = ["default"]
-                            if let explicitlyEnabledTraits
-                            {
-                                allEnabledTraits = Set(explicitlyEnabledTraits)
-                            }
 
                             return try GraphLoadingNode(
                                 identity: dependency.identity,
                                 manifest: manifest,
                                 productFilter: dependency.productFilter,
-                                enabledTraits: allEnabledTraits
+                                enabledTraits: enabledTraitsSet
                             )
                         }
                     }
@@ -566,10 +555,9 @@ extension Workspace {
                     return condition.isSatisfied(by: parentEnabledTraits)
                 }).map(\.name)
 
-                let enabledTraitsSet = explicitlyEnabledTraits.flatMap({ Set($0) })
-                let enabledTraits = enabledTraitsSet?.union(self.enabledTraitsMap[dep.identity]) ?? self.enabledTraitsMap[dep.identity]
-
-                self.enabledTraitsMap[dep.identity] = enabledTraits
+                if let enabledTraitsSet = explicitlyEnabledTraits.flatMap({ Set($0) }) {
+                    self.enabledTraitsMap[dep.identity] = enabledTraitsSet
+                }
 
                 let isDepUsed = try manifest.isPackageDependencyUsed(dep, enabledTraits: parentEnabledTraits)
                 return isDepUsed
@@ -612,10 +600,9 @@ extension Workspace {
                     return condition.isSatisfied(by: parentEnabledTraits)
                 }).map(\.name)
 
-                let enabledTraitsSet = explicitlyEnabledTraits.flatMap({ Set($0) })
-                let enabledTraits = enabledTraitsSet?.union(self.enabledTraitsMap[dep.identity]) ?? self.enabledTraitsMap[dep.identity]
-
-                self.enabledTraitsMap[dep.identity] = enabledTraits
+                if let enabledTraitsSet = explicitlyEnabledTraits.flatMap({ Set($0) }) {
+                    self.enabledTraitsMap[dep.identity] = enabledTraitsSet
+                }
 
                 let isDepUsed = try manifest.isPackageDependencyUsed(dep, enabledTraits: parentEnabledTraits)
                 return isDepUsed
@@ -657,27 +644,14 @@ extension Workspace {
                         return condition.isSatisfied(by: node.item.enabledTraits)
                     }.map(\.name)
 
-                    var enabledTraitsSet = explicitlyEnabledTraits.flatMap { Set($0) }
-                    let precomputedTraits = self.enabledTraitsMap[dependency.identity]
-                    // Shouldn't union here if enabledTraitsMap returns "default" and we DO have explicitly enabled traits, since we're meant to flatten the default traits.
-                    if precomputedTraits == ["default"],
-                       let enabledTraitsSet {
-                        self.enabledTraitsMap[dependency.identity] = enabledTraitsSet
-                    } else {
-                        // Unify traits
-                        enabledTraitsSet?.formUnion(precomputedTraits)
-                        if let enabledTraitsSet {
-                            self.enabledTraitsMap[dependency.identity] = enabledTraitsSet
-                        }
+                    if let enabledTraitsSet = explicitlyEnabledTraits.flatMap({ Set($0) }) {
+                        let calculatedTraits = try manifest.enabledTraits(
+                            using: enabledTraitsSet,
+                            .init(node.item.manifest)
+                        )
+                        self.enabledTraitsMap[dependency.identity] = calculatedTraits
                     }
 
-                    let calculatedTraits = try manifest.enabledTraits(
-                        using: self.enabledTraitsMap[dependency.identity],
-                        .init(node.item.manifest)
-                    )
-
-                    self.enabledTraitsMap[dependency.identity] = calculatedTraits
-
                     // 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
@@ -688,7 +662,7 @@ extension Workspace {
                                 identity: dependency.identity,
                                 manifest: manifest,
                                 productFilter: dependency.productFilter,
-                                enabledTraits: calculatedTraits
+                                enabledTraits: self.enabledTraitsMap[dependency.identity]
                             ),
                             key: dependency.identity
                         ) :
@@ -783,26 +757,14 @@ extension Workspace {
                         return condition.isSatisfied(by: parentTraits)
                     }.map(\.name)
 
-                    var enabledTraitsSet = explicitlyEnabledTraits.flatMap { Set($0) }
-                    let precomputedTraits = self.enabledTraitsMap[dependency.identity]
-                    // Shouldn't union here if enabledTraitsMap returns "default" and we DO have explicitly enabled traits, since we're meant to flatten the default traits.
-                    if precomputedTraits == ["default"],
-                       let enabledTraitsSet {
-                        self.enabledTraitsMap[dependency.identity] = enabledTraitsSet
-                    } else {
-                        // Unify traits
-                        enabledTraitsSet?.formUnion(precomputedTraits)
-                        if let enabledTraitsSet {
-                            self.enabledTraitsMap[dependency.identity] = enabledTraitsSet
-                        }
+                    if let enabledTraitsSet = explicitlyEnabledTraits.flatMap({ Set($0) }) {
+                        let calculatedTraits = try manifest.enabledTraits(
+                            using: enabledTraitsSet,
+                            .init(parent)
+                        )
+                        self.enabledTraitsMap[dependency.identity] = calculatedTraits
                     }
 
-                    let calculatedTraits = try manifest.enabledTraits(
-                        using: self.enabledTraitsMap[dependency.identity],
-                        .init(parent)
-                    )
-
-                    self.enabledTraitsMap[dependency.identity] = calculatedTraits
                     let result = visited.insert(dependency.identity)
                     if result.inserted {
                         try dependencies(of: manifest, dependency.productFilter)

Sources/_InternalTestSupport/MockPackage.swift

@@ -35,7 +35,7 @@ public struct MockPackage {
         targets: [MockTarget],
         products: [MockProduct] = [],
         dependencies: [MockDependency] = [],
-        traits: Set<TraitDescription> = [.init(name: "default")],
+        traits: Set<TraitDescription> = [],
         versions: [String?] = [],
         revisionProvider: ((String) -> String)? = nil,
         toolsVersion: ToolsVersion? = nil