Merge pull request swiftlang#74 from neonichu/ignore-failures-after-c…

…ancel Ignore command failures after being cancelled
aciidgh · Jan 25, 2017 · 153a6be · 153a6be
2 parents 9e6d4d1 + b519ec8
commit 153a6be
Show file tree

Hide file tree

Showing 11 changed files with 90 additions and 14 deletions.
diff --git a/examples/c-api/buildsystem/main.c b/examples/c-api/buildsystem/main.c
@@ -178,6 +178,7 @@ static void command_process_had_output(void* context,
 static void command_process_finished(void* context,
                                      llb_buildsystem_command_t* command,
                                      llb_buildsystem_process_t* process,
+                                     llb_buildsystem_command_result_t result,
                                      int exit_status) {
 }
 

diff --git a/include/llbuild/BuildSystem/BuildExecutionQueue.h b/include/llbuild/BuildSystem/BuildExecutionQueue.h
@@ -26,6 +26,7 @@ namespace buildsystem {
 
 class BuildExecutionQueueDelegate;
 class Command;
+enum class CommandResult;
 
 /// Opaque type which allows the queue implementation to maintain additional
 /// state and associate subsequent requests (e.g., \see executeProcess()) with
@@ -221,12 +222,14 @@ class BuildExecutionQueueDelegate {
   /// \param handle - The handle used to identify the process. This handle will
   /// become invalid as soon as the client returns from this API call.
   ///
-  /// \param exitStatus - The exit status of the process, or -1 if an error was
+  /// \param result - Whether the process suceeded, failed or was cancelled.
+  /// \param exitStatus - The raw exit status of the process, or -1 if an error was
   /// encountered.
   //
   // FIXME: Need to include additional information on the status here, e.g., the
   // signal status, and the process output (if buffering).
   virtual void commandProcessFinished(Command*, ProcessHandle handle,
+                                      CommandResult result,
                                       int exitStatus) = 0;
 };
 

diff --git a/include/llbuild/BuildSystem/BuildSystemFrontend.h b/include/llbuild/BuildSystem/BuildSystemFrontend.h
@@ -40,6 +40,7 @@ namespace buildsystem {
 
 class BuildSystemFrontendDelegate;
 class BuildSystemInvocation;
+enum class CommandResult;
 
 /// This provides a standard "frontend" to the build system features, for use in
 /// building bespoke build systems that can still take advantage of desirable
@@ -239,11 +240,13 @@ class BuildSystemFrontendDelegate : public BuildSystemDelegate {
   /// \param handle - The handle used to identify the process. This handle will
   /// become invalid as soon as the client returns from this API call.
   ///
-  /// \param exitStatus - The exit status of the process.
+  /// \param result - Whether the process suceeded, failed or was cancelled.
+  /// \param exitStatus - The raw exit status of the process.
   //
   // FIXME: Need to include additional information on the status here, e.g., the
   // signal status, and the process output (if buffering).
   virtual void commandProcessFinished(Command*, ProcessHandle handle,
+                                      CommandResult result,
                                       int exitStatus);
 
   /// @}

diff --git a/include/llbuild/BuildSystem/CommandResult.h b/include/llbuild/BuildSystem/CommandResult.h
@@ -0,0 +1,34 @@
+//===- CommandResult.h -----------------------------------------------*- C++ -*-===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2015 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See http://swift.org/LICENSE.txt for license information
+// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+//
+// Compiler support and compatibility macros. Liberally taken from LLVM.
+//
+//===----------------------------------------------------------------------===//
+
+
+#ifndef LLBUILD_BUILDSYSTEM_COMMAND_RESULT_H
+#define LLBUILD_BUILDSYSTEM_COMMAND_RESULT_H
+
+namespace llbuild {
+namespace buildsystem {
+
+/// Result of a command execution
+enum class CommandResult {
+  Succeeded = 0,
+  Failed = 1,
+  Cancelled = 2,
+};
+
+}
+}
+
+#endif
diff --git a/lib/BuildSystem/BuildSystemFrontend.cpp b/lib/BuildSystem/BuildSystemFrontend.cpp
@@ -178,11 +178,12 @@ class BuildSystemFrontendExecutionQueueDelegate
   }
 
   virtual void commandProcessFinished(Command* command, ProcessHandle handle,
+                                      CommandResult result,
                                       int exitStatus) override {
     static_cast<BuildSystemFrontendDelegate*>(&getSystem().getDelegate())->
       commandProcessFinished(
           command, BuildSystemFrontendDelegate::ProcessHandle { handle.id },
-          exitStatus);
+          result, exitStatus);
   }
 };
 
@@ -423,6 +424,7 @@ commandProcessHadOutput(Command* command, ProcessHandle handle,
 
 void BuildSystemFrontendDelegate::
 commandProcessFinished(Command*, ProcessHandle handle,
+                       CommandResult result,
                        int exitStatus) {
 }
 

diff --git a/lib/BuildSystem/LaneBasedExecutionQueue.cpp b/lib/BuildSystem/LaneBasedExecutionQueue.cpp
@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llbuild/BuildSystem/BuildExecutionQueue.h"
+#include "llbuild/BuildSystem/CommandResult.h"
 
 #include "llbuild/Basic/LLVM.h"
 #include "llbuild/Basic/PlatformUtility.h"
@@ -284,7 +285,7 @@ class LaneBasedExecutionQueue : public BuildExecutionQueue {
             context.job.getForCommand(), handle,
             Twine("unable to open output pipe (") + strerror(errno) + ")");
         getDelegate().commandProcessFinished(context.job.getForCommand(),
-                                             handle, -1);
+                                             handle, CommandResult::Failed, -1);
         return false;
       }
 
@@ -357,7 +358,7 @@ class LaneBasedExecutionQueue : public BuildExecutionQueue {
             context.job.getForCommand(), handle,
             Twine("unable to spawn process (") + strerror(errno) + ")");
         getDelegate().commandProcessFinished(context.job.getForCommand(), handle,
-                                             -1);
+                                             CommandResult::Failed, -1);
         return false;
       }
 
@@ -410,7 +411,7 @@ class LaneBasedExecutionQueue : public BuildExecutionQueue {
           context.job.getForCommand(), handle,
           Twine("unable to wait for process (") + strerror(errno) + ")");
       getDelegate().commandProcessFinished(context.job.getForCommand(), handle,
-                                           -1);
+                                           CommandResult::Failed, -1);
       return false;
     }
 
@@ -421,11 +422,11 @@ class LaneBasedExecutionQueue : public BuildExecutionQueue {
     }
 
     // Notify of the process completion.
-    //
-    // FIXME: Need to communicate more information on the process exit status.
+    bool cancelled = WIFSIGNALED(status) && (WTERMSIG(status) == SIGINT || WTERMSIG(status) == SIGKILL);
+    CommandResult commandResult = cancelled ? CommandResult::Cancelled : (status == 0) ? CommandResult::Succeeded : CommandResult::Failed;
     getDelegate().commandProcessFinished(context.job.getForCommand(), handle,
-                                         status);
-    return (status == 0);
+                                         commandResult, status);
+    return (status == 0) || cancelled;
   }
 };
 

diff --git a/llbuild.xcodeproj/project.pbxproj b/llbuild.xcodeproj/project.pbxproj
@@ -696,6 +696,8 @@
 		54E187B81CD296EA00F7EC89 /* SwiftTools.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = SwiftTools.h; sourceTree = "<group>"; };
 		9D0A6D7F1E1FFEA800BE636F /* TempDir.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = TempDir.cpp; sourceTree = "<group>"; };
 		9D0A6D801E1FFEA800BE636F /* TempDir.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = TempDir.hpp; sourceTree = "<group>"; };
+		9D2589301E3820E3006C76F4 /* PlatformUtility.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PlatformUtility.h; sourceTree = "<group>"; };
+		9D2589311E38221D006C76F4 /* CommandResult.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CommandResult.h; sourceTree = "<group>"; };
 		9DADBBAC1E256C52005B4869 /* PlatformUtility.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PlatformUtility.cpp; sourceTree = "<group>"; };
 		9DB0478B1DF9D3E2006CDF52 /* LaneBasedExecutionQueueTest.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = LaneBasedExecutionQueueTest.cpp; sourceTree = "<group>"; };
 		9DB047A81DF9D43D006CDF52 /* BuildSystemTests */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.executable"; includeInIndex = 0; path = BuildSystemTests; sourceTree = BUILT_PRODUCTS_DIR; };
@@ -1382,6 +1384,7 @@
 				E138129C1C536CFC000092C0 /* FileSystem.h */,
 				E147DEFD1BA81D0E0032D08E /* Hashing.h */,
 				E1066C091BC5BCE700B892CE /* LLVM.h */,
+				9D2589301E3820E3006C76F4 /* PlatformUtility.h */,
 				E147DEFC1BA81D0E0032D08E /* SerialQueue.h */,
 				E17440C11CE192E30070A30C /* ShellUtility.h */,
 				E1A2245119F997D40059043E /* Version.h */,
@@ -1796,6 +1799,7 @@
 			isa = PBXGroup;
 			children = (
 				54E187B61CD296EA00F7EC89 /* BuildNode.h */,
+				9D2589311E38221D006C76F4 /* CommandResult.h */,
 				54E187B71CD296EA00F7EC89 /* ExternalCommand.h */,
 				54E187B81CD296EA00F7EC89 /* SwiftTools.h */,
 				E10FE0D51B6FF2000059D086 /* BuildExecutionQueue.h */,

diff --git a/products/libllbuild/BuildSystem-C-API.cpp b/products/libllbuild/BuildSystem-C-API.cpp
@@ -17,6 +17,7 @@
 #include "llbuild/BuildSystem/BuildFile.h"
 #include "llbuild/BuildSystem/BuildSystemCommandInterface.h"
 #include "llbuild/BuildSystem/BuildSystemFrontend.h"
+#include "llbuild/BuildSystem/CommandResult.h"
 #include "llbuild/BuildSystem/ExternalCommand.h"
 #include "llbuild/Core/BuildEngine.h"
 
@@ -250,12 +251,30 @@ class CAPIBuildSystemFrontendDelegate : public BuildSystemFrontendDelegate {
   }
 
   virtual void commandProcessFinished(Command* command, ProcessHandle handle,
+                                      CommandResult commandResult,
                                       int exitStatus) override {
     if (cAPIDelegate.command_process_finished) {
+      llb_buildsystem_command_result_t result = llb_buildsystem_command_result_failed;
+      switch (commandResult) {
+        case CommandResult::Succeeded:
+          result = llb_buildsystem_command_result_succeeded;
+          break;
+        case CommandResult::Cancelled:
+          result = llb_buildsystem_command_result_cancelled;
+          break;
+        case CommandResult::Failed:
+          result = llb_buildsystem_command_result_failed;
+          break;
+        default:
+          assert(0 && "unknown command result");
+          break;
+      }
+
       cAPIDelegate.command_process_finished(
           cAPIDelegate.context,
           (llb_buildsystem_command_t*) command,
           (llb_buildsystem_process_t*) handle.id,
+          result,
           exitStatus);
     }
   }

diff --git a/products/libllbuild/public-api/llbuild/buildsystem.h b/products/libllbuild/public-api/llbuild/buildsystem.h
@@ -87,6 +87,13 @@ typedef struct llb_buildsystem_tool_t_ llb_buildsystem_tool_t;
 /// Opaque handle to a build system command's launched process.
 typedef struct llb_buildsystem_process_t_ llb_buildsystem_process_t;
 
+/// Result of a command execution
+typedef enum {
+  llb_buildsystem_command_result_succeeded = 0,
+  llb_buildsystem_command_result_failed = 1,
+  llb_buildsystem_command_result_cancelled = 2,
+} llb_buildsystem_command_result_t;
+
 /// Status change event kinds.
 typedef enum {
   /// Indicates the command is being scanned to determine if it needs to run.
@@ -288,10 +295,12 @@ typedef struct llb_buildsystem_delegate_t_ {
   /// Xparam process The handle used to identify the process. This handle
   /// will become invalid as soon as the client returns from this API call.
   ///
-  /// Xparam exitStatus The exit status of the process.
+  /// Xparam result Whether the process suceeded, failed or was cancelled.
+  /// Xparam exitStatus The raw exit status of the process.
   void (*command_process_finished)(void* context,
                                    llb_buildsystem_command_t* command,
                                    llb_buildsystem_process_t* process,
+                                   llb_buildsystem_command_result_t result,
                                    int exit_status);
 
   /// @}

diff --git a/unittests/BuildSystem/BuildSystemFrontendTest.cpp b/unittests/BuildSystem/BuildSystemFrontendTest.cpp
@@ -177,13 +177,13 @@ class TestBuildSystemFrontendDelegate : public BuildSystemFrontendDelegate {
     super::commandProcessHadError(command, handle, message);
   }
 
-  virtual void commandProcessFinished(Command* command, ProcessHandle handle,
+  virtual void commandProcessFinished(Command* command, ProcessHandle handle, CommandResult result,
                                       int exitStatus) override {
     {
         std::lock_guard<std::mutex> lock(traceMutex);
         traceStream << __FUNCTION__ << ": " << command->getName() << ": " << exitStatus << "\n";
     }
-    super::commandProcessFinished(command, handle, exitStatus);
+    super::commandProcessFinished(command, handle, result, exitStatus);
   }
 
   virtual FileSystem& getFileSystem() override {

diff --git a/unittests/BuildSystem/LaneBasedExecutionQueueTest.cpp b/unittests/BuildSystem/LaneBasedExecutionQueueTest.cpp
@@ -37,7 +37,7 @@ namespace {
     virtual void commandProcessStarted(Command* command, ProcessHandle handle) override {}
     virtual void commandProcessHadError(Command* command, ProcessHandle handle, const Twine& message) override {}
     virtual void commandProcessHadOutput(Command* command, ProcessHandle handle, StringRef data) override {}
-    virtual void commandProcessFinished(Command* command, ProcessHandle handle, int exitStatus) override {}
+    virtual void commandProcessFinished(Command* command, ProcessHandle handle, CommandResult result, int exitStatus) override {}
   };
 
   TEST(LaneBasedExecutionQueueTest, basic) {