From 0e6f30d3bc1ee50dd4bf41a32522912597c49e32 Mon Sep 17 00:00:00 2001 From: David Hart Date: Mon, 24 Jul 2017 23:07:01 +0200 Subject: [PATCH] Allow build & run to reference unreachable targets --- .../UnreachableTargets/A/Package.swift | 17 ++++++ .../A/Sources/ATarget/main.swift | 3 + .../UnreachableTargets/B/Package.swift | 13 ++++ .../B/Sources/BTarget1/BTarget1.swift | 3 + .../B/Sources/BTarget2/main.swift | 1 + .../UnreachableTargets/C/Package.swift | 11 ++++ .../C/Sources/CTarget/main.swift | 1 + Sources/Build/BuildPlan.swift | 4 +- Sources/Build/llbuild.swift | 41 ++++++++----- Sources/Commands/SwiftRunTool.swift | 27 +++++---- Sources/Commands/SwiftTool.swift | 4 +- Sources/PackageGraph/PackageGraph.swift | 23 +++++--- Tests/BuildTests/BuildPlanTests.swift | 59 +++++++++++++++++++ Tests/CommandsTests/BuildToolTests.swift | 58 +++++++++++++++++- Tests/CommandsTests/RunToolTests.swift | 16 ++++- 15 files changed, 238 insertions(+), 43 deletions(-) create mode 100644 Fixtures/Miscellaneous/UnreachableTargets/A/Package.swift create mode 100644 Fixtures/Miscellaneous/UnreachableTargets/A/Sources/ATarget/main.swift create mode 100644 Fixtures/Miscellaneous/UnreachableTargets/B/Package.swift create mode 100644 Fixtures/Miscellaneous/UnreachableTargets/B/Sources/BTarget1/BTarget1.swift create mode 100644 Fixtures/Miscellaneous/UnreachableTargets/B/Sources/BTarget2/main.swift create mode 100644 Fixtures/Miscellaneous/UnreachableTargets/C/Package.swift create mode 100644 Fixtures/Miscellaneous/UnreachableTargets/C/Sources/CTarget/main.swift diff --git a/Fixtures/Miscellaneous/UnreachableTargets/A/Package.swift b/Fixtures/Miscellaneous/UnreachableTargets/A/Package.swift new file mode 100644 index 00000000000..7c05636fa8a --- /dev/null +++ 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 new file mode 100644 index 00000000000..6abff7a1c58 --- /dev/null +++ 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 new file mode 100644 index 00000000000..dc2553656b8 --- /dev/null +++ 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 new file mode 100644 index 00000000000..9058324cce7 --- /dev/null +++ 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 new file mode 100644 index 00000000000..8b87e07b824 --- /dev/null +++ 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 new file mode 100644 index 00000000000..2c17ea77dde --- /dev/null +++ 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 new file mode 100644 index 00000000000..333d235c2b7 --- /dev/null +++ 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 index 7ea260ff02d..d94d34b38d8 100644 --- 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 index a036d63f3e2..310ce9a8ad0 100644 --- 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 index a2a23a3c541..7e2623311d0 100644 --- a/Sources/Commands/SwiftRunTool.swift +++ b/Sources/Commands/SwiftRunTool.swift @@ -126,27 +126,30 @@ public class SwiftRunTool: SwiftTool { /// 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 index e6f08548279..e27d9e92bcf 100644 --- 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 index 12528f4a22c..1b7cbb6fbca 100644 --- 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 /// 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 + + /// Returns all the targets in the graph, regardless if they are reachable from the root targets or not. + public let allTargets: Set + + /// Returns all the products in the graph, regardless if they are reachable from the root targets or not. + public var allProducts: Set /// Returns true if a given target is present in root packages. public func isInRootPackages(_ target: ResolvedTarget) -> Bool { @@ -38,6 +44,8 @@ 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) @@ -45,18 +53,15 @@ public struct PackageGraph { 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 = [] + 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 index 4ba986a7fe8..bb5eb1470d8 100644 --- 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", "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), @@ -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 index 7c3cb488fca..8858b310dfe 100644 --- 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 index 90167cf324f..ef11c64ed9f 100644 --- a/Tests/CommandsTests/RunToolTests.swift +++ b/Tests/CommandsTests/RunToolTests.swift @@ -28,7 +28,7 @@ final class RunToolTests: XCTestCase { XCTAssert(try execute(["--version"]).contains("Swift Package Manager")) } - func testFunctional() throws { + func testUnkownProductAndArgumentPassing() throws { fixture(name: "Miscellaneous/EchoExecutable") { path in do { _ = try execute(["unknown"], packagePath: path) @@ -43,7 +43,9 @@ final class RunToolTests: XCTestCase { "\(getcwd())" "1" "--hello" "world" """) } + } + func testMultipleExecutableAndExplicitExecutable() throws { fixture(name: "Miscellaneous/MultipleExecutables") { path in do { _ = try execute([], packagePath: path) @@ -61,6 +63,14 @@ final class RunToolTests: XCTestCase { } } + func testUnreachableExecutable() throws { + fixture(name: "Miscellaneous/UnreachableTargets") { path in + let output = try execute(["bexec"], packagePath: path.appending(component: "A")) + let outputLines = output.split(separator: "\n") + XCTAssertEqual(outputLines.last!, "BTarget2") + } + } + func testFileDeprecation() throws { fixture(name: "Miscellaneous/EchoExecutable") { path in let filePath = AbsolutePath(path, "Sources/secho/main.swift").asString @@ -75,7 +85,9 @@ final class RunToolTests: XCTestCase { static var allTests = [ ("testUsage", testUsage), ("testVersion", testVersion), - ("testFunctional", testFunctional), + ("testUnkownProductAndArgumentPassing", testUnkownProductAndArgumentPassing), + ("testMultipleExecutableAndExplicitExecutable", testMultipleExecutableAndExplicitExecutable), + ("testUnreachableExecutable", testUnreachableExecutable), ("testFileDeprecation", testFileDeprecation) ] }