From dae9ec75483e8cda98aa6d6ce207e351eef0306f Mon Sep 17 00:00:00 2001 From: king Date: Thu, 10 Sep 2009 20:59:44 +0000 Subject: [PATCH] Create CMake Policy CMP0015 to fix set(CACHE) The set(CACHE) and option() commands should always expose the cache value. Previously we failed to expose the value when it was already set if a local variable definition hid it. When set to NEW, this policy tells the commands to always remove the local variable definition to expose the cache value. See issue #9008. --- Source/cmMakefile.cxx | 55 ++++++++++++++++++++++++++++++++++++++++++++-- Source/cmMakefile.h | 9 ++++++-- Source/cmOptionCommand.cxx | 5 +++-- Source/cmPolicies.cxx | 31 ++++++++++++++++++++++++++ Source/cmPolicies.h | 5 +++-- Source/cmSetCommand.cxx | 5 +++-- Source/cmSetCommand.h | 10 +++++---- 7 files changed, 106 insertions(+), 14 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 5cfde431e..00b990034 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -3,8 +3,8 @@ Program: CMake - Cross-Platform Makefile Generator Module: $RCSfile: cmMakefile.cxx,v $ Language: C++ - Date: $Date: 2009-09-10 20:59:36 $ - Version: $Revision: 1.518 $ + Date: $Date: 2009-09-10 20:59:44 $ + Version: $Revision: 1.519 $ Copyright (c) 2002 Kitware, Inc., Insight Consortium. All rights reserved. See Copyright.txt or http://www.cmake.org/HTML/Copyright.html for details. @@ -1662,6 +1662,57 @@ void cmMakefile::AddDefinition(const char* name, const char* value) #endif } +//---------------------------------------------------------------------------- +void cmMakefile::UseCacheDefinition(cmCacheManager::CacheIterator const& it) +{ + // Check for a local definition that might hide the cache value. + const char* name = it.GetName(); + const char* def = this->Internal->VarStack.top().Get(name); + if(!def) + { + return; + } + + // If the visible value will change then check policy CMP0015. + const char* cache = it.GetValue(); + if(strcmp(def, cache) != 0) + { + cmOStringStream e; + switch (this->GetPolicyStatus(cmPolicies::CMP0015)) + { + case cmPolicies::WARN: + e << "Local variable \"" << name << "\" is set to\n" + << " " << def << "\n" + << "but the CACHE entry of the same name is set to\n" + << " " << cache << "\n" + << "The local variable is hiding the cache value." + << "\n" + << this->GetPolicies()->GetPolicyWarning(cmPolicies::CMP0015); + this->IssueMessage(cmake::AUTHOR_WARNING, e.str()); + case cmPolicies::OLD: + // OLD behavior is to leave local definition. + return; + case cmPolicies::REQUIRED_IF_USED: + case cmPolicies::REQUIRED_ALWAYS: + e << "Local variable \"" << name << "\" is set to\n" + << " " << def << "\n" + << "but the CACHE entry of the same name is set to\n" + << " " << cache << "\n" + << "This command is removing the local variable to expose " + << "the cache value." + << "\n" + << this->GetPolicies()->GetRequiredPolicyError(cmPolicies::CMP0015); + this->IssueMessage(cmake::FATAL_ERROR, e.str()); + case cmPolicies::NEW: + // NEW behavior is to remove local definition (done below). + break; + } + } + + // Remove the local definition to make the cache value visible. + this->RemoveDefinition(name); +} + void cmMakefile::AddCacheDefinition(const char* name, const char* value, const char* doc, diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index d3db7b7cc..2ae7d1456 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -3,8 +3,8 @@ Program: CMake - Cross-Platform Makefile Generator Module: $RCSfile: cmMakefile.h,v $ Language: C++ - Date: $Date: 2009-09-10 20:59:36 $ - Version: $Revision: 1.258 $ + Date: $Date: 2009-09-10 20:59:44 $ + Version: $Revision: 1.259 $ Copyright (c) 2002 Kitware, Inc., Insight Consortium. All rights reserved. See Copyright.txt or http://www.cmake.org/HTML/Copyright.html for details. @@ -291,6 +291,11 @@ public: bool force = false); /** + * Update the variable scope to make the cache definition visible. + */ + void UseCacheDefinition(cmCacheManager::CacheIterator const& it); + + /** * Add bool variable definition to the build. */ void AddDefinition(const char* name, bool); diff --git a/Source/cmOptionCommand.cxx b/Source/cmOptionCommand.cxx index 44a34dc3d..c39f6ecb9 100644 --- a/Source/cmOptionCommand.cxx +++ b/Source/cmOptionCommand.cxx @@ -3,8 +3,8 @@ Program: CMake - Cross-Platform Makefile Generator Module: $RCSfile: cmOptionCommand.cxx,v $ Language: C++ - Date: $Date: 2009-09-10 20:59:36 $ - Version: $Revision: 1.24 $ + Date: $Date: 2009-09-10 20:59:44 $ + Version: $Revision: 1.25 $ Copyright (c) 2002 Kitware, Inc., Insight Consortium. All rights reserved. See Copyright.txt or http://www.cmake.org/HTML/Copyright.html for details. @@ -58,6 +58,7 @@ bool cmOptionCommand if ( it.GetType() != cmCacheManager::UNINITIALIZED ) { it.SetProperty("HELPSTRING", args[1].c_str()); + this->Makefile->UseCacheDefinition(it); return true; } if ( it.GetValue() ) diff --git a/Source/cmPolicies.cxx b/Source/cmPolicies.cxx index 576ccd7aa..95bf972b3 100644 --- a/Source/cmPolicies.cxx +++ b/Source/cmPolicies.cxx @@ -399,6 +399,37 @@ cmPolicies::cmPolicies() "The OLD behavior for this policy is to silently ignore the problem. " "The NEW behavior for this policy is to report an error.", 2,7,20090902, cmPolicies::WARN); + + this->DefinePolicy( + CMP0015, "CMP0015", + "The set() CACHE mode and option() command make the cache value visible.", + "In CMake 2.6 and below the CACHE mode of the set() command and the " + "option() command did not expose the value from the named cache entry " + "if it was already set both in the cache and as a local variable. " + "This led to subtle differences between first and later configurations " + "because a conflicting local variable would be overridden only when the " + "cache value was first created. " + "For example, the code\n" + " set(x 1)\n" + " set(before ${x})\n" + " set(x 2 CACHE STRING \"X\")\n" + " set(after ${x})\n" + " message(STATUS \"${before},${after}\")\n" + "would print \"1,2\" on the first run and \"1,1\" on future runs." + "\n" + "CMake 2.8.0 and above prefer to expose the cache value in all cases by " + "removing the local variable definition, but this changes behavior in " + "subtle cases when the local variable has a different value than that " + "exposed from the cache. " + "The example above will always print \"1,2\"." + "\n" + "This policy determines whether the commands should always expose the " + "cache value. " + "The OLD behavior for this policy is to leave conflicting local " + "variable values untouched and hide the true cache value. " + "The NEW behavior for this policy is to always expose the cache value.", + 2,7,20090910, cmPolicies::WARN); + } cmPolicies::~cmPolicies() diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h index 71d308fe7..02368e65b 100644 --- a/Source/cmPolicies.h +++ b/Source/cmPolicies.h @@ -3,8 +3,8 @@ Program: CMake - Cross-Platform Makefile Generator Module: $RCSfile: cmPolicies.h,v $ Language: C++ - Date: $Date: 2009-09-03 12:27:12 $ - Version: $Revision: 1.25 $ + Date: $Date: 2009-09-10 20:59:44 $ + Version: $Revision: 1.26 $ Copyright (c) 2002 Kitware, Inc., Insight Consortium. All rights reserved. See Copyright.txt or http://www.cmake.org/HTML/Copyright.html for details. @@ -55,6 +55,7 @@ public: CMP0012, // Strong handling of boolean constants CMP0013, // Duplicate binary directories not allowed CMP0014, // Input directories must have CMakeLists.txt + CMP0015, // set(CACHE) and option() make CACHE value visible // Always the last entry. Useful mostly to avoid adding a comma // the last policy when adding a new one. diff --git a/Source/cmSetCommand.cxx b/Source/cmSetCommand.cxx index 467bb4dcb..3d125fea8 100644 --- a/Source/cmSetCommand.cxx +++ b/Source/cmSetCommand.cxx @@ -3,8 +3,8 @@ Program: CMake - Cross-Platform Makefile Generator Module: $RCSfile: cmSetCommand.cxx,v $ Language: C++ - Date: $Date: 2009-05-10 20:07:34 $ - Version: $Revision: 1.35 $ + Date: $Date: 2009-09-10 20:59:44 $ + Version: $Revision: 1.36 $ Copyright (c) 2002 Kitware, Inc., Insight Consortium. All rights reserved. See Copyright.txt or http://www.cmake.org/HTML/Copyright.html for details. @@ -160,6 +160,7 @@ bool cmSetCommand // or the makefile if(cache && type != cmCacheManager::INTERNAL && !force) { + this->Makefile->UseCacheDefinition(it); return true; } } diff --git a/Source/cmSetCommand.h b/Source/cmSetCommand.h index 6df288406..229f6395a 100644 --- a/Source/cmSetCommand.h +++ b/Source/cmSetCommand.h @@ -3,8 +3,8 @@ Program: CMake - Cross-Platform Makefile Generator Module: $RCSfile: cmSetCommand.h,v $ Language: C++ - Date: $Date: 2009-09-03 19:29:29 $ - Version: $Revision: 1.22 $ + Date: $Date: 2009-09-10 20:59:45 $ + Version: $Revision: 1.23 $ Copyright (c) 2002 Kitware, Inc., Insight Consortium. All rights reserved. See Copyright.txt or http://www.cmake.org/HTML/Copyright.html for details. @@ -69,8 +69,10 @@ public: " set( \n" " [[CACHE [FORCE]] | PARENT_SCOPE])\n" "Within CMake sets to the value . is expanded" - " before is set to it. If CACHE is present, then the " - " is put in the cache. and are then " + " before is set to it. If CACHE is present and " + "is not yet in the cache, then is put in the cache. If it is " + "already in the cache, is assigned the value stored in the " + "cache. If CACHE is present, also and are " "required. is used by the CMake GUI to choose a widget with " "which the user sets a value. The value for may be one of\n" " FILEPATH = File chooser dialog.\n" -- 2.11.4.GIT