[Traits] Change TraitConfiguration to be an enum instead of struct

Sources/Commands/PackageCommands/EditCommands.swift

@@ -36,7 +36,7 @@ extension SwiftPackageCommand {
         var packageIdentity: String
 
         func run(_ swiftCommandState: SwiftCommandState) async throws {
-            try await swiftCommandState.resolve(nil)
+            try await swiftCommandState.resolve()
             let workspace = try swiftCommandState.getActiveWorkspace()
 
             // Put the dependency in edit mode.
@@ -65,7 +65,7 @@ extension SwiftPackageCommand {
         var packageIdentity: String
 
         func run(_ swiftCommandState: SwiftCommandState) async throws {
-            try await swiftCommandState.resolve(nil)
+            try await swiftCommandState.resolve()
             let workspace = try swiftCommandState.getActiveWorkspace()
 
             try await workspace.unedit(

Sources/Commands/PackageCommands/Resolve.swift

@@ -14,7 +14,7 @@ import ArgumentParser
 import CoreCommands
 import TSCUtility
 
-import struct PackageGraph.TraitConfiguration
+import enum PackageGraph.TraitConfiguration
 
 extension SwiftPackageCommand {
     struct ResolveOptions: ParsableArguments {

Sources/CoreCommands/Options.swift

@@ -27,7 +27,7 @@ import class PackageModel.Manifest
 import enum PackageModel.Sanitizer
 @_spi(SwiftPMInternal) import struct PackageModel.SwiftSDK
 
-import struct PackageGraph.TraitConfiguration
+import enum PackageGraph.TraitConfiguration
 
 import struct SPMBuildCore.BuildParameters
 import struct SPMBuildCore.BuildSystemProvider

Sources/CoreCommands/SwiftCommandState.swift

@@ -203,12 +203,14 @@ public final class SwiftCommandState {
     }
 
     /// Get the current workspace root object.
-    public func getWorkspaceRoot(traitConfiguration: TraitConfiguration? = nil) throws -> PackageGraphRootInput {
-        let packages: [AbsolutePath] = if let workspace = options.locations.multirootPackageDataFile {
-            try self.workspaceLoaderProvider(self.fileSystem, self.observabilityScope)
+    public func getWorkspaceRoot(traitConfiguration: TraitConfiguration = .default) throws -> PackageGraphRootInput {
+        let packages: [AbsolutePath]
+
+        if let workspace = options.locations.multirootPackageDataFile {
+            packages = try self.workspaceLoaderProvider(self.fileSystem, self.observabilityScope)
                 .load(workspace: workspace)
         } else {
-            try [self.getPackageRoot()]
+            packages = try [self.getPackageRoot()]
         }
 
         return PackageGraphRootInput(packages: packages, traitConfiguration: traitConfiguration)
@@ -425,10 +427,7 @@ public final class SwiftCommandState {
     }
 
     /// Returns the currently active workspace.
-    public func getActiveWorkspace(
-        emitDeprecatedConfigurationWarning: Bool = false,
-        traitConfiguration: TraitConfiguration? = nil
-    ) throws -> Workspace {
+    public func getActiveWorkspace(emitDeprecatedConfigurationWarning: Bool = false, traitConfiguration: TraitConfiguration = .default) throws -> Workspace {
         if let workspace = _workspace {
             return workspace
         }
@@ -493,9 +492,7 @@ public final class SwiftCommandState {
         return workspace
     }
 
-    public func getRootPackageInformation(traitConfiguration: TraitConfiguration? = nil) async throws
-        -> (dependencies: [PackageIdentity: [PackageIdentity]], targets: [PackageIdentity: [String]])
-    {
+    public func getRootPackageInformation(traitConfiguration: TraitConfiguration = .default) async throws -> (dependencies: [PackageIdentity: [PackageIdentity]], targets: [PackageIdentity: [String]]) {
         let workspace = try self.getActiveWorkspace(traitConfiguration: traitConfiguration)
         let root = try self.getWorkspaceRoot(traitConfiguration: traitConfiguration)
         let rootManifests = try await workspace.loadRootManifests(
@@ -603,7 +600,7 @@ public final class SwiftCommandState {
     }
 
     /// Resolve the dependencies.
-    public func resolve(_ traitConfiguration: TraitConfiguration?) async throws {
+    public func resolve(_ traitConfiguration: TraitConfiguration = .default) async throws {
         let workspace = try getActiveWorkspace(traitConfiguration: traitConfiguration)
         let root = try getWorkspaceRoot(traitConfiguration: traitConfiguration)
 
@@ -633,7 +630,7 @@ public final class SwiftCommandState {
     ) async throws -> ModulesGraph {
         try await self.loadPackageGraph(
             explicitProduct: explicitProduct,
-            traitConfiguration: nil,
+            traitConfiguration: .default,
             testEntryPointPath: testEntryPointPath
         )
     }
@@ -646,7 +643,7 @@ public final class SwiftCommandState {
     @discardableResult
     package func loadPackageGraph(
         explicitProduct: String? = nil,
-        traitConfiguration: TraitConfiguration? = nil,
+        traitConfiguration: TraitConfiguration = .default,
         testEntryPointPath: AbsolutePath? = nil
     ) async throws -> ModulesGraph {
         do {

Sources/PackageGraph/ModulesGraph+Loading.swift

@@ -966,7 +966,8 @@ private func calculateEnabledTraits(
         }
     }
 
-    if let parentPackage, !(explictlyEnabledTraits == nil || areDefaultsEnabled) && manifest.traits.isEmpty {
+    // explicitlyEnabledTraits != nil && !areDefaultsEnabled
+    if let parentPackage, !(explictlyEnabledTraits == nil || areDefaultsEnabled) && !manifest.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 ModuleError.disablingDefaultTraitsOnEmptyTraits(

Sources/PackageGraph/ModulesGraph.swift

@@ -426,7 +426,7 @@ public func loadModulesGraph(
     useXCBuildFileRules: Bool = false,
     customXCTestMinimumDeploymentTargets: [PackageModel.Platform: PlatformVersion]? = .none,
     observabilityScope: ObservabilityScope,
-    traitConfiguration: TraitConfiguration? = nil
+    traitConfiguration: TraitConfiguration = .default
 ) throws -> ModulesGraph {
     let rootManifests = manifests.filter(\.packageKind.isRoot).spm_createDictionary { ($0.path, $0) }
     let externalManifests = try manifests.filter { !$0.packageKind.isRoot }
@@ -439,7 +439,7 @@ public func loadModulesGraph(
 
     let packages = Array(rootManifests.keys)
     let input = PackageGraphRootInput(packages: packages, traitConfiguration: traitConfiguration)
-    let graphRoot = PackageGraphRoot(
+    let graphRoot = try PackageGraphRoot(
         input: input,
         manifests: rootManifests,
         explicitProduct: explicitProduct,

Sources/PackageGraph/PackageContainer.swift

@@ -102,7 +102,7 @@ public protocol PackageContainer {
     /// Fetch the enabled traits of a package container.
     ///
     /// NOTE: This method should only be called on root packages.
-    func getEnabledTraits(traitConfiguration: TraitConfiguration?, version: Version?) async throws -> Set<String>
+    func getEnabledTraits(traitConfiguration: TraitConfiguration, version: Version?) async throws -> Set<String>
 }
 
 extension PackageContainer {
@@ -118,7 +118,7 @@ extension PackageContainer {
         return true
     }
 
-    public func getEnabledTraits(traitConfiguration: TraitConfiguration?, version: Version? = nil) async throws -> Set<String> {
+    public func getEnabledTraits(traitConfiguration: TraitConfiguration, version: Version? = nil) async throws -> Set<String> {
         return []
     }
 }

Sources/PackageGraph/PackageGraphRoot.swift

@@ -24,13 +24,13 @@ public struct PackageGraphRootInput {
     public let dependencies: [PackageDependency]
 
     /// The trait configuration for the root packages.
-    public let traitConfiguration: TraitConfiguration?
+    public let traitConfiguration: TraitConfiguration
 
     /// Create a package graph root.
     public init(
         packages: [AbsolutePath],
         dependencies: [PackageDependency] = [],
-        traitConfiguration: TraitConfiguration? = nil
+        traitConfiguration: TraitConfiguration = .default
     ) {
         self.packages = packages
         self.dependencies = dependencies
@@ -49,6 +49,7 @@ public struct PackageGraphRoot {
         return self.packages.compactMapValues { $0.manifest }
     }
 
+    /// The root manifest(s)'s enabled traits (and their transitively enabled traits).
     public var enabledTraits: [PackageIdentity: Set<String>]
 
     /// The root package references.
@@ -94,7 +95,7 @@ public struct PackageGraphRoot {
         explicitProduct: String? = nil,
         dependencyMapper: DependencyMapper? = nil,
         observabilityScope: ObservabilityScope
-    ) {
+    ) throws {
         self.packages = input.packages.reduce(into: .init(), { partial, inputPath in
             if let manifest = manifests[inputPath]  {
                 let packagePath = manifest.path.parentDirectory
@@ -103,20 +104,19 @@ public struct PackageGraphRoot {
             }
         })
 
-        do {
-            // Calculate the enabled traits for root.
-            self.enabledTraits = try packages.reduce(into: [PackageIdentity: Set<String>]()) { traitsMap, package in
-                let manifest = package.value.manifest
-                let traitConfiguration = input.traitConfiguration
-
-                let enabledTraits = try manifest.enabledTraits(using: traitConfiguration?.enabledTraits, enableAllTraits: traitConfiguration?.enableAllTraits ?? false)
+        // Calculate the enabled traits for root.
+        var enableTraitsMap: [PackageIdentity: Set<String>] = [:]
+        enableTraitsMap = try packages.reduce(into: [PackageIdentity: Set<String>]()) { traitsMap, package in
+            let manifest = package.value.manifest
+            let traitConfiguration = input.traitConfiguration
 
-                traitsMap[package.key] = enabledTraits
-            }
-        } catch {
-            self.enabledTraits = [:]
+            // Should only ever have to use trait configuration here for roots.
+            let enabledTraits = try manifest.enabledTraits2(using: traitConfiguration)
+            traitsMap[package.key] = enabledTraits
         }
 
+        self.enabledTraits = enableTraitsMap
+
         // FIXME: Deprecate special casing once the manifest supports declaring used executable products.
         // Special casing explicit products like this is necessary to pass the test suite and satisfy backwards compatibility.
         // However, changing the dependencies based on the command line arguments may force `Package.resolved` to temporarily change,
@@ -129,18 +129,26 @@ public struct PackageGraphRoot {
             // Check that the dependency is used in at least one of the manifests.
             // If not, then we can omit this dependency if pruning unused dependencies
             // is enabled.
-            return manifests.values.reduce(false) {
-                guard $1.pruneDependencies else { return $0 || true }
-                if let isUsed = try? $1.isPackageDependencyUsed(dep, enabledTraits: input.traitConfiguration?.enabledTraits, enableAllTraits: input.traitConfiguration?.enableAllTraits ?? false) {
-                    return $0 || isUsed
+            return manifests.values.reduce(false) { result, manifest in
+                guard manifest.pruneDependencies else { return true }
+                let enabledTraits: Set<String>? = enableTraitsMap[manifest.packageIdentity]
+                if let isUsed = try? manifest.isPackageDependencyUsed(dep, enabledTraits: enabledTraits) {
+                    return result || isUsed
                 }
+
                 return true
             }
         })
 
         if let explicitProduct {
             // FIXME: `dependenciesRequired` modifies manifests and prevents conversion of `Manifest` to a value type
-            let deps = try? manifests.values.lazy.map({ try $0.dependenciesRequired(for: .everything, input.traitConfiguration?.enabledTraits, enableAllTraits: input.traitConfiguration?.enableAllTraits ?? false) }).flatMap({ $0 })
+            let deps = try? manifests.values.lazy
+                .map({ manifest -> [PackageDependency] in
+                    let enabledTraits: Set<String>? = enableTraitsMap[manifest.packageIdentity]
+                    return try manifest.dependenciesRequired(for: .everything, enabledTraits)
+                })
+                .flatMap({ $0 })
+
             for dependency in deps ?? [] {
                 adjustedDependencies.append(dependency.filtered(by: .specific([explicitProduct])))
             }
@@ -224,3 +232,49 @@ extension PackageDependency.Registry.Requirement {
         }
     }
 }
+
+// TODO: bp to move to Manifest+Traits.swift file
+extension Manifest {
+    /// Calculates the set of all transitive traits that are enabled for this manifest using the passed set of
+    /// explicitly enabled traits and a flag that
+    /// determines whether all traits are enabled.
+    public func enabledTraits2(using traitConfiguration: TraitConfiguration) throws -> Set<String>? {
+        guard supportsTraits else {
+            // If this manifest does not support traits, but the passed configuration either
+            // disables default traits or enables non-default traits (i.e. traits that would
+            // not exist for this manifest) then we must throw an error.
+
+            if !traitConfiguration.enablesDefaultTraits && traitConfiguration.enablesNonDefaultTraits {
+                // TODO: bp add parent package to error message like in modulesgraph+loading.swift
+                // TODO: bp fix explicitlyEnabledTraits value passed here
+                throw TraitError.traitsNotSupported(
+                    package: displayName,
+                    explicitlyEnabledTraits: traits.map(\.name)
+                )
+            }
+
+            return nil
+        }
+
+        var enabledTraits: Set<String> = []
+
+        switch traitConfiguration {
+        case .enableAllTraits:
+            enabledTraits = Set(traits.map(\.name))
+        case .noConfiguration:
+            if let defaultTraits = defaultTraits?.map(\.name) {
+                enabledTraits = Set(defaultTraits)
+            }
+        case .noEnabledTraits:
+            return []
+        case .enabledTraits(let explicitlyEnabledTraits):
+            enabledTraits = explicitlyEnabledTraits
+        }
+
+        if let allEnabledTraits = try? calculateAllEnabledTraits(explictlyEnabledTraits: enabledTraits) {
+            enabledTraits = allEnabledTraits
+        }
+
+        return enabledTraits
+    }
+}

Sources/PackageGraph/Resolution/DependencyResolutionNode.swift

@@ -58,7 +58,7 @@ public enum DependencyResolutionNode {
     ///   It is a warning condition, and builds do not actually need these dependencies.
     ///   However, forcing the graph to resolve and fetch them anyway allows the diagnostics passes access
     ///   to the information needed in order to provide actionable suggestions to help the user stitch up the dependency declarations properly.
-    case root(package: PackageReference, traitConfiguration: TraitConfiguration? = nil)
+    case root(package: PackageReference, traitConfiguration: TraitConfiguration = .default)
 
     /// The package.
     public var package: PackageReference {
@@ -94,7 +94,14 @@ public enum DependencyResolutionNode {
     public var traits: Set<String>? {
         switch self {
         case .root(_, let config):
-            return config?.enabledTraits
+            switch config {
+            case .enabledTraits(let traits):
+                return traits
+            case .noEnabledTraits:
+                return []
+            case .noConfiguration, .enableAllTraits: // TODO: bp fix
+                return nil
+            }
         case .product(_, _, let enabledTraits):
             return enabledTraits
         default:

Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift

@@ -165,7 +165,7 @@ public struct PubGrubDependencyResolver {
     }
 
     /// Execute the resolution algorithm to find a valid assignment of versions.
-    public func solve(constraints: [Constraint], traitConfiguration: TraitConfiguration? = nil) async -> Result<[DependencyResolverBinding], Error> {
+    public func solve(constraints: [Constraint], traitConfiguration: TraitConfiguration = .default) async -> Result<[DependencyResolverBinding], Error> {
         // the graph resolution root
         let root: DependencyResolutionNode
         if constraints.count == 1, let constraint = constraints.first, constraint.package.kind.isRoot {

Sources/PackageGraph/Resolution/PubGrub/PubGrubPackageContainer.swift

@@ -155,7 +155,7 @@ final class PubGrubPackageContainer {
         node: DependencyResolutionNode,
         overriddenPackages: [PackageReference: (version: BoundVersion, products: ProductFilter)],
         root: DependencyResolutionNode,
-        traitConfiguration: TraitConfiguration? = nil
+        traitConfiguration: TraitConfiguration = .default
     ) async throws -> [Incompatibility] {
         // FIXME: It would be nice to compute bounds for this as well.
         if await !self.underlying.isToolsVersionCompatible(at: version) {