From 71910b3fd49b3ab555333e3737ef2f7c5fc43dae Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 7 Oct 2009 14:37:30 -0400 Subject: Fix find_package() when _DIR is wrong When _DIR is set to an incorrect version we search again and store the result in the variable, even if it is _DIR-NOTFOUND. There was a bug in the case when the new search does not find anything and the old value came from a cache entry with UNINITALIZED type. The command used to try to load a package configuration file from the last place searched, and would leave the old wrong value in the entry. This commit fixes the behavior to avoid trying to load a missing file and to set the value to _DIR-NOTFOUND as expected. --- Source/cmFindPackageCommand.cxx | 27 +++++++++++++++++---------- Source/cmFindPackageCommand.h | 2 +- Tests/FindPackageTest/CMakeLists.txt | 36 +++++++++++++++++++++++++++++++++--- 3 files changed, 51 insertions(+), 14 deletions(-) diff --git a/Source/cmFindPackageCommand.cxx b/Source/cmFindPackageCommand.cxx index 165dbc2..7dbbf6d 100644 --- a/Source/cmFindPackageCommand.cxx +++ b/Source/cmFindPackageCommand.cxx @@ -736,7 +736,7 @@ bool cmFindPackageCommand::HandlePackageMode() } // Try to load the config file if the directory is known - bool cachedDirectoryOk = false; + bool fileFound = false; if(!cmSystemTools::IsOff(def)) { // Get the directory from the variable value. @@ -754,25 +754,30 @@ bool cmFindPackageCommand::HandlePackageMode() if (this->FindConfigFile(dir, file)) { this->FileFound = file; - cachedDirectoryOk = true; + fileFound = true; } def = this->Makefile->GetDefinition(this->Variable.c_str()); } // Search for the config file if it is not already found. - if(cmSystemTools::IsOff(def) || !cachedDirectoryOk) + if(cmSystemTools::IsOff(def) || !fileFound) { - this->FindConfig(); + fileFound = this->FindConfig(); def = this->Makefile->GetDefinition(this->Variable.c_str()); } + // Sanity check. + if(fileFound && this->FileFound.empty()) + { + this->Makefile->IssueMessage( + cmake::INTERNAL_ERROR, "fileFound is true but FileFound is empty!"); + fileFound = false; + } + // If the directory for the config file was found, try to read the file. bool result = true; bool found = false; - // in the following test FileFound should never be empty if def is valid - // but I don't want to put an assert() in there now, in case this still - // makes it into 2.6.3 - if(!cmSystemTools::IsOff(def) && (!this->FileFound.empty())) + if(fileFound) { // Set the version variables before loading the config file. // It may override them. @@ -886,7 +891,7 @@ bool cmFindPackageCommand::HandlePackageMode() } //---------------------------------------------------------------------------- -void cmFindPackageCommand::FindConfig() +bool cmFindPackageCommand::FindConfig() { // Compute the set of search prefixes. this->ComputePrefixes(); @@ -938,9 +943,11 @@ void cmFindPackageCommand::FindConfig() "The directory containing a CMake configuration file for "; help += this->Name; help += "."; + // We force the value since we do not get here if it was already set. this->Makefile->AddCacheDefinition(this->Variable.c_str(), init.c_str(), help.c_str(), - cmCacheManager::PATH); + cmCacheManager::PATH, true); + return found; } //---------------------------------------------------------------------------- diff --git a/Source/cmFindPackageCommand.h b/Source/cmFindPackageCommand.h index 139ca9b..63f4111 100644 --- a/Source/cmFindPackageCommand.h +++ b/Source/cmFindPackageCommand.h @@ -73,7 +73,7 @@ private: void AddFindDefinition(const char* var, const char* val); void RestoreFindDefinitions(); bool HandlePackageMode(); - void FindConfig(); + bool FindConfig(); bool FindPrefixedConfig(); bool FindFrameworkConfig(); bool FindAppBundleConfig(); diff --git a/Tests/FindPackageTest/CMakeLists.txt b/Tests/FindPackageTest/CMakeLists.txt index a62756e..48ca548 100644 --- a/Tests/FindPackageTest/CMakeLists.txt +++ b/Tests/FindPackageTest/CMakeLists.txt @@ -37,6 +37,7 @@ FIND_PACKAGE(VersionTestD 1.2.3.4) SET(PACKAGES foo Foo Bar TFramework Tframework TApp Tapp Special VersionedA VersionedB VersionedC VersionedD VersionedE + WrongA WrongB WrongC WrongD wibbleA wibbleB RecursiveA RecursiveB RecursiveC ) @@ -67,6 +68,24 @@ FIND_PACKAGE(VersionedC 4.0 EXACT NAMES zot) FIND_PACKAGE(VersionedD 1.1 EXACT NAMES Baz) FIND_PACKAGE(VersionedE 1.2 EXACT NAMES Baz) +# Test wrong initial path when result is present. +SET(WrongA_DIR "${VersionedD_DIR}") +FIND_PACKAGE(WrongA 1.2 EXACT NAMES Baz) + +# Test wrong initial cache entry of UNINITIALIZED type when result is present. +SET(WrongB_DIR "${VersionedD_DIR}" CACHE UNINITIALIZED "Wrong Value" FORCE) +GET_PROPERTY(type CACHE WrongB_DIR PROPERTY TYPE) +FIND_PACKAGE(WrongB 1.2 EXACT NAMES Baz) + +# Test wrong initial path when result is missing. +SET(WrongC_DIR "${VersionedD_DIR}") +FIND_PACKAGE(WrongC 1.3 EXACT QUIET NAMES Baz) + +# Test wrong initial cache entry of UNINITIALIZED type when result is missing. +SET(WrongD_DIR "${VersionedD_DIR}" CACHE UNINITIALIZED "Wrong Value" FORCE) +GET_PROPERTY(type CACHE WrongD_DIR PROPERTY TYPE) +FIND_PACKAGE(WrongD 1.3 EXACT QUIET NAMES Baz) + # HINTS should override the system but PATHS should not LIST(INSERT CMAKE_SYSTEM_PREFIX_PATH 0 "${CMAKE_CURRENT_SOURCE_DIR}/A") FIND_PACKAGE(wibbleA NAMES wibble PATHS B) @@ -95,6 +114,10 @@ SET(VersionedB_EXPECTED "lib/zot-3.1/zot-config.cmake") SET(VersionedC_EXPECTED "lib/cmake/zot-4.0/zot-config.cmake") SET(VersionedD_EXPECTED "Baz 1.1/BazConfig.cmake") SET(VersionedE_EXPECTED "Baz 1.2/CMake/BazConfig.cmake") +SET(WrongA_EXPECTED "${VersionedE_EXPECTED}") +SET(WrongB_EXPECTED "${VersionedE_EXPECTED}") +SET(WrongC_MISSING "WrongC_DIR-NOTFOUND") +SET(WrongD_MISSING "WrongD_DIR-NOTFOUND") SET(wibbleA_EXPECTED "A/wibble-config.cmake") SET(wibbleB_EXPECTED "B/wibble-config.cmake") SET(RecursiveA_EXPECTED "lib/RecursiveA/recursivea-config.cmake") @@ -103,7 +126,14 @@ SET(RecursiveC_EXPECTED "lib/zot-3.1/zot-config.cmake") # Check the results. FOREACH(p ${PACKAGES}) - IF(${p}_FOUND) + IF(DEFINED ${p}_MISSING) + # Check and report failure. + IF(NOT "${${p}_DIR}" STREQUAL "${${p}_MISSING}") + MESSAGE(SEND_ERROR + "Package ${p} should have been [${${p}_MISSING}] but " + "was [${${p}_DIR}]") + ENDIF() + ELSEIF(${p}_FOUND) # Convert to relative path for comparison to expected location. FILE(RELATIVE_PATH REL_${p}_CONFIG "${CMAKE_CURRENT_SOURCE_DIR}" "${${p}_CONFIG}") @@ -119,9 +149,9 @@ FOREACH(p ${PACKAGES}) "Package ${p} should have been [${${p}_EXPECTED}] but " "was [${REL_${p}_CONFIG}]") ENDIF(NOT "${REL_${p}_CONFIG}" STREQUAL "${${p}_EXPECTED}") - ELSE(${p}_FOUND) + ELSE() MESSAGE(SEND_ERROR "Package ${p} not found!") - ENDIF(${p}_FOUND) + ENDIF() ENDFOREACH(p) # Check that version information was extracted. -- cgit v0.12