cmExecuteProcessCommand: Replace cmsysProcess with cmUVProcessChain

Source/cmExecuteProcessCommand.cxx

@@ -2,8 +2,8 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmExecuteProcessCommand.h"
 
-#include <algorithm>
 #include <cctype> /* isspace */
+#include <cstdint>
 #include <cstdio>
 #include <iostream>
 #include <map>
@@ -16,7 +16,9 @@
 #include <cmext/algorithm>
 #include <cmext/string_view>
 
-#include "cmsys/Process.h"
+#include <cm3p/uv.h>
+
+#include "cm_fileno.hxx"
 
 #include "cmArgumentParser.h"
 #include "cmExecutionStatus.h"
@@ -26,6 +28,9 @@
 #include "cmProcessOutput.h"
 #include "cmStringAlgorithms.h"
 #include "cmSystemTools.h"
+#include "cmUVHandlePtr.h"
+#include "cmUVProcessChain.h"
+#include "cmUVStream.h"
 
 namespace {
 bool cmExecuteProcessCommandIsWhitespace(char c)
@@ -36,7 +41,7 @@ bool cmExecuteProcessCommandIsWhitespace(char c)
 void cmExecuteProcessCommandFixText(std::vector<char>& output,
                                     bool strip_trailing_whitespace);
 void cmExecuteProcessCommandAppend(std::vector<char>& output, const char* data,
-                                   int length);
+                                   std::size_t length);
 }
 
 // cmExecuteProcessCommand
@@ -143,57 +148,61 @@ bool cmExecuteProcessCommand(std::vector<std::string> const& args,
     }
   }
   // Create a process instance.
-  std::unique_ptr<cmsysProcess, void (*)(cmsysProcess*)> cp_ptr(
-    cmsysProcess_New(), cmsysProcess_Delete);
-  cmsysProcess* cp = cp_ptr.get();
+  cmUVProcessChainBuilder builder;
 
   // Set the command sequence.
   for (std::vector<std::string> const& cmd : arguments.Commands) {
-    std::vector<const char*> argv(cmd.size() + 1);
-    std::transform(cmd.begin(), cmd.end(), argv.begin(),
-                   [](std::string const& s) { return s.c_str(); });
-    argv.back() = nullptr;
-    cmsysProcess_AddCommand(cp, argv.data());
+    builder.AddCommand(cmd);
   }
 
   // Set the process working directory.
   if (!arguments.WorkingDirectory.empty()) {
-    cmsysProcess_SetWorkingDirectory(cp, arguments.WorkingDirectory.c_str());
+    builder.SetWorkingDirectory(arguments.WorkingDirectory);
   }
 
-  // Always hide the process window.
-  cmsysProcess_SetOption(cp, cmsysProcess_Option_HideWindow, 1);
-
   // Check the output variables.
-  bool merge_output = false;
+  std::unique_ptr<FILE, int (*)(FILE*)> inputFile(nullptr, fclose);
   if (!arguments.InputFile.empty()) {
-    cmsysProcess_SetPipeFile(cp, cmsysProcess_Pipe_STDIN,
-                             arguments.InputFile.c_str());
+    inputFile.reset(cmsys::SystemTools::Fopen(arguments.InputFile, "rb"));
+    builder.SetExternalStream(cmUVProcessChainBuilder::Stream_INPUT,
+                              cm_fileno(inputFile.get()));
+  } else {
+    builder.SetExternalStream(cmUVProcessChainBuilder::Stream_INPUT,
+                              cm_fileno(stdin));
   }
+
+  std::unique_ptr<FILE, int (*)(FILE*)> outputFile(nullptr, fclose);
   if (!arguments.OutputFile.empty()) {
-    cmsysProcess_SetPipeFile(cp, cmsysProcess_Pipe_STDOUT,
-                             arguments.OutputFile.c_str());
+    outputFile.reset(cmsys::SystemTools::Fopen(arguments.OutputFile, "wb"));
+    builder.SetExternalStream(cmUVProcessChainBuilder::Stream_OUTPUT,
+                              cm_fileno(outputFile.get()));
+  } else {
+    if (arguments.OutputVariable == arguments.ErrorVariable &&
+        !arguments.ErrorVariable.empty()) {
+      builder.SetMergedBuiltinStreams();
+    } else {
+      builder.SetBuiltinStream(cmUVProcessChainBuilder::Stream_OUTPUT);
+    }
   }
+
+  std::unique_ptr<FILE, int (*)(FILE*)> errorFile(nullptr, fclose);
   if (!arguments.ErrorFile.empty()) {
     if (arguments.ErrorFile == arguments.OutputFile) {
-      merge_output = true;
+      builder.SetExternalStream(cmUVProcessChainBuilder::Stream_ERROR,
+                                cm_fileno(outputFile.get()));
     } else {
-      cmsysProcess_SetPipeFile(cp, cmsysProcess_Pipe_STDERR,
-                               arguments.ErrorFile.c_str());
+      errorFile.reset(cmsys::SystemTools::Fopen(arguments.ErrorFile, "wb"));
+      builder.SetExternalStream(cmUVProcessChainBuilder::Stream_ERROR,
+                                cm_fileno(errorFile.get()));
     }
-  }
-  if (!arguments.OutputVariable.empty() &&
-      arguments.OutputVariable == arguments.ErrorVariable) {
-    merge_output = true;
-  }
-  if (merge_output) {
-    cmsysProcess_SetOption(cp, cmsysProcess_Option_MergeOutput, 1);
+  } else if (arguments.ErrorVariable.empty() ||
+             (!arguments.ErrorVariable.empty() &&
+              arguments.OutputVariable != arguments.ErrorVariable)) {
+    builder.SetBuiltinStream(cmUVProcessChainBuilder::Stream_ERROR);
   }
 
   // Set the timeout if any.
-  if (timeout >= 0) {
-    cmsysProcess_SetTimeout(cp, timeout);
-  }
+  int64_t timeoutMillis = static_cast<int64_t>(timeout * 1000.0);
 
   bool echo_stdout = false;
   bool echo_stderr = false;
@@ -241,36 +250,86 @@ bool cmExecuteProcessCommand(std::vector<std::string> const& args,
     }
   }
   // Start the process.
-  cmsysProcess_Execute(cp);
+  auto chain = builder.Start();
+
+  bool timedOut = false;
+  cm::uv_timer_ptr timer;
+
+  if (timeoutMillis >= 0) {
+    timer.init(chain.GetLoop(), &timedOut);
+    timer.start(
+      [](uv_timer_t* handle) {
+        auto* timeoutPtr = static_cast<bool*>(handle->data);
+        *timeoutPtr = true;
+      },
+      timeoutMillis, 0);
+  }
 
   // Read the process output.
-  std::vector<char> tempOutput;
-  std::vector<char> tempError;
-  int length;
-  char* data;
-  int p;
+  struct ReadData
+  {
+    bool Finished = false;
+    std::vector<char> Output;
+    cm::uv_pipe_ptr Stream;
+  };
+  ReadData outputData;
+  ReadData errorData;
   cmProcessOutput processOutput(
     cmProcessOutput::FindEncoding(arguments.Encoding));
   std::string strdata;
-  while ((p = cmsysProcess_WaitForData(cp, &data, &length, nullptr))) {
-    // Put the output in the right place.
-    if (p == cmsysProcess_Pipe_STDOUT && !arguments.OutputQuiet) {
-      if (arguments.OutputVariable.empty() || arguments.EchoOutputVariable) {
-        processOutput.DecodeText(data, length, strdata, 1);
-        cmSystemTools::Stdout(strdata);
-      }
-      if (!arguments.OutputVariable.empty()) {
-        cmExecuteProcessCommandAppend(tempOutput, data, length);
-      }
-    } else if (p == cmsysProcess_Pipe_STDERR && !arguments.ErrorQuiet) {
-      if (arguments.ErrorVariable.empty() || arguments.EchoErrorVariable) {
-        processOutput.DecodeText(data, length, strdata, 2);
-        cmSystemTools::Stderr(strdata);
-      }
-      if (!arguments.ErrorVariable.empty()) {
-        cmExecuteProcessCommandAppend(tempError, data, length);
-      }
-    }
+
+  std::unique_ptr<cmUVStreamReadHandle> outputHandle;
+  if (chain.OutputStream() >= 0) {
+    outputData.Stream.init(chain.GetLoop(), 0);
+    uv_pipe_open(outputData.Stream, chain.OutputStream());
+    outputHandle = cmUVStreamRead(
+      outputData.Stream,
+      [&arguments, &processOutput, &outputData,
+       &strdata](std::vector<char> data) {
+        if (!arguments.OutputQuiet) {
+          if (arguments.OutputVariable.empty() ||
+              arguments.EchoOutputVariable) {
+            processOutput.DecodeText(data.data(), data.size(), strdata, 1);
+            cmSystemTools::Stdout(strdata);
+          }
+          if (!arguments.OutputVariable.empty()) {
+            cmExecuteProcessCommandAppend(outputData.Output, data.data(),
+                                          data.size());
+          }
+        }
+      },
+      [&outputData]() { outputData.Finished = true; });
+  } else {
+    outputData.Finished = true;
+  }
+  std::unique_ptr<cmUVStreamReadHandle> errorHandle;
+  if (chain.ErrorStream() >= 0 &&
+      chain.ErrorStream() != chain.OutputStream()) {
+    errorData.Stream.init(chain.GetLoop(), 0);
+    uv_pipe_open(errorData.Stream, chain.ErrorStream());
+    errorHandle = cmUVStreamRead(
+      errorData.Stream,
+      [&arguments, &processOutput, &errorData,
+       &strdata](std::vector<char> data) {
+        if (!arguments.ErrorQuiet) {
+          if (arguments.ErrorVariable.empty() || arguments.EchoErrorVariable) {
+            processOutput.DecodeText(data.data(), data.size(), strdata, 2);
+            cmSystemTools::Stderr(strdata);
+          }
+          if (!arguments.ErrorVariable.empty()) {
+            cmExecuteProcessCommandAppend(errorData.Output, data.data(),
+                                          data.size());
+          }
+        }
+      },
+      [&errorData]() { errorData.Finished = true; });
+  } else {
+    errorData.Finished = true;
+  }
+
+  while (chain.Valid() && !timedOut &&
+         !(chain.Finished() && outputData.Finished && errorData.Finished)) {
+    uv_run(&chain.GetLoop(), UV_RUN_ONCE);
   }
   if (!arguments.OutputQuiet &&
       (arguments.OutputVariable.empty() || arguments.EchoOutputVariable)) {
@@ -287,151 +346,102 @@ bool cmExecuteProcessCommand(std::vector<std::string> const& args,
     }
   }
 
-  // All output has been read.  Wait for the process to exit.
-  cmsysProcess_WaitForExit(cp, nullptr);
-  processOutput.DecodeText(tempOutput, tempOutput);
-  processOutput.DecodeText(tempError, tempError);
+  // All output has been read.
+  processOutput.DecodeText(outputData.Output, outputData.Output);
+  processOutput.DecodeText(errorData.Output, errorData.Output);
 
   // Fix the text in the output strings.
-  cmExecuteProcessCommandFixText(tempOutput,
+  cmExecuteProcessCommandFixText(outputData.Output,
                                  arguments.OutputStripTrailingWhitespace);
-  cmExecuteProcessCommandFixText(tempError,
+  cmExecuteProcessCommandFixText(errorData.Output,
                                  arguments.ErrorStripTrailingWhitespace);
 
   // Store the output obtained.
-  if (!arguments.OutputVariable.empty() && !tempOutput.empty()) {
+  if (!arguments.OutputVariable.empty() && !outputData.Output.empty()) {
     status.GetMakefile().AddDefinition(arguments.OutputVariable,
-                                       tempOutput.data());
+                                       outputData.Output.data());
   }
-  if (!merge_output && !arguments.ErrorVariable.empty() &&
-      !tempError.empty()) {
+  if (arguments.ErrorVariable != arguments.OutputVariable &&
+      !arguments.ErrorVariable.empty() && !errorData.Output.empty()) {
     status.GetMakefile().AddDefinition(arguments.ErrorVariable,
-                                       tempError.data());
+                                       errorData.Output.data());
   }
 
   // Store the result of running the process.
   if (!arguments.ResultVariable.empty()) {
-    switch (cmsysProcess_GetState(cp)) {
-      case cmsysProcess_State_Exited: {
-        int v = cmsysProcess_GetExitValue(cp);
-        char buf[16];
-        snprintf(buf, sizeof(buf), "%d", v);
-        status.GetMakefile().AddDefinition(arguments.ResultVariable, buf);
-      } break;
-      case cmsysProcess_State_Exception:
+    if (timedOut) {
+      status.GetMakefile().AddDefinition(arguments.ResultVariable,
+                                         "Process terminated due to timeout");
+    } else {
+      auto const* lastStatus = chain.GetStatus().back();
+      auto exception = lastStatus->GetException();
+      if (exception.first == cmUVProcessChain::ExceptionCode::None) {
         status.GetMakefile().AddDefinition(
-          arguments.ResultVariable, cmsysProcess_GetExceptionString(cp));
-        break;
-      case cmsysProcess_State_Error:
+          arguments.ResultVariable,
+          std::to_string(static_cast<int>(lastStatus->ExitStatus)));
+      } else {
         status.GetMakefile().AddDefinition(arguments.ResultVariable,
-                                           cmsysProcess_GetErrorString(cp));
-        break;
-      case cmsysProcess_State_Expired:
-        status.GetMakefile().AddDefinition(
-          arguments.ResultVariable, "Process terminated due to timeout");
-        break;
+                                           exception.second);
+      }
     }
   }
   // Store the result of running the processes.
   if (!arguments.ResultsVariable.empty()) {
-    switch (cmsysProcess_GetState(cp)) {
-      case cmsysProcess_State_Exited: {
-        std::vector<std::string> res;
-        for (size_t i = 0; i < arguments.Commands.size(); ++i) {
-          switch (cmsysProcess_GetStateByIndex(cp, static_cast<int>(i))) {
-            case kwsysProcess_StateByIndex_Exited: {
-              int exitCode =
-                cmsysProcess_GetExitValueByIndex(cp, static_cast<int>(i));
-              char buf[16];
-              snprintf(buf, sizeof(buf), "%d", exitCode);
-              res.emplace_back(buf);
-            } break;
-            case kwsysProcess_StateByIndex_Exception:
-              res.emplace_back(cmsysProcess_GetExceptionStringByIndex(
-                cp, static_cast<int>(i)));
-              break;
-            case kwsysProcess_StateByIndex_Error:
-            default:
-              res.emplace_back("Error getting the child return code");
-              break;
-          }
+    if (timedOut) {
+      status.GetMakefile().AddDefinition(arguments.ResultsVariable,
+                                         "Process terminated due to timeout");
+    } else {
+      std::vector<std::string> res;
+      for (auto const* processStatus : chain.GetStatus()) {
+        auto exception = processStatus->GetException();
+        if (exception.first == cmUVProcessChain::ExceptionCode::None) {
+          res.emplace_back(
+            std::to_string(static_cast<int>(processStatus->ExitStatus)));
+        } else {
+          res.emplace_back(exception.second);
         }
-        status.GetMakefile().AddDefinition(arguments.ResultsVariable,
-                                           cmList::to_string(res));
-      } break;
-      case cmsysProcess_State_Exception:
-        status.GetMakefile().AddDefinition(
-          arguments.ResultsVariable, cmsysProcess_GetExceptionString(cp));
-        break;
-      case cmsysProcess_State_Error:
-        status.GetMakefile().AddDefinition(arguments.ResultsVariable,
-                                           cmsysProcess_GetErrorString(cp));
-        break;
-      case cmsysProcess_State_Expired:
-        status.GetMakefile().AddDefinition(
-          arguments.ResultsVariable, "Process terminated due to timeout");
-        break;
+      }
+      status.GetMakefile().AddDefinition(arguments.ResultsVariable,
+                                         cmList::to_string(res));
     }
   }
 
-  auto queryProcessStatusByIndex = [&cp](int index) -> std::string {
-    std::string processStatus;
-    switch (cmsysProcess_GetStateByIndex(cp, static_cast<int>(index))) {
-      case kwsysProcess_StateByIndex_Exited: {
-        int exitCode = cmsysProcess_GetExitValueByIndex(cp, index);
-        if (exitCode) {
-          processStatus = "Child return code: " + std::to_string(exitCode);
-        }
-      } break;
-      case kwsysProcess_StateByIndex_Exception: {
-        processStatus = cmStrCat(
-          "Abnormal exit with child return code: ",
-          cmsysProcess_GetExceptionStringByIndex(cp, static_cast<int>(index)));
-        break;
+  auto queryProcessStatusByIndex = [&chain](std::size_t index) -> std::string {
+    auto const& processStatus = chain.GetStatus(index);
+    auto exception = processStatus.GetException();
+    if (exception.first == cmUVProcessChain::ExceptionCode::None) {
+      if (processStatus.ExitStatus) {
+        return cmStrCat("Child return code: ", processStatus.ExitStatus);
       }
-      case kwsysProcess_StateByIndex_Error:
-      default:
-        processStatus = "Error getting the child return code";
-        break;
+      return "";
     }
-    return processStatus;
+    return cmStrCat("Abnormal exit with child return code: ",
+                    exception.second);
   };
 
   if (arguments.CommandErrorIsFatal == "ANY"_s) {
     bool ret = true;
-    switch (cmsysProcess_GetState(cp)) {
-      case cmsysProcess_State_Exited: {
-        std::map<int, std::string> failureIndices;
-        for (int i = 0; i < static_cast<int>(arguments.Commands.size()); ++i) {
-          std::string processStatus = queryProcessStatusByIndex(i);
-          if (!processStatus.empty()) {
-            failureIndices[i] = processStatus;
-          }
-          if (!failureIndices.empty()) {
-            std::ostringstream oss;
-            oss << "failed command indexes:\n";
-            for (auto const& e : failureIndices) {
-              oss << "  " << e.first + 1 << ": \"" << e.second << "\"\n";
-            }
-            status.SetError(oss.str());
-            ret = false;
-          }
+    if (timedOut) {
+      status.SetError("Process terminated due to timeout");
+      ret = false;
+    } else {
+      std::map<std::size_t, std::string> failureIndices;
+      auto statuses = chain.GetStatus();
+      for (std::size_t i = 0; i < statuses.size(); ++i) {
+        std::string processStatus = queryProcessStatusByIndex(i);
+        if (!processStatus.empty()) {
+          failureIndices[i] = processStatus;
         }
-      } break;
-      case cmsysProcess_State_Exception:
-        status.SetError(
-          cmStrCat("abnormal exit: ", cmsysProcess_GetExceptionString(cp)));
-        ret = false;
-        break;
-      case cmsysProcess_State_Error:
-        status.SetError(cmStrCat("error getting child return code: ",
-                                 cmsysProcess_GetErrorString(cp)));
-        ret = false;
-        break;
-      case cmsysProcess_State_Expired:
-        status.SetError("Process terminated due to timeout");
+      }
+      if (!failureIndices.empty()) {
+        std::ostringstream oss;
+        oss << "failed command indexes:\n";
+        for (auto const& e : failureIndices) {
+          oss << "  " << e.first + 1 << ": \"" << e.second << "\"\n";
+        }
+        status.SetError(oss.str());
         ret = false;
-        break;
+      }
     }
 
     if (!ret) {
@@ -442,29 +452,23 @@ bool cmExecuteProcessCommand(std::vector<std::string> const& args,
 
   if (arguments.CommandErrorIsFatal == "LAST"_s) {
     bool ret = true;
-    switch (cmsysProcess_GetState(cp)) {
-      case cmsysProcess_State_Exited: {
+    if (timedOut) {
+      status.SetError("Process terminated due to timeout");
+      ret = false;
+    } else {
+      auto const& lastStatus = chain.GetStatus(arguments.Commands.size() - 1);
+      auto exception = lastStatus.GetException();
+      if (exception.first != cmUVProcessChain::ExceptionCode::None) {
+        status.SetError(cmStrCat("Abnormal exit: ", exception.second));
+        ret = false;
+      } else {
         int lastIndex = static_cast<int>(arguments.Commands.size() - 1);
         const std::string processStatus = queryProcessStatusByIndex(lastIndex);
         if (!processStatus.empty()) {
           status.SetError("last command failed");
           ret = false;
         }
-      } break;
-      case cmsysProcess_State_Exception:
-        status.SetError(
-          cmStrCat("Abnormal exit: ", cmsysProcess_GetExceptionString(cp)));
-        ret = false;
-        break;
-      case cmsysProcess_State_Error:
-        status.SetError(cmStrCat("Error getting child return code: ",
-                                 cmsysProcess_GetErrorString(cp)));
-        ret = false;
-        break;
-      case cmsysProcess_State_Expired:
-        status.SetError("Process terminated due to timeout");
-        ret = false;
-        break;
+      }
     }
     if (!ret) {
       cmSystemTools::SetFatalErrorOccurred();
@@ -507,7 +511,7 @@ void cmExecuteProcessCommandFixText(std::vector<char>& output,
 }
 
 void cmExecuteProcessCommandAppend(std::vector<char>& output, const char* data,
-                                   int length)
+                                   std::size_t length)
 {
 #if defined(__APPLE__)
   // HACK on Apple to work around bug with inserting at the

Tests/RunCMake/CMakeLists.txt

@@ -466,6 +466,7 @@ add_RunCMake_test(build_command)
 add_executable(exit_code exit_code.c)
 set(execute_process_ARGS
   -DEXIT_CODE_EXE=$<TARGET_FILE:exit_code>
+  -DPRINT_STDIN_EXE=$<TARGET_FILE:print_stdin>
   -DPython_EXECUTABLE=${Python_EXECUTABLE}
   )
 if(NOT CMake_TEST_EXTERNAL_CMAKE)

Tests/RunCMake/execute_process/RunCMakeTest.cmake

@@ -34,6 +34,7 @@ run_cmake_command(AnyCommandGood ${CMAKE_COMMAND} -P ${RunCMake_SOURCE_DIR}/AnyC
 run_cmake_command(LastCommandError ${CMAKE_COMMAND} -P ${RunCMake_SOURCE_DIR}/LastCommandError.cmake)
 run_cmake_command(LastCommandTimeout ${CMAKE_COMMAND} -P ${RunCMake_SOURCE_DIR}/LastCommandTimeout.cmake)
 run_cmake_command(LastCommandGood ${CMAKE_COMMAND} -P ${RunCMake_SOURCE_DIR}/LastCommandGood.cmake)
+run_cmake_command(Stdin ${CMAKE_COMMAND} -DPRINT_STDIN_EXE=${PRINT_STDIN_EXE} -P ${RunCMake_SOURCE_DIR}/Stdin.cmake)
 
 if(UNIX AND Python_EXECUTABLE)
   run_cmake_command(AnyCommandAbnormalExit ${CMAKE_COMMAND} -DPython_EXECUTABLE=${Python_EXECUTABLE} -P ${RunCMake_SOURCE_DIR}/AnyCommandAbnormalExit.cmake)

Tests/RunCMake/execute_process/Stdin-stdin.txt

@@ -0,0 +1 @@
+Hello world!

Tests/RunCMake/execute_process/Stdin-stdout.txt

@@ -0,0 +1 @@
+^Hello world!$

Tests/RunCMake/execute_process/Stdin.cmake

@@ -0,0 +1 @@
+execute_process(COMMAND ${PRINT_STDIN_EXE})

bootstrap

@@ -508,6 +508,7 @@ CMAKE_CXX_SOURCES="\
   cmake  \
   cmakemain \
   cmcmd  \
+  cm_fileno \
 "
 
 if ${cmake_system_mingw}; then