ENH: try number two with topological sort

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,64 @@ 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<cmStdString, std::vector<cmStdString> >::iterator i
+        = m_DirectoryToAfterList.begin();
+      i != m_DirectoryToAfterList.end(); ++i)
     {
-    return true;
-    }
-  std::vector<cmStdString>& 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<cmStdString>::iterator i = d2dirs.begin();
-      i != d2dirs.end(); ++i)
-    {
-    if(*i == d1)
-      {
-      return false;
-      }
-    }
-  return true;
-}
-
-// 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 <cmStdString, cmStdString, bool> 
-{
-  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 )
+    const cmStdString& p = i->first;
+    bool found = false;
+    for(std::map<cmStdString, std::vector<cmStdString> >::iterator j 
+          = m_DirectoryToAfterList.begin(); j != m_DirectoryToAfterList.end() 
+          && !found; ++j)
       {
-      // check for the case when both libraries have to come
-      // before each other 
-      This->AddImpossible(right, left);
+      if(j != i)
+        {
+        found = (std::find(j->second.begin(), j->second.end(), p) != j->second.end());
+        }
       }
-    if ( LBeforeR == RBeforeL )
+    if(!found)
       {
-      return strcmp(left.c_str(), right.c_str()) < 0;
+      path = p;
+      m_DirectoryToAfterList.erase(i);
+      return true;
       }
-    return LBeforeR;
-  }
-  cmOrderLinkDirectories* This;
-};
-
-//-------------------------------------------------------------------
-void cmOrderLinkDirectories::AddImpossible(const cmStdString& d1,
-                                           const cmStdString& d2)
-{
-  m_ImposibleDirectories.insert(d1);
-  m_ImposibleDirectories.insert(d2);
+    }
+  path = "";
+  return false;
 }
 
+
 //-------------------------------------------------------------------
 void cmOrderLinkDirectories::OrderPaths(std::vector<cmStdString>&
                                         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<cmStdString, std::vector<cmStdString> >::iterator i
+          = m_DirectoryToAfterList.begin();
+        i != m_DirectoryToAfterList.end(); ++i)
+      {
+      m_ImposibleDirectories.insert(i->first);
+      // still put it in the path list in the order we find them
+      orderedPaths.push_back(i->first);
+      }
+    
+    }
 }
 
 //-------------------------------------------------------------------
@@ -204,9 +188,11 @@ void cmOrderLinkDirectories::SetLinkInformation(const cmTarget& target,
 {
     // collect the search paths from the target into paths set
   const std::vector<std::string>& searchPaths = target.GetLinkDirectories();
+  std::vector<cmStdString> empty;
   for(std::vector<std::string>::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
@@ -250,12 +236,14 @@ bool cmOrderLinkDirectories::DetermineLibraryPathOrder()
   Library aLib;
   cmStdString dir;
   cmStdString file;
+  std::vector<cmStdString> empty;
   for(unsigned int i=0; i < m_RawLinkItems.size(); ++i)
     {
     if(cmSystemTools::FileIsFullPath(m_RawLinkItems[i].c_str()))
       {
       cmSystemTools::SplitProgramPath(m_RawLinkItems[i].c_str(),
                                       dir, file);
+      m_DirectoryToAfterList[dir] = empty;
       m_LinkPathSet.insert(dir);
       aLib.FullPath = m_RawLinkItems[i];
       aLib.File = file;
@@ -284,13 +272,8 @@ bool cmOrderLinkDirectories::DetermineLibraryPathOrder()
     }
   this->FindIndividualLibraryOrders();
   m_SortedSearchPaths.clear();
-  for(std::set<cmStdString>::iterator i = m_LinkPathSet.begin();
-      i != m_LinkPathSet.end(); ++i)
-    {
-    m_SortedSearchPaths.push_back(*i);
-    }
   
-  this->OrderPaths(m_SortedSearchPaths);
+  this->OrderPaths(m_SortedSearchPaths); 
   // now turn libfoo.a into foo and foo.a into foo
   // This will prepare the link items for -litem 
   this->PrepareLinkTargets();
@@ -310,7 +293,7 @@ std::string cmOrderLinkDirectories::GetWarnings()
     {
     warning += "Directory: ";
     warning += *i;
-    warning += " contains ";
+    warning += " contains:\n";
     std::map<cmStdString, std::vector<cmStdString> >::iterator j;
     for(j = m_LibraryToDirectories.begin(); 
         j != m_LibraryToDirectories.end(); ++j)
@@ -323,6 +306,7 @@ std::string cmOrderLinkDirectories::GetWarnings()
         warning += "\n";
         }
       }
+    warning += "\n";
     }
   warning += "\n";
   return warning;

Source/cmOrderLinkDirectories.h

@@ -91,10 +91,7 @@ private:
   void PrintMap(const char* name,
                 std::map<cmStdString, std::vector<cmStdString> >& m);
   void OrderPaths(std::vector<cmStdString>& 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<cmStdString, std::vector<cmStdString> > m_LibraryToDirectories;

Tests/Complex/Executable/complex.cxx

@@ -57,24 +57,24 @@ bool TestLibraryOrder(bool shouldFail)
   orderLibs.SetLinkInformation(target, cmTarget::GENERAL, "A");
   bool ret = orderLibs.DetermineLibraryPathOrder();
   orderLibs.GetLinkerInformation(sortedpaths, linkItems);
-  std::cerr << "Sorted Link Paths:\n";
+  std::cout << "Sorted Link Paths:\n";
   for(std::vector<cmStdString>::iterator i = sortedpaths.begin();
       i != sortedpaths.end(); ++i)
     {
-    std::cerr << *i << "\n";
+    std::cout << *i << "\n";
     }
-  std::cerr << "Link Items: \n";
+  std::cout << "Link Items: \n";
   for(std::vector<cmStdString>::iterator i = linkItems.begin();
       i != linkItems.end(); ++i)
     {
-    std::cerr << *i << "\n";
+    std::cout << *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";
+    std::cout << "fail because link items should be A B C -lm and the are not\n";
     return shouldFail;
     }
   
@@ -82,16 +82,18 @@ bool TestLibraryOrder(bool 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' ))
+    char order[5];
+    order[4] = 0;
+    for(int i =0; i < 4; ++i)
       {
-      std::cerr << "fail because order should be /lib/extra/stuff B C A and it is not\n";
+      order[i] = sortedpaths[i][sortedpaths[i].size()-1];
+      }
+    if(!(strcmp(order, "fBCA") == 0 || strcmp(order, "BCAf") == 0))
+      {
+      std::cout << "fail because order should be /lib/extra/stuff B C A and it is not\n";
       return false;
       }
     }
-  
   return ret;
 }
 
@@ -999,6 +1001,29 @@ int main()
 #else
   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 failed when it should.");
+    }
+  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
 

Tests/ComplexOneConfig/Executable/complex.cxx

@@ -57,24 +57,24 @@ bool TestLibraryOrder(bool shouldFail)
   orderLibs.SetLinkInformation(target, cmTarget::GENERAL, "A");
   bool ret = orderLibs.DetermineLibraryPathOrder();
   orderLibs.GetLinkerInformation(sortedpaths, linkItems);
-  std::cerr << "Sorted Link Paths:\n";
+  std::cout << "Sorted Link Paths:\n";
   for(std::vector<cmStdString>::iterator i = sortedpaths.begin();
       i != sortedpaths.end(); ++i)
     {
-    std::cerr << *i << "\n";
+    std::cout << *i << "\n";
     }
-  std::cerr << "Link Items: \n";
+  std::cout << "Link Items: \n";
   for(std::vector<cmStdString>::iterator i = linkItems.begin();
       i != linkItems.end(); ++i)
     {
-    std::cerr << *i << "\n";
+    std::cout << *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";
+    std::cout << "fail because link items should be A B C -lm and the are not\n";
     return shouldFail;
     }
   
@@ -82,16 +82,18 @@ bool TestLibraryOrder(bool 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' ))
+    char order[5];
+    order[4] = 0;
+    for(int i =0; i < 4; ++i)
       {
-      std::cerr << "fail because order should be /lib/extra/stuff B C A and it is not\n";
+      order[i] = sortedpaths[i][sortedpaths[i].size()-1];
+      }
+    if(!(strcmp(order, "fBCA") == 0 || strcmp(order, "BCAf") == 0))
+      {
+      std::cout << "fail because order should be /lib/extra/stuff B C A and it is not\n";
       return false;
       }
     }
-  
   return ret;
 }
 
@@ -999,6 +1001,29 @@ int main()
 #else
   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 failed when it should.");
+    }
+  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
 

Tests/ComplexRelativePaths/Executable/complex.cxx

@@ -57,24 +57,24 @@ bool TestLibraryOrder(bool shouldFail)
   orderLibs.SetLinkInformation(target, cmTarget::GENERAL, "A");
   bool ret = orderLibs.DetermineLibraryPathOrder();
   orderLibs.GetLinkerInformation(sortedpaths, linkItems);
-  std::cerr << "Sorted Link Paths:\n";
+  std::cout << "Sorted Link Paths:\n";
   for(std::vector<cmStdString>::iterator i = sortedpaths.begin();
       i != sortedpaths.end(); ++i)
     {
-    std::cerr << *i << "\n";
+    std::cout << *i << "\n";
     }
-  std::cerr << "Link Items: \n";
+  std::cout << "Link Items: \n";
   for(std::vector<cmStdString>::iterator i = linkItems.begin();
       i != linkItems.end(); ++i)
     {
-    std::cerr << *i << "\n";
+    std::cout << *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";
+    std::cout << "fail because link items should be A B C -lm and the are not\n";
     return shouldFail;
     }
   
@@ -82,16 +82,18 @@ bool TestLibraryOrder(bool 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' ))
+    char order[5];
+    order[4] = 0;
+    for(int i =0; i < 4; ++i)
       {
-      std::cerr << "fail because order should be /lib/extra/stuff B C A and it is not\n";
+      order[i] = sortedpaths[i][sortedpaths[i].size()-1];
+      }
+    if(!(strcmp(order, "fBCA") == 0 || strcmp(order, "BCAf") == 0))
+      {
+      std::cout << "fail because order should be /lib/extra/stuff B C A and it is not\n";
       return false;
       }
     }
-  
   return ret;
 }
 
@@ -999,6 +1001,29 @@ int main()
 #else
   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 failed when it should.");
+    }
+  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