Skip to content

Update Target Based Resolution #3838

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
wants to merge 55 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
29f37ca
Enabled improved resolution.
SDGGiesbrecht Nov 4, 2021
9eb7447
Fixed most tests.
SDGGiesbrecht Nov 4, 2021
100ea8f
Fixed testUseOfBuildToolPluginProductByExecutableAcrossPackages.
SDGGiesbrecht Nov 6, 2021
7ea7e4d
Fixed PubgrubPackageContainer cache.
SDGGiesbrecht Nov 8, 2021
04d2d3b
Fixed testTestProducts.
SDGGiesbrecht Nov 8, 2021
2330a7b
Fixed leaky product requests.
SDGGiesbrecht Nov 9, 2021
508f3ac
Adjusted manifest to permit switching by envirnoment.
SDGGiesbrecht Nov 9, 2021
6f1de2a
Resolved conflicts.
SDGGiesbrecht Nov 11, 2021
056bdd1
Fixed test expectations.
SDGGiesbrecht Nov 11, 2021
2052e7d
Merge branch 'master' into resolution
SDGGiesbrecht Nov 12, 2021
e453c58
Resolved merge conflicts.
SDGGiesbrecht Nov 12, 2021
00308da
Resolved conflicts.
SDGGiesbrecht Nov 15, 2021
4171fc5
Merge branch 'master' into resolution
SDGGiesbrecht Nov 15, 2021
5570773
Reverted debugging message.
SDGGiesbrecht Nov 15, 2021
aae8505
Merge branch 'master' into resolution
SDGGiesbrecht Nov 16, 2021
d1841e7
Resolved conflicts.
SDGGiesbrecht Nov 19, 2021
df8474d
Fixed testDuplicateTransitiveIdentityWithoutNames.
SDGGiesbrecht Nov 19, 2021
472ac0d
Merge branch 'master' into resolution
SDGGiesbrecht Nov 19, 2021
354ca9c
Merge branch 'master' into resolution
SDGGiesbrecht Jan 7, 2022
654d81b
Merge branch 'master' into resolution
SDGGiesbrecht Jan 9, 2022
9a2b13e
Merge branch 'master' into resolution
SDGGiesbrecht Jan 13, 2022
e6d7870
Merge branch 'master' into resolution
SDGGiesbrecht Jan 18, 2022
cb6e3ee
Merge branch 'master' into resolution
SDGGiesbrecht Jan 20, 2022
be88a4b
Implemented command resolution.
SDGGiesbrecht Jan 21, 2022
ab91e16
Merge branch 'master' into resolution
SDGGiesbrecht Feb 8, 2022
f2b40d5
Immortalized semaphore.
SDGGiesbrecht Feb 9, 2022
b0aaaf6
Merge branch 'master' into resolution
SDGGiesbrecht Feb 11, 2022
e2faedb
Merge branch 'master' into resolution
SDGGiesbrecht Feb 19, 2022
1000f49
Fixed expected order in test.
SDGGiesbrecht Feb 19, 2022
6e89bbb
Reverted crash workaround.
SDGGiesbrecht Feb 19, 2022
feb0615
Resolved merge conflicts.
SDGGiesbrecht Mar 4, 2022
72b9a11
Updated test.
SDGGiesbrecht Mar 4, 2022
fa32f5a
Fixed other tests.
SDGGiesbrecht Mar 5, 2022
0edcbdd
Merge branch 'master' into resolution
SDGGiesbrecht Mar 7, 2022
46a44b0
Fixed remaining test.
SDGGiesbrecht Mar 7, 2022
aede970
Merge branch 'master' into resolution
SDGGiesbrecht Mar 17, 2022
7121cce
Merge branch 'master' into resolution
SDGGiesbrecht Mar 18, 2022
13f8cec
Merge branch 'master' into resolution
SDGGiesbrecht Apr 18, 2022
885456a
Merge branch 'master' into resolution
SDGGiesbrecht May 1, 2022
e243964
Merge branch 'master' into resolution
SDGGiesbrecht May 18, 2022
0374ad6
Added CI.
SDGGiesbrecht May 18, 2022
dcf5c44
Disabled some broken tests.
SDGGiesbrecht May 18, 2022
45fdf77
Merge branch 'master' into resolution
SDGGiesbrecht May 18, 2022
04c9a3c
Merge branch 'master' into ci
SDGGiesbrecht May 18, 2022
6cf85c5
Merge branch 'ci' into resolution
SDGGiesbrecht May 18, 2022
4527994
Revert "Merge branch 'ci' into resolution"
SDGGiesbrecht May 18, 2022
43bbbfb
Fixed test.
SDGGiesbrecht May 19, 2022
1c513ca
Fixed test.
SDGGiesbrecht May 19, 2022
19b894b
Resolved conflicts.
SDGGiesbrecht May 25, 2022
0f092d7
Fixed module alias tests.
SDGGiesbrecht May 25, 2022
c7445f0
Overwrote conflicts from master.
SDGGiesbrecht Jul 13, 2022
1f27f39
Fixed test expectations.
SDGGiesbrecht Jul 13, 2022
8a61a4a
Merge branch 'master' into resolution
SDGGiesbrecht Jul 13, 2022
91b9b0b
Merge branch 'master' into resolution
SDGGiesbrecht Oct 14, 2022
195741a
Blocked setting from system libraries.
SDGGiesbrecht Oct 14, 2022
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
11 changes: 11 additions & 0 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -597,3 +597,14 @@ if ProcessInfo.processInfo.environment["SWIFTCI_USE_LOCAL_DEPS"] == nil {
.package(path: "../swift-collections"),
]
}

// FIXME: Once we are confident target based dependency resolution is reliable, this switch can be removed.
if ProcessInfo.processInfo.environment["DISABLE_TARGET_BASED_DEPENDENCY_RESOLUTION"] == nil { // Currently enabled by default.
for target in package.targets {
var swiftSettings = target.swiftSettings ?? []
defer { target.swiftSettings = swiftSettings }
if target.name != "SPMSQLite3" {
swiftSettings.append(.define("ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION"))
}
}
}
30 changes: 25 additions & 5 deletions Sources/PackageGraph/DependencyResolutionNode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,23 +60,38 @@ public enum DependencyResolutionNode {
/// to the information needed in order to provide actionable suggestions to help the user stitch up the dependency declarations properly.
case root(package: PackageReference)

// FIXME: If command plugins become explicit in the manifest, or are extricated from the main graph, `implicitCommands` should be removed.
/// A node representing any implicit command plugins vended by a package.
///
/// This type of node is equivalent to a product node in all respects,
/// except that the number and names of the products are unkown,
/// because the manifest contains no explicit reference to them.
case implicitCommands(package: PackageReference)

/// The package.
public var package: PackageReference {
switch self {
case .empty(let package), .product(_, let package), .root(let package):
case .empty(let package), .product(_, let package), .root(let package), .implicitCommands(package: let package):
return package
}
}

/// The name of the specific product if the node is a product node, otherwise `nil`.
public var specificProduct: String? {
switch self {
case .empty, .root:
case .empty, .root, .implicitCommands:
return nil
case .product(let product, _):
return product
}
}
public var isImplicitCommandsNode: Bool {
if case .implicitCommands = self {
return true
} else {
return false
}
}

/// Assembles the product filter to use on the manifest for this node to determine its dependencies.
public var productFilter: ProductFilter {
Expand All @@ -87,6 +102,8 @@ public enum DependencyResolutionNode {
return .specific([product])
case .root:
return .everything
case .implicitCommands:
return .specific([], includeCommands: true)
}
}

Expand All @@ -95,7 +112,7 @@ public enum DependencyResolutionNode {
/// This is the constraint that requires all products from a package resolve to the same version.
internal func versionLock(version: Version) -> PackageContainerConstraint? {
// Don’t create a version lock for anything but a product.
guard specificProduct != nil else { return nil }
guard specificProduct != nil || isImplicitCommandsNode else { return nil }
return PackageContainerConstraint(
package: self.package,
versionRequirement: .exact(version),
Expand All @@ -108,7 +125,7 @@ public enum DependencyResolutionNode {
/// This is the constraint that requires all products from a package resolve to the same revision.
internal func revisionLock(revision: String) -> PackageContainerConstraint? {
// Don’t create a revision lock for anything but a product.
guard specificProduct != nil else { return nil }
guard specificProduct != nil || isImplicitCommandsNode else { return nil }
return PackageContainerConstraint(
package: self.package,
requirement: .revision(revision),
Expand All @@ -119,14 +136,17 @@ public enum DependencyResolutionNode {

extension DependencyResolutionNode: Equatable {
public static func ==(lhs: DependencyResolutionNode, rhs: DependencyResolutionNode) -> Bool {
return (lhs.package, lhs.specificProduct) == (rhs.package, rhs.specificProduct)
return lhs.package == rhs.package
&& lhs.specificProduct == rhs.specificProduct
&& lhs.isImplicitCommandsNode == rhs.isImplicitCommandsNode
}
}

extension DependencyResolutionNode: Hashable {
public func hash(into hasher: inout Hasher) {
hasher.combine(self.package)
hasher.combine(self.specificProduct)
hasher.combine(self.isImplicitCommandsNode)
}
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/PackageGraph/GraphLoadingNode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ extension GraphLoadingNode: CustomStringConvertible {
switch productFilter {
case .everything:
return self.identity.description
case .specific(let set):
case .specific(let set, _):
return "\(self.identity.description)[\(set.sorted().joined(separator: ", "))]"
}
}
Expand Down
11 changes: 8 additions & 3 deletions Sources/PackageGraph/PackageModel+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,17 @@ extension PackageContainerConstraint {
case .everything:
assertionFailure("Attempted to enumerate a root package’s product filter; root packages have no filter.")
return []
case .specific(let set):
case .specific(let set, let includeCommands):
var nodes: [DependencyResolutionNode]
if set.isEmpty { // Pointing at the package without a particular product.
return [.empty(package: self.package)]
nodes = [.empty(package: self.package)]
} else {
return set.sorted().map { .product($0, package: self.package) }
nodes = set.sorted().map { .product($0, package: self.package) }
}
if includeCommands {
nodes.append(.implicitCommands(package: self.package))
}
return nodes
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions Sources/PackageLoading/PackageBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1146,8 +1146,11 @@ public final class PackageBuilder {
switch productFilter {
case .everything:
filteredProducts = self.manifest.products
case .specific(let set):
filteredProducts = self.manifest.products.filter { set.contains($0.name) }
case .specific(let set, let includeCommands):
filteredProducts = self.manifest.products.filter { product in
return set.contains(product.name)
|| (includeCommands && self.manifest.productIsCommandPlugin(product))
}
}
for product in filteredProducts {
if product.name.isEmpty {
Expand Down
38 changes: 32 additions & 6 deletions Sources/PackageModel/Manifest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,24 @@ public final class Manifest {
self.targetMap = Dictionary(targets.lazy.map({ ($0.name, $0) }), uniquingKeysWith: { $1 })
}

public func productIsCommandPlugin(_ product: ProductDescription) -> Bool {
return product.type == .plugin
&& self.targets.contains(where: { target in
if case .command = target.pluginCapability,
product.targets.contains(target.name) {
return true
} else {
return false
}
})
}
internal func productIsCommandPlugin(_ product: String) -> Bool {
return products.contains(where: { possibleMatch in
return possibleMatch.name == product
&& self.productIsCommandPlugin(possibleMatch)
})
}

/// Returns the targets required for a particular product filter.
public func targetsRequired(for productFilter: ProductFilter) -> [TargetDescription] {
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
Expand All @@ -163,8 +181,11 @@ public final class Manifest {
switch productFilter {
case .everything:
return self.targets
case .specific(let productFilter):
let products = self.products.filter { productFilter.contains($0.name) }
case .specific(let productFilter, let includeCommands):
let products = self.products.filter { product in
return productFilter.contains(product.name)
|| (includeCommands && productIsCommandPlugin(product))
}
targets = targetsRequired(for: products)
}

Expand All @@ -184,7 +205,11 @@ public final class Manifest {
return dependencies
} else {
let targets = self.targetsRequired(for: productFilter)
let dependencies = self.dependenciesRequired(for: targets, keepUnused: productFilter == .everything)
let dependencies = self.dependenciesRequired(
for: targets,
keepImplicitCommandPlugins: productFilter == .everything,
keepUnused: productFilter == .everything
)
self._requiredDependencies[productFilter] = dependencies
return dependencies
}
Expand Down Expand Up @@ -264,6 +289,7 @@ public final class Manifest {
/// The returned dependencies have their particular product filters registered. (To determine product filters without removing any dependencies from the list, specify `keepUnused: true`.)
private func dependenciesRequired(
for targets: [TargetDescription],
keepImplicitCommandPlugins: Bool,
keepUnused: Bool = false
) -> [PackageDependency] {

Expand Down Expand Up @@ -291,13 +317,13 @@ public final class Manifest {

return dependencies.compactMap { dependency in
if let filter = associations[dependency.identity] {
return dependency.filtered(by: filter)
return dependency.filtered(by: keepImplicitCommandPlugins ? filter.includingImplicitCommands() : filter)
} else if keepUnused {
// Register that while the dependency was kept, no products are needed.
return dependency.filtered(by: .nothing)
return dependency.filtered(by: keepImplicitCommandPlugins ? .nothing.includingImplicitCommands() : .nothing)
} else {
// Dependencies known to not have any relevant products are discarded.
return nil
return keepImplicitCommandPlugins ? dependency.filtered(by: .nothing.includingImplicitCommands()) : nil
}
}
}
Expand Down
62 changes: 22 additions & 40 deletions Sources/PackageModel/Product.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,18 @@ public enum ProductType: Equatable, Hashable {
/// Any product which matches the filter will be used for dependency resolution, whereas unrequested products will be ignored.
///
/// Requested products need not actually exist in the package. Under certain circumstances, the resolver may request names whose package of origin are unknown. The intended package will recognize and fulfill the request; packages that do not know what it is will simply ignore it.
public enum ProductFilter: Equatable, Hashable {
public enum ProductFilter: Codable, Equatable, Hashable {

/// All products, targets, and tests are requested.
///
/// This is used for root packages.
case everything

// FIXME: If command plugins become explicit in the manifest, or are extricated from the main graph, `includeCommands` should be removed.
/// A set of specific products requested by one or more client packages.
case specific(Set<String>)
///
/// `includeCommands` is used by first‐level dependencies to also request any command plugins, regardless of whether they are referenced anywhere.
case specific(Set<String>, includeCommands: Bool = false)

/// No products, targets, or tests are requested.
public static var nothing: ProductFilter { .specific([]) }
Expand All @@ -130,12 +133,15 @@ public enum ProductFilter: Equatable, Hashable {
switch self {
case .everything:
return .everything
case .specific(let set):
case .specific(let set, let includeCommands):
switch other {
case .everything:
return .everything
case .specific(let otherSet):
return .specific(set.union(otherSet))
case .specific(let otherSet, let otherIncludeCommands):
return .specific(
set.union(otherSet),
includeCommands: includeCommands || otherIncludeCommands
)
}
}
}
Expand All @@ -144,23 +150,22 @@ public enum ProductFilter: Equatable, Hashable {
self = self.union(other)
}

public func contains(_ product: String) -> Bool {
public func contains(_ product: String, isCommandPlugin: (String) -> Bool) -> Bool {
switch self {
case .everything:
return true
case .specific(let set):
case .specific(let set, let includeCommands):
return set.contains(product)
|| (includeCommands && isCommandPlugin(product))
}
}

public func merge(_ other: ProductFilter) -> ProductFilter {
switch (self, other) {
case (.everything, _):
return .everything
case (_, .everything):
return .everything
case (.specific(let mine), .specific(let other)):
return .specific(mine.union(other))
internal func includingImplicitCommands() -> ProductFilter {
switch self {
case .everything:
return self
case .specific(let set, _):
return .specific(set, includeCommands: true)
}
}
}
Expand Down Expand Up @@ -203,8 +208,8 @@ extension ProductFilter: CustomStringConvertible {
switch self {
case .everything:
return "[everything]"
case .specific(let set):
return "[\(set.sorted().joined(separator: ", "))]"
case .specific(let set, let includeCommands):
return "[\(set.sorted().joined(separator: ", "))\(includeCommands ? " (including commands)" : "")]"
}
}
}
Expand Down Expand Up @@ -254,26 +259,3 @@ extension ProductType: Codable {
}
}
}

extension ProductFilter: Codable {
public func encode(to encoder: Encoder) throws {
let optionalSet: Set<String>?
switch self {
case .everything:
optionalSet = nil
case .specific(let set):
optionalSet = set
}
var container = encoder.singleValueContainer()
try container.encode(optionalSet?.sorted())
}

public init(from decoder: Decoder) throws {
let container = try decoder.singleValueContainer()
if container.decodeNil() {
self = .everything
} else {
self = .specific(Set(try container.decode([String].self)))
}
}
}
2 changes: 1 addition & 1 deletion Sources/Workspace/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1946,7 +1946,7 @@ extension Workspace {
var allManifests = [(identity: PackageIdentity, manifest: Manifest, productFilter: ProductFilter)]()
for pair in allManifestsWithPossibleDuplicates {
if let index = deduplication[pair.key.identity] {
let productFilter = allManifests[index].productFilter.merge(pair.key.productFilter)
let productFilter = allManifests[index].productFilter.union(pair.key.productFilter)
allManifests[index] = (pair.key.identity, pair.item, productFilter)
} else {
deduplication[pair.key.identity] = allManifests.count
Expand Down
16 changes: 16 additions & 0 deletions Tests/BuildTests/ModuleAliasingBuildTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2165,13 +2165,21 @@ final class ModuleAliasingBuildTests: XCTestCase {
))

result.checkProductsCount(1)
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
result.checkTargetsCount(6)
#else
result.checkTargetsCount(7)
#endif

XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "Lib" && $0.target.moduleAliases?["Utils"] == "LibUtils" && $0.target.moduleAliases?["Render"] == "LibRender" })
XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "LibRender" && $0.target.moduleAliases?["Render"] == "LibRender" })
XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "LibUtils" && $0.target.moduleAliases?["Utils"] == "LibUtils" })
XCTAssertFalse(result.targetMap.values.contains { $0.target.name == "DrawRender" || $0.target.moduleAliases?["Render"] == "DrawRender" })
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
XCTAssertFalse(result.targetMap.values.contains { $0.target.name == "Game" })
#else
XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "Game" && $0.target.moduleAliases?["Utils"] == "LibUtils" })
#endif
XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "Render" && $0.target.moduleAliases == nil })
XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "Utils" && $0.target.moduleAliases == nil })
XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "App" && $0.target.moduleAliases == nil })
Expand Down Expand Up @@ -2280,12 +2288,20 @@ final class ModuleAliasingBuildTests: XCTestCase {
))

result.checkProductsCount(1)
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
result.checkTargetsCount(8)
#else
result.checkTargetsCount(9)
#endif

XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "Lib" && $0.target.moduleAliases?["Utils"] == "LibUtils" && $0.target.moduleAliases?["Render"] == "LibRender" })
XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "LibRender" && $0.target.moduleAliases?["Render"] == "LibRender" })
XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "LibUtils" && $0.target.moduleAliases?["Utils"] == "LibUtils" })
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
XCTAssertFalse(result.targetMap.values.contains { $0.target.name == "Game" })
#else
XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "Game" && $0.target.moduleAliases?["Utils"] == "LibUtils" })
#endif
XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "Scene" && $0.target.moduleAliases?["Render"] == "DrawRender" })
XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "DrawRender" && $0.target.moduleAliases?["Render"] == "DrawRender" })
XCTAssertTrue(result.targetMap.values.contains { $0.target.name == "Render" && $0.target.moduleAliases == nil })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ extension ProductFilter: JSONSerializable, JSONMappable {
switch self {
case .everything:
return "all".toJSON()
case .specific(let products):
case .specific(let products, _):
return products.sorted().toJSON()
}
}
Expand Down
Loading