BUG: Correct some of the dependency analysis code.

- Make sure the original link line is untouched
- Avoid duplicating the link line when supporting version < 1.4
- Make sure the cyclic dependencies and such are output correctly in
  complicated cases.
- Avoid outputing dependencies that are already satisfied on the original
  link line when possible.

3 files changed, 82 insertions, 51 deletions

diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt
index 56713da..f21807b 100644
--- a/Source/CMakeLists.txt
+++ b/Source/CMakeLists.txt
@@ -177,6 +177,34 @@ IF(BUILD_TESTING)
       ${CMake_BINARY_DIR}/Tests/Dependency/WOLibOut/Exec
       Dependency)
 
+    ADD_TEST(dependency2 ${CMake_BINARY_DIR}/Source/cmaketest 
+      ${CMake_SOURCE_DIR}/Tests/Dependency 
+      ${CMake_BINARY_DIR}/Tests/Dependency/WithLibOut
+      exec2
+      ${CMake_BINARY_DIR}/Tests/Dependency/WithLibOut/Exec2
+      Dependency CMAKE_ARGS -DLIBRARY_OUTPUT_PATH:PATH=${CMake_BINARY_DIR}/Tests/Dependency/WithLibOut/Lib)
+
+    ADD_TEST(dependency3 ${CMake_BINARY_DIR}/Source/cmaketest 
+      ${CMake_SOURCE_DIR}/Tests/Dependency 
+      ${CMake_BINARY_DIR}/Tests/Dependency/WithLibOut
+      exec3
+      ${CMake_BINARY_DIR}/Tests/Dependency/WithLibOut/Exec3
+      Dependency CMAKE_ARGS -DLIBRARY_OUTPUT_PATH:PATH=${CMake_BINARY_DIR}/Tests/Dependency/WithLibOut/Lib)
+
+    ADD_TEST(dependency4 ${CMake_BINARY_DIR}/Source/cmaketest 
+      ${CMake_SOURCE_DIR}/Tests/Dependency 
+      ${CMake_BINARY_DIR}/Tests/Dependency/WithLibOut
+      exec4
+      ${CMake_BINARY_DIR}/Tests/Dependency/WithLibOut/Exec4
+      Dependency CMAKE_ARGS -DLIBRARY_OUTPUT_PATH:PATH=${CMake_BINARY_DIR}/Tests/Dependency/WithLibOut/Lib)
+
+    ADD_TEST(linkline ${CMake_BINARY_DIR}/Source/cmaketest 
+      ${CMake_SOURCE_DIR}/Tests/LinkLine
+      ${CMake_BINARY_DIR}/Tests/LinkLine
+      Exec
+      ${CMake_BINARY_DIR}/Tests/LinkLine
+      LinkLine)
+
   ENDIF (DART_ROOT)
 ENDIF(BUILD_TESTING)
 
diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx
index 62da8cf..13a5358 100644
--- a/Source/cmTarget.cxx
+++ b/Source/cmTarget.cxx
@@ -19,7 +19,7 @@
 
 #include <map>
 #include <set>
-
+#include <cassert>
 
 
 void cmTarget::GenerateSourceFilesFromSourceLists( cmMakefile &mf)
@@ -73,15 +73,17 @@ void cmTarget::MergeLinkLibraries( cmMakefile& mf,
                                    const char *selfname,
                                    const LinkLibraries& libs )
 {
-  for( LinkLibraries::const_iterator i = libs.begin();
-       i != libs.end(); ++i )
+  // Only add on libraries we haven't added on before.
+  // Assumption: the global link libraries could only grow, never shrink
+  assert( libs.size() >= m_PrevLinkedLibraries.size() );
+  LinkLibraries::const_iterator i = libs.begin();
+  i += m_PrevLinkedLibraries.size();
+  for( ; i != libs.end(); ++i )
     {
-    if(m_PrevLinkedLibraries.insert(i->first).second)
-      {
-      // We call this so that the dependencies get written to the cache
-      this->AddLinkLibrary( mf, selfname, i->first.c_str(), i->second );
-      }
+    // We call this so that the dependencies get written to the cache
+    this->AddLinkLibrary( mf, selfname, i->first.c_str(), i->second );
     }
+  m_PrevLinkedLibraries = libs;
 }
 
 void cmTarget::AddLinkDirectory(const char* d)
@@ -231,8 +233,8 @@ cmTarget::AnalyzeLibDependencies( const cmMakefile& mf )
   // 3. Create the new link line by simply emitting any dependencies that are
   // missing.  Start from the back and keep adding.
   
-  LinkLibraries newLinkLibraries = m_LinkLibraries;
   std::set<cmStdString> done, visited;
+  std::vector<std::string> newLinkLibraries;
   for(LinkLibraries::reverse_iterator lib = m_LinkLibraries.rbegin();
       lib != m_LinkLibraries.rend(); ++lib)
     {
@@ -240,56 +242,57 @@ cmTarget::AnalyzeLibDependencies( const cmMakefile& mf )
     // if a variable expands to nothing.
     if (lib->first.size() == 0) continue;
 
-    std::vector<std::string> link_line;
-    Emit( lib->first, dep_map, done, visited, link_line );
-    if( link_line.size() == 0 )
+    // Emit all the dependencies that are not already satisfied on the
+    // original link line.
+    if( dep_map.find(lib->first) != dep_map.end() ) // does it have dependencies?
       {
-      // everything for this library is already on the link line, but since
-      // we are not going to touch the user's link line, we will output the
-      // library anyway.
-      newLinkLibraries.push_back( *lib );
+      const std::set<cmStdString>& dep_on = dep_map.find( lib->first )->second;
+      std::set<cmStdString>::iterator i;
+      for( i = dep_on.begin(); i != dep_on.end(); ++i )
+        {
+        if( satisfied[lib->first].end() == satisfied[lib->first].find( *i ) )
+          {
+          Emit( *i, dep_map, done, visited, newLinkLibraries );
+          }
+        }
       }
-    else
+    }
+
+  // 4. Add the new libraries to the link line.
+
+  for( std::vector<std::string>::reverse_iterator k = newLinkLibraries.rbegin();
+       k != newLinkLibraries.rend(); ++k )
+    {
+    if( addLibDirs )
       {
-      for( std::vector<std::string>::reverse_iterator k = link_line.rbegin();
-           k != link_line.rend(); ++k )
+      const char* libpath = mf.GetDefinition( k->c_str() );
+      if( libpath )
         {
-        if( satisfied[lib->first].insert( *k ).second )
+        // Don't add a link directory that is already present.
+        if(std::find(m_LinkDirectories.begin(),
+                     m_LinkDirectories.end(), libpath) == m_LinkDirectories.end())
           {
-          if( addLibDirs )
-            {
-            const char* libpath = mf.GetDefinition( k->c_str() );
-            if( libpath )
-              {
-              // Don't add a link directory that is already present.
-              if(std::find(m_LinkDirectories.begin(),
-                           m_LinkDirectories.end(), libpath) == m_LinkDirectories.end())
-                {
-                m_LinkDirectories.push_back(libpath);
-                }
-              }
-            }
-          std::string linkType = *k;
-          linkType += "_LINK_TYPE";
-          cmTarget::LinkLibraryType llt = cmTarget::GENERAL;
-          const char* linkTypeString = mf.GetDefinition( linkType.c_str() );
-          if(linkTypeString)
-            {
-            if(strcmp(linkTypeString, "debug") == 0)
-              {
-              llt = cmTarget::DEBUG;
-              }
-            if(strcmp(linkTypeString, "optimized") == 0)
-              {
-              llt = cmTarget::OPTIMIZED;
-              }
-            }
-          newLinkLibraries.push_back( std::make_pair(*k,llt) );
+          m_LinkDirectories.push_back(libpath);
           }
         }
       }
+    std::string linkType = *k;
+    linkType += "_LINK_TYPE";
+    cmTarget::LinkLibraryType llt = cmTarget::GENERAL;
+    const char* linkTypeString = mf.GetDefinition( linkType.c_str() );
+    if(linkTypeString)
+      {
+      if(strcmp(linkTypeString, "debug") == 0)
+        {
+        llt = cmTarget::DEBUG;
+        }
+      if(strcmp(linkTypeString, "optimized") == 0)
+        {
+        llt = cmTarget::OPTIMIZED;
+        }
+      }
+    m_LinkLibraries.push_back( std::make_pair(*k,llt) );
     }
-  m_LinkLibraries = newLinkLibraries;
 }
 
 
diff --git a/Source/cmTarget.h b/Source/cmTarget.h
index a4e2ac5..cbd2dd6 100644
--- a/Source/cmTarget.h
+++ b/Source/cmTarget.h
@@ -169,7 +169,7 @@ private:
   TargetType m_TargetType;
   std::vector<cmSourceFile*> m_SourceFiles;
   LinkLibraries m_LinkLibraries;
-  std::set<cmStdString> m_PrevLinkedLibraries;
+  LinkLibraries m_PrevLinkedLibraries;
   std::vector<std::string> m_LinkDirectories;
   bool m_InAll;
   std::string m_InstallPath;