Revert "Allow build & run to reference unreachable targets"

Sources/Build/BuildPlan.swift

@@ -474,7 +474,7 @@ public class BuildPlan {
 
         // Create build target description for each target which we need to plan.
         var targetMap = [ResolvedTarget: TargetDescription]()
-        for target in graph.allTargets {
+        for target in graph.targets {
              switch target.underlyingTarget {
              case is SwiftTarget:
                  targetMap[target] = .swift(SwiftTargetDescription(target: target, buildParameters: buildParameters))
@@ -511,7 +511,7 @@ public class BuildPlan {
         var productMap: [ResolvedProduct: ProductBuildDescription] = [:]
         // Create product description for each product we have in the package graph except
         // for automatic libraries because they don't produce any output.
-        for product in graph.allProducts where product.type != .library(.automatic) {
+        for product in graph.products where product.type != .library(.automatic) {
             productMap[product] = ProductBuildDescription(
                 product: product, buildParameters: buildParameters)
         }

Sources/Build/llbuild.swift

@@ -51,7 +51,7 @@ public struct LLBuildManifestGenerator {
         private var otherTargets: [Target] = []
 
         /// Append a command.
-        mutating func append(_ target: Target, buildByDefault: Bool, isTest: Bool) {
+        mutating func append(_ target: Target, isTest: Bool) {
             // Create a phony command with a virtual output node that represents the target.
             let virtualNodeName = "<\(target.name)>"
             let phonyTool = PhonyTool(inputs: target.outputs, outputs: [virtualNodeName])
@@ -63,17 +63,14 @@ public struct LLBuildManifestGenerator {
             newTarget.cmds.insert(phonyCommand)
             otherTargets.append(newTarget)
 
-            if buildByDefault {
-                if !isTest {
-                    main.outputs += newTarget.outputs
-                    main.cmds += newTarget.cmds
-                }
-
-                // Always build everything for the test target.
-                test.outputs += newTarget.outputs
-                test.cmds += newTarget.cmds
+            if !isTest {
+                main.outputs += newTarget.outputs
+                main.cmds += newTarget.cmds
             }
 
+            // Always build everything for the test target.
+            test.outputs += newTarget.outputs
+            test.cmds += newTarget.cmds
             allCommands += newTarget.cmds
         }
     }
@@ -83,26 +80,18 @@ public struct LLBuildManifestGenerator {
         var targets = Targets()
 
         // Create commands for all target description in the plan.
-        for (target, description) in plan.targetMap {
-            switch description {
-            case .swift(let description):
-                // Only build targets by default if they are reachabe from a root target.
-                targets.append(createSwiftCompileTarget(description),
-                    buildByDefault: plan.graph.targets.contains(target),
-                    isTest: description.isTestTarget)
-            case .clang(let description):
-                targets.append(createClangCompileTarget(description),
-                    buildByDefault: plan.graph.targets.contains(target),
-                    isTest: description.isTestTarget)
+        for buildTarget in plan.targets {
+            switch buildTarget {
+            case .swift(let target):
+                targets.append(createSwiftCompileTarget(target), isTest: target.isTestTarget)
+            case .clang(let target):
+                targets.append(createClangCompileTarget(target), isTest: target.isTestTarget)
             }
         }
 
         // Create command for all products in the plan.
-        for (product, description) in plan.productMap {
-            // Only build products by default if they are reachabe from a root target.
-            targets.append(createProductTarget(description),
-                buildByDefault: plan.graph.products.contains(product),
-                isTest: product.type == .test)
+        for buildProduct in plan.buildProducts {
+            targets.append(createProductTarget(buildProduct), isTest: buildProduct.product.type == .test)
         }
 
         // Write the manifest.

Sources/Commands/SwiftRunTool.swift

@@ -126,30 +126,27 @@ public class SwiftRunTool: SwiftTool<RunToolOptions> {
 
     /// Returns the path to the correct executable based on options.
     private func findProduct(in plan: BuildPlan) throws -> ResolvedProduct {
+        let executableProducts = plan.graph.products.filter({ $0.type == .executable })
+
+        // Error out if the product contains no executable.        
+        guard executableProducts.count > 0 else {
+            throw RunError.noExecutableFound
+        }
+
         if let executable = options.executable {
-            // If the exectuable is explicitly specified, search through all products.
-            guard let executableProduct = plan.graph.allProducts.first(where: {
-                $0.type == .executable && $0.name == executable
-            }) else {
+            // If the exectuable is explicitly specified, verify that it exists.
+            guard let executableProduct = executableProducts.first(where: { $0.name == executable }) else {
                 throw RunError.executableNotFound(executable)
             }
 
             return executableProduct
         } else {
-            // If the executably is implicit, search through root products.
-            let rootExecutables = plan.graph.rootPackages.flatMap({ $0.products }).filter({ $0.type == .executable })
-
-            // Error out if the package contains no executables.
-            guard rootExecutables.count > 0 else {
-                throw RunError.noExecutableFound
-            }
-
             // Only implicitly deduce the executable if it is the only one.
-            guard rootExecutables.count == 1 else {
-                throw RunError.multipleExecutables(rootExecutables.map({ $0.name }))
+            guard executableProducts.count == 1 else {
+                throw RunError.multipleExecutables(executableProducts.map({ $0.name }))
             }
 
-            return rootExecutables[0]
+            return executableProducts[0]
         }
     }
 

Sources/Commands/SwiftTool.swift

@@ -568,13 +568,13 @@ extension BuildSubset {
         case .allIncludingTests:
             return LLBuildManifestGenerator.llbuildTestTargetName
         case .product(let productName):
-            guard let product = graph.allProducts.first(where: { $0.name == productName }) else {
+            guard let product = graph.products.first(where: { $0.name == productName }) else {
                 diagnostics.emit(data: ProductNotFoundDiagnostic(productName: productName))
                 return nil
             }
             return product.llbuildTargetName
         case .target(let targetName):
-            guard let target = graph.allTargets.first(where: { $0.name == targetName }) else {
+            guard let target = graph.targets.first(where: { $0.name == targetName }) else {
                 diagnostics.emit(data: TargetNotFoundDiagnostic(targetName: targetName))
                 return nil
             }

Sources/PackageGraph/PackageGraph.swift

@@ -21,17 +21,11 @@ public struct PackageGraph {
     public let packages: [ResolvedPackage]
 
     /// Returns list of all targets (reachable from root targets) in the graph.
-    public let targets: Set<ResolvedTarget>
+    public let targets: [ResolvedTarget]
 
     /// Contains all the products of the root packages and the product dependencies of the root targets.
     /// i.e. this array will not contain the products which are not needed to be built.
-    public let products: Set<ResolvedProduct>
-
-    /// Returns all the targets in the graph, regardless if they are reachable from the root targets or not.
-    public let allTargets: Set<ResolvedTarget>
-
-    /// Returns all the products in the graph, regardless if they are reachable from the root targets or not.
-    public var allProducts: Set<ResolvedProduct>
+    public let products: [ResolvedProduct]
 
     /// Returns true if a given target is present in root packages.
     public func isInRootPackages(_ target: ResolvedTarget) -> Bool {
@@ -44,24 +38,25 @@ public struct PackageGraph {
         self.rootPackages = rootPackages
         let inputPackages = rootPackages + rootDependencies
         self.packages = try! topologicalSort(inputPackages, successors: { $0.dependencies })
-        allTargets = Set(packages.flatMap({ $0.targets }))
-        allProducts = Set(packages.flatMap({ $0.products }))
 
         // Compute the input targets.
         let inputTargets = inputPackages.flatMap({ $0.targets }).map(ResolvedTarget.Dependency.target)
         // Find all the dependencies of the root targets.
         let dependencies = try! topologicalSort(inputTargets, successors: { $0.dependencies })
 
         // Separate out the products and targets but maintain their topological order.
-        var targets: Set<ResolvedTarget> = []
-        var products = Set(inputPackages.flatMap({ $0.products }))
+        var targets: [ResolvedTarget] = []
+        var products = inputPackages.flatMap({ $0.products })
+        let rootDependencyProductSet = Set(rootDependencies.flatMap({ $0.products }))
 
         for dependency in dependencies {
             switch dependency {
             case .target(let target):
-                targets.insert(target)
+                targets.append(target)
             case .product(let product):
-                products.insert(product)
+                // Avoid re-adding any product from the root dependencies.
+                if rootDependencyProductSet.contains(product) { continue }
+                products.append(product)
             }
         }
 

Tests/BuildTests/BuildPlanTests.swift

@@ -458,64 +458,6 @@ final class BuildPlanTests: XCTestCase {
 
     }
 
-    func testNonReachableProductsAndTargets() throws {
-        let fileSystem = InMemoryFileSystem(emptyFiles:
-            "/A/Sources/ATarget/main.swift",
-            "/B/Sources/BTarget1/BTarget1.swift",
-            "/B/Sources/BTarget2/main.swift",
-            "/C/Sources/CTarget/main.swift"
-        )
-
-        let diagnostics = DiagnosticsEngine()
-        let graph = loadMockPackageGraph4([
-            "/A": Package(
-                name: "A",
-                products: [
-                    .executable(name: "aexec", targets: ["ATarget"])
-                ],
-                dependencies: [
-                    .package(url: "/B", from: "1.0.0"),
-                    .package(url: "/C", from: "1.0.0")
-                ],
-                targets: [
-                    .target(name: "ATarget", dependencies: [
-                        .product(name: "BLibrary")
-                    ])
-                ]),
-            "/B": Package(
-                name: "B",
-                products: [
-                    .library(name: "BLibrary", type: .static, targets: ["BTarget1"]),
-                    .executable(name: "bexec", targets: ["BTarget2"])
-                ],
-                targets: [
-                    .target(name: "BTarget1", dependencies: []),
-                    .target(name: "BTarget2", dependencies: [])
-                ]),
-            "/C": Package(
-                name: "C",
-                products: [
-                    .executable(name: "cexec", targets: ["CTarget"])
-                ],
-                targets: [
-                    .target(name: "CTarget", dependencies: [])
-                ])
-        ], root: "/A", diagnostics: diagnostics, in: fileSystem)
-
-        XCTAssertEqual(Set(graph.products.map({ $0.name })), ["aexec", "BLibrary"])
-        XCTAssertEqual(Set(graph.targets.map({ $0.name })), ["ATarget", "BTarget1"])
-        XCTAssertEqual(Set(graph.allProducts.map({ $0.name })), ["aexec", "BLibrary", "bexec", "cexec"])
-        XCTAssertEqual(Set(graph.allTargets.map({ $0.name })), ["ATarget", "BTarget1", "BTarget2", "CTarget"])
-
-        let result = BuildPlanResult(plan: try BuildPlan(
-            buildParameters: mockBuildParameters(),
-            graph: graph,
-            fileSystem: fileSystem))
-
-        XCTAssertEqual(Set(result.productMap.keys), ["aexec", "BLibrary", "bexec", "cexec"])
-        XCTAssertEqual(Set(result.targetMap.keys), ["ATarget", "BTarget1", "BTarget2", "CTarget"])
-    }
-
     static var allTests = [
         ("testBasicClangPackage", testBasicClangPackage),
         ("testBasicReleasePackage", testBasicReleasePackage),
@@ -526,7 +468,6 @@ final class BuildPlanTests: XCTestCase {
         ("testDynamicProducts", testDynamicProducts),
         ("testSwiftCMixed", testSwiftCMixed),
         ("testTestModule", testTestModule),
-        ("testNonReachableProductsAndTargets", testNonReachableProductsAndTargets)
     ]
 }
 

Tests/CommandsTests/BuildToolTests.swift

@@ -111,67 +111,11 @@ final class BuildToolTests: XCTestCase {
         }
     }
 
-    func testNonReachableProductsAndTargetsFunctional() {
-        fixture(name: "Miscellaneous/UnreachableTargets") { path in
-            do {
-                let aPath = path.appending(component: "A")
-                let output = try execute([], packagePath: aPath)
-                XCTAssert(!output.contains("bexec"))
-                XCTAssert(!output.contains("BTarget2"))
-                XCTAssert(!output.contains("cexec"))
-                XCTAssert(!output.contains("CTarget"))
-            } catch SwiftPMProductError.executionFailure(_, _, let stderr) {
-                XCTFail(stderr)
-            }
-
-            let buildPath = path.appending(component: ".build")
-            try localFileSystem.removeFileTree(buildPath)
-
-            // Dependency contains a dependent product
-
-            do {
-                let aPath = path.appending(component: "A")
-                let output = try execute(["--product", "bexec"], packagePath: aPath)
-                XCTAssert(output.contains("Compile") && output.contains("BTarget2"))
-                XCTAssert(output.contains("Linking") && output.contains("bexec"))
-                XCTAssert(!output.contains("aexec"))
-                XCTAssert(!output.contains("ATarget"))
-                XCTAssert(!output.contains("BLibrary"))
-                XCTAssert(!output.contains("BTarget1"))
-                XCTAssert(!output.contains("cexec"))
-                XCTAssert(!output.contains("CTarget"))
-            } catch SwiftPMProductError.executionFailure(_, _, let stderr) {
-                XCTFail(stderr)
-            }
-
-            try localFileSystem.removeFileTree(buildPath)
-
-            // Dependency does not contain a dependent product
-
-            do {
-                let aPath = path.appending(component: "A")
-                let output = try execute(["--target", "CTarget"], packagePath: aPath)
-                XCTAssert(output.contains("Compile") && output.contains("CTarget"))
-                XCTAssert(!output.contains("Linking"))
-                XCTAssert(!output.contains("aexec"))
-                XCTAssert(!output.contains("ATarget"))
-                XCTAssert(!output.contains("BLibrary"))
-                XCTAssert(!output.contains("bexec"))
-                XCTAssert(!output.contains("BTarget1"))
-                XCTAssert(!output.contains("BTarget2"))
-                XCTAssert(!output.contains("cexec"))
-            } catch SwiftPMProductError.executionFailure(_, _, let stderr) {
-                XCTFail(stderr)
-            }
-        }
-    }
-
     static var allTests = [
         ("testUsage", testUsage),
         ("testVersion", testVersion),
         ("testBinPath", testBinPath),
         ("testBackwardsCompatibilitySymlink", testBackwardsCompatibilitySymlink),
-        ("testProductAndTarget", testProductAndTarget),
-        ("testNonReachableProductsAndTargetsFunctional", testNonReachableProductsAndTargetsFunctional)
+        ("testProductAndTarget", testProductAndTarget)
     ]
 }