Fix pre-computation of traits when loading dependency manifests

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

@@ -505,26 +505,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

@@ -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))
             }

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