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

@@ -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"

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");

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;
 };

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())

Source/cmIncludeCommand.h

@@ -69,8 +69,8 @@ public:
   virtual const char* GetFullDocumentation()
     {
     return
-      "  include(file1 [OPTIONAL] [RESULT_VARIABLE <VAR>])\n"
+      "  include(<file|module> [OPTIONAL] [RESULT_VARIABLE <VAR>]\n"
-      "  include(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);

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):
+cmMakefile::IncludeScope::IncludeScope(cmMakefile* mf, const char* fname,
-  Makefile(mf), ReportError(true)
+                                       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();

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

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()

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.

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)

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.
@@ -9,6 +9,37 @@ function(check msg lhs rhs)
   endif()
 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.
 

Tests/PolicyScope/FindFoo.cmake

@@ -0,0 +1,2 @@
+cmake_minimum_required(VERSION 2.6.3)
+cmake_policy(SET CMP0003 OLD)