From 5bb3d40a289c44f350aa68f32e5ef0c1ad7f13b1 Mon Sep 17 00:00:00 2001
From: Robert Maynard <robert.maynard@kitware.com>
Date: Mon, 18 Jun 2018 10:40:36 -0400
Subject: cmOption: Remove VTK 4.0 workarounds
CMake has no reason to have special logic to fix bad logic
within VTK 4.0.
---
Source/cmOptionCommand.cxx | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/Source/cmOptionCommand.cxx b/Source/cmOptionCommand.cxx
index 00a2d2b..13bcd03 100644
--- a/Source/cmOptionCommand.cxx
+++ b/Source/cmOptionCommand.cxx
@@ -14,19 +14,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, " ");
--
cgit v0.12
From 12e6f83319d089b3295b4211d0d4810575e8f7d5 Mon Sep 17 00:00:00 2001
From: Robert Maynard <robert.maynard@kitware.com>
Date: Thu, 21 Jun 2018 14:52:12 -0400
Subject: Option: Add a test that verifies interaction with normal variables
---
Tests/CMakeOnly/CMakeLists.txt | 1 +
Tests/CMakeOnly/option/CMakeLists.txt | 16 ++++++++++++++++
2 files changed, 17 insertions(+)
create mode 100644 Tests/CMakeOnly/option/CMakeLists.txt
diff --git a/Tests/CMakeOnly/CMakeLists.txt b/Tests/CMakeOnly/CMakeLists.txt
index 204d54c..8a3a83b 100644
--- a/Tests/CMakeOnly/CMakeLists.txt
+++ b/Tests/CMakeOnly/CMakeLists.txt
@@ -49,6 +49,7 @@ add_CMakeOnly_test(TargetScope)
add_CMakeOnly_test(find_library)
add_CMakeOnly_test(find_path)
+add_CMakeOnly_test(option)
add_test(CMakeOnly.ProjectInclude ${CMAKE_CMAKE_COMMAND}
-DTEST=ProjectInclude
diff --git a/Tests/CMakeOnly/option/CMakeLists.txt b/Tests/CMakeOnly/option/CMakeLists.txt
new file mode 100644
index 0000000..d040fb5
--- /dev/null
+++ b/Tests/CMakeOnly/option/CMakeLists.txt
@@ -0,0 +1,16 @@
+cmake_minimum_required (VERSION 2.8)
+project(OptionTest NONE)
+
+#Verify that normal variable take precedence over cache variables
+option(OPT_LOCAL_VAR1 "TEST_VAR" ON)
+set(OPT_LOCAL_VAR1 FALSE)
+if(OPT_LOCAL_VAR1)
+ message(FATAL_ERROR "local variable didn't take precedence over cache variable")
+endif()
+
+#Verify that option overwrites existing normal variable
+set(OPT_LOCAL_VAR2 FALSE)
+option(OPT_LOCAL_VAR2 "TEST_VAR" ON)
+if(NOT OPT_LOCAL_VAR2)
+ message(FATAL_ERROR "option failed to overwrite existing normal variable")
+endif()
--
cgit v0.12
From 2a5f5c0e316d415e1b8207348b34761d34f191ae Mon Sep 17 00:00:00 2001
From: Robert Maynard <robert.maynard@kitware.com>
Date: Thu, 21 Jun 2018 14:53:11 -0400
Subject: option: respect existing normal variable
Add policy CMP0077 to change this behavior in a compatible way.
---
Help/command/option.rst | 4 ++-
Help/manual/cmake-policies.7.rst | 1 +
Help/policy/CMP0077.rst | 16 +++++++++
Help/release/dev/option-normal-variable.rst | 5 +++
Source/cmOptionCommand.cxx | 52 ++++++++++++++++++++++-----
Source/cmPolicies.h | 4 ++-
Tests/CMakeOnly/CMakeLists.txt | 1 -
Tests/CMakeOnly/option/CMakeLists.txt | 16 ---------
Tests/RunCMake/CMakeLists.txt | 1 +
Tests/RunCMake/option/CMP0077-NEW.cmake | 14 ++++++++
Tests/RunCMake/option/CMP0077-OLD.cmake | 9 +++++
Tests/RunCMake/option/CMP0077-WARN-stderr.txt | 7 ++++
Tests/RunCMake/option/CMP0077-WARN.cmake | 5 +++
Tests/RunCMake/option/CMakeLists.txt | 3 ++
Tests/RunCMake/option/RunCMakeTest.cmake | 5 +++
15 files changed, 115 insertions(+), 28 deletions(-)
create mode 100644 Help/policy/CMP0077.rst
create mode 100644 Help/release/dev/option-normal-variable.rst
delete mode 100644 Tests/CMakeOnly/option/CMakeLists.txt
create mode 100644 Tests/RunCMake/option/CMP0077-NEW.cmake
create mode 100644 Tests/RunCMake/option/CMP0077-OLD.cmake
create mode 100644 Tests/RunCMake/option/CMP0077-WARN-stderr.txt
create mode 100644 Tests/RunCMake/option/CMP0077-WARN.cmake
create mode 100644 Tests/RunCMake/option/CMakeLists.txt
create mode 100644 Tests/RunCMake/option/RunCMakeTest.cmake
diff --git a/Help/command/option.rst b/Help/command/option.rst
index 91cd0a7..4fabb87 100644
--- a/Help/command/option.rst
+++ b/Help/command/option.rst
@@ -9,7 +9,9 @@ Provides an option that the user can optionally select.
[initial value])
Provide an option for the user to select as ``ON`` or ``OFF``. If no
-initial value is provided, ``OFF`` is used.
+initial value is provided, ``OFF`` is used. If the option is already
+set as a normal variable then the command does nothing
+(see policy :policy:`CMP0077`).
If you have options that depend on the values of other options, see
the module help for :module:`CMakeDependentOption`.
diff --git a/Help/manual/cmake-policies.7.rst b/Help/manual/cmake-policies.7.rst
index 6179a7c..b2eeb3a 100644
--- a/Help/manual/cmake-policies.7.rst
+++ b/Help/manual/cmake-policies.7.rst
@@ -57,6 +57,7 @@ Policies Introduced by CMake 3.13
.. toctree::
:maxdepth: 1
+ CMP0077: option() honors normal variables. </policy/CMP0077>
CMP0076: target_sources() command converts relative paths to absolute. </policy/CMP0076>
Policies Introduced by CMake 3.12
diff --git a/Help/policy/CMP0077.rst b/Help/policy/CMP0077.rst
new file mode 100644
index 0000000..8efe198
--- /dev/null
+++ b/Help/policy/CMP0077.rst
@@ -0,0 +1,16 @@
+CMP0077
+-------
+
+:command:`option` honors normal variables.
+
+The ``OLD`` behavior for this policy is to clear any existing normal variables
+with the same name. The ``NEW`` behavior for this policy is to not create
+a cache entry or modify any existing normal variables if a normal variable
+with the same name already exists.
+
+This policy was introduced in CMake version 3.13. CMake version
+|release| warns when the policy is not set and uses ``OLD`` behavior.
+Use the :command:`cmake_policy` command to set it to ``OLD`` or ``NEW``
+explicitly.
+
+.. include:: DEPRECATED.txt
diff --git a/Help/release/dev/option-normal-variable.rst b/Help/release/dev/option-normal-variable.rst
new file mode 100644
index 0000000..19b9a64
--- /dev/null
+++ b/Help/release/dev/option-normal-variable.rst
@@ -0,0 +1,5 @@
+option-normal-variable
+----------------------
+
+* The :command:`option` command now honors existing normal variables instead
+ of replacing them with a cache entry. See policy :policy:`CMP0077`.
diff --git a/Source/cmOptionCommand.cxx b/Source/cmOptionCommand.cxx
index 13bcd03..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;
@@ -22,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) \
diff --git a/Tests/CMakeOnly/CMakeLists.txt b/Tests/CMakeOnly/CMakeLists.txt
index 8a3a83b..204d54c 100644
--- a/Tests/CMakeOnly/CMakeLists.txt
+++ b/Tests/CMakeOnly/CMakeLists.txt
@@ -49,7 +49,6 @@ add_CMakeOnly_test(TargetScope)
add_CMakeOnly_test(find_library)
add_CMakeOnly_test(find_path)
-add_CMakeOnly_test(option)
add_test(CMakeOnly.ProjectInclude ${CMAKE_CMAKE_COMMAND}
-DTEST=ProjectInclude
diff --git a/Tests/CMakeOnly/option/CMakeLists.txt b/Tests/CMakeOnly/option/CMakeLists.txt
deleted file mode 100644
index d040fb5..0000000
--- a/Tests/CMakeOnly/option/CMakeLists.txt
+++ /dev/null
@@ -1,16 +0,0 @@
-cmake_minimum_required (VERSION 2.8)
-project(OptionTest NONE)
-
-#Verify that normal variable take precedence over cache variables
-option(OPT_LOCAL_VAR1 "TEST_VAR" ON)
-set(OPT_LOCAL_VAR1 FALSE)
-if(OPT_LOCAL_VAR1)
- message(FATAL_ERROR "local variable didn't take precedence over cache variable")
-endif()
-
-#Verify that option overwrites existing normal variable
-set(OPT_LOCAL_VAR2 FALSE)
-option(OPT_LOCAL_VAR2 "TEST_VAR" ON)
-if(NOT OPT_LOCAL_VAR2)
- message(FATAL_ERROR "option failed to overwrite existing normal variable")
-endif()
diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt
index daf3940..e55d97d 100644
--- a/Tests/RunCMake/CMakeLists.txt
+++ b/Tests/RunCMake/CMakeLists.txt
@@ -239,6 +239,7 @@ add_RunCMake_test(include_directories)
add_RunCMake_test(include_guard)
add_RunCMake_test(list)
add_RunCMake_test(message)
+add_RunCMake_test(option)
add_RunCMake_test(project -DCMake_TEST_RESOURCES=${CMake_TEST_RESOURCES})
add_RunCMake_test(return)
add_RunCMake_test(separate_arguments)
diff --git a/Tests/RunCMake/option/CMP0077-NEW.cmake b/Tests/RunCMake/option/CMP0077-NEW.cmake
new file mode 100644
index 0000000..d4c518b
--- /dev/null
+++ b/Tests/RunCMake/option/CMP0077-NEW.cmake
@@ -0,0 +1,14 @@
+
+#Verify that option DOESN'T overwrite existing normal variable when the policy
+#is set to NEW
+cmake_policy(SET CMP0077 NEW)
+set(OPT_LOCAL_VAR FALSE)
+option(OPT_LOCAL_VAR "TEST_VAR" ON)
+if(OPT_LOCAL_VAR)
+ message(FATAL_ERROR "option failed to overwrite existing normal variable")
+endif()
+
+get_property(_exists_in_cache CACHE OPT_LOCAL_VAR PROPERTY VALUE SET)
+if(_exists_in_cache)
+ message(FATAL_ERROR "value should not exist in cache as it was already a local variable")
+endif()
diff --git a/Tests/RunCMake/option/CMP0077-OLD.cmake b/Tests/RunCMake/option/CMP0077-OLD.cmake
new file mode 100644
index 0000000..4c52d4b
--- /dev/null
+++ b/Tests/RunCMake/option/CMP0077-OLD.cmake
@@ -0,0 +1,9 @@
+
+#Verify that option overwrites existing normal variable when the policy
+#is set to OLD
+cmake_policy(SET CMP0077 OLD)
+set(OPT_LOCAL_VAR FALSE)
+option(OPT_LOCAL_VAR "TEST_VAR" ON)
+if(NOT OPT_LOCAL_VAR)
+ message(FATAL_ERROR "option failed to overwrite existing normal variable")
+endif()
diff --git a/Tests/RunCMake/option/CMP0077-WARN-stderr.txt b/Tests/RunCMake/option/CMP0077-WARN-stderr.txt
new file mode 100644
index 0000000..0d02ffb
--- /dev/null
+++ b/Tests/RunCMake/option/CMP0077-WARN-stderr.txt
@@ -0,0 +1,7 @@
+CMake Warning \(dev\) at CMP0077-WARN.cmake:5 \(option\):
+ Policy CMP0077 is not set: option\(\) honors normal variables. Run "cmake
+ --help-policy CMP0077" for policy details. Use the cmake_policy command to
+ set the policy and suppress this warning.
+
+ For compatibility with older versions of CMake, option is clearing the
+ normal variable 'OPT_LOCAL_VAR'.
diff --git a/Tests/RunCMake/option/CMP0077-WARN.cmake b/Tests/RunCMake/option/CMP0077-WARN.cmake
new file mode 100644
index 0000000..7f99456
--- /dev/null
+++ b/Tests/RunCMake/option/CMP0077-WARN.cmake
@@ -0,0 +1,5 @@
+
+#Verify that option overwrites existing normal variable when the policy
+#is set to OLD
+set(OPT_LOCAL_VAR FALSE)
+option(OPT_LOCAL_VAR "TEST_VAR" ON)
diff --git a/Tests/RunCMake/option/CMakeLists.txt b/Tests/RunCMake/option/CMakeLists.txt
new file mode 100644
index 0000000..11dc49a
--- /dev/null
+++ b/Tests/RunCMake/option/CMakeLists.txt
@@ -0,0 +1,3 @@
+cmake_minimum_required(VERSION 3.12)
+project(${RunCMake_TEST} CXX)
+include(${RunCMake_TEST}.cmake)
diff --git a/Tests/RunCMake/option/RunCMakeTest.cmake b/Tests/RunCMake/option/RunCMakeTest.cmake
new file mode 100644
index 0000000..0501624
--- /dev/null
+++ b/Tests/RunCMake/option/RunCMakeTest.cmake
@@ -0,0 +1,5 @@
+include(RunCMake)
+
+run_cmake(CMP0077-OLD)
+run_cmake(CMP0077-NEW)
+run_cmake(CMP0077-WARN)
--
cgit v0.12