find_package: Use PackageName_ROOT variables as search prefixes

This feature was originally added by commit v3.9.0-rc1~71^2~2 (find_*:
Add a new PackageRoot search path group, 2017-05-03) and documented by
commit v3.9.0-rc1~71^2 (find_*: Add docs for PackageRoot search path
group, 2017-05-03).  However, we had to disable the feature and remove
the documentation in commit v3.9.1~2^2 (find_*: Disable the PACKAGE_ROOT
search path group for CMake 3.9, 2017-08-08) due to breaking projects
that used `PackageName_ROOT` variables themselves.

Add policy `CMP0074` to restore the `PackageName_ROOT` variable behavior
in a compatible way.  Also revise the stack of root paths to store the
paths themselves rather than the package names.  This way the policy can
be considered at the `find_package` call site instead of individual
`find_` calls inside a find module.

Co-Author: Chuck Atkins <chuck.atkins@kitware.com>
Issue: #17144

8 files changed, 79 insertions, 44 deletions

diff --git a/Source/cmFindBase.cxx b/Source/cmFindBase.cxx
index 7069386..865595b 100644
--- a/Source/cmFindBase.cxx
+++ b/Source/cmFindBase.cxx
@@ -67,8 +67,6 @@ bool cmFindBase::ParseArguments(std::vector<std::string> const& argsIn)
   }
   this->AlreadyInCache = false;
 
-  this->SelectDefaultNoPackageRootPath();
-
   // Find the current root path mode.
   this->SelectDefaultRootPathMode();
 
@@ -206,16 +204,12 @@ void cmFindBase::FillPackageRootPath()
 {
   cmSearchPath& paths = this->LabeledPaths[PathLabel::PackageRoot];
 
-  // Add package specific search prefixes
-  // NOTE: This should be using const_reverse_iterator but HP aCC and
-  //       Oracle sunCC both currently have standard library issues
-  //       with the reverse iterator APIs.
-  for (std::deque<std::string>::reverse_iterator pkg =
-         this->Makefile->FindPackageModuleStack.rbegin();
-       pkg != this->Makefile->FindPackageModuleStack.rend(); ++pkg) {
-    std::string varName = *pkg + "_ROOT";
-    paths.AddCMakePrefixPath(varName);
-    paths.AddEnvPrefixPath(varName);
+  // Add the PACKAGE_ROOT_PATH from each enclosing find_package call.
+  for (std::deque<std::vector<std::string>>::const_reverse_iterator pkgPaths =
+         this->Makefile->FindPackageRootPathStack.rbegin();
+       pkgPaths != this->Makefile->FindPackageRootPathStack.rend();
+       ++pkgPaths) {
+    paths.AddPrefixPaths(*pkgPaths);
   }
 
   paths.AddSuffixes(this->SearchPathSuffixes);
diff --git a/Source/cmFindCommon.cxx b/Source/cmFindCommon.cxx
index 4a467f3..64108d7 100644
--- a/Source/cmFindCommon.cxx
+++ b/Source/cmFindCommon.cxx
@@ -88,13 +88,6 @@ void cmFindCommon::InitializeSearchPathGroups()
     std::make_pair(PathLabel::Guess, cmSearchPath(this)));
 }
 
-void cmFindCommon::SelectDefaultNoPackageRootPath()
-{
-  if (!this->Makefile->IsOn("__UNDOCUMENTED_CMAKE_FIND_PACKAGE_ROOT")) {
-    this->NoPackageRootPath = true;
-  }
-}
-
 void cmFindCommon::SelectDefaultRootPathMode()
 {
   // Check the policy variable for this find command type.
diff --git a/Source/cmFindCommon.h b/Source/cmFindCommon.h
index b237f1b..89ff174 100644
--- a/Source/cmFindCommon.h
+++ b/Source/cmFindCommon.h
@@ -84,9 +84,6 @@ protected:
   /** Compute final search path list (reroot + trailing slash).  */
   void ComputeFinalPaths();
 
-  /** Decide whether to enable the PACKAGE_ROOT search entries.  */
-  void SelectDefaultNoPackageRootPath();
-
   /** Compute the current default root path mode.  */
   void SelectDefaultRootPathMode();
 
diff --git a/Source/cmFindPackageCommand.cxx b/Source/cmFindPackageCommand.cxx
index 2f3a85b..e9ac4a7 100644
--- a/Source/cmFindPackageCommand.cxx
+++ b/Source/cmFindPackageCommand.cxx
@@ -21,6 +21,7 @@
 
 #include "cmAlgorithms.h"
 #include "cmMakefile.h"
+#include "cmPolicies.h"
 #include "cmSearchPath.h"
 #include "cmState.h"
 #include "cmStateTypes.h"
@@ -209,8 +210,6 @@ bool cmFindPackageCommand::InitialPass(std::vector<std::string> const& args,
     this->SortDirection = strcmp(sd, "ASC") == 0 ? Asc : Dec;
   }
 
-  this->SelectDefaultNoPackageRootPath();
-
   // Find the current root path mode.
   this->SelectDefaultRootPathMode();
 
@@ -454,6 +453,38 @@ bool cmFindPackageCommand::InitialPass(std::vector<std::string> const& args,
     return true;
   }
 
+  {
+    // Allocate a PACKAGE_ROOT_PATH for the current find_package call.
+    this->Makefile->FindPackageRootPathStack.emplace_back();
+    std::vector<std::string>& rootPaths =
+      *this->Makefile->FindPackageRootPathStack.rbegin();
+
+    // Add root paths from <PackageName>_ROOT CMake and environment variables,
+    // subject to CMP0074.
+    switch (this->Makefile->GetPolicyStatus(cmPolicies::CMP0074)) {
+      case cmPolicies::WARN:
+        this->Makefile->MaybeWarnCMP0074(this->Name);
+        CM_FALLTHROUGH;
+      case cmPolicies::OLD:
+        // OLD behavior is to ignore the <pkg>_ROOT variables.
+        break;
+      case cmPolicies::REQUIRED_IF_USED:
+      case cmPolicies::REQUIRED_ALWAYS:
+        this->Makefile->IssueMessage(
+          cmake::FATAL_ERROR,
+          cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0074));
+        break;
+      case cmPolicies::NEW: {
+        // NEW behavior is to honor the <pkg>_ROOT variables.
+        std::string const rootVar = this->Name + "_ROOT";
+        if (const char* pkgRoot = this->Makefile->GetDefinition(rootVar)) {
+          cmSystemTools::ExpandListArgument(pkgRoot, rootPaths, false);
+        }
+        cmSystemTools::GetPath(rootPaths, rootVar.c_str());
+      } break;
+    }
+  }
+
   this->SetModuleVariables(components);
 
   // See if there is a Find<package>.cmake module.
@@ -589,9 +620,6 @@ void cmFindPackageCommand::SetModuleVariables(const std::string& components)
     exact += "_FIND_VERSION_EXACT";
     this->AddFindDefinition(exact, this->VersionExact ? "1" : "0");
   }
-
-  // Push on to the package stack
-  this->Makefile->FindPackageModuleStack.push_back(this->Name);
 }
 
 void cmFindPackageCommand::AddFindDefinition(const std::string& var,
@@ -1076,7 +1104,7 @@ void cmFindPackageCommand::AppendSuccessInformation()
   this->RestoreFindDefinitions();
 
   // Pop the package stack
-  this->Makefile->FindPackageModuleStack.pop_back();
+  this->Makefile->FindPackageRootPathStack.pop_back();
 }
 
 void cmFindPackageCommand::ComputePrefixes()
@@ -1116,16 +1144,14 @@ void cmFindPackageCommand::FillPrefixesPackageRoot()
 {
   cmSearchPath& paths = this->LabeledPaths[PathLabel::PackageRoot];
 
-  // Add package specific search prefixes
-  // NOTE: This should be using const_reverse_iterator but HP aCC and
-  //       Oracle sunCC both currently have standard library issues
-  //       with the reverse iterator APIs.
-  for (std::deque<std::string>::reverse_iterator pkg =
-         this->Makefile->FindPackageModuleStack.rbegin();
-       pkg != this->Makefile->FindPackageModuleStack.rend(); ++pkg) {
-    std::string varName = *pkg + "_ROOT";
-    paths.AddCMakePath(varName);
-    paths.AddEnvPath(varName);
+  // Add the PACKAGE_ROOT_PATH from each enclosing find_package call.
+  for (std::deque<std::vector<std::string>>::const_reverse_iterator pkgPaths =
+         this->Makefile->FindPackageRootPathStack.rbegin();
+       pkgPaths != this->Makefile->FindPackageRootPathStack.rend();
+       ++pkgPaths) {
+    for (std::string const& path : *pkgPaths) {
+      paths.AddPath(path);
+    }
   }
 }
 
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index 9e53579..1191490 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -158,6 +158,26 @@ bool cmMakefile::CheckCMP0037(std::string const& targetName,
   return true;
 }
 
+void cmMakefile::MaybeWarnCMP0074(std::string const& pkg)
+{
+  // Warn if a <pkg>_ROOT variable we may use is set.
+  std::string const varName = pkg + "_ROOT";
+  bool const haveVar = this->GetDefinition(varName) != nullptr;
+  bool const haveEnv = cmSystemTools::HasEnv(varName);
+  if ((haveVar || haveEnv) && this->WarnedCMP0074.insert(varName).second) {
+    std::ostringstream w;
+    w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0074) << "\n";
+    if (haveVar) {
+      w << "CMake variable " << varName << " is set.\n";
+    }
+    if (haveEnv) {
+      w << "Environment variable " << varName << " is set.\n";
+    }
+    w << "For compatibility, CMake is ignoring the variable.";
+    this->IssueMessage(cmake::AUTHOR_WARNING, w.str());
+  }
+}
+
 cmStringRange cmMakefile::GetIncludeDirectoriesEntries() const
 {
   return this->StateSnapshot.GetDirectory().GetIncludeDirectoriesEntries();
diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index 95ba53a..aafe60b 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -832,9 +832,11 @@ public:
   void RemoveExportBuildFileGeneratorCMP0024(cmExportBuildFileGenerator* gen);
   void AddExportBuildFileGenerator(cmExportBuildFileGenerator* gen);
 
-  // Maintain a stack of package names to determine the depth of find modules
-  // we are currently being called with
-  std::deque<std::string> FindPackageModuleStack;
+  // Maintain a stack of package roots to allow nested PACKAGE_ROOT_PATH
+  // searches
+  std::deque<std::vector<std::string>> FindPackageRootPathStack;
+
+  void MaybeWarnCMP0074(std::string const& pkg);
 
 protected:
   // add link libraries and directories to the target
@@ -1001,6 +1003,7 @@ private:
   bool WarnUnused;
   bool CheckSystemVars;
   bool CheckCMP0000;
+  std::set<std::string> WarnedCMP0074;
   bool IsSourceFileTryCompile;
   mutable bool SuppressWatches;
 };
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index a784f98..34fe09d 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -217,7 +217,9 @@ class cmMakefile;
          cmPolicies::WARN)                                                    \
   SELECT(POLICY, CMP0073,                                                     \
          "Do not produce legacy _LIB_DEPENDS cache entries.", 3, 12, 0,       \
-         cmPolicies::WARN)
+         cmPolicies::WARN)                                                    \
+  SELECT(POLICY, CMP0074, "find_package uses PackageName_ROOT variables.", 3, \
+         12, 0, cmPolicies::WARN)
 
 #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
 #define CM_FOR_EACH_POLICY_ID(POLICY)                                         \
diff --git a/Source/cmSearchPath.h b/Source/cmSearchPath.h
index fd0c7c5..2a576ed 100644
--- a/Source/cmSearchPath.h
+++ b/Source/cmSearchPath.h
@@ -39,10 +39,10 @@ public:
   void AddCMakePrefixPath(const std::string& variable);
   void AddEnvPrefixPath(const std::string& variable, bool stripBin = false);
   void AddSuffixes(const std::vector<std::string>& suffixes);
-
-protected:
   void AddPrefixPaths(const std::vector<std::string>& paths,
                       const char* base = nullptr);
+
+protected:
   void AddPathInternal(const std::string& path, const char* base = nullptr);
 
   cmFindCommon* FC;