cmFindPackageCommand: Refactor CMP0074 logic to de-duplicate lookups

Source/cmFindPackageCommand.cxx

@@ -1814,26 +1814,40 @@ void cmFindPackageCommand::PushFindPackageRootPathStack()
 
   // Add root paths from <PackageName>_ROOT CMake and environment variables,
   // subject to CMP0074.
+  std::string const rootVar = this->Name + "_ROOT";
+  cmValue rootDef = this->Makefile->GetDefinition(rootVar);
+  if (rootDef && rootDef.IsEmpty()) {
+    rootDef = nullptr;
+  }
+  cm::optional<std::string> rootEnv = cmSystemTools::GetEnvVar(rootVar);
+  if (rootEnv && rootEnv->empty()) {
+    rootEnv = cm::nullopt;
+  }
   switch (this->Makefile->GetPolicyStatus(cmPolicies::CMP0074)) {
     case cmPolicies::WARN:
-      this->Makefile->MaybeWarnCMP0074(this->Name);
+      this->Makefile->MaybeWarnCMP0074(rootVar, rootDef, rootEnv);
       CM_FALLTHROUGH;
     case cmPolicies::OLD:
-      // OLD behavior is to ignore the <pkg>_ROOT variables.
-      break;
+      // OLD behavior is to ignore the <PackageName>_ROOT variables.
+      return;
     case cmPolicies::REQUIRED_IF_USED:
     case cmPolicies::REQUIRED_ALWAYS:
       this->Makefile->IssueMessage(
         MessageType::FATAL_ERROR,
         cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0074));
-      break;
+      return;
     case cmPolicies::NEW: {
-      // NEW behavior is to honor the <pkg>_ROOT variables.
-      std::string const rootVar = this->Name + "_ROOT";
-      this->Makefile->GetDefExpandList(rootVar, rootPaths, false);
-      cmSystemTools::GetPath(rootPaths, rootVar.c_str());
+      // NEW behavior is to honor the <PackageName>_ROOT variables.
     } break;
   }
+
+  if (rootDef) {
+    cmExpandList(*rootDef, rootPaths);
+  }
+  if (rootEnv) {
+    std::vector<std::string> p = cmSystemTools::SplitEnvPath(*rootEnv);
+    std::move(p.begin(), p.end(), std::back_inserter(rootPaths));
+  }
 }
 
 void cmFindPackageCommand::PopFindPackageRootPathStack()

Source/cmMakefile.cxx

@@ -208,26 +208,20 @@ bool cmMakefile::CheckCMP0037(std::string const& targetName,
   return true;
 }
 
-void cmMakefile::MaybeWarnCMP0074(std::string const& pkg)
+void cmMakefile::MaybeWarnCMP0074(std::string const& rootVar, cmValue rootDef,
+                                  cm::optional<std::string> const& rootEnv)
 {
-  // Warn if a <pkg>_ROOT variable we may use is set.
-  std::string const varName = pkg + "_ROOT";
-  cmValue var = this->GetDefinition(varName);
-  std::string env;
-  cmSystemTools::GetEnv(varName, env);
-
-  bool const haveVar = cmNonempty(var);
-  bool const haveEnv = !env.empty();
-  if ((haveVar || haveEnv) && this->WarnedCMP0074.insert(varName).second) {
+  // Warn if a <PackageName>_ROOT variable we may use is set.
+  if ((rootDef || rootEnv) && this->WarnedCMP0074.insert(rootVar).second) {
     std::ostringstream w;
     w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0074) << "\n";
-    if (haveVar) {
-      w << "CMake variable " << varName << " is set to:\n"
-        << "  " << *var << "\n";
+    if (rootDef) {
+      w << "CMake variable " << rootVar << " is set to:\n"
+        << "  " << *rootDef << "\n";
     }
-    if (haveEnv) {
-      w << "Environment variable " << varName << " is set to:\n"
-        << "  " << env << "\n";
+    if (rootEnv) {
+      w << "Environment variable " << rootVar << " is set to:\n"
+        << "  " << *rootEnv << "\n";
     }
     w << "For compatibility, CMake is ignoring the variable.";
     this->IssueMessage(MessageType::AUTHOR_WARNING, w.str());

Source/cmMakefile.h

@@ -1011,7 +1011,8 @@ public:
 
   bool GetDebugFindPkgMode() const;
 
-  void MaybeWarnCMP0074(std::string const& pkg);
+  void MaybeWarnCMP0074(std::string const& rootVar, cmValue rootDef,
+                        cm::optional<std::string> const& rootEnv);
   void MaybeWarnUninitialized(std::string const& variable,
                               const char* sourceFilename) const;
   bool IsProjectFile(const char* filename) const;