Prefer files from CMAKE_ROOT when including from CMAKE_ROOT

This patch makes include() and find_package() prefer cmake files
located in CMAKE_ROOT over those in CMAKE_MODULE_PATH.
This makes sure that the including file gets that file included
which it expects, i.e. the one from cmake with which it was tested.
It only changes behaviour when such an included file exists both
in CMAKE_MODULE_PATH and in CMAKE_ROOT.
This comes together with a new policy CMP0017, with default
behaviour it behaves as it always did, but warns.
With NEW behaviour it includes the file from CMAKE_ROOT
instead from CMAKE_MODULE_PATH. This fixes (if CMP0017 is set)
building KDE 4.5 with cmake >= 2.8.3.
Also a basic test for this policy in included.

Source/cmIncludeCommand.h

@@ -73,7 +73,12 @@ public:
       "the variable will be set to the full filename which "
       "has been included or NOTFOUND if it failed.\n"
       "If a module is specified instead of a file, the file with name "
-      "<modulename>.cmake is searched in the CMAKE_MODULE_PATH."
+      "<modulename>.cmake is searched first in CMAKE_MODULE_PATH, then in the "
+      "CMake module directory. There is one exception to this: if the file "
+      "which calls include() is located itself in the CMake module directory, "
+      "then first the CMake module directory is searched and "
+      "CMAKE_MODULE_PATH afterwards. This behaviour is controlled by policy "
+      "CMP0017."
       "\n"
       "See the cmake_policy() command documentation for discussion of the "
       "NO_POLICY_SCOPE option."

Source/cmMakefile.cxx

@@ -2822,35 +2822,100 @@ void cmMakefile::DisplayStatus(const char* message, float s)
 
 std::string cmMakefile::GetModulesFile(const char* filename)
 {
-  std::vector<std::string> modulePath;
+  std::string result;
-  const char* def = this->GetDefinition("CMAKE_MODULE_PATH");
-  if(def)
-    {
-    cmSystemTools::ExpandListArgument(def, modulePath);
-    }
 
-  // Also search in the standard modules location.
+  // We search the module always in CMAKE_ROOT and in CMAKE_MODULE_PATH,
-  def = this->GetDefinition("CMAKE_ROOT");
+  // and then decide based on the policy setting which one to return.
-  if(def)
+  // See CMP0017 for more details.
+  // The specific problem was that KDE 4.5.0 installs a
+  // FindPackageHandleStandardArgs.cmake which doesn't have the new features
+  // of FPHSA.cmake introduced in CMake 2.8.3 yet, and by setting
+  // CMAKE_MODULE_PATH also e.g. FindZLIB.cmake from cmake included
+  // FPHSA.cmake from kdelibs and not from CMake, and tried to use the
+  // new features, which were not there in the version from kdelibs, and so
+  // failed ("
+  std::string moduleInCMakeRoot;
+  std::string moduleInCMakeModulePath;
+
+  // Always search in CMAKE_MODULE_PATH:
+  const char* cmakeModulePath = this->GetDefinition("CMAKE_MODULE_PATH");
+  if(cmakeModulePath)
     {
-    std::string rootModules = def;
+    std::vector<std::string> modulePath;
-    rootModules += "/Modules";
+    cmSystemTools::ExpandListArgument(cmakeModulePath, modulePath);
-    modulePath.push_back(rootModules);
+
-    }
+    //Look through the possible module directories.
-  //std::string  Look through the possible module directories.
+    for(std::vector<std::string>::iterator i = modulePath.begin();
-  for(std::vector<std::string>::iterator i = modulePath.begin();
+        i != modulePath.end(); ++i)
-      i != modulePath.end(); ++i)
-    {
-    std::string itempl = *i;
-    cmSystemTools::ConvertToUnixSlashes(itempl);
-    itempl += "/";
-    itempl += filename;
-    if(cmSystemTools::FileExists(itempl.c_str()))
       {
-      return itempl;
+      std::string itempl = *i;
+      cmSystemTools::ConvertToUnixSlashes(itempl);
+      itempl += "/";
+      itempl += filename;
+      if(cmSystemTools::FileExists(itempl.c_str()))
+        {
+        moduleInCMakeModulePath = itempl;
+        break;
+        }
       }
     }
-  return "";
+
+  // Always search in the standard modules location.
+  const char* cmakeRoot = this->GetDefinition("CMAKE_ROOT");
+  if(cmakeRoot)
+    {
+    moduleInCMakeRoot = cmakeRoot;
+    moduleInCMakeRoot += "/Modules/";
+    moduleInCMakeRoot += filename;
+    cmSystemTools::ConvertToUnixSlashes(moduleInCMakeRoot);
+    if(!cmSystemTools::FileExists(moduleInCMakeRoot.c_str()))
+      {
+      moduleInCMakeRoot = "";
+      }
+    }
+
+  // Normally, prefer the files found in CMAKE_MODULE_PATH. Only when the file
+  // from which we are being called is located itself in CMAKE_ROOT, then
+  // prefer results from CMAKE_ROOT depending on the policy setting.
+  result = moduleInCMakeModulePath;
+  if (result.size() == 0)
+    {
+    result = moduleInCMakeRoot;
+    }
+
+  if ((moduleInCMakeModulePath.size()>0) && (moduleInCMakeRoot.size()>0))
+    {
+    const char* currentFile = this->GetDefinition("CMAKE_CURRENT_LIST_FILE");
+    if (currentFile && (strstr(currentFile, cmakeRoot) == currentFile))
+      {
+      switch (this->GetPolicyStatus(cmPolicies::CMP0017))
+        {
+        case cmPolicies::WARN:
+        {
+          cmOStringStream e;
+          e << "File " << currentFile << " includes "
+            << moduleInCMakeModulePath
+            << " (found via CMAKE_MODULE_PATH) which shadows "
+            << moduleInCMakeRoot  << ". This may cause errors later on .\n"
+            << this->GetPolicies()->GetPolicyWarning(cmPolicies::CMP0017);
+
+          this->IssueMessage(cmake::AUTHOR_WARNING, e.str());
+           // break;  // fall through to OLD behaviour
+        }
+        case cmPolicies::OLD:
+          result = moduleInCMakeModulePath;
+          break;
+        case cmPolicies::REQUIRED_IF_USED:
+        case cmPolicies::REQUIRED_ALWAYS:
+        case cmPolicies::NEW:
+        default:
+          result = moduleInCMakeRoot;
+          break;
+        }
+      }
+    }
+
+  return result;
 }
 
 void cmMakefile::ConfigureString(const std::string& input,

Source/cmPolicies.cxx

@@ -446,6 +446,22 @@ cmPolicies::cmPolicies()
     "wasn't a valid target. "
     "In CMake 2.8.3 and above it reports an error in this case.",
     2,8,3,0, cmPolicies::WARN);
+
+    this->DefinePolicy(
+    CMP0017, "CMP0017",
+    "Prefer files from CMAKE_ROOT/ when including from CMAKE_ROOT.",
+    "Starting with CMake 2.8.3, if a cmake-module shipped with CMake (i.e. "
+    "located in CMAKE_ROOT/Modules/) calls include() or find_package(), "
+    "the files located in CMAKE_ROOT/Modules/ are prefered over the files "
+    "in CMAKE_MODULE_PATH.  This makes sure that the modules belonging to "
+    "CMake always get those files included which they expect, and against "
+    "which they were developed and tested.  "
+    "In call other cases, the files found in "
+    "CMAKE_MODULE_PATH still take precedence over the ones in "
+    "CMAKE_ROOT/Modules/.  "
+    "The OLD behaviour is to always prefer files from CMAKE_MODULE_PATH over "
+    "files from CMAKE_ROOT/Modules/.",
+    2,8,3,0, cmPolicies::WARN);
 }
 
 cmPolicies::~cmPolicies()

Source/cmPolicies.h

@@ -52,6 +52,7 @@ public:
     CMP0014, // Input directories must have CMakeLists.txt
     CMP0015, // link_directories() treats paths relative to source dir
     CMP0016, // target_link_libraries() fails if only argument is not a target
+    CMP0017, // Prefer files in CMAKE_ROOT when including from CMAKE_ROOT
 
     // Always the last entry.  Useful mostly to avoid adding a comma
     // the last policy when adding a new one.

Tests/FindPackageTest/CMakeLists.txt

@@ -1,6 +1,15 @@
 cmake_minimum_required (VERSION 2.6)
 PROJECT(FindPackageTest)
 
+LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR})
+
+# Look for a package which uses FindPackageHandleStandardArgs.cmake with the
+# new (as of cmake 2.8.3) syntax. This works only if CMP0017 is set to NEW,
+# because otherwise FindPackageHandleStandardArgs.cmake from the current
+# directory is included (via CMAKE_MODULE_PATH).
+CMAKE_POLICY(SET CMP0017 NEW)
+FIND_PACKAGE(ZLIB)
+
 # Look for a package that has a find module and may be found.
 FIND_PACKAGE(OpenGL QUIET)
 
@@ -23,7 +32,6 @@ IF(NOT FOO_DIR)
   CMAKE_PREFIX_PATH = ${CMAKE_PREFIX_PATH}")
 ENDIF(NOT FOO_DIR)
 
-LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR})
 FIND_PACKAGE(VersionTestA 1)
 FIND_PACKAGE(VersionTestB 1.2)
 FIND_PACKAGE(VersionTestC 1.2.3)

Tests/FindPackageTest/FindPackageHandleStandardArgs.cmake

@@ -0,0 +1 @@
+message(FATAL_ERROR "This file (${CMAKE_CURRENT_LIST_FILE}) must not be included, but FindPackageHandleStandardArgs.cmake from Modules/ instead !")