Merge topic 'changing-toolchain-recover'

ca3d1007f0 Tests: Simplify RunCMake.CompilerChange
ca0bf9848d cmake: Retain cache from presets and command line before re-configure
ee6bfdc1f3 CMAKE_TOOLCHAIN_FILE: Improve recovery from changed compiler or toolchain file

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !12241

Source/cmGlobalGenerator.cxx

@@ -295,17 +295,8 @@ void cmGlobalGenerator::ResolveLanguageCompiler(std::string const& lang,
     }
     cmCMakePath foundPath = path;
     if (foundPath.Normal() != cachedPath.Normal()) {
-      cmValue cvars = this->GetCMakeInstance()->GetState()->GetGlobalProperty(
-        "__CMAKE_DELETE_CACHE_CHANGE_VARS_");
-      if (cvars) {
-        changeVars += *cvars;
-        changeVars += ";";
-      }
-      changeVars += langComp;
-      changeVars += ";";
-      changeVars += *cname;
-      this->GetCMakeInstance()->GetState()->SetGlobalProperty(
-        "__CMAKE_DELETE_CACHE_CHANGE_VARS_", changeVars);
+      this->GetCMakeInstance()->GetState()->AddDeleteCacheChangeVar(langComp,
+                                                                    *cname);
     }
   }
 }
@@ -716,12 +707,12 @@ void cmGlobalGenerator::EnableLanguage(
       cmValue storedToolchainFile =
         mf->GetDefinition("_CMAKE_SYSTEM_TOOLCHAIN_FILE");
       if (toolchainFile && toolchainFile != storedToolchainFile) {
-        mf->GetState()->AppendGlobalProperty(
-          "__CMAKE_DELETE_CACHE_CHANGE_VARS_",
-          "CMAKE_TOOLCHAIN_FILE;" + *toolchainFile);
+        mf->GetState()->AddDeleteCacheChangeVar("CMAKE_TOOLCHAIN_FILE",
+                                                *toolchainFile);
         for (std::string const& lang : cur_languages) {
           this->LanguagesInProgress.erase(lang);
         }
+        cmSystemTools::SetFatalErrorOccurred();
         return;
       }
     }

Source/cmState.h

@@ -5,6 +5,7 @@
 #include "cmConfigure.h" // IWYU pragma: keep
 
 #include <functional>
+#include <map>
 #include <memory>
 #include <set>
 #include <string>
@@ -266,6 +267,22 @@ public:
   }
   bool InTopLevelIncludes() const { return this->ProcessingTopLevelIncludes; }
 
+  void ClearDeleteCacheChangeVars() { this->DeleteCacheChangeVars.clear(); }
+  void AddDeleteCacheChangeVar(std::string var, std::string value)
+  {
+    this->DeleteCacheChangeVars[var] = value;
+  }
+  std::map<std::string, std::string> GetDeleteCacheChangeVars() const
+  {
+    return this->DeleteCacheChangeVars;
+  }
+
+  void SetReconfiguring(bool reconfiguring)
+  {
+    this->Reconfiguring = reconfiguring;
+  }
+  bool IsReconfiguring() const { return this->Reconfiguring; }
+
 private:
   friend class cmake;
   cmStateSnapshot Reset(cmStateSnapshot const& diagnosticState);
@@ -345,4 +362,6 @@ private:
   TryCompile IsTryCompile = TryCompile::No;
   cm::optional<cmDependencyProvider> DependencyProvider;
   bool ProcessingTopLevelIncludes = false;
+  std::map<std::string, std::string> DeleteCacheChangeVars;
+  bool Reconfiguring = false;
 };

Source/cmake.cxx

@@ -2192,6 +2192,9 @@ bool cmake::SetArgsFromPreset(cmCMakePresetsConfigureArgs const& args,
     this->SetTraceFile(expandedPreset->TraceRedirect);
   }
 
+  // Store preset variables in case of cache reset.
+  this->InitialPresetVariables = this->UnprocessedPresetVariables;
+
   return true;
 }
 
@@ -2374,11 +2377,11 @@ struct SaveCacheEntry
   cmStateEnums::CacheEntryType type;
 };
 
-int cmake::HandleDeleteCacheVariables(std::string const& var)
+int cmake::HandleDeleteCacheVariables(
+  std::map<std::string, std::string> const& vars)
 {
-  cmList argsSplit{ var, cmList::EmptyElements::Yes };
-  // erase the property to avoid infinite recursion
-  this->State->SetGlobalProperty("__CMAKE_DELETE_CACHE_CHANGE_VARS_", "");
+  // erase the set to avoid infinite recursion
+  this->State->ClearDeleteCacheChangeVars();
   if (this->GetIsInTryCompile()) {
     return 0;
   }
@@ -2388,18 +2391,11 @@ int cmake::HandleDeleteCacheVariables(std::string const& var)
     << "You have changed variables that require your cache to be deleted.\n"
        "Configure will be re-run and you may have to reset some variables.\n"
        "The following variables have changed:\n";
-  for (auto i = argsSplit.begin(); i != argsSplit.end(); ++i) {
+  for (auto const& var : vars) {
     SaveCacheEntry save;
-    save.key = *i;
-    warning << *i << "= ";
-    i++;
-    if (i != argsSplit.end()) {
-      save.value = *i;
-      warning << *i << '\n';
-    } else {
-      warning << '\n';
-      i -= 1;
-    }
+    save.key = var.first;
+    save.value = var.second;
+    warning << save.key << "= " << save.value << '\n';
     cmValue existingValue = this->State->GetCacheEntryValue(save.key);
     if (existingValue) {
       save.type = this->State->GetCacheEntryType(save.key);
@@ -2417,6 +2413,15 @@ int cmake::HandleDeleteCacheVariables(std::string const& var)
   this->DeleteCache(this->GetHomeOutputDirectory());
   // load the empty cache
   this->LoadCache();
+#ifndef CMAKE_BOOTSTRAP
+  // Restore preset cache variables.
+  this->UnprocessedPresetVariables = this->InitialPresetVariables;
+  this->ProcessPresetVariables();
+#endif
+  // Restore command line cache variables (from this invocation cmake only).
+  bool resetArgsSuccess = this->SetCacheArgs(this->cmdArgs);
+  assert(resetArgsSuccess);
+  (void)resetArgsSuccess;
   // restore the changed compilers
   for (SaveCacheEntry const& i : saved) {
     this->AddCacheEntry(i.key, i.value, i.help, i.type);
@@ -2425,6 +2430,15 @@ int cmake::HandleDeleteCacheVariables(std::string const& var)
   // avoid reconfigure if there were errors
   if (!cmSystemTools::GetErrorOccurredFlag()) {
     // re-run configure
+    this->State->SetReconfiguring(true);
+    return this->Configure();
+  }
+
+  // Toolchain changes trigger a fatal error, but reconfiguring with the new
+  // toolchain should fix them.
+  if (vars.count("CMAKE_TOOLCHAIN_FILE") && !this->State->IsReconfiguring()) {
+    cmSystemTools::ResetErrorOccurredFlag();
+    this->State->SetReconfiguring(true);
     return this->Configure();
   }
   return 0;
@@ -2523,10 +2537,10 @@ int cmake::Configure()
                       cmStateEnums::INTERNAL);
 
   int ret = this->ActualConfigure();
-  cmValue delCacheVars =
-    this->State->GetGlobalProperty("__CMAKE_DELETE_CACHE_CHANGE_VARS_");
-  if (delCacheVars && !delCacheVars->empty()) {
-    return this->HandleDeleteCacheVariables(*delCacheVars);
+  std::map<std::string, std::string> delCacheVars =
+    this->State->GetDeleteCacheChangeVars();
+  if (!delCacheVars.empty()) {
+    return this->HandleDeleteCacheVariables(delCacheVars);
   }
   return ret;
 }

Source/cmake.h

@@ -710,7 +710,8 @@ public:
 
 protected:
   void RunCheckForUnusedVariables();
-  int HandleDeleteCacheVariables(std::string const& var);
+  int HandleDeleteCacheVariables(
+    std::map<std::string, std::string> const& var);
 
   using RegisteredGeneratorsVector =
     std::vector<std::unique_ptr<cmGlobalGeneratorFactory>>;
@@ -805,6 +806,8 @@ private:
     UnprocessedPresetVariables;
   std::map<std::string, cm::optional<std::string>>
     UnprocessedPresetEnvironment;
+  std::map<std::string, cm::optional<cmCMakePresetsGraph::CacheVariable>>
+    InitialPresetVariables;
 #endif
 
 #if !defined(CMAKE_BOOTSTRAP)

Tests/RunCMake/CompilerChange/CMakeLists.txt

@@ -1,7 +1,4 @@
 cmake_minimum_required(VERSION 3.10)
-if(NOT RunCMake_TEST)
-  set(RunCMake_TEST "$ENV{RunCMake_TEST}") # needed when cache is deleted
-endif()
 if(RunCMake_TEST MATCHES "^ToolchainFromFile")
   # CMAKE_TOOLCHAIN_FILE should be set before project. The ToolchainFromFile
   # cases test detecting toolchain path changes between CMake invocations and

Tests/RunCMake/CompilerChange/EmptyCompiler-stderr.txt

@@ -10,4 +10,4 @@ CMake Error at EmptyCompiler\.cmake:2 \(enable_language\):
   variable "CC" or the CMake cache entry CMAKE_C_COMPILER to the full path to
   the compiler, or to the compiler name if it is in the PATH\.
 Call Stack \(most recent call first\):
-  CMakeLists\.txt:14 \(include\)$
+  CMakeLists\.txt:11 \(include\)$

Tests/RunCMake/CompilerChange/RunCMakeTest.cmake

@@ -27,12 +27,9 @@ configure_file(${ccIn} ${cc2} @ONLY)
 
 block()
   set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/ChangeCompiler-build)
-  set(ENV{RunCMake_TEST} "FirstCompiler")
   run_cmake_with_options(FirstCompiler -DCMAKE_C_COMPILER=${cc1})
   set(RunCMake_TEST_NO_CLEAN 1)
-  set(ENV{RunCMake_TEST} "SecondCompiler")
   run_cmake_with_options(SecondCompiler -DCMAKE_C_COMPILER=${cc2})
-  set(ENV{RunCMake_TEST} "EmptyCompiler")
   run_cmake_with_options(EmptyCompiler -DCMAKE_C_COMPILER=)
 endblock()
 
@@ -55,10 +52,18 @@ block()
   set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/ToolchainFromFile-build)
   run_cmake(ToolchainFromFile-step1)
   set(RunCMake_TEST_NO_CLEAN 1)
-  set(ENV{RunCMake_TEST} "ToolchainFromFile-step2")
   run_cmake(ToolchainFromFile-step2)
 endblock()
 
+# New toolchain path comes from file but old toolchain has been deleted
+file(WRITE "${RunCMake_BINARY_DIR}/baz.cmake" "set(toolchain_var baz)\n")
+block()
+  set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/ToolchainFromFileDeleted-build)
+  run_cmake(ToolchainFromFileDeleted-step1)
+  set(RunCMake_TEST_NO_CLEAN 1)
+  run_cmake(ToolchainFromFileDeleted-step2)
+endblock()
+
 # New toolchain path comes from command line
 block()
   set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/ToolchainFromCmdline-build)
@@ -66,7 +71,6 @@ block()
     "-DCMAKE_TOOLCHAIN_FILE=${RunCMake_BINARY_DIR}/foo.cmake"
   )
   set(RunCMake_TEST_NO_CLEAN 1)
-  set(ENV{RunCMake_TEST} "ToolchainFromCmdline-step2")
   run_cmake_with_options(ToolchainFromCmdline-step2
     "-DCMAKE_TOOLCHAIN_FILE=${RunCMake_BINARY_DIR}/bar.cmake"
   )

Tests/RunCMake/CompilerChange/ToolchainFromFileDeleted-step1-prep.cmake

@@ -0,0 +1,4 @@
+file(WRITE
+  "${RunCMake_TEST_BINARY_DIR}/toolchain.txt"
+  "${RunCMake_BINARY_DIR}/baz.cmake\n"
+)

Tests/RunCMake/CompilerChange/ToolchainFromFileDeleted-step1-stdout.txt

@@ -0,0 +1,3 @@
+-- Toolchain file: [^
+]*/Tests/RunCMake/CompilerChange/baz\.cmake
+-- Using toolchain: baz

Tests/RunCMake/CompilerChange/ToolchainFromFileDeleted-step1.cmake

@@ -0,0 +1,2 @@
+message(STATUS "Toolchain file: ${CMAKE_TOOLCHAIN_FILE}")
+message(STATUS "Using toolchain: ${toolchain_var}")

Tests/RunCMake/CompilerChange/ToolchainFromFileDeleted-step2-prep.cmake

@@ -0,0 +1,5 @@
+file(REMOVE "${RunCMake_BINARY_DIR}/baz.cmake")
+file(WRITE
+  "${RunCMake_TEST_BINARY_DIR}/toolchain.txt"
+  "${RunCMake_BINARY_DIR}/bar.cmake\n"
+)

Tests/RunCMake/CompilerChange/ToolchainFromFileDeleted-step2-stderr.txt

@@ -0,0 +1,5 @@
+You have changed variables that require your cache to be deleted.
+Configure will be re-run and you may have to reset some variables.
+The following variables have changed:
+CMAKE_TOOLCHAIN_FILE= [^
+]*/Tests/RunCMake/CompilerChange/bar\.cmake

Tests/RunCMake/CompilerChange/ToolchainFromFileDeleted-step2-stdout.txt

@@ -0,0 +1,3 @@
+-- Toolchain file: [^
+]*/Tests/RunCMake/CompilerChange/bar\.cmake
+-- Using toolchain: bar

Tests/RunCMake/CompilerChange/ToolchainFromFileDeleted-step2.cmake

@@ -0,0 +1,2 @@
+message(STATUS "Toolchain file: ${CMAKE_TOOLCHAIN_FILE}")
+message(STATUS "Using toolchain: ${toolchain_var}")