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.

9 files changed, 168 insertions, 18 deletions

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.