Install: Exit non-zero when an install script fails

`cmake --install` did not propagate per-script failures into its
process exit code.

In parallel mode, cmInstallScriptHandler::Install spawned each install
script as a child process but never inspected the child exit status or
termination signal, and always returned 0.  It now reads each child's
status after the event loop, prints the failing script's exit code or
signal, and returns non-zero if any script failed.  A failed parallel
install also no longer writes the combined install_manifest.txt, so a
partial manifest is not mistaken for a complete install.

In serial mode, GetScripts() returns the top-level cmake_install.cmake
once per component and per configuration, so multiple scripts run only
when installing several components or configurations at once.  The loop
overwrote its result on every iteration, so an earlier script that
failed via cmake_language(EXIT) was masked by a later one that
succeeded.  The loop now stops at the first failure.  This is a no-op
for a single component and configuration; it changes only installs of
multiple components or configurations, which now stop at the first
failed script and report it instead of attempting the rest.

Fixes: #27906

Help/release/dev/install-exit-code-fidelity.rst

@@ -0,0 +1,9 @@
+install-exit-code-fidelity
+---------------------------
+
+* The :option:`cmake --install` command now reports a non-zero exit code
+  when an install script fails.  Previously, a parallel install (enabled by
+  the :prop_gbl:`INSTALL_PARALLEL` global property) always reported success,
+  and a serial install could lose the failure of an earlier component that
+  exited via :command:`cmake_language(EXIT) <cmake_language>` when a later
+  component succeeded.

Source/cmInstallScriptHandler.cxx

@@ -155,49 +155,67 @@ int cmInstallScriptHandler::Install(unsigned int j,
     for (auto queue = std::min(j - working, runners.size() - i); queue > 0;
          --queue) {
       ++working;
-      runners[i].start(loop,
-                       [&runners, &working, &installed, i, &queueScripts]() {
-                         runners[i].printResult(++installed, runners.size());
-                         --working;
-                         queueScripts();
-                       });
+      bool started = runners[i].start(
+        loop, [&runners, &working, &installed, i, &queueScripts]() {
+          runners[i].printResult(++installed, runners.size());
+          --working;
+          queueScripts();
+        });
+      if (!started) {
+        // The child could not be spawned.  Release its scheduling slot so the
+        // remaining scripts still run; it is counted as a failure after the
+        // event loop completes.
+        --working;
+      }
       ++i;
     }
   };
   queueScripts();
   uv_run(loop, UV_RUN_DEFAULT);
 
-  // Write install manifest
-  std::string installManifest;
-  for (auto const& component : this->Components) {
-    if (component.empty()) {
-      installManifest = "install_manifest.txt";
-    } else {
-      cmsys::RegularExpression regEntry;
-      if (regEntry.compile("^[a-zA-Z0-9_.+-]+$") && regEntry.find(component)) {
-        installManifest = cmStrCat("install_manifest_", component, ".txt");
-      } else {
-        cmCryptoHash md5(cmCryptoHash::AlgoMD5);
-        md5.Initialize();
-        installManifest =
-          cmStrCat("install_manifest_", md5.HashString(component), ".txt");
-      }
+  int result = 0;
+  for (auto& runner : runners) {
+    if (runner.Failed()) {
+      runner.printFailure();
+      result = 1;
     }
-    cmGeneratedFileStream fout(
-      cmStrCat(this->BinaryDir, '/', installManifest));
-    fout.SetCopyIfDifferent(true);
-    for (auto const& dir : this->Directories) {
-      auto localManifest = cmStrCat(dir, "/install_local_manifest.txt");
-      if (cmSystemTools::FileExists(localManifest)) {
-        cmsys::ifstream fin(localManifest.c_str());
-        std::string line;
-        while (std::getline(fin, line)) {
-          fout << line << "\n";
+  }
+
+  // Write the install manifest only when every script succeeded.  A failed
+  // install must not leave behind a manifest that looks complete.
+  if (result == 0) {
+    std::string installManifest;
+    for (auto const& component : this->Components) {
+      if (component.empty()) {
+        installManifest = "install_manifest.txt";
+      } else {
+        cmsys::RegularExpression regEntry;
+        if (regEntry.compile("^[a-zA-Z0-9_.+-]+$") &&
+            regEntry.find(component)) {
+          installManifest = cmStrCat("install_manifest_", component, ".txt");
+        } else {
+          cmCryptoHash md5(cmCryptoHash::AlgoMD5);
+          md5.Initialize();
+          installManifest =
+            cmStrCat("install_manifest_", md5.HashString(component), ".txt");
+        }
+      }
+      cmGeneratedFileStream fout(
+        cmStrCat(this->BinaryDir, '/', installManifest));
+      fout.SetCopyIfDifferent(true);
+      for (auto const& dir : this->Directories) {
+        auto localManifest = cmStrCat(dir, "/install_local_manifest.txt");
+        if (cmSystemTools::FileExists(localManifest)) {
+          cmsys::ifstream fin(localManifest.c_str());
+          std::string line;
+          while (std::getline(fin, line)) {
+            fout << line << "\n";
+          }
         }
       }
     }
   }
-  return 0;
+  return result;
 }
 
 InstallScriptRunner::InstallScriptRunner(InstallScript const& script)
@@ -207,7 +225,7 @@ InstallScriptRunner::InstallScriptRunner(InstallScript const& script)
   this->Command = script.command;
 }
 
-void InstallScriptRunner::start(cm::uv_loop_ptr& loop,
+bool InstallScriptRunner::start(cm::uv_loop_ptr& loop,
                                 std::function<void()> callback)
 {
   cmUVProcessChainBuilder builder;
@@ -215,6 +233,9 @@ void InstallScriptRunner::start(cm::uv_loop_ptr& loop,
     .SetExternalLoop(*loop)
     .SetMergedBuiltinStreams();
   this->Chain = cm::make_unique<cmUVProcessChain>(builder.Start());
+  if (!this->Chain->Valid()) {
+    return false;
+  }
   this->StreamHandler = cmUVStreamRead(
     this->Chain->OutputStream(),
     [this](std::vector<char> data) {
@@ -224,6 +245,7 @@ void InstallScriptRunner::start(cm::uv_loop_ptr& loop,
       this->Output.push_back(strdata);
     },
     std::move(callback));
+  return true;
 }
 
 void InstallScriptRunner::printResult(std::size_t n, std::size_t total)
@@ -233,3 +255,39 @@ void InstallScriptRunner::printResult(std::size_t n, std::size_t total)
     cmSystemTools::Stdout(line);
   }
 }
+
+bool InstallScriptRunner::Failed() const
+{
+  if (!this->Chain || !this->Chain->Valid()) {
+    return true;
+  }
+  auto const& status = this->Chain->GetStatus(0);
+  return status.SpawnResult != 0 || status.TermSignal != 0 ||
+    status.ExitStatus != 0;
+}
+
+void InstallScriptRunner::printFailure()
+{
+  std::string detail;
+  if (!this->Chain || !this->Chain->Valid()) {
+    // The chain never spawned a process (e.g. pipe/loop setup failed).
+    detail = "failed to start";
+  } else {
+    auto const& status = this->Chain->GetStatus(0);
+    auto exception = status.GetException();
+    switch (exception.first) {
+      case cmUVProcessChain::ExceptionCode::None:
+        detail = cmStrCat("exited with code ", status.ExitStatus);
+        break;
+      case cmUVProcessChain::ExceptionCode::Spawn:
+        // Prepared, but the process could not be executed.
+        detail = cmStrCat("failed to start: ", exception.second);
+        break;
+      default:
+        detail = exception.second;
+        break;
+    }
+  }
+  cmSystemTools::Stderr(
+    cmStrCat("CMake Error: install script '", this->Name, "' ", detail, '\n'));
+}

Source/cmInstallScriptHandler.h

@@ -36,8 +36,10 @@ public:
   {
   public:
     InstallScriptRunner(InstallScript const&);
-    void start(cm::uv_loop_ptr&, std::function<void()>);
+    bool start(cm::uv_loop_ptr&, std::function<void()>);
     void printResult(std::size_t n, std::size_t total);
+    bool Failed() const;
+    void printFailure();
 
   private:
     std::vector<std::string> Command;

Source/cmakemain.cxx

@@ -969,6 +969,11 @@ int do_install(int ac, char const* const* av)
         });
         cm.SetDebugOutputOn(verbose);
         ret_ = int(bool(cm.Run(cmd)));
+        if (ret_ != 0) {
+          // Serial install is fail-fast: stop at the first failed script
+          // instead of attempting the remaining components or configs.
+          break;
+        }
       }
     }
     return int(ret_ > 0);

Tests/RunCMake/InstallParallel/RunCMakeTest.cmake

@@ -58,3 +58,45 @@ if(RunCMake_GENERATOR MATCHES "Ninja")
   install_test(ninja-parallel ARGS "-t install/parallel" NINJA PARALLEL)
   install_test(ninja-no-parallel ARGS "-t install" NINJA)
 endif()
+
+# Exit-code fidelity: a failing install must report a non-zero exit code.
+function(install_fail_test test fixture)
+  cmake_parse_arguments(ARG "PARALLEL" "FAIL_MODE" "INSTALL_ARGS" ${ARGN})
+  set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${test}-build)
+  set(RunCMake_TEST_OPTIONS
+    -DINSTALL_PARALLEL=${ARG_PARALLEL}
+    -DCMAKE_INSTALL_PREFIX=install)
+  if (ARG_FAIL_MODE)
+    list(APPEND RunCMake_TEST_OPTIONS -DFAIL_MODE=${ARG_FAIL_MODE})
+  endif()
+  set(RunCMake_TEST_OUTPUT_MERGE 1)
+  if (NOT RunCMake_GENERATOR_IS_MULTI_CONFIG)
+    list(APPEND RunCMake_TEST_OPTIONS -DCMAKE_BUILD_TYPE=Debug)
+  endif()
+  run_cmake(${fixture})
+  set(RunCMake_TEST_NO_CLEAN 1)
+  if (ARG_PARALLEL)
+    # The install runs in parallel only when CMakeFiles/InstallScripts.json is
+    # at least as new as CMakeFiles/cmake.check_cache; otherwise the handler
+    # falls back to running the top-level cmake_install.cmake serially.  Both
+    # files are written during the same configuration, and their relative
+    # modification times are not reliable on every filesystem, so make the JSON
+    # newest explicitly to keep this test from intermittently exercising the
+    # serial fallback (which omits the per-script parallel diagnostics).
+    file(TOUCH_NOCREATE
+      ${RunCMake_TEST_BINARY_DIR}/CMakeFiles/InstallScripts.json)
+  endif()
+  run_cmake_command(${test}
+    ${CMAKE_COMMAND} -E env --unset=NINJA_STATUS
+    ${CMAKE_COMMAND} --install . ${ARG_INSTALL_ARGS})
+endfunction()
+
+# Parallel: any failing child must make the install exit non-zero.
+install_fail_test(parallel-fatal install-fail PARALLEL FAIL_MODE fatal INSTALL_ARGS -j 4)
+install_fail_test(parallel-exit install-fail PARALLEL FAIL_MODE exit INSTALL_ARGS -j 4)
+# Serial: fail-fast is preserved on a fatal error (regression guard) ...
+install_fail_test(serial-fatal install-fail FAIL_MODE fatal)
+# ... and a status-only failure of an earlier component is no longer masked
+# by a later success (and the later component is not installed).
+install_fail_test(serial-mask install-component-mask
+  INSTALL_ARGS --component comp_fail --component comp_ok)

Tests/RunCMake/InstallParallel/fail-subdir/CMakeLists.txt

@@ -0,0 +1,5 @@
+if (FAIL_MODE STREQUAL "exit")
+  install(CODE [[cmake_language(EXIT 7)]])
+else ()
+  install(CODE [[message(FATAL_ERROR "intentional install failure")]])
+endif ()

Tests/RunCMake/InstallParallel/install-component-mask.cmake

@@ -0,0 +1,3 @@
+install(CODE [[cmake_language(EXIT 7)]] COMPONENT comp_fail)
+install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/CMakeLists.txt TYPE DATA RENAME ok.txt
+        COMPONENT comp_ok)

Tests/RunCMake/InstallParallel/install-fail.cmake

@@ -0,0 +1,6 @@
+install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/CMakeLists.txt TYPE DATA RENAME root.txt)
+if (INSTALL_PARALLEL)
+  set_property(GLOBAL PROPERTY INSTALL_PARALLEL ON)
+endif()
+add_subdirectory(fail-subdir)
+add_subdirectory(ok-subdir)

Tests/RunCMake/InstallParallel/ok-subdir/CMakeLists.txt

@@ -0,0 +1 @@
+install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/CMakeLists.txt TYPE DATA RENAME ok.txt)

Tests/RunCMake/InstallParallel/parallel-exit-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/InstallParallel/parallel-exit-stdout.txt

@@ -0,0 +1 @@
+install script '[^']*' exited with code 7

Tests/RunCMake/InstallParallel/parallel-fatal-check.cmake

@@ -0,0 +1,4 @@
+if (EXISTS "${RunCMake_TEST_BINARY_DIR}/install_manifest.txt")
+  set(RunCMake_TEST_FAILED
+    "install_manifest.txt was written despite a failed parallel install")
+endif ()

Tests/RunCMake/InstallParallel/parallel-fatal-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/InstallParallel/serial-fatal-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/InstallParallel/serial-mask-check.cmake

@@ -0,0 +1,5 @@
+file(GLOB_RECURSE ok_files "${RunCMake_TEST_BINARY_DIR}/install/*ok.txt")
+if (ok_files)
+  set(RunCMake_TEST_FAILED
+    "comp_ok was installed after comp_fail failed; serial fail-fast not honored: ${ok_files}")
+endif ()

Tests/RunCMake/InstallParallel/serial-mask-result.txt

@@ -0,0 +1 @@
+1