Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Sources/PackageGraph/PackageContainer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ public protocol PackageContainer {
/// after the container is available. The updated identifier is returned in result of the
/// dependency resolution.
func loadPackageReference(at boundVersion: BoundVersion) async throws -> PackageReference

/// Get the updated set of traits at a bound version.
///
func loadPackageTraits(at boundVersion: BoundVersion) async throws -> Set<TraitDescription>
}

extension PackageContainer {
Expand Down
5 changes: 4 additions & 1 deletion Sources/PackageGraph/PackageModel+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ extension PackageDependency {

extension Manifest {
/// Constructs constraints of the dependencies in the raw package.
public func dependencyConstraints(productFilter: ProductFilter, _ enabledTraits: EnabledTraits = ["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 }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@ public struct DependencyResolverBinding {
public let package: PackageReference
public let boundVersion: BoundVersion
public let products: ProductFilter
public let traits: Set<String>
}
Original file line number Diff line number Diff line change
Expand Up @@ -240,12 +240,11 @@ public struct PubGrubDependencyResolver {
try await self.run(state: state)

let decisions = state.solution.assignments.filter(\.isDecision)
var flattenedAssignments: [PackageReference: (binding: BoundVersion, products: ProductFilter)] = [:]
var flattenedAssignments: [PackageReference: (binding: BoundVersion, products: ProductFilter, traits: Set<String>)] = [:]
for assignment in decisions {
if assignment.term.node == state.root {
continue
}

let boundVersion: BoundVersion
switch assignment.term.requirement {
case .exact(let version):
Expand All @@ -264,6 +263,7 @@ public struct PubGrubDependencyResolver {
)
}
let updatePackage = try await container.underlying.loadPackageReference(at: boundVersion)
let updatedTraits = try await container.underlying.loadPackageTraits(at: boundVersion).map(\.name)

if var existing = flattenedAssignments[updatePackage] {
guard existing.binding == boundVersion else {
Expand All @@ -272,13 +272,22 @@ public struct PubGrubDependencyResolver {
existing.products.formUnion(products)
flattenedAssignments[updatePackage] = existing
} else {
flattenedAssignments[updatePackage] = (binding: boundVersion, products: products)
flattenedAssignments[updatePackage] = (
binding: boundVersion,
products: products,
traits: Set(updatedTraits)
)
}
}
var finalAssignments: [DependencyResolverBinding]
= flattenedAssignments.keys.sorted(by: { $0.deprecatedName < $1.deprecatedName }).map { package in
let details = flattenedAssignments[package]!
return .init(package: package, boundVersion: details.binding, products: details.products)
return .init(
package: package,
boundVersion: details.binding,
products: details.products,
traits: details.traits
)
}

// Add overridden packages to the result.
Expand All @@ -289,10 +298,12 @@ public struct PubGrubDependencyResolver {
})
}
let updatePackage = try await container.underlying.loadPackageReference(at: override.version)
let updatedTraits = try await container.underlying.loadPackageTraits(at: override.version).map(\.name)
finalAssignments.append(.init(
package: updatePackage,
boundVersion: override.version,
products: override.products
products: override.products,
traits: Set(updatedTraits)
))
}

Expand Down
1 change: 1 addition & 0 deletions Sources/PackageLoading/ManifestLoader+Validation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public struct ManifestValidator {
}

private func validateTraits() -> [Basics.Diagnostic] {
// TODO bp add trait validation here.
var diagnostics = [Basics.Diagnostic]()

if self.manifest.traits.count > 300 {
Expand Down
20 changes: 12 additions & 8 deletions Sources/PackageModel/Manifest/Manifest+Traits.swift
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,9 @@ extension Manifest {
/// Determines if a trait is enabled with a given set of enabled traits.
public func isTraitEnabled(_ trait: TraitDescription, _ enabledTraits: EnabledTraits) throws -> Bool {
// First, check that the queried trait is valid.
try validateTrait(trait)
// try validateTrait(trait)
// Then, check that the list of enabled traits is valid.
try validateEnabledTraits(enabledTraits)
// try validateEnabledTraits(enabledTraits)

// Special case for dealing with whether a default trait is enabled.
guard !trait.isDefault else {
Expand Down Expand Up @@ -271,7 +271,7 @@ extension Manifest {

/// Calculates and returns a set of all enabled traits, beginning with a set of explicitly enabled traits (which can either be the default traits of a manifest, or a configuration of enabled traits determined from a user-generated trait configuration) and determines which traits are transitively enabled.
private func calculateAllEnabledTraits(explicitlyEnabledTraits: EnabledTraits) throws -> EnabledTraits {
try validateEnabledTraits(explicitlyEnabledTraits)
// try validateEnabledTraits(explicitlyEnabledTraits)
// This the point where we flatten the enabled traits and resolve the recursive traits
var enabledTraits = explicitlyEnabledTraits
let areDefaultsEnabled = enabledTraits.remove("default") != nil
Expand All @@ -296,10 +296,13 @@ extension Manifest {
// we have to check if default traits should be used and then flatten all the enabled traits.
let transitivelyEnabledTraits = try enabledTraits.flatMap { trait in
guard let traitDescription = traitsMap[trait.name] else {
throw TraitError.invalidTrait(
package: .init(self),
trait: trait
)
// TODO bp do somehting more meaningful here.
// testing for now.
return .init([], setBy: .trait("<unknown>"))
// throw TraitError.invalidTrait(
// package: .init(self),
// trait: trait
// )
}
return EnabledTraits(
traitDescription.enabledTraits,
Expand Down Expand Up @@ -447,7 +450,8 @@ extension Manifest {
/// Given a set of enabled traits, determine whether a package dependecy of this manifest is
/// guarded by traits.
private func isPackageDependencyTraitGuarded(_ dependency: PackageDependency, enabledTraits: EnabledTraits) throws -> Bool {
try validateEnabledTraits(enabledTraits)
// TODO bp try doing trait validation after resolution to ensure that updated dependencies (which implicitly means a possible update to trait configurations in package manifests) are accounted for here.
// try validateEnabledTraits(enabledTraits)

let targetDependenciesForPackageDependency = self.targets.flatMap({ $0.dependencies })
.filter({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ public struct FileSystemPackageContainer: PackageContainer {
return package.withName(manifest.displayName)
}

public func loadPackageTraits(at boundVersion: BoundVersion) async throws -> Set<TraitDescription> {
assert(boundVersion == .unversioned, "Unexpected bound version \(boundVersion)")
let manifest = try await loadManifest()
return manifest.traits
}

public func isToolsVersionCompatible(at version: Version) -> Bool {
fatalError("This should never be called")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,14 @@ public class RegistryPackageContainer: PackageContainer {
return self.package
}

public func loadPackageTraits(at boundVersion: BoundVersion) async throws -> Set<TraitDescription> {
guard case .version(let version) = boundVersion else {
throw InternalError("loadPackageTraits expects an exact version")
}
let manifest = try await self.loadManifest(version: version)
return manifest.traits
}

// marked internal for testing
internal func loadManifest(version: Version) async throws -> Manifest {
let result = try await self.getAvailableManifestsFilesystem(version: version)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,27 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
return self.package.withName(manifest.displayName)
}

public func loadPackageTraits(at boundVersion: BoundVersion) async throws -> Set<TraitDescription> {
let revision: Revision
var version: Version?
switch boundVersion {
case .version(let v):
guard let tag = try self.knownVersions()[v] else {
throw StringError("unknown tag \(v)")
}
version = v
revision = try repository.resolveRevision(tag: tag)
case .revision(let identifier, _):
revision = try repository.resolveRevision(identifier: identifier)
case .unversioned, .excluded:
assertionFailure("Unexpected type requirement \(boundVersion)")
return []
}

let manifest = try await self.loadManifest(at: revision, version: version)
return manifest.traits
}

/// Returns true if the tools version is valid and can be used by this
/// version of the package manager.
private func isValidToolsVersion(_ toolsVersion: ToolsVersion) -> Bool {
Expand Down
4 changes: 4 additions & 0 deletions Sources/Workspace/ResolverPrecomputationProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -172,4 +172,8 @@ private struct LocalPackageContainer: PackageContainer {
return .root(identity: self.package.identity, path: self.manifest.path)
}
}

func loadPackageTraits(at boundVersion: BoundVersion) async throws -> Set<TraitDescription> {
return manifest.traits
}
}
36 changes: 35 additions & 1 deletion Sources/Workspace/Workspace+Dependencies.swift
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,13 @@ extension Workspace {
observabilityScope: observabilityScope
)

// Update traits; validation check.
try await self.updateTraits(
manifests: updatedDependencyManifests,
addedOrUpdatedPackages: addedOrUpdatedPackages,
observabilityScope: observabilityScope
)

return packageStateChanges
}

Expand Down Expand Up @@ -479,6 +486,13 @@ extension Workspace {
observabilityScope: observabilityScope
)

// Update traits; validation check
try await self.updateTraits(
manifests: currentManifests,
addedOrUpdatedPackages: [],
observabilityScope: observabilityScope
)

let precomputationResult = try await self.precomputeResolution(
root: graphRoot,
dependencyManifests: currentManifests,
Expand Down Expand Up @@ -586,6 +600,12 @@ extension Workspace {
observabilityScope: observabilityScope
)

try await self.updateTraits(
manifests: currentManifests,
addedOrUpdatedPackages: [],
observabilityScope: observabilityScope
)

return currentManifests
case .required(let reason):
delegate?.willResolveDependencies(reason: reason)
Expand All @@ -608,13 +628,18 @@ extension Workspace {
observabilityScope: observabilityScope
)

// TODO bp; some possible trait validation here.

// Reset the active resolver.
self.activeResolver = nil

guard !observabilityScope.errorsReported else {
return currentManifests
}

// TODO bp: during an update to dependencies checkout, must
// also update/validate the traits here if previous versions of dependencies have been changed.

// Update the checkouts with dependency resolution result.
let packageStateChanges = await self.updateDependenciesCheckouts(
root: graphRoot,
Expand Down Expand Up @@ -661,6 +686,13 @@ extension Workspace {
observabilityScope: observabilityScope
)

// Update traits; validation check.
try await self.updateTraits(
manifests: updatedDependencyManifests,
addedOrUpdatedPackages: addedOrUpdatedPackages,
observabilityScope: observabilityScope
)

return updatedDependencyManifests
}

Expand Down Expand Up @@ -907,7 +939,9 @@ extension Workspace {
}

guard let requiredDependencies = observabilityScope
.trap({ try dependencyManifests.requiredPackages.filter(\.kind.isResolvable) })
.trap({ try
// TODO bp; don't do preemptive trait validation at this step; could be loading from cache re: old trait version
dependencyManifests.requiredPackages.filter(\.kind.isResolvable) })
else {
return nil
}
Expand Down
5 changes: 3 additions & 2 deletions Sources/Workspace/Workspace+Manifests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,7 @@ extension Workspace {
>] = { node in
// optimization: preload manifest we know about in parallel
// avoid loading dependencies that are trait-guarded here since this is redundant.
// todo bp note that traits could have changed here if the package dependency is slated for an update and the user references the new trait but we still have an old checkout. avoid validating traits as a result, and wait until we have updated manifests after resolution.
let dependenciesRequired = try node.item.manifest.dependenciesRequired(
for: node.item.productFilter,
node.item.enabledTraits
Expand Down Expand Up @@ -681,7 +682,7 @@ extension Workspace {
// before the dependency manifest is loaded and its default traits are known.
let allManifests = allNodes.mapValues(\.manifest)
for (_, manifest) in allManifests {
try updateEnabledTraits(for: manifest)
try await updateEnabledTraits(for: manifest, observabilityScope: observabilityScope)
}

let dependencyManifests = allNodes.filter { !$0.value.manifest.packageKind.isRoot }
Expand Down Expand Up @@ -891,7 +892,7 @@ extension Workspace {
}

// Upon loading a new manifest, update enabled traits.
try self.updateEnabledTraits(for: manifest)
try await self.updateEnabledTraits(for: manifest, observabilityScope: observabilityScope)

self.delegate?.didLoadManifest(
packageIdentity: packageIdentity,
Expand Down
19 changes: 18 additions & 1 deletion Sources/Workspace/Workspace+Traits.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@

import class PackageModel.Manifest
import struct PackageModel.PackageIdentity
import struct PackageModel.PackageReference
import enum PackageModel.ProductFilter
import enum PackageModel.PackageDependency
import struct PackageModel.EnabledTrait
import struct PackageModel.EnabledTraits
import class Basics.ObservabilityScope

extension Workspace {
/// Given a loaded `Manifest`, determine the traits that are enabled for it and
Expand All @@ -24,7 +26,7 @@ extension Workspace {
///
/// If the package defines a dependency with an explicit set of enabled traits, it will also
/// add them to the enabled traits map.
public func updateEnabledTraits(for manifest: Manifest) throws {
public func updateEnabledTraits(for manifest: Manifest, observabilityScope: ObservabilityScope) async throws {
// If the `Manifest` is a root, then we should default to using
// the trait configuration set in the `Workspace`. Otherwise,
// check the enabled traits map to see if there are traits
Expand Down Expand Up @@ -92,3 +94,18 @@ extension Workspace {
}
}
}

extension Workspace {
internal func updateTraits(
manifests: DependencyManifests,
addedOrUpdatedPackages: [PackageReference],
observabilityScope: ObservabilityScope
) async throws {
// TODO bp complete implementation
for manifest in addedOrUpdatedPackages {
// do validation check on the traits here.
// take in the enabled traits map; use validation methods
// available on the manifest, referenced from the DependencyManifests.
}
}
}
3 changes: 3 additions & 0 deletions Sources/Workspace/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ public enum WorkspaceResolveReason: Equatable {
/// Requirements were added for new packages.
case newPackages(packages: [PackageReference])

// TODO bp search for instances of this to determine whether a package
// needs to be updated, and therefore possibly has a change in its
// traits.
/// The requirement of a dependency has changed.
case packageRequirementChange(
package: PackageReference,
Expand Down
5 changes: 5 additions & 0 deletions Sources/_InternalTestSupport/MockPackageContainer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ public class MockPackageContainer: CustomPackageContainer {
return self.package
}

public func loadPackageTraits(at boundVersion: BoundVersion) async throws -> Set<TraitDescription> {
// TODO bp complete implementation
return []
}

public func isToolsVersionCompatible(at version: Version) -> Bool {
return true
}
Expand Down
5 changes: 5 additions & 0 deletions Tests/PackageGraphTests/PubGrubTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3198,6 +3198,11 @@ public class MockContainer: PackageContainer {
return self.package
}

public func loadPackageTraits(at boundVersion: BoundVersion) async throws -> Set<TraitDescription> {
// TODO bp complete implementation
return []
}

func appendVersion(_ version: BoundVersion) {
self._versions.append(version)
self._versions = self._versions
Expand Down
Loading