From d96eb5528276a19d79116d842389f3ea165ef21b Mon Sep 17 00:00:00 2001 From: Marc Chevrier Date: Thu, 20 May 2021 15:14:30 +0200 Subject: set(CACHE): do not remove normal variable Fixes: #22038 --- Help/command/set.rst | 10 ++++++--- Help/manual/cmake-policies.7.rst | 1 + Help/policy/CMP0126.rst | 20 +++++++++++++++++ Help/release/dev/set-cache-variable.rst | 5 +++++ Source/cmFindBase.cxx | 35 ++++++++++++++++++++++++----- Source/cmFindPackageCommand.cxx | 5 +++++ Source/cmMakefile.cxx | 8 +++---- Source/cmOptionCommand.cxx | 7 ++++++ Source/cmPolicies.h | 5 ++++- Tests/RunCMake/CMP0126/CMP0126-NEW.cmake | 28 +++++++++++++++++++++++ Tests/RunCMake/CMP0126/CMP0126-NEW_CL.cmake | 9 ++++++++ Tests/RunCMake/CMP0126/CMP0126-OLD.cmake | 25 +++++++++++++++++++++ Tests/RunCMake/CMP0126/CMP0126-OLD_CL.cmake | 9 ++++++++ Tests/RunCMake/CMP0126/CMakeLists.txt | 3 +++ Tests/RunCMake/CMP0126/RunCMakeTest.cmake | 6 +++++ Tests/RunCMake/CMakeLists.txt | 1 + 16 files changed, 164 insertions(+), 13 deletions(-) create mode 100644 Help/policy/CMP0126.rst create mode 100644 Help/release/dev/set-cache-variable.rst create mode 100644 Tests/RunCMake/CMP0126/CMP0126-NEW.cmake create mode 100644 Tests/RunCMake/CMP0126/CMP0126-NEW_CL.cmake create mode 100644 Tests/RunCMake/CMP0126/CMP0126-OLD.cmake create mode 100644 Tests/RunCMake/CMP0126/CMP0126-OLD_CL.cmake create mode 100644 Tests/RunCMake/CMP0126/CMakeLists.txt create mode 100644 Tests/RunCMake/CMP0126/RunCMakeTest.cmake diff --git a/Help/command/set.rst b/Help/command/set.rst index c0e02e2..af862e4 100644 --- a/Help/command/set.rst +++ b/Help/command/set.rst @@ -68,9 +68,13 @@ users. If the cache entry does not exist prior to the call or the ``FORCE`` option is given then the cache entry will be set to the given value. -Furthermore, any normal variable binding in the current scope will -be removed to expose the newly cached value to any immediately -following evaluation. + +.. note:: + + The content of the cache variable will not be directly accessible if a normal + variable of the same name already exists (see :ref:`rules of variable + evaluation `). If policy :policy:`CMP0126` is set + to ``OLD``, any normal variable binding in the current scope will be removed. It is possible for the cache entry to exist prior to the call but have no type set if it was created on the :manual:`cmake(1)` command diff --git a/Help/manual/cmake-policies.7.rst b/Help/manual/cmake-policies.7.rst index 62ccb01..b9e3d45 100644 --- a/Help/manual/cmake-policies.7.rst +++ b/Help/manual/cmake-policies.7.rst @@ -57,6 +57,7 @@ Policies Introduced by CMake 3.21 .. toctree:: :maxdepth: 1 + CMP0126: set(CACHE) does not remove a normal variable of the same name. CMP0125: find_(path|file|library|program) have consistent behavior for cache variables. CMP0124: foreach() loop variables are only available in the loop scope. CMP0123: ARMClang cpu/arch compile and link flags must be set explicitly. diff --git a/Help/policy/CMP0126.rst b/Help/policy/CMP0126.rst new file mode 100644 index 0000000..4c8e928 --- /dev/null +++ b/Help/policy/CMP0126.rst @@ -0,0 +1,20 @@ +CMP0126 +------- + +.. versionadded:: 3.21 + +The :command:`set(CACHE)` does not remove a normal variable of the same name. + +Starting with CMake 3.21, the :command:`set(CACHE)` does not remove, in the +current scope, any normal variable with the same name. + +The ``OLD`` behavior for this policy is to have the :command:`set(CACHE)` +command removing the normal variable of the same name, if any. The ``NEW`` +behavior for this policy is to keep the normal variable of the same name. + +This policy was introduced in CMake version 3.21. Use the +:command:`cmake_policy` command to set it to ``OLD`` or ``NEW`` explicitly. +Unlike many policies, CMake version |release| does *not* warn when the policy +is not set and simply uses ``OLD`` behavior. + +.. include:: DEPRECATED.txt diff --git a/Help/release/dev/set-cache-variable.rst b/Help/release/dev/set-cache-variable.rst new file mode 100644 index 0000000..a96242c --- /dev/null +++ b/Help/release/dev/set-cache-variable.rst @@ -0,0 +1,5 @@ +set-cache-variable +------------------ + +* The :command:`set(CACHE)` command no longer removes a normal variable of the + same name, if any. See policy :policy:`CMP0126`. diff --git a/Source/cmFindBase.cxx b/Source/cmFindBase.cxx index 6296d06..c1281e3 100644 --- a/Source/cmFindBase.cxx +++ b/Source/cmFindBase.cxx @@ -357,11 +357,17 @@ void cmFindBase::NormalizeFindResult() this->Makefile->GetCMakeInstance()->AddCacheEntry( this->VariableName, value.c_str(), this->VariableDocumentation.c_str(), this->VariableType); - // if there was a definition then remove it - // This is required to ensure same behavior as - // cmMakefile::AddCacheDefinition. - // See #22038 for problems raised by this behavior. - this->Makefile->RemoveDefinition(this->VariableName); + if (this->Makefile->GetPolicyStatus(cmPolicies::CMP0126) == + cmPolicies::NEW) { + if (this->Makefile->IsNormalDefinitionSet(this->VariableName)) { + this->Makefile->AddDefinition(this->VariableName, value); + } + } else { + // if there was a definition then remove it + // This is required to ensure same behavior as + // cmMakefile::AddCacheDefinition. + this->Makefile->RemoveDefinition(this->VariableName); + } } } else { // If the user specifies the entry on the command line without a @@ -371,6 +377,14 @@ void cmFindBase::NormalizeFindResult() this->Makefile->AddCacheDefinition(this->VariableName, "", this->VariableDocumentation.c_str(), this->VariableType); + if (this->Makefile->GetPolicyStatus(cmPolicies::CMP0126) == + cmPolicies::NEW && + this->Makefile->IsNormalDefinitionSet(this->VariableName)) { + this->Makefile->AddDefinition( + this->VariableName, + *this->Makefile->GetCMakeInstance()->GetCacheDefinition( + this->VariableName)); + } } } } @@ -379,17 +393,28 @@ void cmFindBase::StoreFindResult(const std::string& value) { bool force = this->Makefile->GetPolicyStatus(cmPolicies::CMP0125) == cmPolicies::NEW; + bool updateNormalVariable = + this->Makefile->GetPolicyStatus(cmPolicies::CMP0126) == cmPolicies::NEW; if (!value.empty()) { this->Makefile->AddCacheDefinition(this->VariableName, value, this->VariableDocumentation.c_str(), this->VariableType, force); + if (updateNormalVariable && + this->Makefile->IsNormalDefinitionSet(this->VariableName)) { + this->Makefile->AddDefinition(this->VariableName, value); + } return; } this->Makefile->AddCacheDefinition( this->VariableName, cmStrCat(this->VariableName, "-NOTFOUND"), this->VariableDocumentation.c_str(), this->VariableType, force); + if (updateNormalVariable && + this->Makefile->IsNormalDefinitionSet(this->VariableName)) { + this->Makefile->AddDefinition(this->VariableName, + cmStrCat(this->VariableName, "-NOTFOUND")); + } if (this->Required) { this->Makefile->IssueMessage( diff --git a/Source/cmFindPackageCommand.cxx b/Source/cmFindPackageCommand.cxx index 3719fe1..fba736e 100644 --- a/Source/cmFindPackageCommand.cxx +++ b/Source/cmFindPackageCommand.cxx @@ -1138,6 +1138,11 @@ bool cmFindPackageCommand::FindConfig() // We force the value since we do not get here if it was already set. this->Makefile->AddCacheDefinition(this->Variable, init, help.c_str(), cmStateEnums::PATH, true); + if (this->Makefile->GetPolicyStatus(cmPolicies::CMP0126) == + cmPolicies::NEW && + this->Makefile->IsNormalDefinitionSet(this->Variable)) { + this->Makefile->AddDefinition(this->Variable, init); + } return found; } diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 4ffd47b..d7987c2 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1962,10 +1962,10 @@ void cmMakefile::AddCacheDefinition(const std::string& name, const char* value, } } this->GetCMakeInstance()->AddCacheEntry(name, value, doc, type); - // if there was a definition then remove it - // The method cmFindBase::NormalizeFindResult also apply same workflow. - // See #22038 for problems raised by this behavior. - this->StateSnapshot.RemoveDefinition(name); + if (this->GetPolicyStatus(cmPolicies::CMP0126) != cmPolicies::NEW) { + // if there was a definition then remove it + this->StateSnapshot.RemoveDefinition(name); + } } void cmMakefile::MarkVariableAsUsed(const std::string& var) diff --git a/Source/cmOptionCommand.cxx b/Source/cmOptionCommand.cxx index a58e2f8..bae67e0 100644 --- a/Source/cmOptionCommand.cxx +++ b/Source/cmOptionCommand.cxx @@ -68,6 +68,13 @@ bool cmOptionCommand(std::vector const& args, bool init = cmIsOn(initialValue); status.GetMakefile().AddCacheDefinition(args[0], init ? "ON" : "OFF", args[1].c_str(), cmStateEnums::BOOL); + if (status.GetMakefile().GetPolicyStatus(cmPolicies::CMP0077) != + cmPolicies::NEW && + status.GetMakefile().GetPolicyStatus(cmPolicies::CMP0126) == + cmPolicies::NEW) { + // if there was a definition then remove it + status.GetMakefile().GetStateSnapshot().RemoveDefinition(args[0]); + } if (checkAndWarn) { const auto* existsAfterSet = diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h index 3ebb17d..f7c0d25 100644 --- a/Source/cmPolicies.h +++ b/Source/cmPolicies.h @@ -376,7 +376,10 @@ class cmMakefile; SELECT(POLICY, CMP0125, \ "find_(path|file|library|program) have consistent behavior for " \ "cache variables.", \ - 3, 21, 0, cmPolicies::WARN) + 3, 21, 0, cmPolicies::WARN) \ + SELECT(POLICY, CMP0126, \ + "set(CACHE) does not remove a normal variable of the same name.", 3, \ + 21, 0, cmPolicies::WARN) #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1) #define CM_FOR_EACH_POLICY_ID(POLICY) \ diff --git a/Tests/RunCMake/CMP0126/CMP0126-NEW.cmake b/Tests/RunCMake/CMP0126/CMP0126-NEW.cmake new file mode 100644 index 0000000..2f8562b --- /dev/null +++ b/Tests/RunCMake/CMP0126/CMP0126-NEW.cmake @@ -0,0 +1,28 @@ + +# enforce policy CMP0125 to ensure predictable result of find_* commands +cmake_policy(SET CMP0125 NEW) + +cmake_policy(SET CMP0126 NEW) + +set(VAR 1) +set(VAR 2 CACHE STRING "") + +if (NOT VAR EQUAL 1) + message(FATAL_ERROR "normal variable does not exist anymore.") +endif() + + +file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/file.txt" "") +set(VAR file.txt) +set(VAR "" CACHE STRING "" FORCE) +set_property(CACHE VAR PROPERTY TYPE UNINITIALIZED) + +find_file(VAR NAMES file.txt PATHS "${CMAKE_CURRENT_BINARY_DIR}") + +unset(VAR CACHE) +if (NOT DEFINED VAR) + message(FATAL_ERROR "find_file: normal variable does not exist anymore.") +endif() +if (NOT VAR STREQUAL "${CMAKE_CURRENT_BINARY_DIR}/file.txt") + message(FATAL_ERROR "find_file: failed to set normal variable.") +endif() diff --git a/Tests/RunCMake/CMP0126/CMP0126-NEW_CL.cmake b/Tests/RunCMake/CMP0126/CMP0126-NEW_CL.cmake new file mode 100644 index 0000000..cfaa1e3 --- /dev/null +++ b/Tests/RunCMake/CMP0126/CMP0126-NEW_CL.cmake @@ -0,0 +1,9 @@ + +cmake_policy(SET CMP0126 NEW) + +set(VAR 1) +set(VAR 2 CACHE STRING "") + +if (NOT VAR EQUAL 1) + message(FATAL_ERROR "normal variable does not exist anymore.") +endif() diff --git a/Tests/RunCMake/CMP0126/CMP0126-OLD.cmake b/Tests/RunCMake/CMP0126/CMP0126-OLD.cmake new file mode 100644 index 0000000..22a5037 --- /dev/null +++ b/Tests/RunCMake/CMP0126/CMP0126-OLD.cmake @@ -0,0 +1,25 @@ + +# enforce policy CMP0125 to ensure predictable result of find_* commands +cmake_policy(SET CMP0125 NEW) + +cmake_policy(SET CMP0126 OLD) + +set(VAR 1) +set(VAR 2 CACHE STRING "") + +if (VAR EQUAL 1) + message(FATAL_ERROR "normal variable still exist.") +endif() + + +file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/file.txt" "") +set(VAR file.txt) +set(VAR "" CACHE STRING "" FORCE) +set_property(CACHE VAR PROPERTY TYPE UNINITIALIZED) + +find_file(VAR NAMES file.txt PATHS "${CMAKE_CURRENT_BINARY_DIR}") + +unset(VAR CACHE) +if (DEFINED VAR) + message(FATAL_ERROR "find_file: normal variable still exist.") +endif() diff --git a/Tests/RunCMake/CMP0126/CMP0126-OLD_CL.cmake b/Tests/RunCMake/CMP0126/CMP0126-OLD_CL.cmake new file mode 100644 index 0000000..5d72a87 --- /dev/null +++ b/Tests/RunCMake/CMP0126/CMP0126-OLD_CL.cmake @@ -0,0 +1,9 @@ + +cmake_policy(SET CMP0126 OLD) + +set(VAR 1) +set(VAR 2 CACHE STRING "") + +if (NOT VAR EQUAL 3) + message(FATAL_ERROR "normal variable still exist.") +endif() diff --git a/Tests/RunCMake/CMP0126/CMakeLists.txt b/Tests/RunCMake/CMP0126/CMakeLists.txt new file mode 100644 index 0000000..7cabeb6 --- /dev/null +++ b/Tests/RunCMake/CMP0126/CMakeLists.txt @@ -0,0 +1,3 @@ +cmake_minimum_required(VERSION 3.20) +project(${RunCMake_TEST} NONE) +include(${RunCMake_TEST}.cmake) diff --git a/Tests/RunCMake/CMP0126/RunCMakeTest.cmake b/Tests/RunCMake/CMP0126/RunCMakeTest.cmake new file mode 100644 index 0000000..ae988f4 --- /dev/null +++ b/Tests/RunCMake/CMP0126/RunCMakeTest.cmake @@ -0,0 +1,6 @@ +include(RunCMake) + +run_cmake(CMP0126-OLD) +run_cmake_with_options(CMP0126-OLD_CL -DVAR=3) +run_cmake(CMP0126-NEW) +run_cmake_with_options(CMP0126-NEW_CL -DVAR=3) diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt index afaeaef..14a7fa3 100644 --- a/Tests/RunCMake/CMakeLists.txt +++ b/Tests/RunCMake/CMakeLists.txt @@ -140,6 +140,7 @@ if (CMAKE_SYSTEM_NAME MATCHES "(Linux|Darwin)") add_RunCMake_test(CMP0125 -DCMAKE_SHARED_LIBRARY_PREFIX=${CMAKE_SHARED_LIBRARY_PREFIX} -DCMAKE_SHARED_LIBRARY_SUFFIX=${CMAKE_SHARED_LIBRARY_SUFFIX}) endif() +add_RunCMake_test(CMP0126) # The test for Policy 65 requires the use of the # CMAKE_SHARED_LIBRARY_LINK_CXX_FLAGS variable, which both the VS and Xcode -- cgit v0.12