Allow build & run to reference unreachable targets

hartbit · hartbit · commit cb71bd7ce41b · 2017-07-26T13:23:23.000+02:00
diff --git a/Fixtures/Miscellaneous/UnreachableTargets/A/Package.swift b/Fixtures/Miscellaneous/UnreachableTargets/A/Package.swift
@@ -0,0 +1,17 @@
+// swift-tools-version:4.0
+import PackageDescription
+
+let package = 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")
+		])
+    ])
diff --git a/Fixtures/Miscellaneous/UnreachableTargets/A/Sources/ATarget/main.swift b/Fixtures/Miscellaneous/UnreachableTargets/A/Sources/ATarget/main.swift
@@ -0,0 +1,3 @@
+import BTarget1
+
+BTarget1()
diff --git a/Fixtures/Miscellaneous/UnreachableTargets/B/Package.swift b/Fixtures/Miscellaneous/UnreachableTargets/B/Package.swift
@@ -0,0 +1,13 @@
+// swift-tools-version:4.0
+import PackageDescription
+
+let package = Package(
+    name: "B",
+    products: [
+        .library(name: "BLibrary", targets: ["BTarget1"]),
+		.executable(name: "bexec", targets: ["BTarget2"])
+    ],
+    targets: [
+        .target(name: "BTarget1", dependencies: []),
+		.target(name: "BTarget2", dependencies: [])
+    ])
diff --git a/Fixtures/Miscellaneous/UnreachableTargets/B/Sources/BTarget1/BTarget1.swift b/Fixtures/Miscellaneous/UnreachableTargets/B/Sources/BTarget1/BTarget1.swift
@@ -0,0 +1,3 @@
+public struct BTarget1 {
+	public init() {}
+}
diff --git a/Fixtures/Miscellaneous/UnreachableTargets/B/Sources/BTarget2/main.swift b/Fixtures/Miscellaneous/UnreachableTargets/B/Sources/BTarget2/main.swift
@@ -0,0 +1 @@
+print("BTarget2")
diff --git a/Fixtures/Miscellaneous/UnreachableTargets/C/Package.swift b/Fixtures/Miscellaneous/UnreachableTargets/C/Package.swift
@@ -0,0 +1,11 @@
+// swift-tools-version:4.0
+import PackageDescription
+
+let package = Package(
+    name: "C",
+    products: [
+		.executable(name: "cexec", targets: ["CTarget"])
+    ],
+    targets: [
+        .target(name: "CTarget", dependencies: [])
+    ])
diff --git a/Fixtures/Miscellaneous/UnreachableTargets/C/Sources/CTarget/main.swift b/Fixtures/Miscellaneous/UnreachableTargets/C/Sources/CTarget/main.swift
@@ -0,0 +1 @@
+print("CTarget")
diff --git a/Sources/Build/BuildPlan.swift b/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.targets {
+        for target in graph.allTargets {
              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.products where product.type != .library(.automatic) {
+        for product in graph.allProducts where product.type != .library(.automatic) {
             productMap[product] = ProductBuildDescription(
                 product: product, buildParameters: buildParameters)
         }
diff --git a/Sources/Build/llbuild.swift b/Sources/Build/llbuild.swift
@@ -51,7 +51,7 @@ public struct LLBuildManifestGenerator {
         private var otherTargets: [Target] = []
 
         /// Append a command.
-        mutating func append(_ target: Target, isTest: Bool) {
+        mutating func append(_ target: Target, buildByDefault: Bool, 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,14 +63,17 @@ public struct LLBuildManifestGenerator {
             newTarget.cmds.insert(phonyCommand)
             otherTargets.append(newTarget)
 
-            if !isTest {
-                main.outputs += newTarget.outputs
-                main.cmds += newTarget.cmds
+            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
             }
 
-            // Always build everything for the test target.
-            test.outputs += newTarget.outputs
-            test.cmds += newTarget.cmds
             allCommands += newTarget.cmds
         }
     }
@@ -80,18 +83,26 @@ public struct LLBuildManifestGenerator {
         var targets = Targets()
 
         // Create commands for all target description in the plan.
-        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)
+        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)
             }
         }
 
         // Create command for all products in the plan.
-        for buildProduct in plan.buildProducts {
-            targets.append(createProductTarget(buildProduct), isTest: buildProduct.product.type == .test)
+        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)
         }
 
         // Write the manifest.
diff --git a/Sources/Commands/SwiftRunTool.swift b/Sources/Commands/SwiftRunTool.swift
@@ -126,27 +126,30 @@ 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, verify that it exists.
-            guard let executableProduct = executableProducts.first(where: { $0.name == executable }) else {
+            // 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 {
                 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 executableProducts.count == 1 else {
-                throw RunError.multipleExecutables(executableProducts.map({ $0.name }))
+            guard rootExecutables.count == 1 else {
+                throw RunError.multipleExecutables(rootExecutables.map({ $0.name }))
             }
             
-            return executableProducts[0]
+            return rootExecutables[0]
         }
     }
     
diff --git a/Sources/Commands/SwiftTool.swift b/Sources/Commands/SwiftTool.swift
@@ -568,13 +568,13 @@ extension BuildSubset {
         case .allIncludingTests:
             return LLBuildManifestGenerator.llbuildTestTargetName
         case .product(let productName):
-            guard let product = graph.products.first(where: { $0.name == productName }) else {
+            guard let product = graph.allProducts.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.targets.first(where: { $0.name == targetName }) else {
+            guard let target = graph.allTargets.first(where: { $0.name == targetName }) else {
                 diagnostics.emit(data: TargetNotFoundDiagnostic(targetName: targetName))
                 return nil
             }
diff --git a/Sources/PackageGraph/PackageGraph.swift b/Sources/PackageGraph/PackageGraph.swift
@@ -21,11 +21,17 @@ public struct PackageGraph {
     public let packages: [ResolvedPackage]
 
     /// Returns list of all targets (reachable from root targets) in the graph.
-    public let targets: [ResolvedTarget]
+    public let targets: Set<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: [ResolvedProduct]
+    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>
 
     /// Returns true if a given target is present in root packages.
     public func isInRootPackages(_ target: ResolvedTarget) -> Bool {
@@ -38,25 +44,24 @@ 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: [ResolvedTarget] = []
-        var products = inputPackages.flatMap({ $0.products })
-        let rootDependencyProductSet = Set(rootDependencies.flatMap({ $0.products }))
+        var targets: Set<ResolvedTarget> = []
+        var products = Set(inputPackages.flatMap({ $0.products }))
 
         for dependency in dependencies {
             switch dependency {
             case .target(let target):
-                targets.append(target)
+                targets.insert(target)
             case .product(let product):
-                // Avoid re-adding any product from the root dependencies.
-                if rootDependencyProductSet.contains(product) { continue }
-                products.append(product)
+                products.insert(product)
             }
         }
 
diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift
@@ -458,6 +458,64 @@ 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", "ATarget", "BLibrary"])
+        XCTAssertEqual(Set(graph.targets.map({ $0.name })), ["ATarget", "BTarget1"])
+        XCTAssertEqual(Set(graph.allProducts.map({ $0.name })), ["aexec", "ATarget", "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", "ATarget", "BLibrary", "bexec", "cexec"])
+        XCTAssertEqual(Set(result.targetMap.keys), ["ATarget", "BTarget1", "BTarget2", "CTarget"])
+    }
+
     static var allTests = [
         ("testBasicClangPackage", testBasicClangPackage),
         ("testBasicReleasePackage", testBasicReleasePackage),
@@ -468,6 +526,7 @@ final class BuildPlanTests: XCTestCase {
         ("testDynamicProducts", testDynamicProducts),
         ("testSwiftCMixed", testSwiftCMixed),
         ("testTestModule", testTestModule),
+        ("testNonReachableProductsAndTargets", testNonReachableProductsAndTargets)
     ]
 }
 
diff --git a/Tests/CommandsTests/BuildToolTests.swift b/Tests/CommandsTests/BuildToolTests.swift
@@ -111,11 +111,67 @@ 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)
+        ("testProductAndTarget", testProductAndTarget),
+        ("testNonReachableProductsAndTargetsFunctional", testNonReachableProductsAndTargetsFunctional)
     ]
 }
diff --git a/Tests/CommandsTests/RunToolTests.swift b/Tests/CommandsTests/RunToolTests.swift

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+import BTarget1`
	`2`	`+`
	`3`	`+BTarget1()`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+public struct BTarget1 {`
	`2`	`+ public init() {}`
	`3`	`+}`