Skip to content

Commit d19f9b2

Browse files
tomerdtomerd
committed
unwind target based dependencies
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) #2749 * adjust call-sites and test which were added or modified after Finish SE‐0226 (Ignore Unused Products) #2749 was merged
1 parent ad067fa commit d19f9b2

39 files changed

+979
-1070
lines changed

‎Sources/Commands/Snippets/Cards/SnippetCard.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ struct SnippetCard: Card {
8787

8888
func runExample() throws {
8989
print("Building '\(snippet.path)'\n")
90-
let buildSystem = try swiftTool.createBuildSystem(explicitProduct: snippet.name)
90+
let buildSystem = try swiftTool.createBuildSystem()
9191
try buildSystem.build(subset: .product(snippet.name))
9292
let executablePath = try swiftTool.buildParameters().buildPath.appending(component: snippet.name)
9393
if let exampleTarget = try buildSystem.getPackageGraph().allTargets.first(where: { $0.name == snippet.name }) {

‎Sources/Commands/SwiftBuildTool.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ public struct SwiftBuildTool: SwiftCommand {
104104
guard let subset = options.buildSubset(observabilityScope: swiftTool.observabilityScope) else {
105105
throw ExitCode.failure
106106
}
107-
let buildSystem = try swiftTool.createBuildSystem(explicitProduct: options.product)
107+
let buildSystem = try swiftTool.createBuildSystem()
108108
do {
109109
try buildSystem.build(subset: subset)
110110
} catch _ as Diagnostics {

‎Sources/Commands/SwiftRunTool.swift

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,7 @@ public struct SwiftRunTool: SwiftCommand {
111111
case .repl:
112112
// Load a custom package graph which has a special product for REPL.
113113
let graphLoader = {
114-
try swiftTool.loadPackageGraph(
115-
explicitProduct: self.options.executable,
116-
createREPLProduct: true)
114+
try swiftTool.loadPackageGraph(createREPLProduct: true)
117115
}
118116
let buildParameters = try swiftTool.buildParameters()
119117

@@ -145,7 +143,7 @@ public struct SwiftRunTool: SwiftCommand {
145143

146144
case .debugger:
147145
do {
148-
let buildSystem = try swiftTool.createBuildSystem(explicitProduct: options.executable)
146+
let buildSystem = try swiftTool.createBuildSystem()
149147
let productName = try findProductName(in: buildSystem.getPackageGraph())
150148
if options.shouldBuildTests {
151149
try buildSystem.build(subset: .allIncludingTests)
@@ -189,7 +187,7 @@ public struct SwiftRunTool: SwiftCommand {
189187
swiftTool.redirectStdoutToStderr()
190188

191189
do {
192-
let buildSystem = try swiftTool.createBuildSystem(explicitProduct: options.executable)
190+
let buildSystem = try swiftTool.createBuildSystem()
193191
let productName = try findProductName(in: buildSystem.getPackageGraph())
194192
if options.shouldBuildTests {
195193
try buildSystem.build(subset: .allIncludingTests)

‎Sources/Commands/SwiftTool.swift

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -694,11 +694,8 @@ public class SwiftTool {
694694

695695
/// Fetch and load the complete package graph.
696696
///
697-
/// - Parameters:
698-
/// - 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.
699697
@discardableResult
700698
func loadPackageGraph(
701-
explicitProduct: String? = nil,
702699
createMultipleTestProducts: Bool = false,
703700
createREPLProduct: Bool = false
704701
) throws -> PackageGraph {
@@ -708,7 +705,6 @@ public class SwiftTool {
708705
// Fetch and load the package graph.
709706
let graph = try workspace.loadPackageGraph(
710707
rootInput: getWorkspaceRoot(),
711-
explicitProduct: explicitProduct,
712708
createMultipleTestProducts: createMultipleTestProducts,
713709
createREPLProduct: createREPLProduct,
714710
forceResolvedVersions: options.forceResolvedVersions,
@@ -806,9 +802,9 @@ public class SwiftTool {
806802
return true
807803
}
808804

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

813809
// Construct the build operation.
814810
let buildOp = try BuildOperation(
@@ -827,11 +823,11 @@ public class SwiftTool {
827823
return buildOp
828824
}
829825

830-
func createBuildSystem(explicitProduct: String? = nil, buildParameters: BuildParameters? = nil) throws -> BuildSystem {
826+
func createBuildSystem(buildParameters: BuildParameters? = nil) throws -> BuildSystem {
831827
let buildSystem: BuildSystem
832828
switch options.buildSystem {
833829
case .native:
834-
let graphLoader = { try self.loadPackageGraph(explicitProduct: explicitProduct) }
830+
let graphLoader = { try self.loadPackageGraph() }
835831
let pluginInvoker = { try self.invokePlugins(graph: $0) }
836832
buildSystem = try BuildOperation(
837833
buildParameters: buildParameters ?? self.buildParameters(),
@@ -844,7 +840,7 @@ public class SwiftTool {
844840
observabilityScope: self.observabilityScope
845841
)
846842
case .xcode:
847-
let graphLoader = { try self.loadPackageGraph(explicitProduct: explicitProduct, createMultipleTestProducts: true) }
843+
let graphLoader = { try self.loadPackageGraph(createMultipleTestProducts: true) }
848844
// FIXME: Implement the custom build command provider also.
849845
buildSystem = try XcodeBuildSystem(
850846
buildParameters: buildParameters ?? self.buildParameters(),

‎Sources/PackageGraph/DependencyResolutionNode.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import TSCBasic
1212
import PackageModel
1313
import struct TSCUtility.Version
1414

15+
/*
1516
/// A node in the dependency resolution graph.
1617
///
1718
/// See the documentation of each case for more detailed descriptions of each kind and how they interact.
@@ -133,3 +134,4 @@ extension DependencyResolutionNode: CustomStringConvertible {
133134
return "\(package.name)\(productFilter)"
134135
}
135136
}
137+
*/

‎Sources/PackageGraph/DependencyResolver.swift

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import PackageModel
1313
import TSCBasic
1414

1515
public protocol DependencyResolver {
16-
typealias Binding = (package: PackageReference, binding: BoundVersion, products: ProductFilter)
16+
typealias Binding = (package: PackageReference, binding: BoundVersion)
1717
typealias Delegate = DependencyResolverDelegate
1818
}
1919

@@ -55,15 +55,15 @@ public struct TracingDependencyResolverDelegate: DependencyResolverDelegate {
5555
}
5656

5757
public func willResolve(term: Term) {
58-
self.log("resolving: \(term.node.package.identity)")
58+
self.log("resolving: \(term.package.identity)")
5959
}
6060

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

6565
public func derived(term: Term) {
66-
self.log("derived: \(term.node.package.identity)")
66+
self.log("derived: \(term.package.identity)")
6767
}
6868

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

8989
public func solved(result: [DependencyResolver.Binding]) {
9090
self.log("solved:")
91-
for (package, binding, _) in result {
91+
for (package, binding) in result {
9292
self.log("\(package) \(binding)")
9393
}
9494
}
@@ -134,8 +134,7 @@ public struct MultiplexResolverDelegate: DependencyResolverDelegate {
134134
underlying.forEach { $0.failedToResolve(incompatibility: incompatibility) }
135135
}
136136

137-
public func solved(result: [(package: PackageReference, binding: BoundVersion, products: ProductFilter)]) {
137+
public func solved(result: [(package: PackageReference, binding: BoundVersion)]) {
138138
underlying.forEach { $0.solved(result: result) }
139139
}
140-
141140
}

‎Sources/PackageGraph/GraphLoadingNode.swift

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import TSCBasic
1717
/// This node uses the product filter that was already finalized during resolution.
1818
///
1919
/// - SeeAlso: DependencyResolutionNode
20+
// FIXME: tomer deprecate or replace withe some other manifest envelope
2021
public struct GraphLoadingNode: Equatable, Hashable {
2122

2223
/// The package identity.
@@ -25,28 +26,20 @@ public struct GraphLoadingNode: Equatable, Hashable {
2526
/// The package manifest.
2627
public let manifest: Manifest
2728

28-
/// The product filter applied to the package.
29-
public let productFilter: ProductFilter
30-
31-
public init(identity: PackageIdentity, manifest: Manifest, productFilter: ProductFilter) {
29+
public init(identity: PackageIdentity, manifest: Manifest) {
3230
self.identity = identity
3331
self.manifest = manifest
34-
self.productFilter = productFilter
32+
//self.productFilter = productFilter
3533
}
3634

3735
/// Returns the dependencies required by this node.
3836
internal func requiredDependencies() -> [PackageDependency] {
39-
return manifest.dependenciesRequired(for: productFilter)
37+
return self.manifest.requiredDependencies()
4038
}
4139
}
4240

4341
extension GraphLoadingNode: CustomStringConvertible {
4442
public var description: String {
45-
switch productFilter {
46-
case .everything:
47-
return self.manifest.name
48-
case .specific(let set):
49-
return "\(self.manifest.name)[\(set.sorted().joined(separator: ", "))]"
50-
}
43+
return self.identity.description
5144
}
5245
}

‎Sources/PackageGraph/PackageContainer.swift

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public protocol PackageContainer {
6868
/// - Precondition: `versions.contains(version)`
6969
/// - Throws: If the version could not be resolved; this will abort
7070
/// dependency resolution completely.
71-
func getDependencies(at version: Version, productFilter: ProductFilter) throws -> [PackageContainerConstraint]
71+
func getDependencies(at version: Version) throws -> [PackageContainerConstraint]
7272

7373
/// Fetch the declared dependencies for a particular revision.
7474
///
@@ -77,12 +77,12 @@ public protocol PackageContainer {
7777
///
7878
/// - Throws: If the revision could not be resolved; this will abort
7979
/// dependency resolution completely.
80-
func getDependencies(at revision: String, productFilter: ProductFilter) throws -> [PackageContainerConstraint]
80+
func getDependencies(at revision: String) throws -> [PackageContainerConstraint]
8181

8282
/// Fetch the dependencies of an unversioned package container.
8383
///
8484
/// NOTE: This method should not be called on a versioned container.
85-
func getUnversionedDependencies(productFilter: ProductFilter) throws -> [PackageContainerConstraint]
85+
func getUnversionedDependencies() throws -> [PackageContainerConstraint]
8686

8787
/// Get the updated identifier at a bound version.
8888
///
@@ -113,27 +113,23 @@ public struct PackageContainerConstraint: Equatable, Hashable {
113113
/// The constraint requirement.
114114
public let requirement: PackageRequirement
115115

116-
/// The required products.
117-
public let products: ProductFilter
118-
119116
/// Create a constraint requiring the given `container` satisfying the
120117
/// `requirement`.
121-
public init(package: PackageReference, requirement: PackageRequirement, products: ProductFilter) {
118+
public init(package: PackageReference, requirement: PackageRequirement) {
122119
self.package = package
123120
self.requirement = requirement
124-
self.products = products
125121
}
126122

127123
/// Create a constraint requiring the given `container` satisfying the
128124
/// `versionRequirement`.
129-
public init(package: PackageReference, versionRequirement: VersionSetSpecifier, products: ProductFilter) {
130-
self.init(package: package, requirement: .versionSet(versionRequirement), products: products)
125+
public init(package: PackageReference, versionRequirement: VersionSetSpecifier) {
126+
self.init(package: package, requirement: .versionSet(versionRequirement))
131127
}
132128
}
133129

134130
extension PackageContainerConstraint: CustomStringConvertible {
135131
public var description: String {
136-
return "Constraint(\(self.package), \(requirement), \(products)"
132+
return "Constraint(\(self.package), \(requirement)"
137133
}
138134
}
139135

‎Sources/PackageGraph/PackageGraph+Loading.swift

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ extension PackageGraph {
4343
let successors: (GraphLoadingNode) -> [GraphLoadingNode] = { node in
4444
node.requiredDependencies().compactMap{ dependency in
4545
return manifestMap[dependency.identity].map { manifest in
46-
GraphLoadingNode(identity: dependency.identity, manifest: manifest, productFilter: dependency.productFilter)
46+
GraphLoadingNode(identity: dependency.identity, manifest: manifest)
4747
}
4848
}
4949
}
@@ -54,11 +54,11 @@ extension PackageGraph {
5454
manifestMap[$0.identity]
5555
})
5656
let rootManifestNodes = root.packages.map { identity, package in
57-
GraphLoadingNode(identity: identity, manifest: package.manifest, productFilter: .everything)
57+
GraphLoadingNode(identity: identity, manifest: package.manifest)
5858
}
5959
let rootDependencyNodes = root.dependencies.lazy.compactMap { (dependency: PackageDependency) -> GraphLoadingNode? in
6060
manifestMap[dependency.identity].map {
61-
GraphLoadingNode(identity: dependency.identity, manifest: $0, productFilter: dependency.productFilter)
61+
GraphLoadingNode(identity: dependency.identity, manifest: $0)
6262
}
6363
}
6464
let inputManifests = rootManifestNodes + rootDependencyNodes
@@ -76,13 +76,15 @@ extension PackageGraph {
7676
allNodes = try topologicalSort(inputManifests, successors: successors)
7777
}
7878

79+
// FIXME: tomer simplify
80+
/*
7981
var flattenedManifests: [PackageIdentity: GraphLoadingNode] = [:]
8082
for node in allNodes {
8183
if let existing = flattenedManifests[node.identity] {
8284
let merged = GraphLoadingNode(
8385
identity: node.identity,
84-
manifest: node.manifest,
85-
productFilter: existing.productFilter.union(node.productFilter)
86+
manifest: node.manifest//,
87+
//productFilter: existing.productFilter.union(node.productFilter)
8688
)
8789
flattenedManifests[node.identity] = merged
8890
} else {
@@ -91,7 +93,8 @@ extension PackageGraph {
9193
}
9294
// sort by identity
9395
allNodes = flattenedManifests.keys.sorted().map { flattenedManifests[$0]! } // force unwrap fine since we are iterating on keys
94-
96+
*/
97+
9598
// Create the packages.
9699
var manifestToPackage: [Manifest: Package] = [:]
97100
for node in allNodes {
@@ -110,7 +113,7 @@ extension PackageGraph {
110113
let builder = PackageBuilder(
111114
identity: node.identity,
112115
manifest: manifest,
113-
productFilter: node.productFilter,
116+
//productFilter: node.productFilter,
114117
path: packagePath,
115118
additionalFileRules: additionalFileRules,
116119
binaryArtifacts: binaryArtifacts,
@@ -209,7 +212,7 @@ private func createResolvedPackages(
209212
let allowedToOverride = rootManifestSet.contains(node.manifest)
210213
return ResolvedPackageBuilder(
211214
package,
212-
productFilter: node.productFilter,
215+
//productFilter: node.productFilter,
213216
isAllowedToVendUnsafeProducts: isAllowedToVendUnsafeProducts,
214217
allowedToOverride: allowedToOverride
215218
)
@@ -234,7 +237,7 @@ private func createResolvedPackages(
234237
var dependenciesByNameForTargetDependencyResolution = [String: ResolvedPackageBuilder]()
235238

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

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

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

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

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

597603
/// The package reference.
598604
let package: Package
599605

600-
/// The product filter applied to the package.
601-
let productFilter: ProductFilter
602-
603606
/// The targets in the package.
604607
var targets: [ResolvedTargetBuilder] = []
605608

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

614617
let allowedToOverride: Bool
615618

616-
init(_ package: Package, productFilter: ProductFilter, isAllowedToVendUnsafeProducts: Bool, allowedToOverride: Bool) {
619+
init(_ package: Package, isAllowedToVendUnsafeProducts: Bool, allowedToOverride: Bool) {
617620
self.package = package
618-
self.productFilter = productFilter
619621
self.isAllowedToVendUnsafeProducts = isAllowedToVendUnsafeProducts
620622
self.allowedToOverride = allowedToOverride
621623
}
@@ -624,8 +626,8 @@ private final class ResolvedPackageBuilder: ResolvedBuilder<ResolvedPackage> {
624626
return ResolvedPackage(
625627
package: package,
626628
dependencies: try dependencies.map{ try $0.construct() },
627-
targets: try targets.map{ try $0.construct() },
628-
products: try products.map{ try $0.construct() }
629+
targets: try self.targets.map{ try $0.construct() },
630+
products: try self.products.map{ try $0.construct() }
629631
)
630632
}
631633
}

0 commit comments

Comments
 (0)