From f50ed1fd8897eae39da6ed9f3af2bc3eab2c35af Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 18 Aug 2008 16:29:00 -0400 Subject: [PATCH] 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. --- Source/cmMakefile.cxx | 138 +++++++++++++++++++++----------------- Source/cmMakefile.h | 1 + Source/cmPolicies.cxx | 152 ++++++++++++++++-------------------------- Source/cmPolicies.h | 16 ++--- 4 files changed, 142 insertions(+), 165 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 7e035e188..5e9a64040 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(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::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 780b2fe07..f9d10b6ca 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -900,6 +900,7 @@ private: typedef std::map PolicyMap; std::vector PolicyStack; + cmPolicies::PolicyStatus GetPolicyStatusInternal(cmPolicies::PolicyID id); bool CheckCMP0000; diff --git a/Source/cmPolicies.cxx b/Source/cmPolicies.cxx index 1b9ab96d5..640e0e7d6 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 ancientPolicies; std::map::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& 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 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::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 7c9be18e3..5284034cb 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& v); @@ -96,7 +91,10 @@ public: // might have to make these internal for VS6 not sure yet std::map Policies; std::map PolicyStringMap; - + + void DiagnoseAncientPolicies(std::vector const& ancient, + unsigned int majorVer, unsigned int minorVer, + unsigned int patchVer, cmMakefile* mf); }; #endif