Fix Command Resolution

Sources/PackageGraph/DependencyResolutionNode.swift

@@ -58,23 +58,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 {
@@ -85,6 +100,8 @@ public enum DependencyResolutionNode {
             return .specific([product])
         case .root:
             return .everything
+        case .implicitCommands:
+            return .specific([], includeCommandPlugins: true)
         }
     }
 
@@ -93,7 +110,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),
@@ -106,7 +123,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),
@@ -117,14 +134,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)
     }
 }
 

Sources/PackageGraph/GraphLoadingNode.swift

@@ -59,7 +59,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: ", "))]"
         }
     }

Sources/PackageGraph/PackageModel+Extensions.swift

@@ -55,12 +55,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 includeCommandPlugins):
+                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 includeCommandPlugins {
+                    nodes.append(.implicitCommands(package: self.package))
+                }
+                return nodes
             }
         }
     }

Sources/PackageLoading/PackageBuilder.swift

@@ -1231,8 +1231,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 includeCommandPlugins):
+            filteredProducts = self.manifest.products.filter { product in
+                return set.contains(product.name)
+                || (includeCommandPlugins && self.manifest.productIsCommandPlugin(product))
+            }
         }
         for product in filteredProducts {
             let targets = try modulesFrom(targetNames: product.targets, product: product.name)

Sources/PackageModel/Manifest.swift

@@ -150,6 +150,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
@@ -161,8 +179,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 includeCommandPlugins):
+                let products = self.products.filter { product in
+                    return productFilter.contains(product.name)
+                    || (includeCommandPlugins && productIsCommandPlugin(product))
+                }
                 targets = targetsRequired(for: products)
             }
 
@@ -182,7 +203,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
         }
@@ -249,6 +274,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] {
 
@@ -276,13 +302,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
             }
         }
     }

Sources/PackageModel/Product.swift

@@ -107,15 +107,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, `includeCommandPlugins` should be removed.
     /// A set of specific products requested by one or more client packages.
-    case specific(Set<String>)
+    ///
+    /// `includeCommandPlugins` is used by first‐level dependencies to also request any command plugins, regardless of whether they are referenced anywhere.
+    case specific(Set<String>, includeCommandPlugins: Bool = false)
 
     /// No products, targets, or tests are requested.
     public static var nothing: ProductFilter { .specific([]) }
@@ -124,12 +127,15 @@ public enum ProductFilter: Equatable, Hashable {
         switch self {
         case .everything:
             return .everything
-        case .specific(let set):
+        case .specific(let set, let includeCommandPlugins):
             switch other {
             case .everything:
                 return .everything
-            case .specific(let otherSet):
-                return .specific(set.union(otherSet))
+            case .specific(let otherSet, let otherIncludeCommandPlugins):
+                return .specific(
+                    set.union(otherSet),
+                    includeCommandPlugins: includeCommandPlugins || otherIncludeCommandPlugins
+                )
             }
         }
     }
@@ -138,23 +144,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 includeCommandPlugins):
             return set.contains(product)
+            || (includeCommandPlugins && 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, includeCommandPlugins: true)
         }
     }
 }
@@ -197,8 +202,8 @@ extension ProductFilter: CustomStringConvertible {
         switch self {
         case .everything:
             return "[everything]"
-        case .specific(let set):
-            return "[\(set.sorted().joined(separator: ", "))]"
+        case .specific(let set, let includeCommandPlugins):
+            return "[\(set.sorted().joined(separator: ", "))\(includeCommandPlugins ? " (including command plugins)" : "")]"
         }
     }
 }
@@ -248,26 +253,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)))
-        }
-    }
-}

Sources/Workspace/Workspace.swift

@@ -1999,7 +1999,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

Tests/PackageGraphPerformanceTests/DependencyResolverPerfTests.swift

@@ -178,14 +178,17 @@ extension ProductFilter: JSONSerializable, JSONMappable {
         switch self {
         case .everything:
             return "all".toJSON()
-        case .specific(let products):
+        case .specific(var products, let includeCommandPlugins):
+            if includeCommandPlugins { products.insert("_command_plugins_") }
             return products.sorted().toJSON()
         }
     }
 
     public init(json: JSON) throws {
         if let products = try? [String](json: json) {
-            self = .specific(Set(products))
+            var set = Set(products)
+            let includeCommandPlugins = set.remove("_commands_plugins_") != nil
+            self = .specific(set, includeCommandPlugins: includeCommandPlugins)
         } else {
             self = .everything
         }

Tests/PackageGraphTests/PackageGraphTests.swift

@@ -1889,7 +1889,7 @@ class PackageGraphTests: XCTestCase {
         let requestedProducts = unrelated.productFilter
         #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
         // Unrelated should not have been asked for Product, because it should know Product comes from Identity.
-        XCTAssertFalse(requestedProducts.contains("Product"), "Product requests are leaking.")
+        XCTAssertFalse(requestedProducts.contains("Product", isCommandPlugin: { _ in false }), "Product requests are leaking.")
         #endif
     }
 }

Tests/PackageGraphTests/PubgrubTests.swift

@@ -2680,7 +2680,8 @@ public class MockContainer: PackageContainer {
             throw _MockLoadingError.unknownRevision
         }
         var filteredDependencies: [MockContainer.Dependency] = []
-        for (product, productDependencies) in revisionDependencies where productFilter.contains(product) {
+        for (product, productDependencies) in revisionDependencies
+        where productFilter.contains(product, isCommandPlugin: { _ in false /* not distinguished by MockContainer */ }) {
             filteredDependencies.append(contentsOf: productDependencies)
         }
         return filteredDependencies.map({ value in