From db44848f441673909e909755d7b833aa474791c7 Mon Sep 17 00:00:00 2001 From: Alex Neundorf Date: Wed, 17 Nov 2010 22:32:53 +0100 Subject: 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 | 7 +- Source/cmMakefile.cxx | 105 +++++++++++++++++---- Source/cmPolicies.cxx | 16 ++++ Source/cmPolicies.h | 1 + Tests/FindPackageTest/CMakeLists.txt | 10 +- .../FindPackageHandleStandardArgs.cmake | 1 + 6 files changed, 118 insertions(+), 22 deletions(-) create mode 100644 Tests/FindPackageTest/FindPackageHandleStandardArgs.cmake diff --git a/Source/cmIncludeCommand.h b/Source/cmIncludeCommand.h index 8704750..a6d43ba 100644 --- a/Source/cmIncludeCommand.h +++ b/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 " - ".cmake is searched in the CMAKE_MODULE_PATH." + ".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." diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 56e0ed9..ccf4d7a 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -2822,35 +2822,100 @@ void cmMakefile::DisplayStatus(const char* message, float s) std::string cmMakefile::GetModulesFile(const char* filename) { - std::vector modulePath; - const char* def = this->GetDefinition("CMAKE_MODULE_PATH"); - if(def) + std::string result; + + // We search the module always in CMAKE_ROOT and in CMAKE_MODULE_PATH, + // and then decide based on the policy setting which one to return. + // 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::vector modulePath; + cmSystemTools::ExpandListArgument(cmakeModulePath, modulePath); + + //Look through the possible module directories. + for(std::vector::iterator i = modulePath.begin(); + i != modulePath.end(); ++i) + { + std::string itempl = *i; + cmSystemTools::ConvertToUnixSlashes(itempl); + itempl += "/"; + itempl += filename; + if(cmSystemTools::FileExists(itempl.c_str())) + { + moduleInCMakeModulePath = itempl; + break; + } + } + } + + // Always search in the standard modules location. + const char* cmakeRoot = this->GetDefinition("CMAKE_ROOT"); + if(cmakeRoot) { - cmSystemTools::ExpandListArgument(def, modulePath); + moduleInCMakeRoot = cmakeRoot; + moduleInCMakeRoot += "/Modules/"; + moduleInCMakeRoot += filename; + cmSystemTools::ConvertToUnixSlashes(moduleInCMakeRoot); + if(!cmSystemTools::FileExists(moduleInCMakeRoot.c_str())) + { + moduleInCMakeRoot = ""; + } } - // Also search in the standard modules location. - def = this->GetDefinition("CMAKE_ROOT"); - if(def) + // 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) { - std::string rootModules = def; - rootModules += "/Modules"; - modulePath.push_back(rootModules); + result = moduleInCMakeRoot; } - //std::string Look through the possible module directories. - for(std::vector::iterator i = modulePath.begin(); - i != modulePath.end(); ++i) + + if ((moduleInCMakeModulePath.size()>0) && (moduleInCMakeRoot.size()>0)) { - std::string itempl = *i; - cmSystemTools::ConvertToUnixSlashes(itempl); - itempl += "/"; - itempl += filename; - if(cmSystemTools::FileExists(itempl.c_str())) + const char* currentFile = this->GetDefinition("CMAKE_CURRENT_LIST_FILE"); + if (currentFile && (strstr(currentFile, cmakeRoot) == currentFile)) { - return itempl; + 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 ""; + + return result; } void cmMakefile::ConfigureString(const std::string& input, diff --git a/Source/cmPolicies.cxx b/Source/cmPolicies.cxx index fccf0cc..34c74c2 100644 --- a/Source/cmPolicies.cxx +++ b/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() diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h index 87eb646..2160f37 100644 --- a/Source/cmPolicies.h +++ b/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. diff --git a/Tests/FindPackageTest/CMakeLists.txt b/Tests/FindPackageTest/CMakeLists.txt index a472bea..fb12121 100644 --- a/Tests/FindPackageTest/CMakeLists.txt +++ b/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) diff --git a/Tests/FindPackageTest/FindPackageHandleStandardArgs.cmake b/Tests/FindPackageTest/FindPackageHandleStandardArgs.cmake new file mode 100644 index 0000000..7e41c96 --- /dev/null +++ b/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 !") -- cgit v0.12