Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft
2 changes: 2 additions & 0 deletions Sources/Build/LLBuildDescription.swift
Original file line number Diff line number Diff line change
@@ -16,6 +16,8 @@ import LLBuildManifest
import SPMBuildCore
import PackageGraph

import enum PackageModel.TraitConfiguration

import struct TSCBasic.ByteString

/// Contains the description of the build that is needed during the execution.
4 changes: 2 additions & 2 deletions Sources/Commands/PackageCommands/EditCommands.swift
Original file line number Diff line number Diff line change
@@ -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(
2 changes: 1 addition & 1 deletion Sources/Commands/PackageCommands/Resolve.swift
Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@ import ArgumentParser
import CoreCommands
import TSCUtility

import struct PackageGraph.TraitConfiguration
import enum PackageModel.TraitConfiguration

extension SwiftPackageCommand {
struct ResolveOptions: ParsableArguments {
1 change: 1 addition & 0 deletions Sources/CoreCommands/BuildSystemSupport.swift
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@ import class Basics.ObservabilityScope
import struct PackageGraph.ModulesGraph
import struct PackageLoading.FileRuleDescription
import protocol TSCBasic.OutputByteStream
import enum PackageModel.TraitConfiguration

private struct NativeBuildSystemFactory: BuildSystemFactory {
let swiftCommandState: SwiftCommandState
2 changes: 1 addition & 1 deletion Sources/CoreCommands/Options.swift
Original file line number Diff line number Diff line change
@@ -27,7 +27,7 @@ import struct PackageModel.PackageIdentity
import enum PackageModel.Sanitizer
@_spi(SwiftPMInternal) import struct PackageModel.SwiftSDK

import struct PackageGraph.TraitConfiguration
import enum PackageModel.TraitConfiguration

import struct SPMBuildCore.BuildParameters
import struct SPMBuildCore.BuildSystemProvider
21 changes: 8 additions & 13 deletions Sources/CoreCommands/SwiftCommandState.swift
Original file line number Diff line number Diff line change
@@ -214,14 +214,14 @@ public final class SwiftCommandState {
}

/// Get the current workspace root object.
public func getWorkspaceRoot(traitConfiguration: TraitConfiguration? = nil) throws -> PackageGraphRootInput {
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 {
packages = [try getPackageRoot()]
packages = try [self.getPackageRoot()]
}

return PackageGraphRootInput(packages: packages, traitConfiguration: traitConfiguration)
@@ -457,10 +457,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
}
@@ -525,9 +522,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(
@@ -652,7 +647,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)

@@ -682,7 +677,7 @@ public final class SwiftCommandState {
) async throws -> ModulesGraph {
try await self.loadPackageGraph(
explicitProduct: explicitProduct,
traitConfiguration: nil,
traitConfiguration: .default,
testEntryPointPath: testEntryPointPath
)
}
@@ -695,7 +690,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 {
@@ -750,7 +745,7 @@ public final class SwiftCommandState {
try self._manifestLoader.get()
}

public func canUseCachedBuildManifest(_ traitConfiguration: TraitConfiguration? = nil) async throws -> Bool {
public func canUseCachedBuildManifest(_ traitConfiguration: TraitConfiguration = .default) async throws -> Bool {
if !self.options.caching.cacheBuildManifest {
return false
}
1 change: 0 additions & 1 deletion Sources/PackageGraph/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -19,7 +19,6 @@ add_library(PackageGraph
PackageModel+Extensions.swift
PackageRequirement.swift
ResolvedPackagesStore.swift
TraitConfiguration.swift
Resolution/PubGrub/Assignment.swift
Resolution/PubGrub/ContainerProvider.swift
Resolution/PubGrub/DiagnosticReportBuilder.swift
2 changes: 1 addition & 1 deletion Sources/PackageGraph/ModulesGraph+Loading.swift
Original file line number Diff line number Diff line change
@@ -1034,7 +1034,7 @@ private func calculateEnabledTraits(
}
}

if let parentPackage, !(explictlyEnabledTraits == nil || areDefaultsEnabled) && manifest.traits.isEmpty {
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(
4 changes: 2 additions & 2 deletions Sources/PackageGraph/ModulesGraph.swift
Original file line number Diff line number Diff line change
@@ -456,7 +456,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 }
@@ -469,7 +469,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,
4 changes: 2 additions & 2 deletions Sources/PackageGraph/PackageContainer.swift
Original file line number Diff line number Diff line change
@@ -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 []
}
}
55 changes: 36 additions & 19 deletions Sources/PackageGraph/PackageGraphRoot.swift
Original file line number Diff line number Diff line change
@@ -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,27 @@ 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)

traitsMap[package.key] = enabledTraits
// 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

// Should only ever have to use trait configuration here for roots.
let enabledTraits = try manifest.enabledTraits(using: traitConfiguration)
traitsMap[package.key] = enabledTraits

// Calculate the enabled traits for each dependency of this root:
manifest.dependencies.forEach { dependency in
if let traits = dependency.traits {
let traitNames = traits.map(\.name)
traitsMap[dependency.identity, default: []].formUnion(Set(traitNames))
}
}
} catch {
self.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 +137,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 +240,4 @@ extension PackageDependency.Registry.Requirement {
}
}
}

15 changes: 13 additions & 2 deletions Sources/PackageGraph/Resolution/DependencyResolutionNode.swift
Original file line number Diff line number Diff line change
@@ -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,14 +94,25 @@ public enum DependencyResolutionNode {
public var traits: Set<String>? {
switch self {
case .root(_, let config):
return config?.enabledTraits
return config.enabledTraits
case .product(_, _, let enabledTraits):
return enabledTraits
default:
return nil
}
}

public var traitConfiguration: TraitConfiguration {
switch self {
case .root(_, let config):
return config
case .product(_, _, let enabledTraits):
return .init(enabledTraits: enabledTraits)
case .empty:
return .default
}
}

/// Returns the dependency that a product has on its own package, if relevant.
///
/// This is the constraint that requires all products from a package resolve to the same version.
Original file line number Diff line number Diff line change
@@ -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 {
Original file line number Diff line number Diff line change
@@ -154,8 +154,7 @@ final class PubGrubPackageContainer {
at version: Version,
node: DependencyResolutionNode,
overriddenPackages: [PackageReference: (version: BoundVersion, products: ProductFilter)],
root: DependencyResolutionNode,
traitConfiguration: TraitConfiguration? = nil
root: DependencyResolutionNode
) async throws -> [Incompatibility] {
// FIXME: It would be nice to compute bounds for this as well.
if await !self.underlying.isToolsVersionCompatible(at: version) {
@@ -168,10 +167,11 @@ final class PubGrubPackageContainer {
)]
}

let enabledTraits = node.package.kind.isRoot ? try await self.underlying.getEnabledTraits(traitConfiguration: node.traitConfiguration) : node.traits
var unprocessedDependencies = try await self.underlying.getDependencies(
at: version,
productFilter: node.productFilter,
node.traits
enabledTraits
)
if let sharedVersion = node.versionLock(version: version) {
unprocessedDependencies.append(sharedVersion)
28 changes: 0 additions & 28 deletions Sources/PackageGraph/TraitConfiguration.swift

This file was deleted.

1 change: 1 addition & 0 deletions Sources/PackageLoading/ManifestLoader.swift
Original file line number Diff line number Diff line change
@@ -440,6 +440,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {

let manifest = Manifest(
displayName: parsedManifest.name,
packageIdentity: packageIdentity,
path: manifestPath,
packageKind: packageKind,
packageLocation: packageLocation,
Loading