From f7d49b362ce797d6fc4ca8e78a8b5e961d0be470 Mon Sep 17 00:00:00 2001
From: Steven Wu <stevenwu@apple.com>
Date: Thu, 27 Feb 2025 13:18:57 -0800
Subject: [PATCH] Re-land [Planning] Avoid batching compile job twice

This re-land the change in 7ba9d485ab44cca5c61cf974679d6fba91feb1ca 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.
---
 Sources/SwiftDriver/Jobs/Planning.swift       | 22 ++++++++++---------
 .../IncrementalCompilationTests.swift         | 19 ++++++++++++++++
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/Sources/SwiftDriver/Jobs/Planning.swift b/Sources/SwiftDriver/Jobs/Planning.swift
index 53462039d..ae8c9b5d9 100644
--- a/Sources/SwiftDriver/Jobs/Planning.swift
+++ b/Sources/SwiftDriver/Jobs/Planning.swift
@@ -90,16 +90,18 @@ extension Driver {
       incrementalCompilationState = nil
     }
 
-    return try (
-      // For compatibility with swiftpm, the driver produces batched jobs
-      // for every job, even when run in incremental mode, so that all jobs
-      // can be returned from `planBuild`.
-      // But in that case, don't emit lifecycle messages.
-      formBatchedJobs(jobsInPhases.allJobs,
-                      showJobLifecycle: showJobLifecycle && incrementalCompilationState == nil,
-                      jobCreatingPch: jobsInPhases.allJobs.first(where: {$0.kind == .generatePCH})),
-      incrementalCompilationState
-    )
+    let batchedJobs: [Job]
+    // If the jobs are batched during the incremental build, reuse the computation rather than computing the batches again.
+    if let incrementalState = incrementalCompilationState {
+      // For compatibility reasons, all the jobs planned will be returned, even the incremental state suggests the job is not mandatory.
+      batchedJobs = incrementalState.skippedJobs + incrementalState.mandatoryJobsInOrder + incrementalState.jobsAfterCompiles
+    } else {
+      batchedJobs = try formBatchedJobs(jobsInPhases.allJobs,
+                                        showJobLifecycle: showJobLifecycle,
+                                        jobCreatingPch: jobsInPhases.allJobs.first(where: {$0.kind == .generatePCH}))
+    }
+
+    return (batchedJobs, incrementalCompilationState)
   }
 
   /// If performing an explicit module build, compute an inter-module dependency graph.
diff --git a/Tests/SwiftDriverTests/IncrementalCompilationTests.swift b/Tests/SwiftDriverTests/IncrementalCompilationTests.swift
index dbddd257e..12aff3f7c 100644
--- a/Tests/SwiftDriverTests/IncrementalCompilationTests.swift
+++ b/Tests/SwiftDriverTests/IncrementalCompilationTests.swift
@@ -217,6 +217,25 @@ extension IncrementalCompilationTests {
     let expected = try AbsolutePath(validating: "\(module).autolink", relativeTo: derivedDataPath)
     XCTAssertEqual(outputs.first!.file.absolutePath, expected)
   }
+
+  // Null planning should not return an empty compile job for compatibility reason.
+  // `swift-build` wraps the jobs returned by swift-driver in `Executor` so returning an empty list of compile job will break build system.
+  func testNullPlanningCompatility() throws {
+    guard let sdkArgumentsForTesting = try Driver.sdkArgumentsForTesting() else {
+      throw XCTSkip("Cannot perform this test on this host")
+    }
+    var driver = try Driver(args: commonArgs + sdkArgumentsForTesting)
+    let initialJobs = try driver.planBuild()
+    try driver.run(jobs: initialJobs)
+
+    // Plan the build again without touching any file. This should be a null build but for compatibility reason,
+    // planBuild() should return all the jobs and supported build system will query incremental state for the actual
+    // jobs need to be executed.
+    let replanJobs = try driver.planBuild()
+    XCTAssertFalse(
+      replanJobs.filter { $0.kind == .compile }.isEmpty,
+      "more than one compile job needs to be planned")
+  }
 }
 
 // MARK: - Dot file tests