diff --git a/Source/cmOrderLinkDirectories.cxx b/Source/cmOrderLinkDirectories.cxx index 09f1f85f7..36486eb22 100644 --- a/Source/cmOrderLinkDirectories.cxx +++ b/Source/cmOrderLinkDirectories.cxx @@ -52,7 +52,7 @@ void cmOrderLinkDirectories::FindLibrariesInSeachPaths() { if(lib->second.Path != *dir) { - if(LibraryInDirectory(dir->c_str(), lib->second.File.c_str())) + if(this->LibraryInDirectory(dir->c_str(), lib->second.File.c_str())) { m_LibraryToDirectories[lib->second.FullPath].push_back(*dir); } @@ -120,80 +120,68 @@ void cmOrderLinkDirectories::PrepareLinkTargets() } //------------------------------------------------------------------- -bool cmOrderLinkDirectories::CanBeBefore(const cmStdString& d1, - const cmStdString& d2) +bool cmOrderLinkDirectories::FindPathNotInDirectoryToAfterList( + cmStdString& path) { - if(m_DirectoryToAfterList.count(d2) == 0) + for(std::map >::iterator i + = m_DirectoryToAfterList.begin(); + i != m_DirectoryToAfterList.end(); ++i) { - return true; - } - std::vector& d2dirs = m_DirectoryToAfterList[d2]; - // is d1 in the d2's list of directories that d2 must be before - // if so, then d1 can not come before d2 - for(std::vector::iterator i = d2dirs.begin(); - i != d2dirs.end(); ++i) - { - if(*i == d1) + const cmStdString& p = i->first; + bool found = false; + for(std::map >::iterator j + = m_DirectoryToAfterList.begin(); j != m_DirectoryToAfterList.end() + && !found; ++j) { - return false; + if(j != i) + { + found = (std::find(j->second.begin(), j->second.end(), p) != j->second.end()); + } + } + if(!found) + { + path = p; + m_DirectoryToAfterList.erase(i); + return true; } } - return true; + return false; } -// This is a stl function object used to sort -// the vector of library paths. It returns true -// if left directory can be before right directory (no swap). -// It also checks for the impossible case of two libraries and -// two directories that have both libraries. -struct cmOrderLinkDirectoriesCompare - : public std::binary_function -{ - cmOrderLinkDirectoriesCompare() - { - This = 0; - } - bool operator()( - const cmStdString& left, - const cmStdString& right - ) const - { - bool LBeforeR= This->CanBeBefore(left, right); - bool RBeforeL= This->CanBeBefore(right, left); - - if ( !LBeforeR && !RBeforeL ) - { - // check for the case when both libraries have to come - // before each other - This->AddImpossible(right, left); - } - if ( LBeforeR == RBeforeL ) - { - return strcmp(left.c_str(), right.c_str()) < 0; - } - return LBeforeR; - } - cmOrderLinkDirectories* This; -}; - -//------------------------------------------------------------------- -void cmOrderLinkDirectories::AddImpossible(const cmStdString& d1, - const cmStdString& d2) -{ - m_ImposibleDirectories.insert(d1); - m_ImposibleDirectories.insert(d2); -} //------------------------------------------------------------------- void cmOrderLinkDirectories::OrderPaths(std::vector& orderedPaths) { - cmOrderLinkDirectoriesCompare comp; - comp.This = this; - // for some reason when cmake is run on InsightApplication - // if std::sort is used here cmake crashes, but stable_sort works - // - std::sort(orderedPaths.begin(), orderedPaths.end(), comp); + cmStdString path; + // This is a topological sort implementation + // One at a time find paths that are not in any other paths after list + // and put them into the orderedPaths vector in that order + // FindPathNotInDirectoryToAfterList removes the path from the + // m_DirectoryToAfterList once it is found + while(this->FindPathNotInDirectoryToAfterList(path)) + { + orderedPaths.push_back(path); + } + // at this point if there are still paths in m_DirectoryToAfterList + // then there is a cycle and we are stuck + if(m_DirectoryToAfterList.size()) + { + for(std::map >::iterator i + = m_DirectoryToAfterList.begin(); + i != m_DirectoryToAfterList.end(); ++i) + { + m_ImposibleDirectories.insert(i->first); + } + // if it failed, then fall back to original order + orderedPaths.clear(); + for(std::set::iterator dir = m_LinkPathSet.begin(); + dir != m_LinkPathSet.end(); ++dir) + { + orderedPaths.push_back(*dir); + } + + } } //------------------------------------------------------------------- @@ -204,9 +192,11 @@ void cmOrderLinkDirectories::SetLinkInformation(const cmTarget& target, { // collect the search paths from the target into paths set const std::vector& searchPaths = target.GetLinkDirectories(); + std::vector empty; for(std::vector::const_iterator p = searchPaths.begin(); p != searchPaths.end(); ++p) { + m_DirectoryToAfterList[*p] = empty; m_LinkPathSet.insert(*p); } // collect the link items from the target and put it into libs @@ -284,12 +274,6 @@ bool cmOrderLinkDirectories::DetermineLibraryPathOrder() } this->FindIndividualLibraryOrders(); m_SortedSearchPaths.clear(); - for(std::set::iterator i = m_LinkPathSet.begin(); - i != m_LinkPathSet.end(); ++i) - { - m_SortedSearchPaths.push_back(*i); - } - this->OrderPaths(m_SortedSearchPaths); // now turn libfoo.a into foo and foo.a into foo // This will prepare the link items for -litem @@ -310,7 +294,7 @@ std::string cmOrderLinkDirectories::GetWarnings() { warning += "Directory: "; warning += *i; - warning += " contains "; + warning += " contains:\n"; std::map >::iterator j; for(j = m_LibraryToDirectories.begin(); j != m_LibraryToDirectories.end(); ++j) @@ -323,6 +307,7 @@ std::string cmOrderLinkDirectories::GetWarnings() warning += "\n"; } } + warning += "\n"; } warning += "\n"; return warning; diff --git a/Source/cmOrderLinkDirectories.h b/Source/cmOrderLinkDirectories.h index 6634f2347..09441b97a 100644 --- a/Source/cmOrderLinkDirectories.h +++ b/Source/cmOrderLinkDirectories.h @@ -91,10 +91,7 @@ private: void PrintMap(const char* name, std::map >& m); void OrderPaths(std::vector& paths); - bool CanBeBefore(const cmStdString& d1, - const cmStdString& d2); - void AddImpossible(const cmStdString& , - const cmStdString& ); + bool FindPathNotInDirectoryToAfterList(cmStdString& path); private: // map from library to directories that it is in other than its full path std::map > m_LibraryToDirectories; diff --git a/Tests/Complex/Executable/CMakeLists.txt b/Tests/Complex/Executable/CMakeLists.txt index a7ee95db5..55b02e11a 100644 --- a/Tests/Complex/Executable/CMakeLists.txt +++ b/Tests/Complex/Executable/CMakeLists.txt @@ -81,3 +81,9 @@ SOURCE_GROUP(A_GROUP ".cxx") SOURCE_GROUP(B_GROUP REGULAR_EXPRESSION "cxx") SOURCE_GROUP(C_GROUP FILES complex.cxx) +FILE(WRITE ${Complex_BINARY_DIR}/A/libA.a "test") +FILE(WRITE ${Complex_BINARY_DIR}/A/libC.a "test") +FILE(WRITE ${Complex_BINARY_DIR}/B/libB.a "test") +FILE(WRITE ${Complex_BINARY_DIR}/B/libA.a "test") +FILE(WRITE ${Complex_BINARY_DIR}/C/libC.a "test") +FILE(WRITE ${Complex_BINARY_DIR}/C/libB.a "test") diff --git a/Tests/Complex/Executable/complex.cxx b/Tests/Complex/Executable/complex.cxx index 906616e78..78f5b7306 100644 --- a/Tests/Complex/Executable/complex.cxx +++ b/Tests/Complex/Executable/complex.cxx @@ -11,6 +11,8 @@ extern "C" { #include "cmStandardIncludes.h" #include "cmSystemTools.h" #include "cmDynamicLoader.h" +#include "cmSystemTools.h" +#include "cmOrderLinkDirectories.h" int cm_passed = 0; int cm_failed = 0; @@ -21,6 +23,77 @@ This is a problem. Looks like ADD_DEFINITIONS and REMOVE_DEFINITIONS does not wo // Here is a stupid function that tries to use std::string methods // so that the dec cxx compiler will instantiate the stuff that // we are using from the CMakeLib library.... +bool TestLibraryOrder(bool shouldFail) +{ + std::string Adir = std::string(BINARY_DIR) + std::string("/A"); + std::string Bdir = std::string(BINARY_DIR) + std::string("/B"); + std::string Cdir = std::string(BINARY_DIR) + std::string("/C"); + + if(!shouldFail) + { + std::string rm = Bdir; + rm += "/libA.a"; + cmSystemTools::RemoveFile(rm.c_str()); + } + cmTarget target; + target.AddLinkDirectory(Adir.c_str()); + target.AddLinkDirectory(Bdir.c_str()); + target.AddLinkDirectory(Cdir.c_str()); + target.AddLinkDirectory("/lib/extra/stuff"); + + Adir += "/libA.a"; + Bdir += "/libB.a"; + Cdir += "/libC.a"; + + target.AddLinkLibrary(Adir.c_str(), cmTarget::GENERAL); + target.AddLinkLibrary(Bdir.c_str(), cmTarget::GENERAL); + target.AddLinkLibrary(Cdir.c_str(), cmTarget::GENERAL); + target.AddLinkLibrary("-lm", cmTarget::GENERAL); + std::vector sortedpaths; + std::vector linkItems; + cmOrderLinkDirectories orderLibs; + orderLibs.AddLinkExtension(".so"); + orderLibs.AddLinkExtension(".a"); + orderLibs.SetLinkInformation(target, cmTarget::GENERAL, "A"); + bool ret = orderLibs.DetermineLibraryPathOrder(); + orderLibs.GetLinkerInformation(sortedpaths, linkItems); + std::cerr << "Sorted Link Paths:\n"; + for(std::vector::iterator i = sortedpaths.begin(); + i != sortedpaths.end(); ++i) + { + std::cerr << *i << "\n"; + } + std::cerr << "Link Items: \n"; + for(std::vector::iterator i = linkItems.begin(); + i != linkItems.end(); ++i) + { + std::cerr << *i << "\n"; + } + if(!(linkItems[0] == "A" && + linkItems[1] == "B" && + linkItems[2] == "C" && + linkItems[3] == "-lm" )) + { + std::cerr << "fail because link items should be A B C -lm and the are not\n"; + return shouldFail; + } + + + // if this is not the fail test then the order should be f B C A + if(!shouldFail) + { + if(!(sortedpaths[0][sortedpaths[0].size()-1] == 'f' && + sortedpaths[1][sortedpaths[1].size()-1] == 'B' && + sortedpaths[2][sortedpaths[2].size()-1] == 'C' && + sortedpaths[3][sortedpaths[3].size()-1] == 'A' )) + { + std::cerr << "fail because order should be /lib/extra/stuff B C A and it is not\n"; + return false; + } + } + + return ret; +} void ForceStringUse() { @@ -927,6 +1000,28 @@ int main() cmPassed("CMake SET CACHE FORCE"); #endif + // first run with shouldFail = true, this will + // run with A B C as set by the CMakeList.txt file. + if(!TestLibraryOrder(true)) + { + cmPassed("CMake cmOrderLinkDirectories worked."); + } + else + { + cmFailed("CMake cmOrderLinkDirectories failed to fail when given an impossible set of paths."); + } + // next run with shouldPass = true, this will + // run with B/libA.a removed and should create the order + // B C A + if(TestLibraryOrder(false)) + { + cmPassed("CMake cmOrderLinkDirectories worked."); + } + else + { + cmFailed("CMake cmOrderLinkDirectories failed."); + } + // ---------------------------------------------------------------------- // Summary @@ -936,6 +1031,5 @@ int main() std::cout << "Failed: " << cm_failed << "\n"; return cm_failed; } - return 0; } diff --git a/Tests/ComplexOneConfig/Executable/CMakeLists.txt b/Tests/ComplexOneConfig/Executable/CMakeLists.txt index a7ee95db5..55b02e11a 100644 --- a/Tests/ComplexOneConfig/Executable/CMakeLists.txt +++ b/Tests/ComplexOneConfig/Executable/CMakeLists.txt @@ -81,3 +81,9 @@ SOURCE_GROUP(A_GROUP ".cxx") SOURCE_GROUP(B_GROUP REGULAR_EXPRESSION "cxx") SOURCE_GROUP(C_GROUP FILES complex.cxx) +FILE(WRITE ${Complex_BINARY_DIR}/A/libA.a "test") +FILE(WRITE ${Complex_BINARY_DIR}/A/libC.a "test") +FILE(WRITE ${Complex_BINARY_DIR}/B/libB.a "test") +FILE(WRITE ${Complex_BINARY_DIR}/B/libA.a "test") +FILE(WRITE ${Complex_BINARY_DIR}/C/libC.a "test") +FILE(WRITE ${Complex_BINARY_DIR}/C/libB.a "test") diff --git a/Tests/ComplexOneConfig/Executable/complex.cxx b/Tests/ComplexOneConfig/Executable/complex.cxx index 906616e78..78f5b7306 100644 --- a/Tests/ComplexOneConfig/Executable/complex.cxx +++ b/Tests/ComplexOneConfig/Executable/complex.cxx @@ -11,6 +11,8 @@ extern "C" { #include "cmStandardIncludes.h" #include "cmSystemTools.h" #include "cmDynamicLoader.h" +#include "cmSystemTools.h" +#include "cmOrderLinkDirectories.h" int cm_passed = 0; int cm_failed = 0; @@ -21,6 +23,77 @@ This is a problem. Looks like ADD_DEFINITIONS and REMOVE_DEFINITIONS does not wo // Here is a stupid function that tries to use std::string methods // so that the dec cxx compiler will instantiate the stuff that // we are using from the CMakeLib library.... +bool TestLibraryOrder(bool shouldFail) +{ + std::string Adir = std::string(BINARY_DIR) + std::string("/A"); + std::string Bdir = std::string(BINARY_DIR) + std::string("/B"); + std::string Cdir = std::string(BINARY_DIR) + std::string("/C"); + + if(!shouldFail) + { + std::string rm = Bdir; + rm += "/libA.a"; + cmSystemTools::RemoveFile(rm.c_str()); + } + cmTarget target; + target.AddLinkDirectory(Adir.c_str()); + target.AddLinkDirectory(Bdir.c_str()); + target.AddLinkDirectory(Cdir.c_str()); + target.AddLinkDirectory("/lib/extra/stuff"); + + Adir += "/libA.a"; + Bdir += "/libB.a"; + Cdir += "/libC.a"; + + target.AddLinkLibrary(Adir.c_str(), cmTarget::GENERAL); + target.AddLinkLibrary(Bdir.c_str(), cmTarget::GENERAL); + target.AddLinkLibrary(Cdir.c_str(), cmTarget::GENERAL); + target.AddLinkLibrary("-lm", cmTarget::GENERAL); + std::vector sortedpaths; + std::vector linkItems; + cmOrderLinkDirectories orderLibs; + orderLibs.AddLinkExtension(".so"); + orderLibs.AddLinkExtension(".a"); + orderLibs.SetLinkInformation(target, cmTarget::GENERAL, "A"); + bool ret = orderLibs.DetermineLibraryPathOrder(); + orderLibs.GetLinkerInformation(sortedpaths, linkItems); + std::cerr << "Sorted Link Paths:\n"; + for(std::vector::iterator i = sortedpaths.begin(); + i != sortedpaths.end(); ++i) + { + std::cerr << *i << "\n"; + } + std::cerr << "Link Items: \n"; + for(std::vector::iterator i = linkItems.begin(); + i != linkItems.end(); ++i) + { + std::cerr << *i << "\n"; + } + if(!(linkItems[0] == "A" && + linkItems[1] == "B" && + linkItems[2] == "C" && + linkItems[3] == "-lm" )) + { + std::cerr << "fail because link items should be A B C -lm and the are not\n"; + return shouldFail; + } + + + // if this is not the fail test then the order should be f B C A + if(!shouldFail) + { + if(!(sortedpaths[0][sortedpaths[0].size()-1] == 'f' && + sortedpaths[1][sortedpaths[1].size()-1] == 'B' && + sortedpaths[2][sortedpaths[2].size()-1] == 'C' && + sortedpaths[3][sortedpaths[3].size()-1] == 'A' )) + { + std::cerr << "fail because order should be /lib/extra/stuff B C A and it is not\n"; + return false; + } + } + + return ret; +} void ForceStringUse() { @@ -927,6 +1000,28 @@ int main() cmPassed("CMake SET CACHE FORCE"); #endif + // first run with shouldFail = true, this will + // run with A B C as set by the CMakeList.txt file. + if(!TestLibraryOrder(true)) + { + cmPassed("CMake cmOrderLinkDirectories worked."); + } + else + { + cmFailed("CMake cmOrderLinkDirectories failed to fail when given an impossible set of paths."); + } + // next run with shouldPass = true, this will + // run with B/libA.a removed and should create the order + // B C A + if(TestLibraryOrder(false)) + { + cmPassed("CMake cmOrderLinkDirectories worked."); + } + else + { + cmFailed("CMake cmOrderLinkDirectories failed."); + } + // ---------------------------------------------------------------------- // Summary @@ -936,6 +1031,5 @@ int main() std::cout << "Failed: " << cm_failed << "\n"; return cm_failed; } - return 0; } diff --git a/Tests/ComplexRelativePaths/Executable/CMakeLists.txt b/Tests/ComplexRelativePaths/Executable/CMakeLists.txt index a7ee95db5..55b02e11a 100644 --- a/Tests/ComplexRelativePaths/Executable/CMakeLists.txt +++ b/Tests/ComplexRelativePaths/Executable/CMakeLists.txt @@ -81,3 +81,9 @@ SOURCE_GROUP(A_GROUP ".cxx") SOURCE_GROUP(B_GROUP REGULAR_EXPRESSION "cxx") SOURCE_GROUP(C_GROUP FILES complex.cxx) +FILE(WRITE ${Complex_BINARY_DIR}/A/libA.a "test") +FILE(WRITE ${Complex_BINARY_DIR}/A/libC.a "test") +FILE(WRITE ${Complex_BINARY_DIR}/B/libB.a "test") +FILE(WRITE ${Complex_BINARY_DIR}/B/libA.a "test") +FILE(WRITE ${Complex_BINARY_DIR}/C/libC.a "test") +FILE(WRITE ${Complex_BINARY_DIR}/C/libB.a "test") diff --git a/Tests/ComplexRelativePaths/Executable/complex.cxx b/Tests/ComplexRelativePaths/Executable/complex.cxx index 906616e78..78f5b7306 100644 --- a/Tests/ComplexRelativePaths/Executable/complex.cxx +++ b/Tests/ComplexRelativePaths/Executable/complex.cxx @@ -11,6 +11,8 @@ extern "C" { #include "cmStandardIncludes.h" #include "cmSystemTools.h" #include "cmDynamicLoader.h" +#include "cmSystemTools.h" +#include "cmOrderLinkDirectories.h" int cm_passed = 0; int cm_failed = 0; @@ -21,6 +23,77 @@ This is a problem. Looks like ADD_DEFINITIONS and REMOVE_DEFINITIONS does not wo // Here is a stupid function that tries to use std::string methods // so that the dec cxx compiler will instantiate the stuff that // we are using from the CMakeLib library.... +bool TestLibraryOrder(bool shouldFail) +{ + std::string Adir = std::string(BINARY_DIR) + std::string("/A"); + std::string Bdir = std::string(BINARY_DIR) + std::string("/B"); + std::string Cdir = std::string(BINARY_DIR) + std::string("/C"); + + if(!shouldFail) + { + std::string rm = Bdir; + rm += "/libA.a"; + cmSystemTools::RemoveFile(rm.c_str()); + } + cmTarget target; + target.AddLinkDirectory(Adir.c_str()); + target.AddLinkDirectory(Bdir.c_str()); + target.AddLinkDirectory(Cdir.c_str()); + target.AddLinkDirectory("/lib/extra/stuff"); + + Adir += "/libA.a"; + Bdir += "/libB.a"; + Cdir += "/libC.a"; + + target.AddLinkLibrary(Adir.c_str(), cmTarget::GENERAL); + target.AddLinkLibrary(Bdir.c_str(), cmTarget::GENERAL); + target.AddLinkLibrary(Cdir.c_str(), cmTarget::GENERAL); + target.AddLinkLibrary("-lm", cmTarget::GENERAL); + std::vector sortedpaths; + std::vector linkItems; + cmOrderLinkDirectories orderLibs; + orderLibs.AddLinkExtension(".so"); + orderLibs.AddLinkExtension(".a"); + orderLibs.SetLinkInformation(target, cmTarget::GENERAL, "A"); + bool ret = orderLibs.DetermineLibraryPathOrder(); + orderLibs.GetLinkerInformation(sortedpaths, linkItems); + std::cerr << "Sorted Link Paths:\n"; + for(std::vector::iterator i = sortedpaths.begin(); + i != sortedpaths.end(); ++i) + { + std::cerr << *i << "\n"; + } + std::cerr << "Link Items: \n"; + for(std::vector::iterator i = linkItems.begin(); + i != linkItems.end(); ++i) + { + std::cerr << *i << "\n"; + } + if(!(linkItems[0] == "A" && + linkItems[1] == "B" && + linkItems[2] == "C" && + linkItems[3] == "-lm" )) + { + std::cerr << "fail because link items should be A B C -lm and the are not\n"; + return shouldFail; + } + + + // if this is not the fail test then the order should be f B C A + if(!shouldFail) + { + if(!(sortedpaths[0][sortedpaths[0].size()-1] == 'f' && + sortedpaths[1][sortedpaths[1].size()-1] == 'B' && + sortedpaths[2][sortedpaths[2].size()-1] == 'C' && + sortedpaths[3][sortedpaths[3].size()-1] == 'A' )) + { + std::cerr << "fail because order should be /lib/extra/stuff B C A and it is not\n"; + return false; + } + } + + return ret; +} void ForceStringUse() { @@ -927,6 +1000,28 @@ int main() cmPassed("CMake SET CACHE FORCE"); #endif + // first run with shouldFail = true, this will + // run with A B C as set by the CMakeList.txt file. + if(!TestLibraryOrder(true)) + { + cmPassed("CMake cmOrderLinkDirectories worked."); + } + else + { + cmFailed("CMake cmOrderLinkDirectories failed to fail when given an impossible set of paths."); + } + // next run with shouldPass = true, this will + // run with B/libA.a removed and should create the order + // B C A + if(TestLibraryOrder(false)) + { + cmPassed("CMake cmOrderLinkDirectories worked."); + } + else + { + cmFailed("CMake cmOrderLinkDirectories failed."); + } + // ---------------------------------------------------------------------- // Summary @@ -936,6 +1031,5 @@ int main() std::cout << "Failed: " << cm_failed << "\n"; return cm_failed; } - return 0; }