Skip to content

Commit

Permalink
unwind target based dependencies
Browse files Browse the repository at this point in the history
motivation: target based dependencies implementation of unused products never materialized. we need to revisit this topic, but until then we would like to simplify the codebase by removing the partial implementation

changes:
* undo changes from Finish SE‐0226 (Ignore Unused Products) apple#2749
* adjust call-sites and test which were added or modified after Finish SE‐0226 (Ignore Unused Products) apple#2749 was merged
  • Loading branch information
tomerd committed Nov 1, 2021
1 parent ad067fa commit d19f9b2
Show file tree
Hide file tree
Showing 39 changed files with 979 additions and 1,070 deletions.
2 changes: 1 addition & 1 deletion Sources/Commands/Snippets/Cards/SnippetCard.swift
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
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
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
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
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
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
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
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
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

0 comments on commit d19f9b2

Please sign in to comment.