Fix unsafeToInterrupt on Amazon Linux 2

This distro is going EoL in July, but fix the test anyways for better future-proofing.

What happens is that the task fails to execute because there is no working directory support on Amazon Linux 2, so the build fails. However, in that case the background task which waits for the sentinel file to be written will run forever. Ensure that we cancel the background task in cases where the build has completed.

Author: jakepetroules

Sources/SWBBuildService/BuildOperationMessages.swift

@@ -1054,15 +1054,17 @@ final class OperationDelegate: BuildOperationDelegate {
         request.send(BuildOperationReportPathMap(copiedPathMap: copiedPathMap, generatedFilesPathMap: generatedFilesPathMap))
     }
 
-    func buildComplete(_ operation: any BuildSystemOperation, status: BuildOperationEnded.Status?, delegate: any BuildOutputDelegate, metrics: BuildOperationMetrics?) {
+    func buildComplete(_ operation: any BuildSystemOperation, status: BuildOperationEnded.Status?, delegate: any BuildOutputDelegate, metrics: BuildOperationMetrics?) -> BuildOperationEnded.Status {
         if !skipCommandLevelInformation {
             // Kick the target callbacks so that our target-level diagnostics get emitted in the right context in the case of an early build failure due to task construction errors.
             for target in diagnosticsHandler.deferredTargets {
                 targetStarted(operation, configuredTarget: target)
                 targetComplete(operation, configuredTarget: target)
             }
         }
-        activeBuild.completeBuild(status: status ?? taskCompletionBasedStatus, metrics: metrics)
+        let realStatus = status ?? taskCompletionBasedStatus
+        activeBuild.completeBuild(status: realStatus, metrics: metrics)
+        return realStatus
     }
 
     private func getActiveTargetInfo(_ operation: any BuildSystemOperation, _ configuredTarget: ConfiguredTarget) -> TargetInfo {

Sources/SWBBuildSystem/BuildOperation.swift

@@ -52,7 +52,7 @@ package protocol BuildOperationDelegate {
     /// - Parameters:
     ///   - status: Overall build operation status to override the build result with. This can be used when the operation was aborted (the build could not run to completion, e.g. due to discovery of a cycle) or cancelled and wants to propagate that status regardless of the status of the individual build tasks in the underlying (llbuild) build system. `nil` will use the status based on the result status of the tasks in the underlying llbuild build system.
     ///   - delegate: The task output delegate provided by the \see buildStarted() method.
-    func buildComplete(_ operation: any BuildSystemOperation, status: BuildOperationEnded.Status?, delegate: any BuildOutputDelegate, metrics: BuildOperationMetrics?)
+    @discardableResult func buildComplete(_ operation: any BuildSystemOperation, status: BuildOperationEnded.Status?, delegate: any BuildOutputDelegate, metrics: BuildOperationMetrics?) -> BuildOperationEnded.Status
 
     /// Called when an individual task has been started.
     ///
@@ -137,7 +137,7 @@ package protocol BuildSystemOperation: AnyObject, Sendable {
     var subtaskProgressReporter: (any SubtaskProgressReporter)? { get }
     var uuid: UUID { get }
 
-    func build() async
+    @discardableResult func build() async -> BuildOperationEnded.Status
     func cancel()
     func abort()
 }
@@ -285,7 +285,7 @@ package final class BuildOperation: BuildSystemOperation {
     }
 
     /// Perform the build.
-    package func build() async {
+    package func build() async -> BuildOperationEnded.Status {
         // Inform the client the build has started.
         buildOutputDelegate = delegate.buildStarted(self)
 
@@ -305,8 +305,9 @@ package final class BuildOperation: BuildSystemOperation {
         // Diagnose attempts to use "dry run" mode, which we don't support yet.
         if request.useDryRun {
             buildOutputDelegate.error("-dry-run is not yet supported in the new build system")
-            delegate.buildComplete(self, status: .failed, delegate: buildOutputDelegate, metrics: nil)
-            return
+            let effectiveStatus = BuildOperationEnded.Status.failed
+            delegate.buildComplete(self, status: effectiveStatus, delegate: buildOutputDelegate, metrics: nil)
+            return effectiveStatus
         }
 
         // Helper method to emit information to the CAPTURED_BUILD_INFO_DIR.  This is mainly to be able to gracefully return after emitting a warning if something goes wrong, because not being able to emit this info is typically not serious enough to want to abort the whole build.
@@ -367,8 +368,9 @@ package final class BuildOperation: BuildSystemOperation {
 
         // If task construction had errors, fail the build immediately, unless `continueBuildingAfterErrors` is set.
         if !request.continueBuildingAfterErrors && buildDescription.hadErrors {
-            delegate.buildComplete(self, status: .failed, delegate: buildOutputDelegate, metrics: nil)
-            return
+            let effectiveStatus = BuildOperationEnded.Status.failed
+            delegate.buildComplete(self, status: effectiveStatus, delegate: buildOutputDelegate, metrics: nil)
+            return effectiveStatus
         }
 
         if userPreferences.enableDebugActivityLogs {
@@ -410,8 +412,9 @@ package final class BuildOperation: BuildSystemOperation {
         do {
             let isCancelled = await queue.sync{ self.wasCancellationRequested }
             if !UserDefaults.skipEarlyBuildOperationCancellation && isCancelled {
-                delegate.buildComplete(self, status: .cancelled, delegate: buildOutputDelegate, metrics: nil)
-                return
+                let effectiveStatus = BuildOperationEnded.Status.cancelled
+                delegate.buildComplete(self, status: effectiveStatus, delegate: buildOutputDelegate, metrics: nil)
+                return effectiveStatus
             }
         }
 
@@ -421,8 +424,9 @@ package final class BuildOperation: BuildSystemOperation {
             for message in warnings { buildOutputDelegate.warning(message) }
             for message in errors { buildOutputDelegate.error(message) }
             guard errors.isEmpty else {
-                delegate.buildComplete(self, status: .failed, delegate: buildOutputDelegate, metrics: nil)
-                return
+                let effectiveStatus = BuildOperationEnded.Status.failed
+                delegate.buildComplete(self, status: effectiveStatus, delegate: buildOutputDelegate, metrics: nil)
+                return effectiveStatus
             }
         }
 
@@ -776,7 +780,7 @@ package final class BuildOperation: BuildSystemOperation {
         }
 
         // `buildComplete()` should not run within `queue`, otherwise there can be a deadlock during cancelling.
-        delegate.buildComplete(self, status: effectiveStatus, delegate: buildOutputDelegate, metrics: .init(counters: delegate.aggregatedCounters))
+        return delegate.buildComplete(self, status: effectiveStatus, delegate: buildOutputDelegate, metrics: .init(counters: delegate.aggregatedCounters))
     }
 
     func prepareForBuilding() -> ([String], [String])? {

Sources/SWBBuildSystem/CleanOperation.swift

@@ -12,6 +12,7 @@
 
 package import SWBCore
 import SWBTaskExecution
+package import SWBProtocol
 package import SWBUtil
 
 package import class Foundation.FileManager
@@ -61,7 +62,7 @@ package final class CleanOperation: BuildSystemOperation, TargetDependencyResolv
         return try BuildDescriptionManager.cacheDirectory(buildRequest, buildRequestContext: buildRequestContext, workspaceContext: workspaceContext).join("XCBuildData")
     }
 
-    package func build() async {
+    package func build() async -> BuildOperationEnded.Status {
         let buildOutputDelegate = delegate.buildStarted(self)
 
         if workspaceContext.userPreferences.enableDebugActivityLogs {
@@ -103,7 +104,7 @@ package final class CleanOperation: BuildSystemOperation, TargetDependencyResolv
 
         cleanBuildFolders(buildFolders: Set(buildFolders), buildOutputDelegate: buildOutputDelegate)
 
-        delegate.buildComplete(self, status: nil, delegate: buildOutputDelegate, metrics: nil)
+        return delegate.buildComplete(self, status: nil, delegate: buildOutputDelegate, metrics: nil)
     }
 
     package func cancel() {
@@ -205,7 +206,7 @@ package final class CleanOperation: BuildSystemOperation, TargetDependencyResolv
         delegate.targetPreparationStarted(self, configuredTarget: configuredTarget)
         delegate.targetStarted(self, configuredTarget: configuredTarget)
 
-        let (executable, arguments, workingDirectory, environment) = constructCommandLine(for: configuredTarget.target as! ExternalTarget, action: "clean", settings: settings, workspaceContext: workspaceContext, scope: settings.globalScope)
+        let (executable, arguments, workingDirectory, environment) = constructCommandLine(for: configuredTarget.target as! SWBCore.ExternalTarget, action: "clean", settings: settings, workspaceContext: workspaceContext, scope: settings.globalScope)
         let commandLine = [executable] + arguments
 
         let specLookupContext = SpecLookupCtxt(specRegistry: workspaceContext.core.specRegistry, platform: settings.platform)

Sources/SWBTestSupport/BuildOperationTester.swift

@@ -1995,11 +1995,12 @@ private final class BuildOperationTesterDelegate: BuildOperationDelegate {
         return TesterBuildOutputDelegate(delegate: self)
     }
 
-    func buildComplete(_ operation: any BuildSystemOperation, status: BuildOperationEnded.Status?, delegate: any BuildOutputDelegate, metrics: BuildOperationMetrics?) {
+    func buildComplete(_ operation: any BuildSystemOperation, status: BuildOperationEnded.Status?, delegate: any BuildOutputDelegate, metrics: BuildOperationMetrics?) -> BuildOperationEnded.Status {
         queue.async {
             // There is no "build failed" event, so only explicit cancellation needs to map to `buildCancelled`
             self.events.append(status == .cancelled ? .buildCancelled : .buildCompleted)
         }
+        return status ?? .succeeded
     }
 
     func targetPreparationStarted(_ operation: any BuildSystemOperation, configuredTarget: ConfiguredTarget) {

Tests/SWBBuildSystemTests/BuildTaskBehaviorTests.swift

@@ -306,7 +306,7 @@ fileprivate struct BuildTaskBehaviorTests: CoreBasedTests {
     }
 
     /// Check that we honor specs which are unsafe to interrupt.
-    @Test(.requireSDKs(.host), .skipHostOS(.windows, "no bash shell"))
+    @Test(.requireSDKs(.host), .skipHostOS(.windows, "no bash shell"), .requireThreadSafeWorkingDirectory)
     func unsafeToInterrupt() async throws {
         let fs = localFS
         let output = MakePlannedVirtualNode("<WAIT>")
@@ -365,7 +365,13 @@ fileprivate struct BuildTaskBehaviorTests: CoreBasedTests {
                 }
 
                 try await withTaskCancellationHandler {
-                    await operation.build()
+                    switch await operation.build() {
+                    case .cancelled, .failed:
+                        // If the build already failed, cancel the task that waits for the script so the test doesn't hang forever.
+                        task.cancel()
+                    case .succeeded:
+                        break
+                    }
                     try await task.value
                 } onCancel: {
                     task.cancel()