Skip to content

Commit f7d49b3

Browse files
Re-land [Planning] Avoid batching compile job twice
This re-land the change in 7ba9d48 with compatibility fixes and additional tests. The fix makes sure the skipped jobs are returned in the planning as well for compatibility reasons. Not after this change: For the build systems that use swift-driver that doesn't support incremental build (swiftpm), asking for an incremental planning will still get the full list of jobs, but the skipped jobs might not be batched. The planned jobs are still complete, but might not be optimally batched. For the build systems that supports incremental build (swift-build), the build planning is extracted from incremental state directly. But note swift-driver lazily wrapped the compile jobs in Executor so swift-driver can't return an empty list of jobs. Add a test to make sure that is the case.
1 parent a9afe15 commit f7d49b3

File tree

2 files changed

+31
-10
lines changed

2 files changed

+31
-10
lines changed

Sources/SwiftDriver/Jobs/Planning.swift

+12-10
Original file line numberDiff line numberDiff line change
@@ -90,16 +90,18 @@ extension Driver {
9090
incrementalCompilationState = nil
9191
}
9292

93-
return try (
94-
// For compatibility with swiftpm, the driver produces batched jobs
95-
// for every job, even when run in incremental mode, so that all jobs
96-
// can be returned from `planBuild`.
97-
// But in that case, don't emit lifecycle messages.
98-
formBatchedJobs(jobsInPhases.allJobs,
99-
showJobLifecycle: showJobLifecycle && incrementalCompilationState == nil,
100-
jobCreatingPch: jobsInPhases.allJobs.first(where: {$0.kind == .generatePCH})),
101-
incrementalCompilationState
102-
)
93+
let batchedJobs: [Job]
94+
// If the jobs are batched during the incremental build, reuse the computation rather than computing the batches again.
95+
if let incrementalState = incrementalCompilationState {
96+
// For compatibility reasons, all the jobs planned will be returned, even the incremental state suggests the job is not mandatory.
97+
batchedJobs = incrementalState.skippedJobs + incrementalState.mandatoryJobsInOrder + incrementalState.jobsAfterCompiles
98+
} else {
99+
batchedJobs = try formBatchedJobs(jobsInPhases.allJobs,
100+
showJobLifecycle: showJobLifecycle,
101+
jobCreatingPch: jobsInPhases.allJobs.first(where: {$0.kind == .generatePCH}))
102+
}
103+
104+
return (batchedJobs, incrementalCompilationState)
103105
}
104106

105107
/// If performing an explicit module build, compute an inter-module dependency graph.

Tests/SwiftDriverTests/IncrementalCompilationTests.swift

+19
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,25 @@ extension IncrementalCompilationTests {
217217
let expected = try AbsolutePath(validating: "\(module).autolink", relativeTo: derivedDataPath)
218218
XCTAssertEqual(outputs.first!.file.absolutePath, expected)
219219
}
220+
221+
// Null planning should not return an empty compile job for compatibility reason.
222+
// `swift-build` wraps the jobs returned by swift-driver in `Executor` so returning an empty list of compile job will break build system.
223+
func testNullPlanningCompatility() throws {
224+
guard let sdkArgumentsForTesting = try Driver.sdkArgumentsForTesting() else {
225+
throw XCTSkip("Cannot perform this test on this host")
226+
}
227+
var driver = try Driver(args: commonArgs + sdkArgumentsForTesting)
228+
let initialJobs = try driver.planBuild()
229+
try driver.run(jobs: initialJobs)
230+
231+
// Plan the build again without touching any file. This should be a null build but for compatibility reason,
232+
// planBuild() should return all the jobs and supported build system will query incremental state for the actual
233+
// jobs need to be executed.
234+
let replanJobs = try driver.planBuild()
235+
XCTAssertFalse(
236+
replanJobs.filter { $0.kind == .compile }.isEmpty,
237+
"more than one compile job needs to be planned")
238+
}
220239
}
221240

222241
// MARK: - Dot file tests

0 commit comments

Comments
 (0)