Merge topic 'option-normal-variable'

2a5f5c0e31 option: respect existing normal variable
12e6f83319 Option: Add a test that verifies interaction with normal variables
5bb3d40a28 cmOption: Remove VTK 4.0 workarounds

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

2 files changed, 47 insertions, 23 deletions

diff --git a/Source/cmOptionCommand.cxx b/Source/cmOptionCommand.cxx
index 00a2d2b..4ab0e96 100644
--- a/Source/cmOptionCommand.cxx
+++ b/Source/cmOptionCommand.cxx
@@ -2,11 +2,16 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmOptionCommand.h"
 
+#include <sstream>
+
 #include "cmAlgorithms.h"
 #include "cmMakefile.h"
+#include "cmPolicies.h"
 #include "cmState.h"
+#include "cmStateSnapshot.h"
 #include "cmStateTypes.h"
 #include "cmSystemTools.h"
+#include "cmake.h"
 
 class cmExecutionStatus;
 
@@ -14,19 +19,7 @@ class cmExecutionStatus;
 bool cmOptionCommand::InitialPass(std::vector<std::string> const& args,
                                   cmExecutionStatus&)
 {
-  bool argError = false;
-  if (args.size() < 2) {
-    argError = true;
-  }
-  // for VTK 4.0 we have to support the option command with more than 3
-  // arguments if CMAKE_MINIMUM_REQUIRED_VERSION is not defined, if
-  // CMAKE_MINIMUM_REQUIRED_VERSION is defined, then we can have stricter
-  // checking.
-  if (this->Makefile->GetDefinition("CMAKE_MINIMUM_REQUIRED_VERSION")) {
-    if (args.size() > 3) {
-      argError = true;
-    }
-  }
+  const bool argError = (args.size() < 2) || (args.size() > 3);
   if (argError) {
     std::string m = "called with incorrect number of arguments: ";
     m += cmJoin(args, " ");
@@ -34,18 +27,47 @@ bool cmOptionCommand::InitialPass(std::vector<std::string> const& args,
     return false;
   }
 
-  std::string initialValue = "Off";
-  // Now check and see if the value has been stored in the cache
-  // already, if so use that value and don't look for the program
+  // Determine the state of the option policy
+  auto status = this->Makefile->GetPolicyStatus(cmPolicies::CMP0077);
+  const char* exists =
+    this->Makefile->GetStateSnapshot().GetDefinition(args[0]);
+  switch (status) {
+    case cmPolicies::WARN:
+      if (exists) {
+        std::ostringstream w;
+        w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0077)
+          << "\n"
+             "For compatibility with older versions of CMake, option "
+             "is clearing the normal variable '"
+          << args[0] << "'.";
+        this->Makefile->IssueMessage(cmake::AUTHOR_WARNING, w.str());
+      }
+    case cmPolicies::OLD:
+      // OLD behavior does not warn.
+      break;
+    case cmPolicies::REQUIRED_ALWAYS:
+    case cmPolicies::REQUIRED_IF_USED:
+    case cmPolicies::NEW: {
+      // See if a local variable with this name already exists.
+      // If so we ignore the option command.
+      if (exists) {
+        return true;
+      }
+    } break;
+  }
+
+  // See if a cache variable with this name already exists
+  // If so just make sure the doc state is correct
   cmState* state = this->Makefile->GetState();
   const char* existingValue = state->GetCacheEntryValue(args[0]);
-  if (existingValue) {
-    if (state->GetCacheEntryType(args[0]) != cmStateEnums::UNINITIALIZED) {
-      state->SetCacheEntryProperty(args[0], "HELPSTRING", args[1]);
-      return true;
-    }
-    initialValue = existingValue;
+  if (existingValue &&
+      (state->GetCacheEntryType(args[0]) != cmStateEnums::UNINITIALIZED)) {
+    state->SetCacheEntryProperty(args[0], "HELPSTRING", args[1]);
+    return true;
   }
+
+  // Nothing in the cache so add it
+  std::string initialValue = existingValue ? existingValue : "Off";
   if (args.size() == 3) {
     initialValue = args[2];
   }
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index 1732ad6..d0d9307 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -226,7 +226,9 @@ class cmMakefile;
          0, cmPolicies::WARN)                                                 \
   SELECT(POLICY, CMP0076,                                                     \
          "target_sources() command converts relative paths to absolute.", 3,  \
-         13, 0, cmPolicies::WARN)
+         13, 0, cmPolicies::WARN)                                             \
+  SELECT(POLICY, CMP0077, "option() honors normal variables.", 3, 13, 0,      \
+         cmPolicies::WARN)
 
 #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
 #define CM_FOR_EACH_POLICY_ID(POLICY)                                         \