From 9579ccc31ed3a2a1a66119cd6cc0f2927cf9ca3d Mon Sep 17 00:00:00 2001 From: Bri Peticca Date: Tue, 21 Oct 2025 14:50:51 -0400 Subject: [PATCH 1/3] Fix for required deps calculation with traits The enabled traits property was essentially being erased to defaults rather than propagating what was calculated earlier due to how we were returning the DependencyResolutionNode during the pub grub stage. This fix should now assure that we are passing the appropriate enabled traits to these nodes, and are being considered when making calls to PackageContainer's dependency-related methods. --- .../PackageModel+Extensions.swift | 4 ++-- .../PubGrub/PubGrubDependencyResolver.swift | 5 +++-- .../Workspace/Workspace+Dependencies.swift | 19 ++++++++++++++++++- Sources/Workspace/Workspace+Manifests.swift | 15 +++++++++++++++ Sources/Workspace/Workspace+Traits.swift | 17 +++++++++++++++++ 5 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 Sources/Workspace/Workspace+Traits.swift diff --git a/Sources/PackageGraph/PackageModel+Extensions.swift b/Sources/PackageGraph/PackageModel+Extensions.swift index 19f94f929b0..fbb9888039b 100644 --- a/Sources/PackageGraph/PackageModel+Extensions.swift +++ b/Sources/PackageGraph/PackageModel+Extensions.swift @@ -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) } } } } diff --git a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift index 801162636d8..b78be2fae5b 100644 --- a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift +++ b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift @@ -356,8 +356,9 @@ public struct PubGrubDependencyResolver { ) } + // TODO bp must update how we fetch dependencies wrt traits? for dependency in try await container.underlying - .getUnversionedDependencies(productFilter: node.productFilter, constraint.enabledTraits) + .getUnversionedDependencies(productFilter: node.productFilter, node.enabledTraits) // TODO bp replace constraint.enabledTraits with node.enabledTraits { if let versionedBasedConstraints = VersionBasedConstraint.constraints(dependency) { for constraint in versionedBasedConstraints { @@ -431,7 +432,7 @@ public struct PubGrubDependencyResolver { var unprocessedDependencies = try await container.underlying.getDependencies( at: revisionForDependencies, productFilter: constraint.products, - constraint.enabledTraits + node.enabledTraits // TODO bp replace constraint.enabledTraits with node.enabledTraits ) if let sharedRevision = node.revisionLock(revision: revision) { unprocessedDependencies.append(sharedRevision) diff --git a/Sources/Workspace/Workspace+Dependencies.swift b/Sources/Workspace/Workspace+Dependencies.swift index 64eb37227b8..816946b72af 100644 --- a/Sources/Workspace/Workspace+Dependencies.swift +++ b/Sources/Workspace/Workspace+Dependencies.swift @@ -507,6 +507,7 @@ extension Workspace { // Ensure the cache path exists and validate that edited dependencies. self.createCacheDirectories(observabilityScope: observabilityScope) + // Load the root manifests and currently checked out manifests. let rootManifests = try await self.loadRootManifests( packages: root.packages, @@ -515,6 +516,15 @@ extension Workspace { let rootManifestsMinimumToolsVersion = rootManifests.values.map(\.toolsVersion).min() ?? ToolsVersion.current let resolvedFileOriginHash = try self.computeResolvedFileOriginHash(root: root) + // Precompute enabled traits, beginning with + // root manifests, if we haven't already done so. + if self.enabledTraitsMap.dictionaryLiteral.isEmpty { + let rootManifestMap = rootManifests.values.reduce(into: [PackageIdentity: Manifest]()) { manifestMap, manifest in + manifestMap[manifest.packageIdentity] = manifest + } + self.enabledTraitsMap = .init(try precomputeTraits(rootManifests.values.map({ $0 }), rootManifestMap)) + } + // Load the current manifests. let graphRoot = try PackageGraphRoot( input: root, @@ -525,12 +535,15 @@ extension Workspace { enabledTraitsMap: self.enabledTraitsMap ) - // Of the enabled dependencies of targets, only consider these for dependency resolution let currentManifests = try await self.loadDependencyManifests( root: graphRoot, observabilityScope: observabilityScope ) + // Update traits map here; before we make call to + // resolveDependencies below, which will check out + // the depenedencies we need. + guard !observabilityScope.errorsReported else { return currentManifests } @@ -593,11 +606,15 @@ extension Workspace { } } + try await self.updateEnabledTraitsMap() + // Create the constraints; filter unused dependencies. var computedConstraints = [PackageContainerConstraint]() computedConstraints += currentManifests.editedPackagesConstraints computedConstraints += try graphRoot.constraints(self.enabledTraitsMap) + constraints + manifestLoader + // Perform dependency resolution. let resolver = try self.createResolver(resolvedPackages: resolvedPackagesStore.resolvedPackages, observabilityScope: observabilityScope) self.activeResolver = resolver diff --git a/Sources/Workspace/Workspace+Manifests.swift b/Sources/Workspace/Workspace+Manifests.swift index 0a1f5c95c88..4cf45e75ad8 100644 --- a/Sources/Workspace/Workspace+Manifests.swift +++ b/Sources/Workspace/Workspace+Manifests.swift @@ -624,17 +624,27 @@ extension Workspace { >] = { node in // optimization: preload manifest we know about in parallel // avoid loading dependencies that are trait-guarded here since this is redundant. + // load conditional traits, if any let dependenciesRequired = try node.item.manifest.dependenciesRequired( for: node.item.productFilter, node.item.enabledTraits ) + if node.item.identity.description.contains("apple-configuration") { + print("enabled traits for apple config: \(node.item.enabledTraits)") + } let dependenciesToLoad = dependenciesRequired.map(\.packageRef) .filter { !loadedManifests.keys.contains($0.identity) } + if node.item.identity.description.contains("apple-configuration") { + print("dependencies to load: \(dependenciesToLoad.map(\.identity.description))") + } try await prepopulateManagedDependencies(dependenciesToLoad) let dependenciesManifests = await self.loadManagedManifests( for: dependenciesToLoad, observabilityScope: observabilityScope ) + if node.item.identity.description.contains("apple-configuration") { + print("dependencies manifests loaded: \(dependenciesManifests.map(\.key.description))") + } dependenciesManifests.forEach { loadedManifests[$0.key] = $0.value } return try dependenciesRequired.compactMap { dependency in return try loadedManifests[dependency.identity].flatMap { manifest in @@ -730,6 +740,7 @@ extension Workspace { dependencies.append((node.manifest, dependency, node.productFilter, fileSystem ?? self.fileSystem)) } + // dependency manifests returned here return DependencyManifests( root: root, dependencies: dependencies, @@ -962,6 +973,10 @@ extension Workspace { diagnostics: manifestLoadingDiagnostics, duration: duration ) + // update enabled traits map here + // Look for conditionally enabled traits in the manifest. +// let enabledTraits = manifest.enabledTraits(using: self.enabledTraitsMap[manifest.packageIdentity]) + return manifest } diff --git a/Sources/Workspace/Workspace+Traits.swift b/Sources/Workspace/Workspace+Traits.swift new file mode 100644 index 00000000000..40043699eb5 --- /dev/null +++ b/Sources/Workspace/Workspace+Traits.swift @@ -0,0 +1,17 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2025 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +extension Workspace { + package func updateEnabledTraitsMap() { + + } +} From 6e524b6fd18468731f9941bf8819f830e2611118 Mon Sep 17 00:00:00 2001 From: Bri Peticca Date: Wed, 22 Oct 2025 11:33:23 -0400 Subject: [PATCH 2/3] Cleanup --- .../PubGrub/PubGrubDependencyResolver.swift | 5 +- Sources/Workspace/CMakeLists.txt | 3 +- .../Workspace/Workspace+Dependencies.swift | 7 --- Sources/Workspace/Workspace+Manifests.swift | 61 ------------------- Sources/Workspace/Workspace+Traits.swift | 53 +++++++++++++++- 5 files changed, 55 insertions(+), 74 deletions(-) diff --git a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift index b78be2fae5b..a8ae0399409 100644 --- a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift +++ b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift @@ -356,9 +356,8 @@ public struct PubGrubDependencyResolver { ) } - // TODO bp must update how we fetch dependencies wrt traits? for dependency in try await container.underlying - .getUnversionedDependencies(productFilter: node.productFilter, node.enabledTraits) // TODO bp replace constraint.enabledTraits with node.enabledTraits + .getUnversionedDependencies(productFilter: node.productFilter, node.enabledTraits) { if let versionedBasedConstraints = VersionBasedConstraint.constraints(dependency) { for constraint in versionedBasedConstraints { @@ -432,7 +431,7 @@ public struct PubGrubDependencyResolver { var unprocessedDependencies = try await container.underlying.getDependencies( at: revisionForDependencies, productFilter: constraint.products, - node.enabledTraits // TODO bp replace constraint.enabledTraits with node.enabledTraits + node.enabledTraits ) if let sharedRevision = node.revisionLock(revision: revision) { unprocessedDependencies.append(sharedRevision) diff --git a/Sources/Workspace/CMakeLists.txt b/Sources/Workspace/CMakeLists.txt index d0b033eb1cf..cdc939112f2 100644 --- a/Sources/Workspace/CMakeLists.txt +++ b/Sources/Workspace/CMakeLists.txt @@ -40,7 +40,8 @@ add_library(Workspace Workspace+ResolvedPackages.swift Workspace+Signing.swift Workspace+SourceControl.swift - Workspace+State.swift) + Workspace+State.swift + Workspace+Traits.swift) target_link_libraries(Workspace PUBLIC TSCBasic TSCUtility diff --git a/Sources/Workspace/Workspace+Dependencies.swift b/Sources/Workspace/Workspace+Dependencies.swift index 816946b72af..592329e88ab 100644 --- a/Sources/Workspace/Workspace+Dependencies.swift +++ b/Sources/Workspace/Workspace+Dependencies.swift @@ -540,10 +540,6 @@ extension Workspace { observabilityScope: observabilityScope ) - // Update traits map here; before we make call to - // resolveDependencies below, which will check out - // the depenedencies we need. - guard !observabilityScope.errorsReported else { return currentManifests } @@ -606,14 +602,11 @@ extension Workspace { } } - try await self.updateEnabledTraitsMap() - // Create the constraints; filter unused dependencies. var computedConstraints = [PackageContainerConstraint]() computedConstraints += currentManifests.editedPackagesConstraints computedConstraints += try graphRoot.constraints(self.enabledTraitsMap) + constraints - manifestLoader // Perform dependency resolution. let resolver = try self.createResolver(resolvedPackages: resolvedPackagesStore.resolvedPackages, observabilityScope: observabilityScope) diff --git a/Sources/Workspace/Workspace+Manifests.swift b/Sources/Workspace/Workspace+Manifests.swift index 4cf45e75ad8..72d2e6a67aa 100644 --- a/Sources/Workspace/Workspace+Manifests.swift +++ b/Sources/Workspace/Workspace+Manifests.swift @@ -624,27 +624,17 @@ extension Workspace { >] = { node in // optimization: preload manifest we know about in parallel // avoid loading dependencies that are trait-guarded here since this is redundant. - // load conditional traits, if any let dependenciesRequired = try node.item.manifest.dependenciesRequired( for: node.item.productFilter, node.item.enabledTraits ) - if node.item.identity.description.contains("apple-configuration") { - print("enabled traits for apple config: \(node.item.enabledTraits)") - } let dependenciesToLoad = dependenciesRequired.map(\.packageRef) .filter { !loadedManifests.keys.contains($0.identity) } - if node.item.identity.description.contains("apple-configuration") { - print("dependencies to load: \(dependenciesToLoad.map(\.identity.description))") - } try await prepopulateManagedDependencies(dependenciesToLoad) let dependenciesManifests = await self.loadManagedManifests( for: dependenciesToLoad, observabilityScope: observabilityScope ) - if node.item.identity.description.contains("apple-configuration") { - print("dependencies manifests loaded: \(dependenciesManifests.map(\.key.description))") - } dependenciesManifests.forEach { loadedManifests[$0.key] = $0.value } return try dependenciesRequired.compactMap { dependency in return try loadedManifests[dependency.identity].flatMap { manifest in @@ -749,54 +739,6 @@ extension Workspace { ) } - public func precomputeTraits( - _ topLevelManifests: [Manifest], - _ manifestMap: [PackageIdentity: Manifest] - ) throws -> [PackageIdentity: Set] { - var visited: Set = [] - - func dependencies(of parent: Manifest, _ productFilter: ProductFilter = .everything) throws { - let parentTraits = self.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) - ) - self.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 self.enabledTraitsMap.dictionaryLiteral - } - /// Loads the given manifests, if it is present in the managed dependencies. /// @@ -973,9 +915,6 @@ extension Workspace { diagnostics: manifestLoadingDiagnostics, duration: duration ) - // update enabled traits map here - // Look for conditionally enabled traits in the manifest. -// let enabledTraits = manifest.enabledTraits(using: self.enabledTraitsMap[manifest.packageIdentity]) return manifest } diff --git a/Sources/Workspace/Workspace+Traits.swift b/Sources/Workspace/Workspace+Traits.swift index 40043699eb5..5811d6da9c4 100644 --- a/Sources/Workspace/Workspace+Traits.swift +++ b/Sources/Workspace/Workspace+Traits.swift @@ -10,8 +10,57 @@ // //===----------------------------------------------------------------------===// +import class PackageModel.Manifest +import struct PackageModel.PackageIdentity +import enum PackageModel.ProductFilter + extension Workspace { - package func updateEnabledTraitsMap() { - + public func precomputeTraits( + _ topLevelManifests: [Manifest], + _ manifestMap: [PackageIdentity: Manifest] + ) throws -> [PackageIdentity: Set] { + var visited: Set = [] + + func dependencies(of parent: Manifest, _ productFilter: ProductFilter = .everything) throws { + let parentTraits = self.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) + ) + self.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 self.enabledTraitsMap.dictionaryLiteral } + } From adc81e1f235f4b96119b3a096a5fac4fb1add31a Mon Sep 17 00:00:00 2001 From: Bri Peticca Date: Wed, 22 Oct 2025 13:00:47 -0400 Subject: [PATCH 3/3] Cleanup --- Sources/Workspace/Workspace+Dependencies.swift | 1 - Sources/Workspace/Workspace+Manifests.swift | 1 - 2 files changed, 2 deletions(-) diff --git a/Sources/Workspace/Workspace+Dependencies.swift b/Sources/Workspace/Workspace+Dependencies.swift index 592329e88ab..90b2593a381 100644 --- a/Sources/Workspace/Workspace+Dependencies.swift +++ b/Sources/Workspace/Workspace+Dependencies.swift @@ -507,7 +507,6 @@ extension Workspace { // Ensure the cache path exists and validate that edited dependencies. self.createCacheDirectories(observabilityScope: observabilityScope) - // Load the root manifests and currently checked out manifests. let rootManifests = try await self.loadRootManifests( packages: root.packages, diff --git a/Sources/Workspace/Workspace+Manifests.swift b/Sources/Workspace/Workspace+Manifests.swift index 72d2e6a67aa..d066afa5f5f 100644 --- a/Sources/Workspace/Workspace+Manifests.swift +++ b/Sources/Workspace/Workspace+Manifests.swift @@ -730,7 +730,6 @@ extension Workspace { dependencies.append((node.manifest, dependency, node.productFilter, fileSystem ?? self.fileSystem)) } - // dependency manifests returned here return DependencyManifests( root: root, dependencies: dependencies,