Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 17 additions & 0 deletions Fixtures/Miscellaneous/UnreachableTargets/A/Package.swift
Original file line number Diff line number Diff line change
@@ -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")
])
])
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import BTarget1

BTarget1()
13 changes: 13 additions & 0 deletions Fixtures/Miscellaneous/UnreachableTargets/B/Package.swift
Original file line number Diff line number Diff line change
@@ -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: [])
])
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
public struct BTarget1 {
public init() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
print("BTarget2")
11 changes: 11 additions & 0 deletions Fixtures/Miscellaneous/UnreachableTargets/C/Package.swift
Original file line number Diff line number Diff line change
@@ -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: [])
])
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
print("CTarget")
4 changes: 2 additions & 2 deletions Sources/Build/BuildPlan.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this start building all the targets when swift build is invoked?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed now.

switch target.underlyingTarget {
case is SwiftTarget:
targetMap[target] = .swift(SwiftTargetDescription(target: target, buildParameters: buildParameters))
Expand Down Expand Up @@ -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)
}
Expand Down
41 changes: 26 additions & 15 deletions Sources/Build/llbuild.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -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
}
}
Expand All @@ -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.
Expand Down
27 changes: 15 additions & 12 deletions Sources/Commands/SwiftRunTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: matchingRootExecutables

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind, but is it really more explicit? The predicate we are matching against is already in the name: it's the list of "root executables".


// 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]
}
}

Expand Down
4 changes: 2 additions & 2 deletions Sources/Commands/SwiftTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
23 changes: 14 additions & 9 deletions Sources/PackageGraph/PackageGraph.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
}

Expand Down
59 changes: 59 additions & 0 deletions Tests/BuildTests/BuildPlanTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -468,6 +526,7 @@ final class BuildPlanTests: XCTestCase {
("testDynamicProducts", testDynamicProducts),
("testSwiftCMixed", testSwiftCMixed),
("testTestModule", testTestModule),
("testNonReachableProductsAndTargets", testNonReachableProductsAndTargets)
]
}

Expand Down
58 changes: 57 additions & 1 deletion Tests/CommandsTests/BuildToolTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
]
}
Loading