Allow build & run to reference unreachable targets #1381
Conversation
Sources/Commands/SwiftRunTool.swift
Outdated
|
|
||
| return executableProduct | ||
| } else { | ||
| // If the executably is implicit, search through root products. |
|
|
||
| /// Returns list of all targets (reachable from root targets) in the graph. | ||
| public let targets: [ResolvedTarget] | ||
| public let targets: Set<ResolvedTarget> |
There was a problem hiding this comment.
Should we call this reachableTargets?
There was a problem hiding this comment.
Good idea. Also renamed products to reachableProducts.
| "BTargetTests", | ||
| "ATargetTests", | ||
| "BTarget" | ||
| ]) |
There was a problem hiding this comment.
We should sort the keys for deterministic order
There was a problem hiding this comment.
Doesn't the conversion to a Set already handle that?
| 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"]) |
There was a problem hiding this comment.
We should sort these sets for deterministic order
| XCTAssert(!output.contains("bexec")) | ||
| XCTAssert(!output.contains("BTarget2")) | ||
| XCTAssert(!output.contains("cexec")) | ||
| XCTAssert(!output.contains("CTarget")) |
There was a problem hiding this comment.
probably better to check the build data than the output
There was a problem hiding this comment.
As in check that the expected files are present inside the .build directory
| XCTAssert(!output.contains("bexec")) | ||
| XCTAssert(!output.contains("BTarget1")) | ||
| XCTAssert(!output.contains("BTarget2")) | ||
| XCTAssert(!output.contains("cexec")) |
There was a problem hiding this comment.
We should test that we only build things required by CTarget
|
Thanks @hartbit! Looks good, I added some minor feedback. |
hartbit
force-pushed
the
build-unreachable-products-and-targets
branch
from
ab8c5f8 to
178e83a
Compare
| } | ||
|
|
||
| // Always build everything for the test target. | ||
| test.outputs += newTarget.outputs |
There was a problem hiding this comment.
Does this mean we will end up compiling tests from other packages for when building tests?
There was a problem hiding this comment.
No, because the code only runs whenbuildByDefault is true, which is set to plan.graph.reachableTargets.contains(target) on line 91. See below the vapor build manifest output from a master build of swiftpm vs with this branch:
hartbit
force-pushed
the
build-unreachable-products-and-targets
branch
from
178e83a to
527a735
Compare
|
@swift-ci please smoke test |
2 participants
This reintroduces PR #1284 that was reverted before the release of Swift 4 because of a bug referenced in SR-5595. The original code was modified to pass the new test that was added in 4b74cec to support external test targets on Linux.
This resolves SR-5597.