From c332e0bf3c4e619358322bd0d0961af45653eb5b Mon Sep 17 00:00:00 2001
From: Brad King <brad.king@kitware.com>
Date: Thu, 22 Jan 2009 13:18:40 -0500
Subject: ENH: Isolate policy changes in included scripts
Isolation of policy changes inside scripts is important for protecting
the including context. This teaches include() and find_package() to
imply a cmake_policy(PUSH) and cmake_policy(POP) around the scripts they
load, with a NO_POLICY_SCOPE option to disable the behavior. This also
creates CMake Policy CMP0011 to provide compatibility. See issue #8192.
---
Source/cmCMakePolicyCommand.h | 3 ++
Source/cmFindPackageCommand.cxx | 30 ++++++++---
Source/cmFindPackageCommand.h | 4 +-
Source/cmIncludeCommand.cxx | 8 ++-
Source/cmIncludeCommand.h | 10 ++--
Source/cmMakefile.cxx | 107 +++++++++++++++++++++++++++++++++++++--
Source/cmMakefile.h | 3 +-
Source/cmPolicies.cxx | 20 ++++++++
Source/cmPolicies.h | 1 +
Tests/PolicyScope/Bar.cmake | 8 +++
Tests/PolicyScope/CMakeLists.txt | 33 +++++++++++-
Tests/PolicyScope/FindFoo.cmake | 2 +
12 files changed, 210 insertions(+), 19 deletions(-)
create mode 100644 Tests/PolicyScope/Bar.cmake
create mode 100644 Tests/PolicyScope/FindFoo.cmake
diff --git a/Source/cmCMakePolicyCommand.h b/Source/cmCMakePolicyCommand.h
index e74a6cf..1f70247 100644
--- a/Source/cmCMakePolicyCommand.h
+++ b/Source/cmCMakePolicyCommand.h
@@ -117,6 +117,9 @@ public:
"cmake_policy command affect only the top of the stack. "
"A new entry on the policy stack is managed automatically for each "
"subdirectory to protect its parents and siblings. "
+ "CMake also manages a new entry for scripts loaded by include() and "
+ "find_package() commands except when invoked with the NO_POLICY_SCOPE "
+ "option (see also policy CMP0011). "
"The cmake_policy command provides an interface to manage custom "
"entries on the policy stack:\n"
" cmake_policy(PUSH)\n"
diff --git a/Source/cmFindPackageCommand.cxx b/Source/cmFindPackageCommand.cxx
index a57a4f1..5b61180 100644
--- a/Source/cmFindPackageCommand.cxx
+++ b/Source/cmFindPackageCommand.cxx
@@ -64,6 +64,7 @@ cmFindPackageCommand::cmFindPackageCommand()
this->NoModule = false;
this->DebugMode = false;
this->UseLib64Paths = false;
+ this->PolicyScope = true;
this->VersionMajor = 0;
this->VersionMinor = 0;
this->VersionPatch = 0;
@@ -77,7 +78,8 @@ cmFindPackageCommand::cmFindPackageCommand()
this->VersionFoundCount = 0;
this->CommandDocumentation =
" find_package(<package> [version] [EXACT] [QUIET]\n"
- " [[REQUIRED|COMPONENTS] [components...]])\n"
+ " [[REQUIRED|COMPONENTS] [components...]]\n"
+ " [NO_POLICY_SCOPE])\n"
"Finds and loads settings from an external project. "
"<package>_FOUND will be set to indicate whether the package was found. "
"When the package is found package-specific information is provided "
@@ -116,6 +118,7 @@ cmFindPackageCommand::cmFindPackageCommand()
"The complete Config mode command signature is:\n"
" find_package(<package> [version] [EXACT] [QUIET]\n"
" [[REQUIRED|COMPONENTS] [components...]] [NO_MODULE]\n"
+ " [NO_POLICY_SCOPE]\n"
" [NAMES name1 [name2 ...]]\n"
" [CONFIGS config1 [config2 ...]]\n"
" [HINTS path1 [path2 ... ]]\n"
@@ -290,6 +293,11 @@ cmFindPackageCommand::cmFindPackageCommand()
this->CommandDocumentation += this->GenericDocumentationMacPolicy;
this->CommandDocumentation += this->GenericDocumentationRootPath;
this->CommandDocumentation += this->GenericDocumentationPathsOrder;
+ this->CommandDocumentation +=
+ "\n"
+ "See the cmake_policy() command documentation for discussion of the "
+ "NO_POLICY_SCOPE option."
+ ;
}
//----------------------------------------------------------------------------
@@ -406,6 +414,12 @@ bool cmFindPackageCommand
this->Compatibility_1_6 = false;
doing = DoingConfigs;
}
+ else if(args[i] == "NO_POLICY_SCOPE")
+ {
+ this->PolicyScope = false;
+ this->Compatibility_1_6 = false;
+ doing = DoingNone;
+ }
else if(args[i] == "NO_CMAKE_BUILDS_PATH")
{
this->NoBuilds = true;
@@ -675,7 +689,7 @@ bool cmFindPackageCommand::FindModule(bool& found)
std::string var = this->Name;
var += "_FIND_MODULE";
this->Makefile->AddDefinition(var.c_str(), "1");
- bool result = this->ReadListFile(mfile.c_str());
+ bool result = this->ReadListFile(mfile.c_str(), DoPolicyScope);
this->Makefile->RemoveDefinition(var.c_str());
return result;
}
@@ -753,7 +767,7 @@ bool cmFindPackageCommand::HandlePackageMode()
this->StoreVersionFound();
// Parse the configuration file.
- if(this->ReadListFile(this->FileFound.c_str()))
+ if(this->ReadListFile(this->FileFound.c_str(), DoPolicyScope))
{
// The package has been found.
found = true;
@@ -963,9 +977,10 @@ bool cmFindPackageCommand::FindAppBundleConfig()
}
//----------------------------------------------------------------------------
-bool cmFindPackageCommand::ReadListFile(const char* f)
+bool cmFindPackageCommand::ReadListFile(const char* f, PolicyScopeRule psr)
{
- if(this->Makefile->ReadListFile(this->Makefile->GetCurrentListFile(),f))
+ if(this->Makefile->ReadListFile(this->Makefile->GetCurrentListFile(), f, 0,
+ !this->PolicyScope || psr == NoPolicyScope))
{
return true;
}
@@ -1304,9 +1319,10 @@ bool cmFindPackageCommand::CheckVersionFile(std::string const& version_file)
sprintf(buf, "%u", this->VersionCount);
this->Makefile->AddDefinition("PACKAGE_FIND_VERSION_COUNT", buf);
- // Load the version check file.
+ // Load the version check file. Pass NoPolicyScope because we do
+ // our own policy push/pop independent of CMP0011.
bool suitable = false;
- if(this->ReadListFile(version_file.c_str()))
+ if(this->ReadListFile(version_file.c_str(), NoPolicyScope))
{
// Check the output variables.
bool okay = this->Makefile->IsOn("PACKAGE_VERSION_EXACT");
diff --git a/Source/cmFindPackageCommand.h b/Source/cmFindPackageCommand.h
index 3fc8f65..faeb04d 100644
--- a/Source/cmFindPackageCommand.h
+++ b/Source/cmFindPackageCommand.h
@@ -82,7 +82,8 @@ private:
bool FindPrefixedConfig();
bool FindFrameworkConfig();
bool FindAppBundleConfig();
- bool ReadListFile(const char* f);
+ enum PolicyScopeRule { NoPolicyScope, DoPolicyScope };
+ bool ReadListFile(const char* f, PolicyScopeRule psr);
void StoreVersionFound();
void ComputePrefixes();
@@ -132,6 +133,7 @@ private:
bool NoBuilds;
bool DebugMode;
bool UseLib64Paths;
+ bool PolicyScope;
std::vector<std::string> Names;
std::vector<std::string> Configs;
};
diff --git a/Source/cmIncludeCommand.cxx b/Source/cmIncludeCommand.cxx
index f8d6dc2..d1a51ef 100644
--- a/Source/cmIncludeCommand.cxx
+++ b/Source/cmIncludeCommand.cxx
@@ -28,6 +28,7 @@ bool cmIncludeCommand
return false;
}
bool optional = false;
+ bool noPolicyScope = false;
std::string fname = args[0];
std::string resultVarName;
@@ -60,6 +61,10 @@ bool cmIncludeCommand
return false;
}
}
+ else if(args[i] == "NO_POLICY_SCOPE")
+ {
+ noPolicyScope = true;
+ }
else if(i > 1) // compat.: in previous cmake versions the second
// parameter was ignore if it wasn't "OPTIONAL"
{
@@ -84,7 +89,8 @@ bool cmIncludeCommand
std::string fullFilePath;
bool readit =
this->Makefile->ReadListFile( this->Makefile->GetCurrentListFile(),
- fname.c_str(), &fullFilePath );
+ fname.c_str(), &fullFilePath,
+ noPolicyScope);
// add the location of the included file if a result variable was given
if (resultVarName.size())
diff --git a/Source/cmIncludeCommand.h b/Source/cmIncludeCommand.h
index aaeea39..89df06c 100644
--- a/Source/cmIncludeCommand.h
+++ b/Source/cmIncludeCommand.h
@@ -69,8 +69,8 @@ public:
virtual const char* GetFullDocumentation()
{
return
- " include(file1 [OPTIONAL] [RESULT_VARIABLE <VAR>])\n"
- " include(module [OPTIONAL] [RESULT_VARIABLE <VAR>])\n"
+ " include(<file|module> [OPTIONAL] [RESULT_VARIABLE <VAR>]\n"
+ " [NO_POLICY_SCOPE])\n"
"Reads CMake listfile code from the given file. Commands in the file "
"are processed immediately as if they were written in place of the "
"include command. If OPTIONAL is present, then no error "
@@ -78,7 +78,11 @@ public:
"the variable will be set to the full filename which "
"has been included or NOTFOUND if it failed.\n"
"If a module is specified instead of a file, the file with name "
- "<modulename>.cmake is searched in the CMAKE_MODULE_PATH.";
+ "<modulename>.cmake is searched in the CMAKE_MODULE_PATH."
+ "\n"
+ "See the cmake_policy() command documentation for discussion of the "
+ "NO_POLICY_SCOPE option."
+ ;
}
cmTypeMacro(cmIncludeCommand, cmCommand);
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index 4442bd3..fabfd35 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -448,18 +448,53 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
class cmMakefile::IncludeScope
{
public:
- IncludeScope(cmMakefile* mf);
+ IncludeScope(cmMakefile* mf, const char* fname, bool noPolicyScope);
~IncludeScope();
void Quiet() { this->ReportError = false; }
private:
cmMakefile* Makefile;
+ const char* File;
+ bool NoPolicyScope;
+ bool CheckCMP0011;
bool ReportError;
+ void EnforceCMP0011();
};
//----------------------------------------------------------------------------
-cmMakefile::IncludeScope::IncludeScope(cmMakefile* mf):
- Makefile(mf), ReportError(true)
+cmMakefile::IncludeScope::IncludeScope(cmMakefile* mf, const char* fname,
+ bool noPolicyScope):
+ Makefile(mf), File(fname), NoPolicyScope(noPolicyScope),
+ CheckCMP0011(false), ReportError(true)
{
+ if(!this->NoPolicyScope)
+ {
+ // Check CMP0011 to determine the policy scope type.
+ switch (this->Makefile->GetPolicyStatus(cmPolicies::CMP0011))
+ {
+ case cmPolicies::WARN:
+ // We need to push a scope to detect whether the script sets
+ // any policies that would affect the includer and therefore
+ // requires a warning. We use a weak scope to simulate OLD
+ // behavior by allowing policy changes to affect the includer.
+ this->Makefile->PushPolicy(true);
+ this->CheckCMP0011 = true;
+ break;
+ case cmPolicies::OLD:
+ // OLD behavior is to not push a scope at all.
+ this->NoPolicyScope = true;
+ break;
+ case cmPolicies::REQUIRED_IF_USED:
+ case cmPolicies::REQUIRED_ALWAYS:
+ // We should never make this policy required, but we handle it
+ // here just in case.
+ this->CheckCMP0011 = true;
+ case cmPolicies::NEW:
+ // NEW behavior is to push a (strong) scope.
+ this->Makefile->PushPolicy();
+ break;
+ }
+ }
+
// The included file cannot pop our policy scope.
this->Makefile->PushPolicyBarrier();
}
@@ -469,6 +504,67 @@ cmMakefile::IncludeScope::~IncludeScope()
{
// Enforce matching policy scopes inside the included file.
this->Makefile->PopPolicyBarrier(this->ReportError);
+
+ if(!this->NoPolicyScope)
+ {
+ // If we need to enforce policy CMP0011 then the top entry is the
+ // one we pushed above. If the entry is empty, then the included
+ // script did not set any policies that might affect the includer so
+ // we do not need to enforce the policy.
+ if(this->CheckCMP0011 && this->Makefile->PolicyStack.back().empty())
+ {
+ this->CheckCMP0011 = false;
+ }
+
+ // Pop the scope we pushed for the script.
+ this->Makefile->PopPolicy();
+
+ // We enforce the policy after the script's policy stack entry has
+ // been removed.
+ if(this->CheckCMP0011)
+ {
+ this->EnforceCMP0011();
+ }
+ }
+}
+
+//----------------------------------------------------------------------------
+void cmMakefile::IncludeScope::EnforceCMP0011()
+{
+ // We check the setting of this policy again because the included
+ // script might actually set this policy for its includer.
+ cmPolicies* policies = this->Makefile->GetPolicies();
+ switch (this->Makefile->GetPolicyStatus(cmPolicies::CMP0011))
+ {
+ case cmPolicies::WARN:
+ // Warn because the user did not set this policy.
+ {
+ cmOStringStream w;
+ w << policies->GetPolicyWarning(cmPolicies::CMP0011) << "\n"
+ << "The included script\n " << this->File << "\n"
+ << "affects policy settings. "
+ << "CMake is implying the NO_POLICY_SCOPE option for compatibility, "
+ << "so the effects are applied to the including context.";
+ this->Makefile->IssueMessage(cmake::AUTHOR_WARNING, w.str());
+ }
+ break;
+ case cmPolicies::REQUIRED_IF_USED:
+ case cmPolicies::REQUIRED_ALWAYS:
+ {
+ cmOStringStream e;
+ e << policies->GetRequiredPolicyError(cmPolicies::CMP0011) << "\n"
+ << "The included script\n " << this->File << "\n"
+ << "affects policy settings, so it requires this policy to be set.";
+ this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str());
+ }
+ break;
+ case cmPolicies::OLD:
+ case cmPolicies::NEW:
+ // The script set this policy. We assume the purpose of the
+ // script is to initialize policies for its includer, and since
+ // the policy is now set for later scripts, we do not warn.
+ break;
+ }
}
//----------------------------------------------------------------------------
@@ -476,7 +572,8 @@ cmMakefile::IncludeScope::~IncludeScope()
//
bool cmMakefile::ReadListFile(const char* filename_in,
const char *external_in,
- std::string* fullPath)
+ std::string* fullPath,
+ bool noPolicyScope)
{
std::string currentParentFile
= this->GetSafeDefinition("CMAKE_PARENT_LIST_FILE");
@@ -564,7 +661,7 @@ bool cmMakefile::ReadListFile(const char* filename_in,
// Enforce balanced blocks (if/endif, function/endfunction, etc.).
{
LexicalPushPop lexScope(this);
- IncludeScope incScope(this);
+ IncludeScope incScope(this, filenametoread, noPolicyScope);
// Run the parsed commands.
const size_t numberFunctions = cacheFile.Functions.size();
diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index 1c59313..51136f1 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -84,7 +84,8 @@ public:
*/
bool ReadListFile(const char* listfile,
const char* external= 0,
- std::string* fullPath= 0);
+ std::string* fullPath= 0,
+ bool noPolicyScope = true);
/**
* Add a function blocker to this makefile
diff --git a/Source/cmPolicies.cxx b/Source/cmPolicies.cxx
index be546f8..169814a 100644
--- a/Source/cmPolicies.cxx
+++ b/Source/cmPolicies.cxx
@@ -335,6 +335,26 @@ cmPolicies::cmPolicies()
"the string untouched, and continue. "
"The NEW behavior for this policy is to report an error.",
2,6,3, cmPolicies::WARN);
+
+ this->DefinePolicy(
+ CMP0011, "CMP0011",
+ "Included scripts do automatic cmake_policy PUSH and POP.",
+ "In CMake 2.6.2 and below, CMake Policy settings in scripts loaded by "
+ "the include() and find_package() commands would affect the includer. "
+ "Explicit invocations of cmake_policy(PUSH) and cmake_policy(POP) were "
+ "required to isolate policy changes and protect the includer. "
+ "While some scripts intend to affect the policies of their includer, "
+ "most do not. "
+ "In CMake 2.6.3 and above, include() and find_package() by default PUSH "
+ "and POP an entry on the policy stack around an included script, "
+ "but provide a NO_POLICY_SCOPE option to disable it. "
+ "This policy determines whether or not to imply NO_POLICY_SCOPE for "
+ "compatibility. "
+ "The OLD behavior for this policy is to imply NO_POLICY_SCOPE for "
+ "include() and find_package() commands. "
+ "The NEW behavior for this policy is to allow the commands to do their "
+ "default cmake_policy PUSH and POP.",
+ 2,6,3, cmPolicies::WARN);
}
cmPolicies::~cmPolicies()
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index 8dc7f31..939d6d0 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -51,6 +51,7 @@ public:
CMP0008, // Full-path libraries must be a valid library file name
CMP0009, // GLOB_RECURSE should not follow symlinks by default
CMP0010, // Bad variable reference syntax is an error
+ CMP0011, // Strong policy scope for include and find_package
// Always the last entry. Useful mostly to avoid adding a comma
// the last policy when adding a new one.
diff --git a/Tests/PolicyScope/Bar.cmake b/Tests/PolicyScope/Bar.cmake
new file mode 100644
index 0000000..cf1904c
--- /dev/null
+++ b/Tests/PolicyScope/Bar.cmake
@@ -0,0 +1,8 @@
+cmake_minimum_required(VERSION 2.6.3)
+
+# Make sure a policy set differently by our includer is now correct.
+cmake_policy(GET CMP0003 cmp)
+check(CMP0003 "NEW" "${cmp}")
+
+# Test allowing the top-level file to not have cmake_minimum_required.
+cmake_policy(SET CMP0000 OLD)
diff --git a/Tests/PolicyScope/CMakeLists.txt b/Tests/PolicyScope/CMakeLists.txt
index ccb64de..89a89ee 100644
--- a/Tests/PolicyScope/CMakeLists.txt
+++ b/Tests/PolicyScope/CMakeLists.txt
@@ -1,5 +1,5 @@
-cmake_minimum_required(VERSION 2.6.3)
project(PolicyScope C)
+# No cmake_minimum_required(VERSION), it's in FindFoo.
#-----------------------------------------------------------------------------
# Helper function to report results.
@@ -10,6 +10,37 @@ function(check msg lhs rhs)
endfunction(check)
#-----------------------------------------------------------------------------
+# Test using a development framework that sets policies for us.
+
+# Policy CMP0011 should not be set at this point.
+cmake_policy(GET CMP0011 cmp)
+check(CMP0011 "" "${cmp}")
+
+# Put the test modules in the search path.
+list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR})
+
+# The included file should set policies for us.
+find_package(Foo)
+
+# Check policies set by the package.
+cmake_policy(GET CMP0003 cmp)
+check(CMP0003 "OLD" "${cmp}")
+cmake_policy(GET CMP0002 cmp)
+check(CMP0002 "NEW" "${cmp}")
+cmake_policy(GET CMP0011 cmp)
+check(CMP0011 "NEW" "${cmp}")
+
+# Make sure an included file cannot change policies.
+include(Bar)
+cmake_policy(GET CMP0003 cmp)
+check(CMP0003 "OLD" "${cmp}")
+
+# Allow the included file to change policies.
+include(Bar NO_POLICY_SCOPE)
+cmake_policy(GET CMP0003 cmp)
+check(CMP0003 "NEW" "${cmp}")
+
+#-----------------------------------------------------------------------------
# Test function and macro policy recording.
# Create the functions in an isolated scope in which we change policies.
diff --git a/Tests/PolicyScope/FindFoo.cmake b/Tests/PolicyScope/FindFoo.cmake
new file mode 100644
index 0000000..5b441e2
--- /dev/null
+++ b/Tests/PolicyScope/FindFoo.cmake
@@ -0,0 +1,2 @@
+cmake_minimum_required(VERSION 2.6.3)
+cmake_policy(SET CMP0003 OLD)
--
cgit v0.12