From 3ec82b713e51711df0c70f539aea62fc16dd3060 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 19 Dec 2019 08:46:50 -0500 Subject: cmMarkAsAdvancedCommand: ignore variables which don't exist in the cache Fixes: #18331 --- Help/command/mark_as_advanced.rst | 6 +++ Help/manual/cmake-policies.7.rst | 1 + Help/policy/CMP0102.rst | 25 +++++++++ Help/variable/CMAKE_POLICY_WARNING_CMPNNNN.rst | 2 + Source/cmMarkAsAdvancedCommand.cxx | 62 +++++++++++++++++++++-- Source/cmPolicies.h | 5 +- Tests/RunCMake/CMP0102/CMP0102-Common.cmake | 2 + Tests/RunCMake/CMP0102/CMP0102-NEW.cmake | 13 +++++ Tests/RunCMake/CMP0102/CMP0102-OLD.cmake | 18 +++++++ Tests/RunCMake/CMP0102/CMP0102-WARN-Default.cmake | 16 ++++++ Tests/RunCMake/CMP0102/CMP0102-WARN-stderr.txt | 10 ++++ Tests/RunCMake/CMP0102/CMP0102-WARN.cmake | 18 +++++++ Tests/RunCMake/CMP0102/CMakeLists.txt | 3 ++ Tests/RunCMake/CMP0102/RunCMakeTest.cmake | 6 +++ Tests/RunCMake/CMakeLists.txt | 1 + 15 files changed, 182 insertions(+), 6 deletions(-) create mode 100644 Help/policy/CMP0102.rst create mode 100644 Tests/RunCMake/CMP0102/CMP0102-Common.cmake create mode 100644 Tests/RunCMake/CMP0102/CMP0102-NEW.cmake create mode 100644 Tests/RunCMake/CMP0102/CMP0102-OLD.cmake create mode 100644 Tests/RunCMake/CMP0102/CMP0102-WARN-Default.cmake create mode 100644 Tests/RunCMake/CMP0102/CMP0102-WARN-stderr.txt create mode 100644 Tests/RunCMake/CMP0102/CMP0102-WARN.cmake create mode 100644 Tests/RunCMake/CMP0102/CMakeLists.txt create mode 100644 Tests/RunCMake/CMP0102/RunCMakeTest.cmake diff --git a/Help/command/mark_as_advanced.rst b/Help/command/mark_as_advanced.rst index 5712fb4..e52e623 100644 --- a/Help/command/mark_as_advanced.rst +++ b/Help/command/mark_as_advanced.rst @@ -22,3 +22,9 @@ If neither ``FORCE`` nor ``CLEAR`` is specified, new values will be marked as advanced, but if a variable already has an advanced/non-advanced state, it will not be changed. + +.. note:: + + Policy :policy:`CMP0102` affects the behavior of the ``mark_as_advanced`` + call. When set to ``NEW``, variables passed to this command which are not + already in the cache are ignored. See policy :policy:`CMP0102`. diff --git a/Help/manual/cmake-policies.7.rst b/Help/manual/cmake-policies.7.rst index 53cf264..c256250 100644 --- a/Help/manual/cmake-policies.7.rst +++ b/Help/manual/cmake-policies.7.rst @@ -57,6 +57,7 @@ Policies Introduced by CMake 3.17 .. toctree:: :maxdepth: 1 + CMP0102: mark_as_advanced() does nothing if a cache entry does not exist. CMP0101: target_compile_options honors BEFORE keyword in all scopes. CMP0100: Let AUTOMOC and AUTOUIC process .hh header files. CMP0099: Link properties are transitive over private dependency on static libraries. diff --git a/Help/policy/CMP0102.rst b/Help/policy/CMP0102.rst new file mode 100644 index 0000000..9859006 --- /dev/null +++ b/Help/policy/CMP0102.rst @@ -0,0 +1,25 @@ +CMP0102 +------- + +The :command:`mark_as_advanced` command no longer creates a cache entry if one +does not already exist. + +In CMake 3.16 and below, if a variable was not defined at all or just defined +locally, the :command:`mark_as_advanced` command would create a new cache +entry with an ``UNINITIALIZED`` type and no value. When a :command:`find_path` +(or other similar ``find_`` command) would next run, it would find this +undefined cache entry and set it up with an empty string value. This process +would end up deleting the local variable in the process (due to the way the +cache works), effectively clearing any stored ``find_`` results that were only +available in the local scope. + +The ``OLD`` behavior for this policy is to create the empty cache definition. +The ``NEW`` behavior of this policy is to ignore variables which do not +already exist in the cache. + +This policy was introduced in CMake version 3.17. Use the +:command:`cmake_policy` command to set it to ``OLD`` or ``NEW`` explicitly. +Unlike many policies, CMake version |release| does *not* warn +when this policy is not set and simply uses ``OLD`` behavior. + +.. include:: DEPRECATED.txt diff --git a/Help/variable/CMAKE_POLICY_WARNING_CMPNNNN.rst b/Help/variable/CMAKE_POLICY_WARNING_CMPNNNN.rst index fc52e7b..de71d0e 100644 --- a/Help/variable/CMAKE_POLICY_WARNING_CMPNNNN.rst +++ b/Help/variable/CMAKE_POLICY_WARNING_CMPNNNN.rst @@ -23,6 +23,8 @@ warn by default: policy :policy:`CMP0082`. * ``CMAKE_POLICY_WARNING_CMP0089`` controls the warning for policy :policy:`CMP0089`. +* ``CMAKE_POLICY_WARNING_CMP0102`` controls the warning for + policy :policy:`CMP0102`. This variable should not be set by a project in CMake code. Project developers running CMake may set this variable in their cache to diff --git a/Source/cmMarkAsAdvancedCommand.cxx b/Source/cmMarkAsAdvancedCommand.cxx index ca46e14..45043fa 100644 --- a/Source/cmMarkAsAdvancedCommand.cxx +++ b/Source/cmMarkAsAdvancedCommand.cxx @@ -4,8 +4,11 @@ #include "cmExecutionStatus.h" #include "cmMakefile.h" +#include "cmMessageType.h" +#include "cmPolicies.h" #include "cmState.h" #include "cmStateTypes.h" +#include "cmStringAlgorithms.h" #include "cmSystemTools.h" #include "cmake.h" @@ -28,14 +31,63 @@ bool cmMarkAsAdvancedCommand(std::vector const& args, } i = 1; } + + cmMakefile& mf = status.GetMakefile(); + cmState* state = mf.GetState(); + for (; i < args.size(); ++i) { std::string const& variable = args[i]; - cmState* state = status.GetMakefile().GetState(); - if (!state->GetCacheEntryValue(variable)) { - status.GetMakefile().GetCMakeInstance()->AddCacheEntry( - variable, nullptr, nullptr, cmStateEnums::UNINITIALIZED); - overwrite = true; + + bool issueMessage = false; + bool oldBehavior = false; + bool ignoreVariable = false; + switch (mf.GetPolicyStatus(cmPolicies::CMP0102)) { + case cmPolicies::WARN: + if (mf.PolicyOptionalWarningEnabled("CMAKE_POLICY_WARNING_CMP0102")) { + if (!state->GetCacheEntryValue(variable)) { + issueMessage = true; + } + } + CM_FALLTHROUGH; + case cmPolicies::OLD: + oldBehavior = true; + break; + case cmPolicies::NEW: + case cmPolicies::REQUIRED_IF_USED: + case cmPolicies::REQUIRED_ALWAYS: + if (!state->GetCacheEntryValue(variable)) { + ignoreVariable = true; + } + break; + } + + // First see if we should issue a message about CMP0102 + if (issueMessage) { + std::string err = cmStrCat( + "Policy CMP0102 is not set: The variable named \"", variable, + "\" is not in the cache. This results in an empty cache entry which " + "is no longer created when policy CMP0102 is set to NEW. Run \"cmake " + "--help-policy CMP0102\" for policy details. Use the cmake_policy " + "command to set the policy and suppress this warning."); + mf.IssueMessage(MessageType::AUTHOR_WARNING, err); } + + // If it's not in the cache and we're using the new behavior, nothing to + // see here. + if (ignoreVariable) { + continue; + } + + // Check if we want the old behavior of making a dummy cache entry. + if (oldBehavior) { + if (!state->GetCacheEntryValue(variable)) { + status.GetMakefile().GetCMakeInstance()->AddCacheEntry( + variable, nullptr, nullptr, cmStateEnums::UNINITIALIZED); + overwrite = true; + } + } + + // We need a cache entry to do this. if (!state->GetCacheEntryValue(variable)) { cmSystemTools::Error("This should never happen..."); return false; diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h index eef41c0..1366ff0 100644 --- a/Source/cmPolicies.h +++ b/Source/cmPolicies.h @@ -302,7 +302,10 @@ class cmMakefile; 17, 0, cmPolicies::WARN) \ SELECT(POLICY, CMP0101, \ "target_compile_options honors BEFORE keyword in all scopes.", 3, \ - 17, 0, cmPolicies::WARN) + 17, 0, cmPolicies::WARN) \ + SELECT(POLICY, CMP0102, \ + "mark_as_advanced() does nothing if a cache entry does not exist.", \ + 3, 17, 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/CMP0102/CMP0102-Common.cmake b/Tests/RunCMake/CMP0102/CMP0102-Common.cmake new file mode 100644 index 0000000..61fdad6 --- /dev/null +++ b/Tests/RunCMake/CMP0102/CMP0102-Common.cmake @@ -0,0 +1,2 @@ + +mark_as_advanced(CMP0102_TEST_VARIABLE) diff --git a/Tests/RunCMake/CMP0102/CMP0102-NEW.cmake b/Tests/RunCMake/CMP0102/CMP0102-NEW.cmake new file mode 100644 index 0000000..bdf769f --- /dev/null +++ b/Tests/RunCMake/CMP0102/CMP0102-NEW.cmake @@ -0,0 +1,13 @@ + +cmake_policy(SET CMP0102 NEW) + +include (CMP0102-Common.cmake) +get_property(is_type_set CACHE CMP0102_TEST_VARIABLE + PROPERTY TYPE SET) +if (is_type_set) + get_property(type CACHE CMP0102_TEST_VARIABLE + PROPERTY TYPE) + message(FATAL_ERROR + "There is a cache entry for an undefined variable after " + "`mark_as_advanced`.") +endif () diff --git a/Tests/RunCMake/CMP0102/CMP0102-OLD.cmake b/Tests/RunCMake/CMP0102/CMP0102-OLD.cmake new file mode 100644 index 0000000..5c20dd3 --- /dev/null +++ b/Tests/RunCMake/CMP0102/CMP0102-OLD.cmake @@ -0,0 +1,18 @@ + +cmake_policy(SET CMP0102 OLD) + +include (CMP0102-Common.cmake) +get_property(is_type_set CACHE CMP0102_TEST_VARIABLE + PROPERTY TYPE SET) +if (NOT is_type_set) + message(FATAL_ERROR + "There is a cache entry for an undefined variable after " + "`mark_as_advanced`.") +endif () +get_property(type CACHE CMP0102_TEST_VARIABLE + PROPERTY TYPE) +if (NOT type STREQUAL "UNINITIALIZED") + message(FATAL_ERROR + "The cache type for CMP0102_TEST_VARIABLE is not " + "UNINITIALIZED") +endif () diff --git a/Tests/RunCMake/CMP0102/CMP0102-WARN-Default.cmake b/Tests/RunCMake/CMP0102/CMP0102-WARN-Default.cmake new file mode 100644 index 0000000..d6ebe4d --- /dev/null +++ b/Tests/RunCMake/CMP0102/CMP0102-WARN-Default.cmake @@ -0,0 +1,16 @@ + +include (CMP0102-Common.cmake) +get_property(is_type_set CACHE CMP0102_TEST_VARIABLE + PROPERTY TYPE SET) +if (NOT is_type_set) + message(FATAL_ERROR + "There is a cache entry for an undefined variable after " + "`mark_as_advanced`.") +endif () +get_property(type CACHE CMP0102_TEST_VARIABLE + PROPERTY TYPE) +if (NOT type STREQUAL "UNINITIALIZED") + message(FATAL_ERROR + "The cache type for CMP0102_TEST_VARIABLE is not " + "UNINITIALIZED") +endif () diff --git a/Tests/RunCMake/CMP0102/CMP0102-WARN-stderr.txt b/Tests/RunCMake/CMP0102/CMP0102-WARN-stderr.txt new file mode 100644 index 0000000..bb56ec2 --- /dev/null +++ b/Tests/RunCMake/CMP0102/CMP0102-WARN-stderr.txt @@ -0,0 +1,10 @@ +CMake Warning \(dev\) at CMP0102-Common.cmake:2 \(mark_as_advanced\): + Policy CMP0102 is not set: The variable named "CMP0102_TEST_VARIABLE" is + not in the cache. This results in an empty cache entry which is no longer + created when policy CMP0102 is set to NEW. Run "cmake --help-policy + CMP0102" for policy details. Use the cmake_policy command to set the + policy and suppress this warning. +Call Stack \(most recent call first\): + CMP0102-WARN.cmake:4 \(include\) + CMakeLists.txt:3 \(include\) +This warning is for project developers. Use -Wno-dev to suppress it. diff --git a/Tests/RunCMake/CMP0102/CMP0102-WARN.cmake b/Tests/RunCMake/CMP0102/CMP0102-WARN.cmake new file mode 100644 index 0000000..e9a45f1 --- /dev/null +++ b/Tests/RunCMake/CMP0102/CMP0102-WARN.cmake @@ -0,0 +1,18 @@ + +set(CMAKE_POLICY_WARNING_CMP0102 1) + +include (CMP0102-Common.cmake) +get_property(is_type_set CACHE CMP0102_TEST_VARIABLE + PROPERTY TYPE SET) +if (NOT is_type_set) + message(FATAL_ERROR + "There is a cache entry for an undefined variable after " + "`mark_as_advanced`.") +endif () +get_property(type CACHE CMP0102_TEST_VARIABLE + PROPERTY TYPE) +if (NOT type STREQUAL "UNINITIALIZED") + message(FATAL_ERROR + "The cache type for CMP0102_TEST_VARIABLE is not " + "UNINITIALIZED") +endif () diff --git a/Tests/RunCMake/CMP0102/CMakeLists.txt b/Tests/RunCMake/CMP0102/CMakeLists.txt new file mode 100644 index 0000000..ef2163c --- /dev/null +++ b/Tests/RunCMake/CMP0102/CMakeLists.txt @@ -0,0 +1,3 @@ +cmake_minimum_required(VERSION 3.1) +project(${RunCMake_TEST} NONE) +include(${RunCMake_TEST}.cmake) diff --git a/Tests/RunCMake/CMP0102/RunCMakeTest.cmake b/Tests/RunCMake/CMP0102/RunCMakeTest.cmake new file mode 100644 index 0000000..9b5df74 --- /dev/null +++ b/Tests/RunCMake/CMP0102/RunCMakeTest.cmake @@ -0,0 +1,6 @@ +include(RunCMake) + +run_cmake(CMP0102-OLD) +run_cmake(CMP0102-NEW) +run_cmake(CMP0102-WARN) +run_cmake(CMP0102-WARN-Default) diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt index 0534974..95eeab8 100644 --- a/Tests/RunCMake/CMakeLists.txt +++ b/Tests/RunCMake/CMakeLists.txt @@ -115,6 +115,7 @@ if(CMAKE_SYSTEM_NAME MATCHES Darwin AND CMAKE_SHARED_LIBRARY_RUNTIME_C_FLAG) endif() add_RunCMake_test(CMP0069) add_RunCMake_test(CMP0081) +add_RunCMake_test(CMP0102) # 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