Fix for .when(traits:) condition not working for multiple traits (swiftlang#9015)

bripeticca · web-flow · commit abd31578fbf3 · 2025-08-26T12:39:32.000-07:00
The culprit here is the fact that we were essentially doing an AND
boolean check across all the traits in the target dependency condition,
when we should be doing an OR boolean check.

If at least one trait is enabled in the list of traits in the target
dependency condition, then the target dependecy should be considered
enabled as well.
diff --git a/Fixtures/Traits/Example/Package.swift b/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"])),
diff --git a/Fixtures/Traits/Example/Sources/Example/Example.swift b/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")
diff --git a/Fixtures/Traits/Package10/Package.swift b/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(
diff --git a/Fixtures/Traits/Package10/Sources/Package10Library2/Library.swift b/Fixtures/Traits/Package10/Sources/Package10Library2/Library.swift
@@ -0,0 +1,3 @@
+public func hello() {
+    print("Package10Library2 has been included.")
+}
diff --git a/Sources/PackageModel/Manifest/Manifest+Traits.swift b/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(
@@ -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
diff --git a/Sources/_InternalTestSupport/SwiftTesting+TraitArgumentData.swift b/Sources/_InternalTestSupport/SwiftTesting+TraitArgumentData.swift
@@ -0,0 +1,27 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift open source project
+//
+// Copyright (c) 2025 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See http://swift.org/LICENSE.txt for license information
+// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import struct SPMBuildCore.BuildSystemProvider
+import enum PackageModel.BuildConfiguration
+
+/// A utility struct that represents a list of traits that would be passed through the command line.
+/// This is used for testing purposes, and its use is currently specific to the `TraitTests.swift`
+public struct TraitArgumentData {
+    public var traitsArgument: String
+    public var expectedOutput: String
+}
+
+public func getTraitCombinations(_ traitsAndMessage: (traits: String, output: String)...) -> [TraitArgumentData] {
+    traitsAndMessage.map { traitListAndMessage in
+        TraitArgumentData(traitsArgument: traitListAndMessage.traits, expectedOutput: traitListAndMessage.output)
+    }
+}
diff --git a/Tests/FunctionalTests/TraitTests.swift b/Tests/FunctionalTests/TraitTests.swift
@@ -111,6 +111,7 @@ struct TraitTests {
             Package10Library1 trait2 enabled
             Package10Library1 trait1 enabled
             Package10Library1 trait2 enabled
+            Package10Library2 has been included.
             DEFINE1 enabled
             DEFINE2 disabled
             DEFINE3 disabled
@@ -362,6 +363,8 @@ struct TraitTests {
             Package10Library1 trait2 enabled
             Package10Library1 trait1 enabled
             Package10Library1 trait2 enabled
+            Package10Library2 has been included.
+            Package10Library2 has been included.
             DEFINE1 enabled
             DEFINE2 enabled
             DEFINE3 enabled
@@ -421,6 +424,8 @@ struct TraitTests {
             Package10Library1 trait2 enabled
             Package10Library1 trait1 enabled
             Package10Library1 trait2 enabled
+            Package10Library2 has been included.
+            Package10Library2 has been included.
             DEFINE1 enabled
             DEFINE2 enabled
             DEFINE3 enabled
@@ -454,7 +459,7 @@ struct TraitTests {
             let json = try JSON(bytes: ByteString(encodingAsUTF8: dumpOutput))
             guard case .dictionary(let contents) = json else { Issue.record("unexpected result"); return }
             guard case .array(let traits)? = contents["traits"] else { Issue.record("unexpected result"); return }
-            #expect(traits.count == 12)
+            #expect(traits.count == 13)
         }
     }
 
@@ -653,4 +658,78 @@ struct TraitTests {
             }
         }
     }
+
+    @Test(
+        .IssueSwiftBuildLinuxRunnable,
+        .IssueProductTypeForObjectLibraries,
+        .tags(
+            Tag.Feature.Command.Run,
+        ),
+        arguments:
+        getBuildData(for: SupportedBuildSystemOnAllPlatforms),
+        getTraitCombinations(
+            ("ExtraTrait",
+            """
+            Package10Library2 has been included.
+            DEFINE1 disabled
+            DEFINE2 disabled
+            DEFINE3 disabled
+            
+            """
+            ),
+            ("Package10",
+            """
+            Package10Library1 trait1 disabled
+            Package10Library1 trait2 enabled
+            Package10Library2 has been included.
+            DEFINE1 disabled
+            DEFINE2 disabled
+            DEFINE3 disabled
+            
+            """
+            ),
+            ("ExtraTrait,Package10",
+            """
+            Package10Library1 trait1 disabled
+            Package10Library1 trait2 enabled
+            Package10Library2 has been included.
+            Package10Library2 has been included.
+            DEFINE1 disabled
+            DEFINE2 disabled
+            DEFINE3 disabled
+            
+            """
+            )
+        )
+    )
+    func traits_whenManyTraitsEnableTargetDependency(
+        data: BuildData,
+        traits: TraitArgumentData
+    ) async throws {
+        try await withKnownIssue(
+            """
+            Linux: https://github.com/swiftlang/swift-package-manager/issues/8416,
+            Windows: https://github.com/swiftlang/swift-build/issues/609
+            """,
+            isIntermittent: (ProcessInfo.hostOperatingSystem == .windows),
+        ) {
+            try await fixture(name: "Traits") { fixturePath in
+                // We expect no warnings to be produced. Specifically no unused dependency warnings.
+                let unusedDependencyRegex = try Regex("warning: '.*': dependency '.*' is not used by any target")
+
+                let (stdout, stderr) = try await executeSwiftRun(
+                    fixturePath.appending("Example"),
+                    "Example",
+                    configuration: data.config,
+                    extraArgs: ["--traits", traits.traitsArgument],
+                    buildSystem: data.buildSystem,
+                )
+                #expect(!stderr.contains(unusedDependencyRegex))
+                #expect(stdout == traits.expectedOutput)
+            }
+        } when: {
+            (ProcessInfo.hostOperatingSystem == .windows && (CiEnvironment.runningInSmokeTestPipeline || data.buildSystem == .swiftbuild))
+            || (data.buildSystem == .swiftbuild && ProcessInfo.hostOperatingSystem == .linux && CiEnvironment.runningInSelfHostedPipeline)
+        }
+    }
 }
diff --git a/Tests/PackageModelTests/ManifestTests.swift b/Tests/PackageModelTests/ManifestTests.swift
@@ -683,6 +683,7 @@ class ManifestTests: XCTestCase {
         let dependencies: [PackageDependency] = [
             .localSourceControl(path: "/Bar", requirement: .upToNextMajor(from: "1.0.0")),
             .localSourceControl(path: "/Baz", requirement: .upToNextMajor(from: "1.0.0")),
+            .localSourceControl(path: "/Cosmic", requirement: .upToNextMajor(from: "1.0.0")),
         ]
 
         let products = try [
@@ -711,13 +712,20 @@ class ManifestTests: XCTestCase {
             package: "Boom"
         )
 
+        let manyTraitsEnableTargetDependency: TargetDescription.Dependency = .product(
+            name: "Supernova",
+            package: "Cosmic",
+            condition: .init(traits: ["Space", "Music"])
+        )
+
         let target = try TargetDescription(
             name: "Foo",
             dependencies: [
                 unguardedTargetDependency,
                 trait3GuardedTargetDependency,
                 defaultTraitGuardedTargetDependency,
                 enabledTargetDependencyWithSamePackage,
+                manyTraitsEnableTargetDependency
             ]
         )
 
@@ -726,6 +734,8 @@ class ManifestTests: XCTestCase {
             TraitDescription(name: "Trait1", enabledTraits: ["Trait2"]),
             TraitDescription(name: "Trait2"),
             TraitDescription(name: "Trait3"),
+            TraitDescription(name: "Space"),
+            TraitDescription(name: "Music"),
         ]
 
         do {
@@ -792,18 +802,47 @@ class ManifestTests: XCTestCase {
                 enabledTargetDependencyWithSamePackage,
                 enabledTraits: []
             ))
+
+            // Test variations of traits that enable a target dependency that is unguarded by many traits.
+            XCTAssertFalse(try manifest.isTargetDependencyEnabled(
+                target: "Foo",
+                manyTraitsEnableTargetDependency,
+                enabledTraits: []
+            ))
+            XCTAssertTrue(try manifest.isTargetDependencyEnabled(
+                target: "Foo",
+                manyTraitsEnableTargetDependency,
+                enabledTraits: ["Space"]
+            ))
+            XCTAssertTrue(try manifest.isTargetDependencyEnabled(
+                target: "Foo",
+                manyTraitsEnableTargetDependency,
+                enabledTraits: ["Music"]
+            ))
+            XCTAssertTrue(try manifest.isTargetDependencyEnabled(
+                target: "Foo",
+                manyTraitsEnableTargetDependency,
+                enabledTraits: ["Music", "Space"]
+            ))
+            XCTAssertTrue(try manifest.isTargetDependencyEnabled(
+                target: "Foo",
+                manyTraitsEnableTargetDependency,
+                enabledTraits: ["Trait3", "Music", "Space", "Trait1", "Trait2"]
+            ))
         }
     }
 
     func testIsPackageDependencyUsed() throws {
         let bar: PackageDependency = .localSourceControl(path: "/Bar", requirement: .upToNextMajor(from: "1.0.0"))
         let baz: PackageDependency = .localSourceControl(path: "/Baz", requirement: .upToNextMajor(from: "1.0.0"))
         let bam: PackageDependency = .localSourceControl(path: "/Bam", requirement: .upToNextMajor(from: "1.0.0"))
+        let drinks: PackageDependency = .localSourceControl(path: "/Drinks", requirement: .upToNextMajor(from: "1.0.0"))
 
         let dependencies: [PackageDependency] = [
             bar,
             baz,
             bam,
+            drinks,
         ]
 
         let products = try [
@@ -832,12 +871,19 @@ class ManifestTests: XCTestCase {
             package: "Bam"
         )
 
+        let manyTraitsGuardingTargetDependency: TargetDescription.Dependency = .product(
+            name: "Coffee",
+            package: "Drinks",
+            condition: .init(traits: ["Sugar", "Cream"])
+        )
+
         let target = try TargetDescription(
             name: "Foo",
             dependencies: [
                 unguardedTargetDependency,
                 trait3GuardedTargetDependency,
                 defaultTraitGuardedTargetDependency,
+                manyTraitsGuardingTargetDependency
             ]
         )
 
@@ -856,6 +902,8 @@ class ManifestTests: XCTestCase {
             TraitDescription(name: "Trait1", enabledTraits: ["Trait2"]),
             TraitDescription(name: "Trait2"),
             TraitDescription(name: "Trait3"),
+            TraitDescription(name: "Sugar"),
+            TraitDescription(name: "Cream"),
         ]
 
         do {
@@ -879,19 +927,19 @@ class ManifestTests: XCTestCase {
                 traits: traits
             )
 
-//            XCTAssertTrue(try manifest.isPackageDependencyUsed(bar))
             XCTAssertTrue(try manifest.isPackageDependencyUsed(bar, enabledTraits: []))
-//            XCTAssertFalse(try manifest.isPackageDependencyUsed(baz))
             XCTAssertTrue(try manifest.isPackageDependencyUsed(baz, enabledTraits: ["Trait3"]))
-//            XCTAssertTrue(try manifest.isPackageDependencyUsed(bam))
             XCTAssertFalse(try manifest.isPackageDependencyUsed(bam, enabledTraits: []))
             XCTAssertFalse(try manifest.isPackageDependencyUsed(bam, enabledTraits: ["Trait3"]))
+            XCTAssertFalse(try manifest.isPackageDependencyUsed(drinks, enabledTraits: []))
+            XCTAssertTrue(try manifest.isPackageDependencyUsed(drinks, enabledTraits: ["Sugar"]))
+            XCTAssertTrue(try manifest.isPackageDependencyUsed(drinks, enabledTraits: ["Cream"]))
+            XCTAssertTrue(try manifest.isPackageDependencyUsed(drinks, enabledTraits: ["Sugar", "Cream"]))
 
             // Configuration of the manifest that includes a case where there exists a target
             // dependency that depends on the same package as another target dependency, but
             // is unguarded by traits; therefore, this package dependency should be considered used
             // in every scenario, regardless of the passed trait configuration.
-//            XCTAssertTrue(try manifestWithBamDependencyAlwaysUsed.isPackageDependencyUsed(bam, enabledTraits: nil))
             XCTAssertTrue(try manifestWithBamDependencyAlwaysUsed.isPackageDependencyUsed(bam, enabledTraits: []))
             XCTAssertTrue(try manifestWithBamDependencyAlwaysUsed.isPackageDependencyUsed(bam, enabledTraits: ["Trait3"]))
         }
diff --git a/Tests/WorkspaceTests/WorkspaceTests.swift b/Tests/WorkspaceTests/WorkspaceTests.swift

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+public func hello() {`
	`2`	`+ print("Package10Library2 has been included.")`
	`3`	`+}`