Fix find_package() when <pkg>_DIR is wrong
When <pkg>_DIR is set to an incorrect version we search again and store the result in the variable, even if it is <pkg>_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 <pkg>_DIR-NOTFOUND as expected.
This commit is contained in:
Brad King
parent
1b5a986a42
commit
71910b3fd4
|
|
@ -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;
|
||||
}
|
||||
|
||||
//----------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Reference in New Issue