diff --git a/Fixtures/Traits/Example/Package.swift b/Fixtures/Traits/Example/Package.swift index 6eaf1479afa..b5358cb9d3b 100644 --- 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 index 5f1d871df50..d1edafb6bb2 100644 --- 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 index 4236e806cff..60767db5e7a 100644 --- 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 new file mode 100644 index 00000000000..8d1dd343c75 --- /dev/null +++ b/Fixtures/Traits/Package10/Sources/Package10Library2/Library.swift @@ -0,0 +1,3 @@ +public func hello() { + print("Package10Library2 has been included.") +} \ No newline at end of file diff --git a/Sources/PackageModel/Manifest/Manifest+Traits.swift b/Sources/PackageModel/Manifest/Manifest+Traits.swift index 9f9364ea3a9..e5258a7a188 100644 --- 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/* = ["default"]*/) throws -> Bool { + public func isPackageDependencyUsed(_ dependency: PackageDependency, enabledTraits: Set) 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 new file mode 100644 index 00000000000..92664e017a6 --- /dev/null +++ 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 index 9475ab9a5d8..39b3615c06b 100644 --- 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 index d42b450bdb1..19b84a0c56d 100644 --- 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,6 +712,12 @@ 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: [ @@ -718,6 +725,7 @@ class ManifestTests: XCTestCase { 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,6 +802,33 @@ 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"] + )) } } @@ -799,11 +836,13 @@ class ManifestTests: XCTestCase { 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 index 667d61aee2f..cdfd563f3ec 100644 --- a/Tests/WorkspaceTests/WorkspaceTests.swift +++ b/Tests/WorkspaceTests/WorkspaceTests.swift @@ -16257,6 +16257,113 @@ final class WorkspaceTests: XCTestCase { } } + func testManyTraitsEnableTargetDependency() async throws { + let sandbox = AbsolutePath("/tmp/ws/") + let fs = InMemoryFileSystem() + + func createMockWorkspace(_ traitConfiguration: TraitConfiguration) async throws -> MockWorkspace { + try await MockWorkspace( + sandbox: sandbox, + fileSystem: fs, + roots: [ + MockPackage( + name: "Cereal", + targets: [ + MockTarget( + name: "Wheat", + dependencies: [ + .product( + name: "Icing", + package: "Sugar", + condition: .init(traits: ["BreakfastOfChampions", "DontTellMom"]) + ), + ] + ), + ], + products: [ + MockProduct(name: "YummyBreakfast", modules: ["Wheat"]) + ], + dependencies: [ + .sourceControl(path: "./Sugar", requirement: .upToNextMajor(from: "1.0.0")), + ], + traits: ["BreakfastOfChampions", "DontTellMom"] + ), + ], + packages: [ + MockPackage( + name: "Sugar", + targets: [ + MockTarget(name: "Icing"), + ], + products: [ + MockProduct(name: "Icing", modules: ["Icing"]), + ], + versions: ["1.0.0", "1.5.0"] + ), + ], + traitConfiguration: traitConfiguration + ) + } + + + let deps: [MockDependency] = [ + .sourceControl(path: "./Sugar", requirement: .exact("1.0.0"), products: .specific(["Icing"])), + ] + + let workspaceOfChampions = try await createMockWorkspace(.enabledTraits(["BreakfastOfChampions"])) + try await workspaceOfChampions.checkPackageGraph(roots: ["Cereal"], deps: deps) { graph, diagnostics in + XCTAssertNoDiagnostics(diagnostics) + PackageGraphTesterXCTest(graph) { result in + result.check(roots: "Cereal") + result.check(packages: "cereal", "sugar") + result.check(modules: "Wheat", "Icing") + result.check(products: "YummyBreakfast", "Icing") + result.checkTarget("Wheat") { result in + result.check(dependencies: "Icing") + } + } + } + + let dontTellMomAboutThisWorkspace = try await createMockWorkspace(.enabledTraits(["DontTellMom"])) + try await dontTellMomAboutThisWorkspace.checkPackageGraph(roots: ["Cereal"], deps: deps) { graph, diagnostics in + XCTAssertNoDiagnostics(diagnostics) + PackageGraphTesterXCTest(graph) { result in + result.check(roots: "Cereal") + result.check(packages: "cereal", "sugar") + result.check(modules: "Wheat", "Icing") + result.check(products: "YummyBreakfast", "Icing") + result.checkTarget("Wheat") { result in + result.check(dependencies: "Icing") + } + } + } + + let allEnabledTraitsWorkspace = try await createMockWorkspace(.enableAllTraits) + try await allEnabledTraitsWorkspace.checkPackageGraph(roots: ["Cereal"], deps: deps) { graph, diagnostics in + XCTAssertNoDiagnostics(diagnostics) + PackageGraphTesterXCTest(graph) { result in + result.check(roots: "Cereal") + result.check(packages: "cereal", "sugar") + result.check(modules: "Wheat", "Icing") + result.check(products: "YummyBreakfast", "Icing") + result.checkTarget("Wheat") { result in + result.check(dependencies: "Icing") + } + } + } + + let noSugarForBreakfastWorkspace = try await createMockWorkspace(.disableAllTraits) + try await noSugarForBreakfastWorkspace.checkPackageGraph(roots: ["Cereal"], deps: deps) { graph, diagnostics in + XCTAssertNoDiagnostics(diagnostics) + PackageGraphTesterXCTest(graph) { result in + result.check(roots: "Cereal") + result.check(packages: "cereal") + result.check(modules: "Wheat") + result.check(products: "YummyBreakfast") + } + } + } + func makeRegistryClient( packageIdentity: PackageIdentity, packageVersion: Version,