Introduce richer EnabledTrait model to handle thread-safe enabled traits computations

Sources/Basics/Collections/IdentifiableSet.swift

@@ -83,7 +83,7 @@ public struct IdentifiableSet<Element: Identifiable>: Collection {
 
     public func intersection(_ otherSequence: some Sequence<Element>) -> Self {
         let keysToRemove = Set(self.storage.keys).subtracting(otherSequence.map(\.id))
-        var result = Self()
+        var result = self
         for key in keysToRemove {
             result.storage.removeValue(forKey: key)
         }
@@ -101,6 +101,14 @@ public struct IdentifiableSet<Element: Identifiable>: Collection {
     public func contains(id: Element.ID) -> Bool {
         self.storage.keys.contains(id)
     }
+
+    public mutating func remove(id: Element.ID) -> Element? {
+        self.storage.removeValue(forKey: id)
+    }
+
+    public mutating func remove(_ member: Element) -> Element? {
+        self.storage.removeValue(forKey: member.id)
+    }
 }
 
 extension OrderedDictionary where Value: Identifiable, Key == Value.ID {

Sources/PackageGraph/GraphLoadingNode.swift

@@ -30,13 +30,13 @@ public struct GraphLoadingNode: Equatable, Hashable {
     public let productFilter: ProductFilter
 
     /// The enabled traits for this package.
-    package var enabledTraits: Set<String>
+    package var enabledTraits: EnabledTraits
 
     public init(
         identity: PackageIdentity,
         manifest: Manifest,
         productFilter: ProductFilter,
-        enabledTraits: Set<String>
+        enabledTraits: EnabledTraits
     ) throws {
         self.identity = identity
         self.manifest = manifest

Sources/PackageGraph/ModulesGraph+Loading.swift

@@ -399,7 +399,7 @@ private func createResolvedPackages(
         return ResolvedPackageBuilder(
             package,
             productFilter: node.productFilter,
-            enabledTraits: node.enabledTraits /*?? []*/,
+            enabledTraits: node.enabledTraits,
             isAllowedToVendUnsafeProducts: isAllowedToVendUnsafeProducts,
             allowedToOverride: allowedToOverride,
             platformVersionProvider: platformVersionProvider
@@ -1438,7 +1438,7 @@ private final class ResolvedPackageBuilder: ResolvedBuilder<ResolvedPackage> {
     var products: [ResolvedProductBuilder] = []
 
     /// The enabled traits of this package.
-    var enabledTraits: Set<String>
+    var enabledTraits: EnabledTraits
 
     /// The dependencies of this package.
     var dependencies: [ResolvedPackageBuilder] = []
@@ -1462,7 +1462,7 @@ private final class ResolvedPackageBuilder: ResolvedBuilder<ResolvedPackage> {
     init(
         _ package: Package,
         productFilter: ProductFilter,
-        enabledTraits: Set<String>,
+        enabledTraits: EnabledTraits,
         isAllowedToVendUnsafeProducts: Bool,
         allowedToOverride: Bool,
         platformVersionProvider: PlatformVersionProvider
@@ -1485,7 +1485,7 @@ private final class ResolvedPackageBuilder: ResolvedBuilder<ResolvedPackage> {
             defaultLocalization: self.defaultLocalization,
             supportedPlatforms: self.supportedPlatforms,
             dependencies: self.dependencies.map(\.package.identity),
-            enabledTraits: self.enabledTraits,
+            enabledTraits: self.enabledTraits.names,
             modules: modules,
             products: products,
             registryMetadata: self.registryMetadata,

Sources/PackageGraph/ModulesGraph.swift

@@ -466,7 +466,8 @@ public func loadModulesGraph(
     useXCBuildFileRules: Bool = false,
     customXCTestMinimumDeploymentTargets: [PackageModel.Platform: PlatformVersion]? = .none,
     observabilityScope: ObservabilityScope,
-    traitConfiguration: TraitConfiguration = .default
+    traitConfiguration: TraitConfiguration = .default,
+    enabledTraitsMap: EnabledTraitsMap = .init()
 ) throws -> ModulesGraph {
     let rootManifests = manifests.filter(\.packageKind.isRoot).spm_createDictionary { ($0.path, $0) }
     let externalManifests = try manifests.filter { !$0.packageKind.isRoot }
@@ -479,70 +480,6 @@ public func loadModulesGraph(
 
     let packages = Array(rootManifests.keys)
 
-    let manifestMap = manifests.reduce(into: [PackageIdentity: Manifest]()) { manifestMap, manifest in
-        manifestMap[manifest.packageIdentity] = manifest
-    }
-
-    // Note: The following is a copy of the existing `Workspace.precomputeTraits` method
-    func precomputeTraits(
-        _ enabledTraitsMap: EnabledTraitsMap,
-        _ topLevelManifests: [Manifest],
-        _ manifestMap: [PackageIdentity: Manifest]
-    ) throws -> [PackageIdentity: Set<String>] {
-        var visited: Set<PackageIdentity> = []
-        var enabledTraitsMap = enabledTraitsMap
-
-        func dependencies(of parent: Manifest, _ productFilter: ProductFilter = .everything) throws {
-            let parentTraits = enabledTraitsMap[parent.packageIdentity]
-            let requiredDependencies = try parent.dependenciesRequired(for: productFilter, parentTraits)
-            let guardedDependencies = parent.dependenciesTraitGuarded(withEnabledTraits: parentTraits)
-
-            _ = try (requiredDependencies + guardedDependencies).compactMap({ dependency in
-                return try manifestMap[dependency.identity].flatMap({ manifest in
-
-                    let explicitlyEnabledTraits = dependency.traits?.filter {
-                        guard let condition = $0.condition else { return true }
-                        return condition.isSatisfied(by: parentTraits)
-                    }.map(\.name)
-
-                    if let enabledTraitsSet = explicitlyEnabledTraits.flatMap({ Set($0) }) {
-                        let calculatedTraits = try manifest.enabledTraits(
-                            using: enabledTraitsSet,
-                            .init(parent)
-                        )
-                        enabledTraitsMap[dependency.identity] = calculatedTraits
-                    }
-
-                    let result = visited.insert(dependency.identity)
-                    if result.inserted {
-                        try dependencies(of: manifest, dependency.productFilter)
-                    }
-
-                    return manifest
-                })
-            })
-        }
-
-        for manifest in topLevelManifests {
-            // Track already-visited manifests to avoid cycles
-            let result = visited.insert(manifest.packageIdentity)
-            if result.inserted {
-                try dependencies(of: manifest)
-            }
-        }
-
-        return enabledTraitsMap.dictionaryLiteral
-    }
-
-
-    // Precompute enabled traits for roots.
-    var enabledTraitsMap: EnabledTraitsMap = [:]
-    for root in rootManifests.values {
-        let enabledTraits = try root.enabledTraits(using: traitConfiguration)
-        enabledTraitsMap[root.packageIdentity] = enabledTraits
-    }
-    enabledTraitsMap = .init(try precomputeTraits(enabledTraitsMap, manifests, manifestMap))
-
     let input = PackageGraphRootInput(packages: packages, traitConfiguration: traitConfiguration)
     let graphRoot = try PackageGraphRoot(
         input: input,

Sources/PackageGraph/PackageContainer.swift

@@ -75,7 +75,7 @@ public protocol PackageContainer {
     /// - Precondition: `versions.contains(version)`
     /// - Throws: If the version could not be resolved; this will abort
     ///   dependency resolution completely.
-    func getDependencies(at version: Version, productFilter: ProductFilter, _ enabledTraits: Set<String>) async throws -> [PackageContainerConstraint]
+    func getDependencies(at version: Version, productFilter: ProductFilter, _ enabledTraits: EnabledTraits) async throws -> [PackageContainerConstraint]
 
     /// Fetch the declared dependencies for a particular revision.
     ///
@@ -84,12 +84,12 @@ public protocol PackageContainer {
     ///
     /// - Throws: If the revision could not be resolved; this will abort
     ///   dependency resolution completely.
-    func getDependencies(at revision: String, productFilter: ProductFilter, _ enabledTraits: Set<String>) async throws -> [PackageContainerConstraint]
+    func getDependencies(at revision: String, productFilter: ProductFilter, _ enabledTraits: EnabledTraits) async throws -> [PackageContainerConstraint]
 
     /// Fetch the dependencies of an unversioned package container.
     ///
     /// NOTE: This method should not be called on a versioned container.
-    func getUnversionedDependencies(productFilter: ProductFilter, _ enabledTraits: Set<String>) async throws -> [PackageContainerConstraint]
+    func getUnversionedDependencies(productFilter: ProductFilter, _ enabledTraits: EnabledTraits) async throws -> [PackageContainerConstraint]
 
     /// Get the updated identifier at a bound version.
     ///
@@ -150,11 +150,11 @@ public struct PackageContainerConstraint: Equatable, Hashable {
     public let products: ProductFilter
 
     /// The traits that have been enabled for the package.
-    public let enabledTraits: Set<String>
+    public let enabledTraits: EnabledTraits
 
     /// Create a constraint requiring the given `container` satisfying the
     /// `requirement`.
-    public init(package: PackageReference, requirement: PackageRequirement, products: ProductFilter, enabledTraits: Set<String> = ["default"]) {
+    public init(package: PackageReference, requirement: PackageRequirement, products: ProductFilter, enabledTraits: EnabledTraits = ["default"]) {
         self.package = package
         self.requirement = requirement
         self.products = products
@@ -163,7 +163,7 @@ public struct PackageContainerConstraint: Equatable, Hashable {
 
     /// Create a constraint requiring the given `container` satisfying the
     /// `versionRequirement`.
-    public init(package: PackageReference, versionRequirement: VersionSetSpecifier, products: ProductFilter, enabledTraits: Set<String> = ["default"]) {
+    public init(package: PackageReference, versionRequirement: VersionSetSpecifier, products: ProductFilter, enabledTraits: EnabledTraits = ["default"]) {
         self.init(package: package, requirement: .versionSet(versionRequirement), products: products, enabledTraits: enabledTraits)
     }
 

Sources/PackageGraph/PackageGraphRoot.swift

@@ -115,7 +115,7 @@ public struct PackageGraphRoot {
             // If not, then we can omit this dependency if pruning unused dependencies
             // is enabled.
             return manifests.values.reduce(false) { result, manifest in
-                let enabledTraits: Set<String> = enabledTraitsMap[manifest.packageIdentity]
+                let enabledTraits = enabledTraitsMap[manifest.packageIdentity]
                 if let isUsed = try? manifest.isPackageDependencyUsed(dep, enabledTraits: enabledTraits) {
                     return result || isUsed
                 }
@@ -128,7 +128,7 @@ public struct PackageGraphRoot {
             // FIXME: `dependenciesRequired` modifies manifests and prevents conversion of `Manifest` to a value type
             let deps = try? manifests.values.lazy
                 .map({ manifest -> [PackageDependency] in
-                    let enabledTraits: Set<String> = enabledTraitsMap[manifest.packageIdentity]
+                    let enabledTraits = enabledTraitsMap[manifest.packageIdentity]
                     return try manifest.dependenciesRequired(for: .everything, enabledTraits)
                 })
                 .flatMap({ $0 })
@@ -145,7 +145,7 @@ public struct PackageGraphRoot {
 
     /// Returns the constraints imposed by root manifests + dependencies.
     public func constraints(_ enabledTraitsMap: EnabledTraitsMap) throws -> [PackageContainerConstraint] {
-        var rootEnabledTraits: Set<String> = []
+        var rootEnabledTraits: EnabledTraits = []
         let constraints = self.packages.map { (identity, package) in
             let enabledTraits = enabledTraitsMap[identity]
             rootEnabledTraits.formUnion(enabledTraits)
@@ -161,11 +161,11 @@ public struct PackageGraphRoot {
             .map { dep in
                 let enabledTraits = dep.traits?.filter {
                     guard let condition = $0.condition else { return true }
-                    return condition.isSatisfied(by: rootEnabledTraits)
-                }.map(\.name)
+                    return condition.isSatisfied(by: rootEnabledTraits.names)
+                }.map({ EnabledTrait(name: $0.name, setBy: .package(.init(identity: "root"))) })
 
                 var enabledTraitsSet = enabledTraitsMap[dep.identity]
-                enabledTraitsSet.formUnion(enabledTraits.flatMap({ Set($0) }) ?? [])
+                enabledTraitsSet.formUnion(enabledTraits ?? [])
 
                 return PackageContainerConstraint(
                     package: dep.packageRef,

Sources/PackageGraph/PackageModel+Extensions.swift

@@ -35,14 +35,14 @@ extension PackageDependency {
 
 extension Manifest {
     /// Constructs constraints of the dependencies in the raw package.
-    public func dependencyConstraints(productFilter: ProductFilter, _ enabledTraits: Set<String> = ["default"]) throws -> [PackageContainerConstraint] {
+    public func dependencyConstraints(productFilter: ProductFilter, _ enabledTraits: EnabledTraits = ["default"]) throws -> [PackageContainerConstraint] {
         return try self.dependenciesRequired(for: productFilter, enabledTraits).map({
             let explicitlyEnabledTraits = $0.traits?.filter {
                 guard let condition = $0.condition else { return true }
-                return condition.isSatisfied(by: enabledTraits)
+                return condition.isSatisfied(by: enabledTraits.names)
             }.map(\.name)
 
-            let enabledTraitsSet = explicitlyEnabledTraits.flatMap({ Set($0) }) ?? ["default"]
+            let enabledTraitsSet = EnabledTraits(explicitlyEnabledTraits ?? [], setBy: .package(.init(identity: self.packageIdentity, name: self.displayName)))
 
             return PackageContainerConstraint(
                 package: $0.packageRef,
@@ -60,7 +60,7 @@ extension PackageContainerConstraint {
     internal func nodes() -> [DependencyResolutionNode] {
         switch products {
         case .everything:
-            return [.root(package: self.package)]
+            return [.root(package: self.package, enabledTraits: self.enabledTraits)]
         case .specific:
             switch products {
             case .everything:
@@ -70,7 +70,7 @@ extension PackageContainerConstraint {
                 if set.isEmpty { // Pointing at the package without a particular product.
                     return [.empty(package: self.package)]
                 } else {
-                    return set.sorted().map { .product($0, package: self.package) }
+                    return set.sorted().map { .product($0, package: self.package, enabledTraits: self.enabledTraits) }
                 }
             }
         }

Sources/PackageGraph/Resolution/DependencyResolutionNode.swift

@@ -44,7 +44,7 @@ public enum DependencyResolutionNode {
     ///   Since a non‐existent product ends up with only its implicit dependency on its own package,
     ///   only whichever package contains the product will end up adding additional constraints.
     ///   See `ProductFilter` and `Manifest.register(...)`.
-    case product(String, package: PackageReference, enabledTraits: Set<String> = ["default"])
+    case product(String, package: PackageReference, enabledTraits: EnabledTraits = ["default"])
 
     /// A root node.
     ///
@@ -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, enabledTraits: Set<String> = ["default"])
+    case root(package: PackageReference, enabledTraits: EnabledTraits = ["default"])
 
     /// The package.
     public var package: PackageReference {
@@ -91,7 +91,7 @@ public enum DependencyResolutionNode {
     }
 
     /// Returns the enabled traits for this node's manifest.
-    public var enabledTraits: Set<String> {
+    public var enabledTraits: EnabledTraits {
         switch self {
         case .root(_, let enabledTraits), .product(_, _, let enabledTraits):
             return enabledTraits

Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift

@@ -357,7 +357,7 @@ public struct PubGrubDependencyResolver {
                 }
 
                 for dependency in try await container.underlying
-                    .getUnversionedDependencies(productFilter: node.productFilter, constraint.enabledTraits)
+                    .getUnversionedDependencies(productFilter: node.productFilter, node.enabledTraits)
                 {
                     if let versionedBasedConstraints = VersionBasedConstraint.constraints(dependency) {
                         for constraint in versionedBasedConstraints {
@@ -431,7 +431,7 @@ public struct PubGrubDependencyResolver {
                 var unprocessedDependencies = try await container.underlying.getDependencies(
                     at: revisionForDependencies,
                     productFilter: constraint.products,
-                    constraint.enabledTraits
+                    node.enabledTraits
                 )
                 if let sharedRevision = node.revisionLock(revision: revision) {
                     unprocessedDependencies.append(sharedRevision)

Sources/PackageLoading/PackageBuilder.swift

@@ -388,7 +388,7 @@ public final class PackageBuilder {
     private var swiftVersionCache: SwiftLanguageVersion? = nil
 
     /// The enabled traits of this package.
-    private let enabledTraits: Set<String>
+    private let enabledTraits: EnabledTraits
 
     /// Create a builder for the given manifest and package `path`.
     ///
@@ -414,7 +414,7 @@ public final class PackageBuilder {
         createREPLProduct: Bool = false,
         fileSystem: FileSystem,
         observabilityScope: ObservabilityScope,
-        enabledTraits: Set<String>
+        enabledTraits: EnabledTraits
     ) {
         self.identity = identity
         self.manifest = manifest
@@ -1151,7 +1151,7 @@ public final class PackageBuilder {
 
         // Process each setting.
         for setting in target.settings {
-            if let traits = setting.condition?.traits, traits.intersection(self.enabledTraits).isEmpty {
+            if let traits = setting.condition?.traits, traits.intersection(self.enabledTraits.names).isEmpty {
                 // The setting is currently not enabled so we should skip it
                 continue
             }

Sources/PackageModel/CMakeLists.txt

@@ -14,7 +14,7 @@ add_library(PackageModel
   BuildSettings.swift
   DependencyMapper.swift
   Diagnostics.swift
-  EnabledTraitsMap.swift
+  EnabledTrait.swift
   IdentityResolver.swift
   InstalledSwiftPMConfiguration.swift
   Manifest/Manifest.swift