ENH: Improve cmake_policy command signature

- Replace NEW and OLD modes with a SET mode for clarity
  - Enforce VERSION argument validity (major.minor[.patch])

Source/cmCMakePolicyCommand.cxx

@@ -24,35 +24,106 @@ bool cmCMakePolicyCommand
 {
   if(args.size() < 1)
     {
-    this->SetError("cmake_policy requires at least one argument.");
+    this->SetError("requires at least one argument.");
     return false;
     }
 
-  if (args[0] == "OLD" && args.size() == 2)
+  if(args[0] == "SET")
     {
-    return this->Makefile->SetPolicy(args[1].c_str(),cmPolicies::OLD);
+    return this->HandleSetMode(args);
     }
-  
-  if (args[0] == "NEW" && args.size() == 2)
+  else if(args[0] == "PUSH")
     {
-    return this->Makefile->SetPolicy(args[1].c_str(),cmPolicies::NEW);
+    if(args.size() > 1)
+      {
+      this->SetError("PUSH may not be given additional arguments.");
+      return false;
       }
-  
-  if (args[0] == "VERSION" && args.size() == 2)
-  {
-    return this->Makefile->SetPolicyVersion(args[1].c_str());
-  }
-
-  if (args[0] == "PUSH" && args.size() == 1)
-  {
     return this->Makefile->PushPolicy();
     }
-  
-  if (args[0] == "POP" && args.size() == 1)
+  else if(args[0] == "POP")
     {
-    return this->Makefile->PopPolicy();
-  }
-
-  this->SetError("incorrect arguments for cmake_policy.");  
+    if(args.size() > 1)
+      {
+      this->SetError("POP may not be given additional arguments.");
       return false;
       }
+    if(this->Makefile->PopPolicy(false))
+      {
+      return true;
+      }
+    else
+      {
+      this->SetError("POP without matching PUSH");
+      return false;
+      }
+    }
+  else if(args[0] == "VERSION")
+    {
+    return this->HandleVersionMode(args);
+    }
+
+  cmOStringStream e;
+  e << "given unknown first argument \"" << args[0] << "\"";
+  this->SetError(e.str().c_str());
+  return false;
+}
+
+//----------------------------------------------------------------------------
+bool cmCMakePolicyCommand::HandleSetMode(std::vector<std::string> const& args)
+{
+  if(args.size() != 3)
+    {
+    this->SetError("SET must be given exactly 2 additional arguments.");
+    return false;
+    }
+
+  cmPolicies::PolicyStatus status;
+  if(args[2] == "OLD")
+    {
+    status = cmPolicies::OLD;
+    }
+  else if(args[2] == "NEW")
+    {
+    status = cmPolicies::NEW;
+    }
+  else
+    {
+    cmOStringStream e;
+    e << "SET given unrecognized policy status \"" << args[2] << "\"";
+    this->SetError(e.str().c_str());
+    return false;
+    }
+
+  if(!this->Makefile->SetPolicy(args[1].c_str(), status))
+    {
+    this->SetError("SET failed to set policy.");
+    return false;
+    }
+  return true;
+}
+
+//----------------------------------------------------------------------------
+bool
+cmCMakePolicyCommand::HandleVersionMode(std::vector<std::string> const& args)
+{
+  if(args.size() <= 1)
+    {
+    this->SetError("VERSION not given an argument");
+    return false;
+    }
+  else if(args.size() >= 3)
+    {
+    this->SetError("VERSION given too many arguments");
+    return false;
+    }
+  if(!this->Makefile->SetPolicyVersion(args[1].c_str()))
+    {
+    cmOStringStream e;
+    e << "VERSION given invalid value \"" << args[1] << "\".  "
+      << "A numeric major.minor[.patch] must be given.";
+    this->SetError(e.str().c_str());
+    return false;
+    }
+  return true;
+}

Source/cmCMakePolicyCommand.h

@@ -58,7 +58,7 @@ public:
    */
   virtual const char* GetTerseDocumentation() 
     {
-    return "Set how CMake should handle policies.";
+    return "Manage CMake policy settings.";
     }
   
   /**
@@ -67,30 +67,39 @@ public:
   virtual const char* GetFullDocumentation()
     {
     return
-      "  cmake_policy(NEW id)\n"
-      "  cmake_policy(OLD id)\n"
-      "  cmake_policy(VERSION version)\n"
+      "  cmake_policy(VERSION major.minor[.patch])\n"
+      "Specify that the current CMake list file is written for the "
+      "given version of CMake.  "
+      "All policies introduced in the specified version or earlier "
+      "will be set NEW.  "
+      "All policies introduced after the specified version will be set "
+      "to WARN, which is like OLD but also produces a warning.  "
+      "This effectively requests behavior preferred as of a given CMake "
+      "version and tells newer CMake versions to warn about their new "
+      "policies."
+      "\n"
+      "  cmake_policy(SET <CMP_NNNN> NEW)\n"
+      "  cmake_policy(SET <CMP_NNNN> OLD)\n"
+      "Tell CMake to use the OLD or NEW behavior for a given policy.  "
+      "Projects depending on the old behavior of a given policy may "
+      "silence a policy warning by setting the policy state to OLD.  "
+      "Alternatively one may fix the project to work with the new behavior "
+      "and set the policy state to NEW."
+      "\n"
       "  cmake_policy(PUSH)\n"
       "  cmake_policy(POP)\n"
-      "The first two forms of this command sets a specified policy to "
-      "use the OLD or NEW implementation respectively. For example "
-      "if a new policy is created in CMake 2.6 then you could use "
-      "this command to tell the running CMake to use the OLD behavior "
-      "(before the change in 2.6) or the NEW behavior.\n"
-      "The third form of this command indicates that the CMake List file "
-      "has been written to the specified version of CMake and to the "
-      "policies of that version of CMake. All policies introduced in "
-      "the specified version of CMake or earlier will be set to NEW. "
-      "All policies introduced after the specified version of CMake will "
-      "be set to WARN (WARN is like OLD but also produces a warning) if "
-      "that is possible.\n"
-      "The last two forms of this command push and pop the current "
-      "handling of policies in CMake. This is useful when mixing multiple "
-      "projects that may have been written to different versions of CMake."
+      "Push and pop the current policy setting state on a stack.  "
+      "Each PUSH must have a matching POP.  "
+      "This is useful when mixing multiple projects, subprojects, and "
+      "files included from external projects that may each have been "
+      "written for a different version of CMake."
       ;
     }
   
   cmTypeMacro(cmCMakePolicyCommand, cmCommand);
+private:
+  bool HandleSetMode(std::vector<std::string> const& args);
+  bool HandleVersionMode(std::vector<std::string> const& args);
 };
 
 

Source/cmMakefile.cxx

@@ -3317,12 +3317,15 @@ bool cmMakefile::PushPolicy()
   return true;
 }
 
-bool cmMakefile::PopPolicy()
+bool cmMakefile::PopPolicy(bool reportError)
 {
-  if (PolicyStack.size() == 1)
+  if(this->PolicyStack.size() == 1)
+    {
+    if(reportError)
       {
       cmSystemTools::Error("Attempt to pop the policy stack past "
                            "it's beginning.");
+      }
     return false;
     }
   this->PolicyStack.pop_back();

Source/cmMakefile.h

@@ -332,7 +332,7 @@ public:
   bool SetPolicy(const char *id, cmPolicies::PolicyStatus status);
   cmPolicies::PolicyStatus GetPolicyStatus(cmPolicies::PolicyID id);
   bool PushPolicy();
-  bool PopPolicy();
+  bool PopPolicy(bool reportError = true);
   bool SetPolicyVersion(const char *version);
   //@}