cmMarkAsAdvancedCommand: ignore variables which don't exist in the cache

Fixes: #18331

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`.

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. </policy/CMP0102>
    CMP0101: target_compile_options honors BEFORE keyword in all scopes. </policy/CMP0101>
    CMP0100: Let AUTOMOC and AUTOUIC process .hh header files. </policy/CMP0100>
    CMP0099: Link properties are transitive over private dependency on static libraries. </policy/CMP0099>

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

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

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<std::string> 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;

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)                                         \

Tests/RunCMake/CMP0102/CMP0102-Common.cmake

@@ -0,0 +1,2 @@
+
+mark_as_advanced(CMP0102_TEST_VARIABLE)

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 ()

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 ()

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 ()

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.

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 ()

Tests/RunCMake/CMP0102/CMakeLists.txt

@@ -0,0 +1,3 @@
+cmake_minimum_required(VERSION 3.1)
+project(${RunCMake_TEST} NONE)
+include(${RunCMake_TEST}.cmake)

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)

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