ENH: Improve errors when a policy is REQUIRED

In the future some policies may be set to REQUIRED_IF_USED or
REQUIRED_ALWAYS.  This change clarifies the error messages users receive
when violating the requirements.

4 files changed, 142 insertions, 165 deletions

diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index 7e035e1..5e9a640 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -3460,50 +3460,59 @@ bool cmMakefile::EnforceUniqueName(std::string const& name, std::string& msg,
   return true;
 }
 
-cmPolicies::PolicyStatus cmMakefile
-::GetPolicyStatus(cmPolicies::PolicyID id)
+//----------------------------------------------------------------------------
+cmPolicies::PolicyStatus
+cmMakefile::GetPolicyStatus(cmPolicies::PolicyID id)
 {
-  cmPolicies::PolicyStatus status = cmPolicies::REQUIRED_IF_USED;
-  PolicyMap::iterator mappos;
-  int vecpos;
-  bool done = false;
+  // Get the current setting of the policy.
+  cmPolicies::PolicyStatus cur = this->GetPolicyStatusInternal(id);
 
-  // check our policy stack first
-  for (vecpos = static_cast<int>(this->PolicyStack.size()) - 1; 
-       vecpos >= 0 && !done; vecpos--)
-  {
-    mappos = this->PolicyStack[vecpos].find(id);
-    if (mappos != this->PolicyStack[vecpos].end())
+  // If the policy is required to be set to NEW but is not, ignore the
+  // current setting and tell the caller.
+  if(cur != cmPolicies::NEW)
     {
-      status = mappos->second;
-      done = true;
+    if(cur == cmPolicies::REQUIRED_ALWAYS ||
+       cur == cmPolicies::REQUIRED_IF_USED)
+      {
+      return cur;
+      }
+    cmPolicies::PolicyStatus def = this->GetPolicies()->GetPolicyStatus(id);
+    if(def == cmPolicies::REQUIRED_ALWAYS ||
+       def == cmPolicies::REQUIRED_IF_USED)
+      {
+      return def;
+      }
     }
-  }
-  
-  // if not found then 
-  if (!done)
-  {
-    // pass the buck to our parent if we have one
-    if (this->LocalGenerator->GetParent())
+
+  // The current setting is okay.
+  return cur;
+}
+
+//----------------------------------------------------------------------------
+cmPolicies::PolicyStatus
+cmMakefile::GetPolicyStatusInternal(cmPolicies::PolicyID id)
+{
+  // Is the policy set in our stack?
+  for(std::vector<PolicyMap>::reverse_iterator
+        psi = this->PolicyStack.rbegin();
+      psi != this->PolicyStack.rend(); ++psi)
     {
-      cmMakefile *parent = 
-        this->LocalGenerator->GetParent()->GetMakefile();
-      return parent->GetPolicyStatus(id);
+    PolicyMap::const_iterator pse = psi->find(id);
+    if(pse != psi->end())
+      {
+      return pse->second;
+      }
     }
-    // otherwise use the default
-    else
+
+  // If we have a parent directory, recurse up to it.
+  if(this->LocalGenerator->GetParent())
     {
-      status = this->GetPolicies()->GetPolicyStatus(id);
+    cmMakefile* parent = this->LocalGenerator->GetParent()->GetMakefile();
+    return parent->GetPolicyStatusInternal(id);
     }
-  }
-  
-  // warn if we see a REQUIRED_IF_USED above a OLD or WARN
-  if (!this->GetPolicies()->IsValidUsedPolicyStatus(id,status))
-  {
-    return cmPolicies::REQUIRED_IF_USED;
-  }
-  
-  return status;
+
+  // The policy is not set.  Use the default for this CMake version.
+  return this->GetPolicies()->GetPolicyStatus(id);
 }
 
 bool cmMakefile::SetPolicy(const char *id, 
@@ -3520,37 +3529,44 @@ bool cmMakefile::SetPolicy(const char *id,
   return this->SetPolicy(pid,status);
 }
 
-bool cmMakefile::SetPolicy(cmPolicies::PolicyID id, 
+//----------------------------------------------------------------------------
+bool cmMakefile::SetPolicy(cmPolicies::PolicyID id,
                            cmPolicies::PolicyStatus status)
 {
-  // setting a REQUIRED_ALWAYS policy to WARN or OLD is an insta error
-  if (this->GetPolicies()->
-      IsValidPolicyStatus(id,status))
-  {
-    this->PolicyStack.back()[id] = status;
+  // A REQUIRED_ALWAYS policy may be set only to NEW.
+  if(status != cmPolicies::NEW &&
+     this->GetPolicies()->GetPolicyStatus(id) ==
+     cmPolicies::REQUIRED_ALWAYS)
+    {
+    std::string msg =
+      this->GetPolicies()->GetRequiredAlwaysPolicyError(id);
+    this->IssueMessage(cmake::FATAL_ERROR, msg.c_str());
+    return false;
+    }
 
-    // Special hook for presenting compatibility variable as soon as
-    // the user requests it.
-    if(id == cmPolicies::CMP0001 &&
-       (status == cmPolicies::WARN || status == cmPolicies::OLD))
+  // Store the setting.
+  this->PolicyStack.back()[id] = status;
+
+  // Special hook for presenting compatibility variable as soon as
+  // the user requests it.
+  if(id == cmPolicies::CMP0001 &&
+     (status == cmPolicies::WARN || status == cmPolicies::OLD))
+    {
+    if(!(this->GetCacheManager()
+         ->GetCacheValue("CMAKE_BACKWARDS_COMPATIBILITY")))
       {
-      if(!(this->GetCacheManager()
-           ->GetCacheValue("CMAKE_BACKWARDS_COMPATIBILITY")))
-        {
-        // Set it to 2.4 because that is the last version where the
-        // variable had meaning.
-        this->AddCacheDefinition
-          ("CMAKE_BACKWARDS_COMPATIBILITY", "2.4",
-           "For backwards compatibility, what version of CMake "
-           "commands and "
-           "syntax should this version of CMake try to support.",
-           cmCacheManager::STRING);
-        }
+      // Set it to 2.4 because that is the last version where the
+      // variable had meaning.
+      this->AddCacheDefinition
+        ("CMAKE_BACKWARDS_COMPATIBILITY", "2.4",
+         "For backwards compatibility, what version of CMake "
+         "commands and "
+         "syntax should this version of CMake try to support.",
+         cmCacheManager::STRING);
       }
+    }
 
-    return true;
-  }
-  return false;
+  return true;
 }
 
 bool cmMakefile::PushPolicy()
diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index 780b2fe..f9d10b6 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -900,6 +900,7 @@ private:
   typedef std::map<cmPolicies::PolicyID,
                    cmPolicies::PolicyStatus> PolicyMap;
   std::vector<PolicyMap> PolicyStack;
+  cmPolicies::PolicyStatus GetPolicyStatusInternal(cmPolicies::PolicyID id);
 
   bool CheckCMP0000;
 
diff --git a/Source/cmPolicies.cxx b/Source/cmPolicies.cxx
index 1b9ab96..640e0e7 100644
--- a/Source/cmPolicies.cxx
+++ b/Source/cmPolicies.cxx
@@ -347,6 +347,7 @@ void cmPolicies::DefinePolicy(cmPolicies::PolicyID iD,
   this->PolicyStringMap[idString] = iD;
 }
 
+//----------------------------------------------------------------------------
 bool cmPolicies::ApplyPolicyVersion(cmMakefile *mf, 
                                     const char *version)
 {
@@ -408,13 +409,18 @@ bool cmPolicies::ApplyPolicyVersion(cmMakefile *mf,
     }
 
   // now loop over all the policies and set them as appropriate
+  std::vector<cmPolicies::PolicyID> ancientPolicies;
   std::map<cmPolicies::PolicyID,cmPolicy *>::iterator i 
     = this->Policies.begin();
   for (;i != this->Policies.end(); ++i)
   {
     if (i->second->IsPolicyNewerThan(majorVer,minorVer,patchVer))
     {
-      if (!mf->SetPolicy(i->second->ID, cmPolicies::WARN))
+      if(i->second->Status == cmPolicies::REQUIRED_ALWAYS)
+      {
+        ancientPolicies.push_back(i->first);
+      }
+      else if (!mf->SetPolicy(i->second->ID, cmPolicies::WARN))
       {
         return false;
       }
@@ -427,105 +433,16 @@ bool cmPolicies::ApplyPolicyVersion(cmMakefile *mf,
       }
     }
   }
-  return true;
-}
 
-// is this a valid status the listfile can set this policy to?
-bool cmPolicies::IsValidPolicyStatus(cmPolicies::PolicyID id, 
-                                     cmPolicies::PolicyStatus status)
-{
-  // if they are setting a feature to anything other than OLD or WARN and the
-  // feature is not known about then that is an error
-  if (this->Policies.find(id) == this->Policies.end())
-  {
-    if (status == cmPolicies::WARN ||
-        status == cmPolicies::OLD)
+  // Make sure the project does not use any ancient policies.
+  if(!ancientPolicies.empty())
     {
-      return true;
-    }
-    cmOStringStream error;
-    error << 
-      "Error: an attempt was made to enable the new behavior for " <<
-      "a new feature that is in a later version of CMake than "
-      "what you are runing, please upgrade to a newer version "
-      "of CMake.";
-    cmSystemTools::Error(error.str().c_str());
-    return false;  
-  }
-
-  // now we know the feature is defined, so the only issue is if someone is
-  // setting it to WARN or OLD when the feature is REQUIRED_ALWAYS
-  if ((status == cmPolicies::WARN || 
-      status == cmPolicies::OLD) && 
-      this->Policies[id]->Status == cmPolicies::REQUIRED_ALWAYS)
-  {
-    cmOStringStream error;
-    error << 
-      "Error: an attempt was made to enable the old behavior for " <<
-      "a feature that is no longer supported. The feature in " <<
-      "question is feature " <<
-      id <<
-      " which had new behavior introduced in CMake version " <<
-      this->Policies[id]->GetVersionString() <<
-      " please either update your CMakeLists files to conform to " <<
-      "the new behavior " <<
-      "or use an older version of CMake that still supports " <<
-      "the old behavior. Run cmake --help-policies " <<
-      id << " for more information.";
-    cmSystemTools::Error(error.str().c_str());
+    this->DiagnoseAncientPolicies(ancientPolicies,
+                                  majorVer, minorVer, patchVer, mf);
+    cmSystemTools::SetFatalErrorOccured();
     return false;
-  }
-  
-  return true;
-}
-
-// is this a valid status the listfile can set this policy to?
-bool cmPolicies::IsValidUsedPolicyStatus(cmPolicies::PolicyID id, 
-                                         cmPolicies::PolicyStatus status)
-{
-  // if they are setting a feature to anything other than OLD or WARN and the
-  // feature is not known about then that is an error
-  if (this->Policies.find(id) == this->Policies.end())
-  {
-    if (status == cmPolicies::WARN ||
-        status == cmPolicies::OLD)
-    {
-      return true;
     }
-    cmOStringStream error;
-    error << 
-      "Error: an attempt was made to enable the new behavior for " <<
-      "a new feature that is in a later version of CMake than "
-      "what you are runing, please upgrade to a newer version "
-      "of CMake.";
-    cmSystemTools::Error(error.str().c_str());
-    return false;  
-  }
 
-  // now we know the feature is defined, so the only issue is if someone is
-  // setting it to WARN or OLD when the feature is REQUIRED_ALWAYS
-  if ((status == cmPolicies::WARN || 
-      status == cmPolicies::OLD) && 
-      (this->Policies[id]->Status == cmPolicies::REQUIRED_ALWAYS ||
-       this->Policies[id]->Status == cmPolicies::REQUIRED_IF_USED))
-  {
-    cmOStringStream error;
-    error << 
-      "Error: an attempt was made to enable the old behavior for " <<
-      "a feature that is no longer supported. The feature in " <<
-      "question is feature " <<
-      id <<
-      " which had new behavior introduced in CMake version " <<
-      this->Policies[id]->GetVersionString() <<
-      " please either update your CMakeLists files to conform to " <<
-      "the new behavior " <<
-      "or use an older version of CMake that still supports " <<
-      "the old behavior. Run cmake --help-policies " <<
-      id << " for more information.";
-    cmSystemTools::Error(error.str().c_str());
-    return false;
-  }
-  
   return true;
 }
 
@@ -671,3 +588,48 @@ void cmPolicies::GetDocumentation(std::vector<cmDocumentationEntry>& v)
     v.push_back(e);
   }
 }
+
+//----------------------------------------------------------------------------
+std::string
+cmPolicies::GetRequiredAlwaysPolicyError(cmPolicies::PolicyID id)
+{
+  std::string pid = this->GetPolicyIDString(id);
+  cmOStringStream e;
+  e << "Policy " << pid << " may not be set to OLD behavior because this "
+    << "version of CMake no longer supports it.  "
+    << "The policy was introduced in "
+    << "CMake version " << this->Policies[id]->GetVersionString()
+    << ", and use of NEW behavior is now required."
+    << "\n"
+    << "Please either update your CMakeLists.txt files to conform to "
+    << "the new behavior or use an older version of CMake that still "
+    << "supports the old behavior.  "
+    << "Run cmake --help-policy " << pid << " for more information.";
+  return e.str();
+}
+
+//----------------------------------------------------------------------------
+void
+cmPolicies::DiagnoseAncientPolicies(std::vector<PolicyID> const& ancient,
+                                    unsigned int majorVer,
+                                    unsigned int minorVer,
+                                    unsigned int patchVer,
+                                    cmMakefile* mf)
+{
+  cmOStringStream e;
+  e << "The project requests behavior compatible with CMake version \""
+    << majorVer << "." << minorVer << "." << patchVer
+    << "\", which requires OLD the behavior for some policies:\n";
+  for(std::vector<PolicyID>::const_iterator
+        i = ancient.begin(); i != ancient.end(); ++i)
+    {
+    cmPolicy const* policy = this->Policies[*i];
+    e << "  " << policy->IDString << ": " << policy->ShortDescription << "\n";
+    }
+  e << "However, this version of CMake no longer supports the OLD "
+    << "behavior for these policies.  "
+    << "Please either update your CMakeLists.txt files to conform to "
+    << "the new behavior or use an older version of CMake that still "
+    << "supports the old behavior.";
+  mf->IssueMessage(cmake::FATAL_ERROR, e.str().c_str());
+}
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index 7c9be18..5284034 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -75,20 +75,15 @@ public:
   ///! Set a policy level for this listfile
   bool ApplyPolicyVersion(cmMakefile *mf, const char *version);
 
-  ///! test to see if setting a policy to a specific value is valid
-  bool IsValidPolicyStatus(cmPolicies::PolicyID id, 
-                           cmPolicies::PolicyStatus status);
-
-  ///! test to see if setting a policy to a specific value is valid, when used
-  bool IsValidUsedPolicyStatus(cmPolicies::PolicyID id, 
-                               cmPolicies::PolicyStatus status);
-
   ///! return a warning string for a given policy
   std::string GetPolicyWarning(cmPolicies::PolicyID id);
   
   ///! return an error string for when a required policy is unspecified
   std::string GetRequiredPolicyError(cmPolicies::PolicyID id);
 
+  ///! return an error string for when a required policy is unspecified
+  std::string GetRequiredAlwaysPolicyError(cmPolicies::PolicyID id);
+
   ///! Get docs for policies
   void GetDocumentation(std::vector<cmDocumentationEntry>& v);
 
@@ -96,7 +91,10 @@ public:
   // might have to make these internal for VS6 not sure yet
   std::map<PolicyID,cmPolicy *> Policies;
   std::map<std::string,PolicyID> PolicyStringMap;
-  
+
+  void DiagnoseAncientPolicies(std::vector<PolicyID> const& ancient,
+                               unsigned int majorVer, unsigned int minorVer,
+                               unsigned int patchVer, cmMakefile* mf);
 };
 
 #endif