Merge topic 'install-better-error'

50c29eae35 cmake --install: Check that install scripts exist before running
3ed425fca5 cmListFileCache: Do not mention internal class name in allocation error
fe6b359067 cmake: Clarify errors when a script cannot be read

Acked-by: Kitware Robot <kwrobot@kitware.com>
Reviewed-by: Ben Boeckel <ben.boeckel@kitware.com>
Merge-request: !12772

Source/cmListFileCache.cxx

@@ -132,7 +132,9 @@ bool cmListFileParser::ParseFile()
   cmListFileLexer_BOM bom;
   if (!cmListFileLexer_SetFileName(this->Lexer.get(), filename->c_str(),
                                    &bom)) {
-    this->IssueFileOpenError("cmListFileCache: error can not open file.");
+    this->IssueFileOpenError(
+      cmStrCat("Cannot open file \"", this->FileName,
+               "\": ", cmSystemTools::GetLastSystemError()));
     return false;
   }
 
@@ -151,7 +153,7 @@ bool cmListFileParser::ParseString(cm::string_view str)
 {
   if (!cmListFileLexer_SetString(this->Lexer.get(), str.data(),
                                  str.length())) {
-    this->IssueFileOpenError("cmListFileCache: cannot allocate buffer.");
+    this->IssueFileOpenError("Out of memory while reading code to parse.");
     return false;
   }
 

Source/cmake.cxx

@@ -898,9 +898,10 @@ void cmake::ReadListFile(std::vector<std::string> const& args,
     }
     if (!cmSystemTools::FileExists(path, true)) {
       cmSystemTools::Error("Not a file: " + path);
+      return;
     }
     if (!mf.ReadListFile(path)) {
-      cmSystemTools::Error("Error processing file: " + path);
+      cmSystemTools::Error("Failed to read file: " + path);
     }
   }
 }

Source/cmakemain.cxx

@@ -1032,6 +1032,13 @@ int do_install(int ac, char const* const* av)
   }
 
   auto doInstall = [&handler, &verbose, &jobs, &instrumentation]() -> int {
+    for (auto const& script : handler.GetScripts()) {
+      if (!cmSystemTools::FileExists(script.path, true)) {
+        cmSystemTools::Error(
+          cmStrCat("Install script does not exist:\n  ", script.path));
+        return 1;
+      }
+    }
     int ret_ = 0;
     if (handler.IsParallel()) {
       ret_ = handler.Install(jobs, instrumentation);

Tests/RunCMake/CommandLine/P_no-file-stderr.txt

@@ -1,4 +1,2 @@
 ^CMake Error: Not a file: [^
-]*/Tests/RunCMake/CommandLine/P_no-file-build/nosuchscriptfile\.cmake
-CMake Error: Error processing file: [^
 ]*/Tests/RunCMake/CommandLine/P_no-file-build/nosuchscriptfile\.cmake$

Tests/RunCMake/CommandLine/P_parse-error-result.txt

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

Tests/RunCMake/CommandLine/P_parse-error-stderr.txt

@@ -0,0 +1,6 @@
+^CMake Error at [^
+]*/P_parse-error\.cmake:2:
+  Parse error\.  Function missing ending "\)"\.  End of file reached\.
++
+CMake Error: Failed to read file: [^
+]*/Tests/RunCMake/CommandLine/P_parse-error\.cmake$

Tests/RunCMake/CommandLine/P_parse-error.cmake

@@ -0,0 +1,2 @@
+message(STATUS "before")
+foo(

Tests/RunCMake/CommandLine/RunCMakeTest.cmake

@@ -123,6 +123,7 @@ run_cmake_command(G_no-arg ${CMAKE_COMMAND} -B DummyBuildDir -G)
 run_cmake_command(G_bad-arg ${CMAKE_COMMAND} -B DummyBuildDir -G NoSuchGenerator)
 run_cmake_command(P_no-arg ${CMAKE_COMMAND} -P)
 run_cmake_command(P_no-file ${CMAKE_COMMAND} -P nosuchscriptfile.cmake)
+run_cmake_command(P_parse-error ${CMAKE_COMMAND} -P "${RunCMake_SOURCE_DIR}/P_parse-error.cmake")
 run_cmake_command(P_args ${CMAKE_COMMAND} -P "${RunCMake_SOURCE_DIR}/P_args.cmake" relative/path "${RunCMake_SOURCE_DIR}")
 run_cmake_command(P_arbitrary_args ${CMAKE_COMMAND} -P "${RunCMake_SOURCE_DIR}/P_arbitrary_args.cmake" -- -DFOO -S -B --fresh --version)
 run_cmake_command(P_P_in_arbitrary_args ${CMAKE_COMMAND} -P "${RunCMake_SOURCE_DIR}/P_arbitrary_args.cmake" -- -P "${RunCMake_SOURCE_DIR}/non_existing.cmake")

Tests/RunCMake/CommandLine/install-bad-dir-stderr.txt

@@ -1 +1,3 @@
-^CMake Error: Not a file:
+^CMake Error: Install script does not exist:
+  [^
+]*/dir-does-not-exist/cmake_install\.cmake$

Tests/RunCMake/InstallParallel/RunCMakeTest.cmake

@@ -116,3 +116,18 @@ function(reconfigure_parallel_test test)
   unset(RunCMake-check-file)
 endfunction()
 reconfigure_parallel_test(out-of-date-json-reconfigure)
+
+# A parallel install does not leave a partial installation behind after a
+# missing install script is reported.
+function(missing_script_test test)
+  set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${test}-build)
+  set(RunCMake_TEST_OPTIONS -DINSTALL_PARALLEL=ON -DCMAKE_INSTALL_PREFIX=install)
+  if (NOT RunCMake_GENERATOR_IS_MULTI_CONFIG)
+    list(APPEND RunCMake_TEST_OPTIONS -DCMAKE_BUILD_TYPE=Debug)
+  endif()
+  run_cmake(install)
+  set(RunCMake_TEST_NO_CLEAN 1)
+  file(REMOVE ${RunCMake_TEST_BINARY_DIR}/subdir-1/cmake_install.cmake)
+  run_cmake_command(${test} ${CMAKE_COMMAND} --install . -j 4)
+endfunction()
+missing_script_test(parallel-missing-script)

Tests/RunCMake/InstallParallel/parallel-missing-script-check.cmake

@@ -0,0 +1,4 @@
+if (EXISTS "${RunCMake_TEST_BINARY_DIR}/install")
+  set(RunCMake_TEST_FAILED
+    "files were installed despite a missing install script")
+endif ()

Tests/RunCMake/InstallParallel/parallel-missing-script-result.txt

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

Tests/RunCMake/InstallParallel/parallel-missing-script-stderr.txt

@@ -0,0 +1,3 @@
+^CMake Error: Install script does not exist:
+  [^
+]*/parallel-missing-script-build/subdir-1/cmake_install\.cmake$

Tests/RunCMake/Instrumentation/RunCMakeTest.cmake

@@ -15,6 +15,7 @@ function(instrument test)
     "INSTALL_PARALLEL"
     "INSTALL_SEAM"
     "INSTALL_INTERRUPT"
+    "INSTALL_MISSING_SCRIPT"
     "TEST"
     "CTEST_SEAM"
     "CTEST_INTERRUPT"
@@ -300,6 +301,11 @@ function(instrument test)
   if (ARGS_INSTALL)
     run_cmake_command(${test}-install ${CMAKE_COMMAND} --install . --prefix install --config Debug)
   endif()
+  if (ARGS_INSTALL_MISSING_SCRIPT)
+    file(REMOVE ${RunCMake_TEST_BINARY_DIR}/cmake_install.cmake)
+    run_cmake_command(${test}-install
+      ${CMAKE_COMMAND} --install . --prefix install --config Debug)
+  endif()
   if (ARGS_INSTALL_SEAM)
     # Drive the cmakeInstall interrupt path deterministically via the test-only
     # injection seam, with no OS signal, so it runs on every generator.  First
@@ -736,6 +742,10 @@ instrument(interrupt-test BUILD CTEST_SEAM
   CHECK_SCRIPT check-test-interrupted.cmake
 )
 
+instrument(install-missing-script INSTALL_MISSING_SCRIPT
+  CHECK_SCRIPT check-install-missing-script.cmake
+)
+
 # Test make/ninja hooks
 if(RunCMake_GENERATOR STREQUAL "FASTBuild")
   # FIXME(#27184): This does not work for FASTBuild.

Tests/RunCMake/Instrumentation/check-install-missing-script.cmake

@@ -0,0 +1,15 @@
+include(${CMAKE_CURRENT_LIST_DIR}/json.cmake)
+
+# The install failed before running any script, but the cmakeInstall snippet
+# should still be written, recording the failure.
+file(GLOB snippets LIST_DIRECTORIES false ${v1}/data/cmakeInstall-*.json)
+list(LENGTH snippets num)
+if (NOT num EQUAL 1)
+  add_error("Expected exactly one cmakeInstall snippet, found ${num}: ${snippets}")
+else()
+  read_json("${snippets}" contents)
+  string(JSON result GET "${contents}" result)
+  if (result EQUAL 0)
+    add_error("Expected nonzero result in cmakeInstall snippet:\n${contents}")
+  endif()
+endif()

Tests/RunCMake/Instrumentation/install-missing-script-install-result.txt

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

Tests/RunCMake/Instrumentation/install-missing-script-install-stderr.txt

@@ -0,0 +1,3 @@
+^CMake Error: Install script does not exist:
+  [^
+]*/install-missing-script/cmake_install\.cmake$