Merge pull request #79 from owenv/multi-job-diag

Diagnose inputs modified during the build for multi-job plans

Author: owenv

Sources/SwiftDriver/Driver/Driver.swift

@@ -669,6 +669,7 @@ extension Driver {
     let jobExecutor = JobExecutor(
         jobs: jobs, resolver: resolver,
         executorDelegate: executorDelegate,
+        diagnosticsEngine: diagnosticEngine,
         numParallelJobs: numParallelJobs,
         processSet: processSet,
         forceResponseFiles: forceResponseFiles,

Sources/SwiftDriver/Execution/JobExecutor.swift

@@ -148,6 +148,9 @@ public final class JobExecutor {
     /// The last time each input file was modified, recorded at the start of the build.
     public let recordedInputModificationDates: [TypedVirtualPath: Date]
 
+    /// The diagnostics engine to use when reporting errors.
+    let diagnosticsEngine: DiagnosticsEngine
+
     init(
       argsResolver: ArgsResolver,
       env: [String: String],
@@ -157,7 +160,8 @@ public final class JobExecutor {
       jobQueue: OperationQueue,
       processSet: ProcessSet?,
       forceResponseFiles: Bool,
-      recordedInputModificationDates: [TypedVirtualPath: Date]
+      recordedInputModificationDates: [TypedVirtualPath: Date],
+      diagnosticsEngine: DiagnosticsEngine
     ) {
       self.producerMap = producerMap
       self.argsResolver = argsResolver
@@ -168,6 +172,7 @@ public final class JobExecutor {
       self.processSet = processSet
       self.forceResponseFiles = forceResponseFiles
       self.recordedInputModificationDates = recordedInputModificationDates
+      self.diagnosticsEngine = diagnosticsEngine
     }
   }
 
@@ -192,10 +197,14 @@ public final class JobExecutor {
   /// The last time each input file was modified, recorded at the start of the build.
   public let recordedInputModificationDates: [TypedVirtualPath: Date]
 
+  /// The diagnostics engine to use when reporting errors.
+  let diagnosticsEngine: DiagnosticsEngine
+
   public init(
     jobs: [Job],
     resolver: ArgsResolver,
     executorDelegate: JobExecutorDelegate,
+    diagnosticsEngine: DiagnosticsEngine,
     numParallelJobs: Int? = nil,
     processSet: ProcessSet? = nil,
     forceResponseFiles: Bool = false,
@@ -204,6 +213,7 @@ public final class JobExecutor {
     self.jobs = jobs
     self.argsResolver = resolver
     self.executorDelegate = executorDelegate
+    self.diagnosticsEngine = diagnosticsEngine
     self.numParallelJobs = numParallelJobs ?? 1
     self.processSet = processSet
     self.forceResponseFiles = forceResponseFiles
@@ -248,7 +258,8 @@ public final class JobExecutor {
       jobQueue: jobQueue,
       processSet: processSet,
       forceResponseFiles: forceResponseFiles,
-      recordedInputModificationDates: recordedInputModificationDates
+      recordedInputModificationDates: recordedInputModificationDates,
+      diagnosticsEngine: diagnosticsEngine
     )
   }
 }
@@ -429,6 +440,9 @@ class ExecuteJobRule: LLBuildRule {
 
       value = .jobExecution(success: success)
     } catch {
+      if error is DiagnosticData {
+        context.diagnosticsEngine.emit(error)
+      }
       context.delegateQueue.async {
         let result = ProcessResult(
           arguments: [],

Tests/SwiftDriverTests/JobExecutorTests.swift

@@ -150,7 +150,7 @@ final class JobExecutorTests: XCTestCase {
       )
 
       let delegate = JobCollectingDelegate()
-      let executor = JobExecutor(jobs: [compileFoo, compileMain, link], resolver: resolver, executorDelegate: delegate)
+      let executor = JobExecutor(jobs: [compileFoo, compileMain, link], resolver: resolver, executorDelegate: delegate, diagnosticsEngine: DiagnosticsEngine())
       try executor.execute(env: toolchain.env, fileSystem: localFileSystem)
 
       let output = try TSCBasic.Process.checkNonZeroExit(args: exec.pathString)
@@ -178,7 +178,8 @@ final class JobExecutorTests: XCTestCase {
     delegate.useStubProcess = true
     let executor = JobExecutor(
       jobs: [job], resolver: try ArgsResolver(),
-      executorDelegate: delegate
+      executorDelegate: delegate,
+      diagnosticsEngine: DiagnosticsEngine()
     )
     try executor.execute(env: ProcessEnv.vars, fileSystem: localFileSystem)
 
@@ -247,27 +248,24 @@ final class JobExecutorTests: XCTestCase {
       try localFileSystem.writeFileContents(other) {
         $0 <<< "let bar = 2"
       }
-
-      var driver = try Driver(args: ["swiftc", main.pathString, other.pathString])
-      let jobs = try driver.planBuild()
-      XCTAssertTrue(jobs.count > 1)
-      let resolver = try ArgsResolver()
-
-      // Change the file
-      try localFileSystem.writeFileContents(other) {
-        $0 <<< "let bar = 3"
-      }
-
-      let delegate = JobCollectingDelegate()
-      delegate.useStubProcess = true
-      XCTAssertThrowsError(try driver.run(jobs: jobs, resolver: resolver,
-                                          executorDelegate: delegate)) {
-        // FIXME: The JobExecutor needs a way of emitting diagnostics or
-        // propagating errors through llbuild.
-        XCTAssertEqual($0 as? Diagnostics, .fatalError)
+      try assertDriverDiagnostics(args: ["swiftc", main.pathString, other.pathString]) {driver, verifier in
+        let jobs = try driver.planBuild()
+        XCTAssertTrue(jobs.count > 1)
+        let resolver = try ArgsResolver()
+
+        // Change the file
+        try localFileSystem.writeFileContents(other) {
+          $0 <<< "let bar = 3"
+        }
+
+        let delegate = JobCollectingDelegate()
+        delegate.useStubProcess = true
+
+        // FIXME: It's unfortunate we diagnose this twice, once for each job which uses the input.
+        verifier.expect(.error("input file '\(other.description)' was modified during the build"))
+        verifier.expect(.error("input file '\(other.description)' was modified during the build"))
+        XCTAssertThrowsError(try driver.run(jobs: jobs, resolver: resolver, executorDelegate: delegate))
       }
-
     }
   }
-
 }