Skip to content

[wip] unwind target based dependencies #3829

New issue

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

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

Already on GitHub? Sign in to your account

Closed
Closed
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
2 changes: 1 addition & 1 deletion Sources/Commands/Snippets/Cards/SnippetCard.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ struct SnippetCard: Card {

func runExample() throws {
print("Building '\(snippet.path)'\n")
let buildSystem = try swiftTool.createBuildSystem(explicitProduct: snippet.name)
let buildSystem = try swiftTool.createBuildSystem()
try buildSystem.build(subset: .product(snippet.name))
let executablePath = try swiftTool.buildParameters().buildPath.appending(component: snippet.name)
if let exampleTarget = try buildSystem.getPackageGraph().allTargets.first(where: { $0.name == snippet.name }) {
Expand Down
2 changes: 1 addition & 1 deletion Sources/Commands/SwiftBuildTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public struct SwiftBuildTool: SwiftCommand {
guard let subset = options.buildSubset(observabilityScope: swiftTool.observabilityScope) else {
throw ExitCode.failure
}
let buildSystem = try swiftTool.createBuildSystem(explicitProduct: options.product)
let buildSystem = try swiftTool.createBuildSystem()
do {
try buildSystem.build(subset: subset)
} catch _ as Diagnostics {
Expand Down
8 changes: 3 additions & 5 deletions Sources/Commands/SwiftRunTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,7 @@ public struct SwiftRunTool: SwiftCommand {
case .repl:
// Load a custom package graph which has a special product for REPL.
let graphLoader = {
try swiftTool.loadPackageGraph(
explicitProduct: self.options.executable,
createREPLProduct: true)
try swiftTool.loadPackageGraph(createREPLProduct: true)
}
let buildParameters = try swiftTool.buildParameters()

Expand Down Expand Up @@ -145,7 +143,7 @@ public struct SwiftRunTool: SwiftCommand {

case .debugger:
do {
let buildSystem = try swiftTool.createBuildSystem(explicitProduct: options.executable)
let buildSystem = try swiftTool.createBuildSystem()
let productName = try findProductName(in: buildSystem.getPackageGraph())
if options.shouldBuildTests {
try buildSystem.build(subset: .allIncludingTests)
Expand Down Expand Up @@ -189,7 +187,7 @@ public struct SwiftRunTool: SwiftCommand {
swiftTool.redirectStdoutToStderr()

do {
let buildSystem = try swiftTool.createBuildSystem(explicitProduct: options.executable)
let buildSystem = try swiftTool.createBuildSystem()
let productName = try findProductName(in: buildSystem.getPackageGraph())
if options.shouldBuildTests {
try buildSystem.build(subset: .allIncludingTests)
Expand Down
14 changes: 5 additions & 9 deletions Sources/Commands/SwiftTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -694,11 +694,8 @@ public class SwiftTool {

/// Fetch and load the complete package graph.
///
/// - Parameters:
/// - explicitProduct: The product specified on the command line to a “swift run” or “swift build” command. This allows executables from dependencies to be run directly without having to hook them up to any particular target.
@discardableResult
func loadPackageGraph(
explicitProduct: String? = nil,
createMultipleTestProducts: Bool = false,
createREPLProduct: Bool = false
) throws -> PackageGraph {
Expand All @@ -708,7 +705,6 @@ public class SwiftTool {
// Fetch and load the package graph.
let graph = try workspace.loadPackageGraph(
rootInput: getWorkspaceRoot(),
explicitProduct: explicitProduct,
createMultipleTestProducts: createMultipleTestProducts,
createREPLProduct: createREPLProduct,
forceResolvedVersions: options.forceResolvedVersions,
Expand Down Expand Up @@ -806,9 +802,9 @@ public class SwiftTool {
return true
}

func createBuildOperation(explicitProduct: String? = nil, cacheBuildManifest: Bool = true) throws -> BuildOperation {
func createBuildOperation(cacheBuildManifest: Bool = true) throws -> BuildOperation {
// Load a custom package graph which has a special product for REPL.
let graphLoader = { try self.loadPackageGraph(explicitProduct: explicitProduct) }
let graphLoader = { try self.loadPackageGraph() }

// Construct the build operation.
let buildOp = try BuildOperation(
Expand All @@ -827,11 +823,11 @@ public class SwiftTool {
return buildOp
}

func createBuildSystem(explicitProduct: String? = nil, buildParameters: BuildParameters? = nil) throws -> BuildSystem {
func createBuildSystem(buildParameters: BuildParameters? = nil) throws -> BuildSystem {
let buildSystem: BuildSystem
switch options.buildSystem {
case .native:
let graphLoader = { try self.loadPackageGraph(explicitProduct: explicitProduct) }
let graphLoader = { try self.loadPackageGraph() }
let pluginInvoker = { try self.invokePlugins(graph: $0) }
buildSystem = try BuildOperation(
buildParameters: buildParameters ?? self.buildParameters(),
Expand All @@ -844,7 +840,7 @@ public class SwiftTool {
observabilityScope: self.observabilityScope
)
case .xcode:
let graphLoader = { try self.loadPackageGraph(explicitProduct: explicitProduct, createMultipleTestProducts: true) }
let graphLoader = { try self.loadPackageGraph(createMultipleTestProducts: true) }
// FIXME: Implement the custom build command provider also.
buildSystem = try XcodeBuildSystem(
buildParameters: buildParameters ?? self.buildParameters(),
Expand Down
2 changes: 2 additions & 0 deletions Sources/PackageGraph/DependencyResolutionNode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import TSCBasic
import PackageModel
import struct TSCUtility.Version

/*
/// A node in the dependency resolution graph.
///
/// See the documentation of each case for more detailed descriptions of each kind and how they interact.
Expand Down Expand Up @@ -133,3 +134,4 @@ extension DependencyResolutionNode: CustomStringConvertible {
return "\(package.name)\(productFilter)"
}
}
*/
13 changes: 6 additions & 7 deletions Sources/PackageGraph/DependencyResolver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import PackageModel
import TSCBasic

public protocol DependencyResolver {
typealias Binding = (package: PackageReference, binding: BoundVersion, products: ProductFilter)
typealias Binding = (package: PackageReference, binding: BoundVersion)
typealias Delegate = DependencyResolverDelegate
}

Expand Down Expand Up @@ -55,15 +55,15 @@ public struct TracingDependencyResolverDelegate: DependencyResolverDelegate {
}

public func willResolve(term: Term) {
self.log("resolving: \(term.node.package.identity)")
self.log("resolving: \(term.package.identity)")
}

public func didResolve(term: Term, version: Version, duration: DispatchTimeInterval) {
self.log("resolved: \(term.node.package.identity) @ \(version)")
self.log("resolved: \(term.package.identity) @ \(version)")
}

public func derived(term: Term) {
self.log("derived: \(term.node.package.identity)")
self.log("derived: \(term.package.identity)")
}

public func conflict(conflict: Incompatibility) {
Expand All @@ -88,7 +88,7 @@ public struct TracingDependencyResolverDelegate: DependencyResolverDelegate {

public func solved(result: [DependencyResolver.Binding]) {
self.log("solved:")
for (package, binding, _) in result {
for (package, binding) in result {
self.log("\(package) \(binding)")
}
}
Expand Down Expand Up @@ -134,8 +134,7 @@ public struct MultiplexResolverDelegate: DependencyResolverDelegate {
underlying.forEach { $0.failedToResolve(incompatibility: incompatibility) }
}

public func solved(result: [(package: PackageReference, binding: BoundVersion, products: ProductFilter)]) {
public func solved(result: [(package: PackageReference, binding: BoundVersion)]) {
underlying.forEach { $0.solved(result: result) }
}

}
17 changes: 5 additions & 12 deletions Sources/PackageGraph/GraphLoadingNode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import TSCBasic
/// This node uses the product filter that was already finalized during resolution.
///
/// - SeeAlso: DependencyResolutionNode
// FIXME: tomer deprecate or replace withe some other manifest envelope
public struct GraphLoadingNode: Equatable, Hashable {

/// The package identity.
Expand All @@ -25,28 +26,20 @@ public struct GraphLoadingNode: Equatable, Hashable {
/// The package manifest.
public let manifest: Manifest

/// The product filter applied to the package.
public let productFilter: ProductFilter

public init(identity: PackageIdentity, manifest: Manifest, productFilter: ProductFilter) {
public init(identity: PackageIdentity, manifest: Manifest) {
self.identity = identity
self.manifest = manifest
self.productFilter = productFilter
//self.productFilter = productFilter
}

/// Returns the dependencies required by this node.
internal func requiredDependencies() -> [PackageDependency] {
return manifest.dependenciesRequired(for: productFilter)
return self.manifest.requiredDependencies()
}
}

extension GraphLoadingNode: CustomStringConvertible {
public var description: String {
switch productFilter {
case .everything:
return self.manifest.name
case .specific(let set):
return "\(self.manifest.name)[\(set.sorted().joined(separator: ", "))]"
}
return self.identity.description
}
}
18 changes: 7 additions & 11 deletions Sources/PackageGraph/PackageContainer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,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) throws -> [PackageContainerConstraint]
func getDependencies(at version: Version) throws -> [PackageContainerConstraint]

/// Fetch the declared dependencies for a particular revision.
///
Expand All @@ -77,12 +77,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) throws -> [PackageContainerConstraint]
func getDependencies(at revision: String) 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) throws -> [PackageContainerConstraint]
func getUnversionedDependencies() throws -> [PackageContainerConstraint]

/// Get the updated identifier at a bound version.
///
Expand Down Expand Up @@ -113,27 +113,23 @@ public struct PackageContainerConstraint: Equatable, Hashable {
/// The constraint requirement.
public let requirement: PackageRequirement

/// The required products.
public let products: ProductFilter

/// Create a constraint requiring the given `container` satisfying the
/// `requirement`.
public init(package: PackageReference, requirement: PackageRequirement, products: ProductFilter) {
public init(package: PackageReference, requirement: PackageRequirement) {
self.package = package
self.requirement = requirement
self.products = products
}

/// Create a constraint requiring the given `container` satisfying the
/// `versionRequirement`.
public init(package: PackageReference, versionRequirement: VersionSetSpecifier, products: ProductFilter) {
self.init(package: package, requirement: .versionSet(versionRequirement), products: products)
public init(package: PackageReference, versionRequirement: VersionSetSpecifier) {
self.init(package: package, requirement: .versionSet(versionRequirement))
}
}

extension PackageContainerConstraint: CustomStringConvertible {
public var description: String {
return "Constraint(\(self.package), \(requirement), \(products)"
return "Constraint(\(self.package), \(requirement)"
}
}

Expand Down
34 changes: 18 additions & 16 deletions Sources/PackageGraph/PackageGraph+Loading.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ extension PackageGraph {
let successors: (GraphLoadingNode) -> [GraphLoadingNode] = { node in
node.requiredDependencies().compactMap{ dependency in
return manifestMap[dependency.identity].map { manifest in
GraphLoadingNode(identity: dependency.identity, manifest: manifest, productFilter: dependency.productFilter)
GraphLoadingNode(identity: dependency.identity, manifest: manifest)
}
}
}
Expand All @@ -54,11 +54,11 @@ extension PackageGraph {
manifestMap[$0.identity]
})
let rootManifestNodes = root.packages.map { identity, package in
GraphLoadingNode(identity: identity, manifest: package.manifest, productFilter: .everything)
GraphLoadingNode(identity: identity, manifest: package.manifest)
}
let rootDependencyNodes = root.dependencies.lazy.compactMap { (dependency: PackageDependency) -> GraphLoadingNode? in
manifestMap[dependency.identity].map {
GraphLoadingNode(identity: dependency.identity, manifest: $0, productFilter: dependency.productFilter)
GraphLoadingNode(identity: dependency.identity, manifest: $0)
}
}
let inputManifests = rootManifestNodes + rootDependencyNodes
Expand All @@ -76,13 +76,15 @@ extension PackageGraph {
allNodes = try topologicalSort(inputManifests, successors: successors)
}

// FIXME: tomer simplify
/*
var flattenedManifests: [PackageIdentity: GraphLoadingNode] = [:]
for node in allNodes {
if let existing = flattenedManifests[node.identity] {
let merged = GraphLoadingNode(
identity: node.identity,
manifest: node.manifest,
productFilter: existing.productFilter.union(node.productFilter)
manifest: node.manifest//,
//productFilter: existing.productFilter.union(node.productFilter)
)
flattenedManifests[node.identity] = merged
} else {
Expand All @@ -91,7 +93,8 @@ extension PackageGraph {
}
// sort by identity
allNodes = flattenedManifests.keys.sorted().map { flattenedManifests[$0]! } // force unwrap fine since we are iterating on keys

*/

// Create the packages.
var manifestToPackage: [Manifest: Package] = [:]
for node in allNodes {
Expand All @@ -110,7 +113,7 @@ extension PackageGraph {
let builder = PackageBuilder(
identity: node.identity,
manifest: manifest,
productFilter: node.productFilter,
//productFilter: node.productFilter,
path: packagePath,
additionalFileRules: additionalFileRules,
binaryArtifacts: binaryArtifacts,
Expand Down Expand Up @@ -209,7 +212,7 @@ private func createResolvedPackages(
let allowedToOverride = rootManifestSet.contains(node.manifest)
return ResolvedPackageBuilder(
package,
productFilter: node.productFilter,
//productFilter: node.productFilter,
isAllowedToVendUnsafeProducts: isAllowedToVendUnsafeProducts,
allowedToOverride: allowedToOverride
)
Expand All @@ -234,7 +237,7 @@ private func createResolvedPackages(
var dependenciesByNameForTargetDependencyResolution = [String: ResolvedPackageBuilder]()

// Establish the manifest-declared package dependencies.
package.manifest.dependenciesRequired(for: packageBuilder.productFilter).forEach { dependency in
package.manifest.requiredDependencies().forEach { dependency in
// FIXME: change this validation logic to use identity instead of location
let dependencyLocation: String
switch dependency {
Expand Down Expand Up @@ -481,6 +484,7 @@ private func createResolvedPackages(
}

/// A generic builder for `Resolved` models.
// FIXME: can we deprecate this? seems like adding very little value
private class ResolvedBuilder<T> {
/// The constructed object, available after the first call to `construct()`.
private var _constructedObject: T?
Expand Down Expand Up @@ -530,6 +534,7 @@ private final class ResolvedProductBuilder: ResolvedBuilder<ResolvedProduct> {
}

/// Builder for resolved target.
// FIXME: can we deprecate this? seems like adding very little value
private final class ResolvedTargetBuilder: ResolvedBuilder<ResolvedTarget> {

/// Enumeration to represent target dependencies.
Expand Down Expand Up @@ -592,14 +597,12 @@ private final class ResolvedTargetBuilder: ResolvedBuilder<ResolvedTarget> {
}

/// Builder for resolved package.
// FIXME: can we deprecate this? seems like adding very little value
private final class ResolvedPackageBuilder: ResolvedBuilder<ResolvedPackage> {

/// The package reference.
let package: Package

/// The product filter applied to the package.
let productFilter: ProductFilter

/// The targets in the package.
var targets: [ResolvedTargetBuilder] = []

Expand All @@ -613,9 +616,8 @@ private final class ResolvedPackageBuilder: ResolvedBuilder<ResolvedPackage> {

let allowedToOverride: Bool

init(_ package: Package, productFilter: ProductFilter, isAllowedToVendUnsafeProducts: Bool, allowedToOverride: Bool) {
init(_ package: Package, isAllowedToVendUnsafeProducts: Bool, allowedToOverride: Bool) {
self.package = package
self.productFilter = productFilter
self.isAllowedToVendUnsafeProducts = isAllowedToVendUnsafeProducts
self.allowedToOverride = allowedToOverride
}
Expand All @@ -624,8 +626,8 @@ private final class ResolvedPackageBuilder: ResolvedBuilder<ResolvedPackage> {
return ResolvedPackage(
package: package,
dependencies: try dependencies.map{ try $0.construct() },
targets: try targets.map{ try $0.construct() },
products: try products.map{ try $0.construct() }
targets: try self.targets.map{ try $0.construct() },
products: try self.products.map{ try $0.construct() }
)
}
}
Expand Down
Loading