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/Fixtures/Traits/PackageConditionalDeps/Package.swift b/Fixtures/Traits/PackageConditionalDeps/Package.swift new file mode 100644 index 00000000000..d5567dc8ffd --- /dev/null +++ b/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"]) + ) + ] + ), + ] +) diff --git a/Fixtures/Traits/PackageConditionalDeps/Sources/PackageConditionalDeps/PackageConditionalDeps.swift b/Fixtures/Traits/PackageConditionalDeps/Sources/PackageConditionalDeps/PackageConditionalDeps.swift new file mode 100644 index 00000000000..231b3efb31b --- /dev/null +++ b/Fixtures/Traits/PackageConditionalDeps/Sources/PackageConditionalDeps/PackageConditionalDeps.swift @@ -0,0 +1,3 @@ +public func nothingHappens() { + // Do nothing. +} diff --git a/Sources/PackageGraph/ModulesGraph.swift b/Sources/PackageGraph/ModulesGraph.swift index 5c9fe3ced47..26cf462e948 100644 --- a/Sources/PackageGraph/ModulesGraph.swift +++ b/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) diff --git a/Sources/PackageGraph/PackageGraphRoot.swift b/Sources/PackageGraph/PackageGraphRoot.swift index 7f7f3b56521..2bdcff664a0 100644 --- a/Sources/PackageGraph/PackageGraphRoot.swift +++ b/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, diff --git a/Sources/PackageGraph/PackageModel+Extensions.swift b/Sources/PackageGraph/PackageModel+Extensions.swift index a32b352fa6a..19f94f929b0 100644 --- a/Sources/PackageGraph/PackageModel+Extensions.swift +++ b/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, diff --git a/Sources/PackageModel/EnabledTraitsMap.swift b/Sources/PackageModel/EnabledTraitsMap.swift index 958aeb15219..a7338095511 100644 --- a/Sources/PackageModel/EnabledTraitsMap.swift +++ b/Sources/PackageModel/EnabledTraitsMap.swift @@ -31,7 +31,22 @@ public struct EnabledTraitsMap: ExpressibleByDictionaryLiteral { public subscript(key: PackageIdentity) -> Set { 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? { + get { storage[key] } } public var dictionaryLiteral: [PackageIdentity: Set] { diff --git a/Sources/PackageModel/Manifest/Manifest+Traits.swift b/Sources/PackageModel/Manifest/Manifest+Traits.swift index 9f9364ea3a9..a5535e828d4 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( @@ -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/* = ["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/Workspace/Workspace+Manifests.swift b/Sources/Workspace/Workspace+Manifests.swift index 334983cbd33..0a1f5c95c88 100644 --- a/Sources/Workspace/Workspace+Manifests.swift +++ b/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 = ["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) diff --git a/Sources/_InternalTestSupport/MockPackage.swift b/Sources/_InternalTestSupport/MockPackage.swift index 0185514e486..1f0c62b1ba3 100644 --- a/Sources/_InternalTestSupport/MockPackage.swift +++ b/Sources/_InternalTestSupport/MockPackage.swift @@ -35,7 +35,7 @@ public struct MockPackage { targets: [MockTarget], products: [MockProduct] = [], dependencies: [MockDependency] = [], - traits: Set = [.init(name: "default")], + traits: Set = [], versions: [String?] = [], revisionProvider: ((String) -> String)? = nil, toolsVersion: ToolsVersion? = nil 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 84eafbe5c3f..fc7c447ebef 100644 --- a/Tests/FunctionalTests/TraitTests.swift +++ b/Tests/FunctionalTests/TraitTests.swift @@ -94,6 +94,7 @@ struct TraitTests { Package10Library1 trait2 enabled Package10Library1 trait1 enabled Package10Library1 trait2 enabled + Package10Library2 has been included. DEFINE1 enabled DEFINE2 disabled DEFINE3 disabled @@ -289,6 +290,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 @@ -336,6 +339,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 @@ -362,7 +367,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) } } @@ -525,4 +530,75 @@ struct TraitTests { #expect(stderr.contains(expectedErr)) } } + + @Test( + .tags( + Tag.Feature.Command.Run, + ), + arguments: + 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( + buildSystem: BuildSystemProvider.Kind, + 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", + extraArgs: ["--traits", traits.traitsArgument], + buildSystem: buildSystem, + ) + #expect(!stderr.contains(unusedDependencyRegex)) + #expect(stdout == traits.expectedOutput) + } + } when: { + (ProcessInfo.hostOperatingSystem == .windows && (CiEnvironment.runningInSmokeTestPipeline || buildSystem == .swiftbuild)) + || (buildSystem == .swiftbuild && ProcessInfo.hostOperatingSystem == .linux && CiEnvironment.runningInSelfHostedPipeline) + } + } } diff --git a/Tests/PackageGraphTests/ModulesGraphTests.swift b/Tests/PackageGraphTests/ModulesGraphTests.swift index 2a93a6bff22..8030254f2f4 100644 --- a/Tests/PackageGraphTests/ModulesGraphTests.swift +++ b/Tests/PackageGraphTests/ModulesGraphTests.swift @@ -4463,6 +4463,164 @@ final class ModulesGraphTests: XCTestCase { } } } + + func testTraits_whenConditionalDependencies() throws { + let fs = InMemoryFileSystem( + emptyFiles: + "/Lunch/Sources/Drink/source.swift", + "/Caffeine/Sources/CoffeeTarget/source.swift", + "/Juice/Sources/AppleJuiceTarget/source.swift", + ) + + let manifests = try [ + Manifest.createRootManifest( + displayName: "Lunch", + path: "/Lunch", + toolsVersion: .v5_9, + dependencies: [ + .localSourceControl( + path: "/Caffeine", + requirement: .upToNextMajor(from: "1.0.0"), + ), + .localSourceControl( + path: "/Juice", + requirement: .upToNextMajor(from: "1.0.0") + ) + ], + targets: [ + TargetDescription( + name: "Drink", + dependencies: [ + .product( + name: "Coffee", + package: "Caffeine", + condition: .init(traits: ["EnableCoffeeDep"]) + ), + .product( + name: "AppleJuice", + package: "Juice", + condition: .init(traits: ["EnableAppleJuiceDep"]) + ) + ], + ), + ], + traits: [ + .init(name: "default", enabledTraits: ["EnableCoffeeDep"]), + .init(name: "EnableCoffeeDep"), + .init(name: "EnableAppleJuiceDep"), + ], + ), + Manifest.createFileSystemManifest( + displayName: "Caffeine", + path: "/Caffeine", + toolsVersion: .v5_9, + products: [ + .init( + name: "Coffee", + type: .library(.automatic), + targets: ["CoffeeTarget"] + ), + ], + targets: [ + TargetDescription( + name: "CoffeeTarget", + ), + ], + ), + Manifest.createFileSystemManifest( + displayName: "Juice", + path: "/Juice", + toolsVersion: .v5_9, + products: [ + .init( + name: "AppleJuice", + type: .library(.automatic), + targets: ["AppleJuiceTarget"] + ), + ], + targets: [ + TargetDescription( + name: "AppleJuiceTarget", + ), + ], + ) + ] + + // Test graph with default trait configuration + let observability = ObservabilitySystem.makeForTesting() + let graph = try loadModulesGraph( + fileSystem: fs, + manifests: manifests, + observabilityScope: observability.topScope + ) + + XCTAssertEqual(observability.diagnostics.count, 0) + PackageGraphTester(graph) { result in + result.checkPackage("Lunch") { package in + XCTAssertEqual(package.enabledTraits, ["EnableCoffeeDep"]) + XCTAssertEqual(package.dependencies.count, 1) + } + result.checkTarget("Drink") { target in + target.check(dependencies: "Coffee") + } + result.checkPackage("Caffeine") { package in + XCTAssertEqual(package.enabledTraits, ["default"]) + } + result.checkPackage("Juice") { package in + XCTAssertEqual(package.enabledTraits, ["default"]) + } + } + + // Test graph when disabling all traits + let graphWithTraitsDisabled = try loadModulesGraph( + fileSystem: fs, + manifests: manifests, + observabilityScope: observability.topScope, + traitConfiguration: .disableAllTraits + ) + XCTAssertEqual(observability.diagnostics.count, 0) + + PackageGraphTester(graphWithTraitsDisabled) { result in + result.checkPackage("Lunch") { package in + XCTAssertEqual(package.enabledTraits, []) + XCTAssertEqual(package.dependencies.count, 0) + } + result.checkTarget("Drink") { target in + XCTAssertTrue(target.target.dependencies.isEmpty) + } + result.checkPackage("Caffeine") { package in + XCTAssertEqual(package.enabledTraits, ["default"]) + } + result.checkPackage("Juice") { package in + XCTAssertEqual(package.enabledTraits, ["default"]) + } + } + + // Test graph when we set a trait configuration that enables different traits than the defaults + let graphWithDifferentEnabledTraits = try loadModulesGraph( + fileSystem: fs, + manifests: manifests, + observabilityScope: observability.topScope, + traitConfiguration: .enabledTraits(["EnableAppleJuiceDep"]) + ) + XCTAssertEqual(observability.diagnostics.count, 0) + + PackageGraphTester(graphWithDifferentEnabledTraits) { result in + result.checkPackage("Lunch") { package in + XCTAssertEqual(package.enabledTraits, ["EnableAppleJuiceDep"]) + XCTAssertEqual(package.dependencies.count, 1) + } + result.checkTarget("Drink") { target in + target.check(dependencies: "AppleJuice") + } + result.checkPackage("Caffeine") { package in + XCTAssertEqual(package.enabledTraits, ["default"]) + } + result.checkPackage("Juice") { package in + XCTAssertEqual(package.enabledTraits, ["default"]) + } + } + } } extension Manifest { diff --git a/Tests/PackageModelTests/ManifestTests.swift b/Tests/PackageModelTests/ManifestTests.swift index 438117b3d6f..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 { @@ -883,6 +931,10 @@ class ManifestTests: XCTestCase { XCTAssertTrue(try manifest.isPackageDependencyUsed(baz, enabledTraits: ["Trait3"])) 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 diff --git a/Tests/WorkspaceTests/WorkspaceTests.swift b/Tests/WorkspaceTests/WorkspaceTests.swift index 3e79d010183..be6377f0c23 100644 --- a/Tests/WorkspaceTests/WorkspaceTests.swift +++ b/Tests/WorkspaceTests/WorkspaceTests.swift @@ -16301,6 +16301,236 @@ 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) + PackageGraphTester(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) + PackageGraphTester(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) + PackageGraphTester(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) + PackageGraphTester(graph) { result in + result.check(roots: "Cereal") + result.check(packages: "cereal") + result.check(modules: "Wheat") + result.check(products: "YummyBreakfast") + } + } + } + + func testTraitsConditionalDependencies() 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"]) + ), + .product( + name: "Raisin", + package: "Fruit", + condition: .init(traits: ["Healthy"]) + ) + ] + ), + ], + products: [ + MockProduct(name: "YummyBreakfast", modules: ["Wheat"]) + ], + dependencies: [ + .sourceControl(path: "./Sugar", requirement: .upToNextMajor(from: "1.0.0")), + .sourceControl(path: "./Fruit", requirement: .upToNextMajor(from: "1.0.0")), + ], + traits: ["Healthy", "BreakfastOfChampions"] + ), + ], + packages: [ + MockPackage( + name: "Sugar", + targets: [ + MockTarget(name: "Icing"), + ], + products: [ + MockProduct(name: "Icing", modules: ["Icing"]), + ], + versions: ["1.0.0", "1.5.0"] + ), + MockPackage( + name: "Fruit", + targets: [ + MockTarget(name: "Raisin"), + ], + products: [ + MockProduct(name: "Raisin", modules: ["Raisin"]), + ], + versions: ["1.0.0", "1.2.0"] + ), + ], + traitConfiguration: traitConfiguration + ) + } + + + let deps: [MockDependency] = [ + .sourceControl(path: "./Sugar", requirement: .exact("1.0.0"), products: .specific(["Icing"])), + .sourceControl(path: "./Fruit", requirement: .exact("1.0.0"), products: .specific(["Raisin"])), + ] + + let workspaceOfChampions = try await createMockWorkspace(.enabledTraits(["BreakfastOfChampions"])) + try await workspaceOfChampions.checkPackageGraph(roots: ["Cereal"], deps: deps) { graph, diagnostics in + XCTAssertNoDiagnostics(diagnostics) + PackageGraphTester(graph) { result in + result.check(roots: "Cereal") + result.check(packages: "cereal", "sugar") + result.check(modules: "Wheat", "Icing") + result.checkTarget("Wheat") { result in + result.check(dependencies: "Icing") + } + } + } + + let healthyWorkspace = try await createMockWorkspace(.enabledTraits(["Healthy"])) + try await healthyWorkspace.checkPackageGraph(roots: ["Cereal"], deps: deps) { graph, diagnostics in + XCTAssertNoDiagnostics(diagnostics) + PackageGraphTester(graph) { result in + result.check(roots: "Cereal") + result.check(packages: "cereal", "fruit") + result.check(modules: "Wheat", "Raisin") + result.check(products: "YummyBreakfast", "Raisin") + result.checkTarget("Wheat") { result in + result.check(dependencies: "Raisin") + } + } + } + + let allEnabledTraitsWorkspace = try await createMockWorkspace(.enableAllTraits) + try await allEnabledTraitsWorkspace.checkPackageGraph(roots: ["Cereal"], deps: deps) { graph, diagnostics in + XCTAssertNoDiagnostics(diagnostics) + PackageGraphTester(graph) { result in + result.check(roots: "Cereal") + result.check(packages: "cereal", "sugar", "fruit") + result.check(modules: "Wheat", "Icing", "Raisin") + result.check(products: "YummyBreakfast", "Icing", "Raisin") + result.checkTarget("Wheat") { result in + result.check(dependencies: "Icing", "Raisin") + } + } + } + + let boringBreakfastWorkspace = try await createMockWorkspace(.disableAllTraits) + try await boringBreakfastWorkspace.checkPackageGraph(roots: ["Cereal"], deps: deps) { graph, diagnostics in + XCTAssertNoDiagnostics(diagnostics) + PackageGraphTester(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,