ENH: updated handling of debug and optimized target link libraries

Source/cmMakefile.cxx

@@ -959,26 +959,6 @@ void cmMakefile::AddLinkLibraryForTarget(const char *target,
           }
         }
       }
-    // make sure the type is correct if it is currently
-    // general.  So if you do a 
-    // target_link_libraries(foo optimized bar) it will stay
-    // optimized and not use the lookup.   As there maybe the
-    // case where someone has specifed that a library is both
-    // debug and optimized.
-    std::string linkType = lib;
-    linkType += "_LINK_TYPE";
-    const char* linkTypeString = this->GetDefinition( linkType.c_str() );
-    if(llt == cmTarget::GENERAL &&  linkTypeString)
-      {
-      if(strcmp(linkTypeString, "debug") == 0)
-        {
-        llt = cmTarget::DEBUG;
-        }
-      if(strcmp(linkTypeString, "optimized") == 0)
-        {
-        llt = cmTarget::OPTIMIZED;
-        }
-      }
     i->second.AddLinkLibrary( *this, target, lib, llt );
     }
   else

Source/cmTarget.cxx

@@ -500,58 +500,6 @@ void cmTarget::AddLinkLibrary(cmMakefile& mf,
   this->LinkLibraries.push_back( std::pair<std::string, 
                                  cmTarget::LinkLibraryType>(lib,llt) );
 
-  if(llt != cmTarget::GENERAL)
-    {
-    // Store the library's link type in the cache.  If it is a
-    // conflicting type then assume it is always used.  This is the
-    // case when the user sets the cache entries for debug and
-    // optimized versions of the library to the same value.
-    std::string linkTypeName = lib;
-    linkTypeName += "_LINK_TYPE";
-    switch(llt)
-      {
-      case cmTarget::DEBUG:
-        {
-        const char* def = mf.GetDefinition(linkTypeName.c_str());
-        if(!def || strcmp(def, "debug") == 0)
-          {
-          mf.AddCacheDefinition(linkTypeName.c_str(),
-                                "debug", 
-                                "Library is used for debug links only",
-                                cmCacheManager::STATIC);
-          }
-        else
-          {
-          mf.AddCacheDefinition
-            (linkTypeName.c_str(), "general", 
-             "Library is used for both debug and optimized links",
-             cmCacheManager::STATIC);
-          }
-        }
-        break;
-      case cmTarget::OPTIMIZED:
-        {
-        const char* def = mf.GetDefinition(linkTypeName.c_str());
-        if(!def || strcmp(def, "optimized") == 0)
-          {
-          mf.AddCacheDefinition
-            (linkTypeName.c_str(), "optimized", 
-             "Library is used for debug links only",
-             cmCacheManager::STATIC);
-          }
-        else
-          {
-          mf.AddCacheDefinition
-            (linkTypeName.c_str(), "general", 
-             "Library is used for both debug and optimized links",
-             cmCacheManager::STATIC);
-          }
-        }
-        break;
-      case cmTarget::GENERAL:
-        break;
-      }
-    }
   // Add the explicit dependency information for this target. This is
   // simply a set of libraries separated by ";". There should always
   // be a trailing ";". These library names are not canonical, in that
@@ -570,6 +518,19 @@ void cmTarget::AddLinkLibrary(cmMakefile& mf,
       {
       dependencies += old_val;
       }
+    switch (llt)
+      {
+      case cmTarget::GENERAL:
+        dependencies += "general";
+        break;
+      case cmTarget::DEBUG:
+        dependencies += "debug";
+        break;
+      case cmTarget::OPTIMIZED:
+        dependencies += "optimized";
+        break;
+      }
+    dependencies += ";";
     dependencies += lib;
     dependencies += ";";
     mf.AddCacheDefinition( targetEntry.c_str(), dependencies.c_str(),
@@ -667,7 +628,7 @@ cmTarget::AnalyzeLibDependencies( const cmMakefile& mf )
         = this->LinkLibraries.rbegin();
       lib != this->LinkLibraries.rend(); ++lib)
     {
-    this->GatherDependencies( mf, lib->first, dep_map );
+    this->GatherDependencies( mf, *lib, dep_map);
     }
 
   // 2. Remove any dependencies that are already satisfied in the original
@@ -679,7 +640,7 @@ cmTarget::AnalyzeLibDependencies( const cmMakefile& mf )
     for( LinkLibraryVectorType::iterator lib2 = lib;
       lib2 != this->LinkLibraries.end(); ++lib2)
       {
-      DeleteDependency( dep_map, lib->first, lib2->first );
+      this->DeleteDependency( dep_map, *lib, *lib2);
       }
     }
 
@@ -687,8 +648,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.
   //
-  std::set<cmStdString> done, visited;
+  std::set<DependencyMap::key_type> done, visited;
-  std::vector<std::string> newLinkLibraries;
+  std::vector<DependencyMap::key_type> newLinkLibraries;
   for(LinkLibraryVectorType::reverse_iterator lib = 
         this->LinkLibraries.rbegin();
       lib != this->LinkLibraries.rend(); ++lib)
@@ -697,47 +658,33 @@ cmTarget::AnalyzeLibDependencies( const cmMakefile& mf )
     // if a variable expands to nothing.
     if (lib->first.size() != 0)
       {
-      Emit( lib->first, dep_map, done, visited, newLinkLibraries );
+      this->Emit( *lib, dep_map, done, visited, newLinkLibraries );
       }
     }
 
   // 4. Add the new libraries to the link line.
   //
-  for( std::vector<std::string>::reverse_iterator k = 
+  for( std::vector<DependencyMap::key_type>::reverse_iterator k = 
          newLinkLibraries.rbegin();
        k != newLinkLibraries.rend(); ++k )
     {
-    std::string linkType = *k;
+    // get the llt from the dep_map    
-    linkType += "_LINK_TYPE";
+    this->LinkLibraries.push_back( std::make_pair(k->first,k->second) );
-    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;
-        }
-      }
-    this->LinkLibraries.push_back( std::make_pair(*k,llt) );
     }
   this->LinkLibrariesAnalyzed = true;
 }
 
 
 void cmTarget::InsertDependency( DependencyMap& depMap,
-                                 const cmStdString& lib,
+                                 const LibraryID& lib,
-                                 const cmStdString& dep ) 
+                                 const LibraryID& dep) 
 {
   depMap[lib].push_back(dep);
 }
 
 void cmTarget::DeleteDependency( DependencyMap& depMap,
-                                 const cmStdString& lib,
+                                 const LibraryID& lib,
-                                 const cmStdString& dep ) 
+                                 const LibraryID& dep) 
 {
   // Make sure there is an entry in the map for lib. If so, delete all
   // dependencies to dep. There may be repeated entries because of
@@ -755,11 +702,11 @@ void cmTarget::DeleteDependency( DependencyMap& depMap,
     }
 }
 
-void cmTarget::Emit( const std::string& lib,
+void cmTarget::Emit(const LibraryID lib,
-                     const DependencyMap& dep_map,
+                    const DependencyMap& dep_map,
-                     std::set<cmStdString>& emitted,
+                    std::set<LibraryID>& emitted,
-                     std::set<cmStdString>& visited,
+                    std::set<LibraryID>& visited,
-                     std::vector<std::string>& link_line )
+                    DependencyList& link_line )
 {
   // It's already been emitted
   if( emitted.find(lib) != emitted.end() )
@@ -790,7 +737,7 @@ void cmTarget::Emit( const std::string& lib,
       // recursive call. This way, if we come across a library that
       // has already been emitted, we repeat it iff it has been
       // emitted here.
-      std::set<cmStdString> emitted_here;
+      std::set<DependencyMap::key_type> emitted_here;
       for( i = dep_on.rbegin(); i != dep_on.rend(); ++i )
         {
         if( emitted_here.find(*i) != emitted_here.end() )
@@ -819,8 +766,8 @@ void cmTarget::Emit( const std::string& lib,
 
 
 void cmTarget::GatherDependencies( const cmMakefile& mf,
-                                   const std::string& lib,
+                                   const LibraryID& lib,
-                                   DependencyMap& dep_map )
+                                   DependencyMap& dep_map)
 {
   // If the library is already in the dependency map, then it has
   // already been fully processed.
@@ -829,7 +776,7 @@ void cmTarget::GatherDependencies( const cmMakefile& mf,
     return;
     }
 
-  const char* deps = mf.GetDefinition( (lib+"_LIB_DEPENDS").c_str() );
+  const char* deps = mf.GetDefinition( (lib.first+"_LIB_DEPENDS").c_str() );
   if( deps && strcmp(deps,"") != 0 )
     {
     // Make sure this library is in the map, even if it has an empty
@@ -837,8 +784,9 @@ void cmTarget::GatherDependencies( const cmMakefile& mf,
     // no dependencies with that of unspecified dependencies.
     dep_map[lib];
 
-    // Parse the dependency information, which is simply a set of
+    // Parse the dependency information, which is a set of
-    // libraries separated by ";". There is always a trailing ";".
+    // type, library pairs separated by ";". There is always a trailing ";".
+    cmTarget::LinkLibraryType llt = cmTarget::GENERAL;
     std::string depline = deps;
     std::string::size_type start = 0;
     std::string::size_type end;
@@ -848,13 +796,31 @@ void cmTarget::GatherDependencies( const cmMakefile& mf,
       std::string l = depline.substr( start, end-start );
       if( l.size() != 0 )
         {
-        InsertDependency( dep_map, lib, l );
+        if (l == "debug")
-        GatherDependencies( mf, l, dep_map );
+          {
+          llt = cmTarget::DEBUG;
+          }
+        else if (l == "optimized")
+          {
+          llt = cmTarget::OPTIMIZED;
+          }
+        else if (l == "general")
+          {
+          llt = cmTarget::GENERAL;
+          }
+        else
+          {
+          LibraryID lib2(l,llt);
+          this->InsertDependency( dep_map, lib, lib2);
+          this->GatherDependencies( mf, lib2, dep_map);
+          llt = cmTarget::GENERAL;
+          }
         }
       start = end+1; // skip the ;
       end = depline.find( ";", start );
       }
-    DeleteDependency( dep_map, lib, lib); // cannot depend on itself
+    // cannot depend on itself
+    this->DeleteDependency( dep_map, lib, lib); 
     }
 }
 

Source/cmTarget.h

@@ -95,8 +95,10 @@ public:
    */
   enum LinkLibraryType {GENERAL, DEBUG, OPTIMIZED};
 
-  typedef std::vector<std::pair<std::string,LinkLibraryType> > 
+  //* how we identify a library, by name and type
-  LinkLibraryVectorType;
+  typedef std::pair<cmStdString, LinkLibraryType> LibraryID;
+
+  typedef std::vector<LibraryID > LinkLibraryVectorType;
   const LinkLibraryVectorType &GetLinkLibraries() {
     return this->LinkLibraries;}
   const LinkLibraryVectorType &GetOriginalLinkLibraries() 
@@ -251,7 +253,7 @@ private:
   /**
    * A list of direct dependencies. Use in conjunction with DependencyMap.
    */
-  typedef std::vector<cmStdString> DependencyList;
+  typedef std::vector< LibraryID > DependencyList;
 
   /**
    * This map holds the dependency graph. map[x] returns a set of
@@ -259,27 +261,21 @@ private:
    * ordered. This is necessary to handle direct dependencies that
    * themselves have no dependency information.
    */
-  typedef std::map< cmStdString, std::vector< cmStdString > > DependencyMap;
+  typedef std::map< LibraryID, DependencyList > DependencyMap;
-
-  /**
-   * Maps a library name to its internal structure
-   */
-  typedef std::map< cmStdString, std::pair<cmStdString,LinkLibraryType> >
-  LibTypeMap;
 
   /**
    * Inserts \a dep at the end of the dependency list of \a lib.
    */
   void InsertDependency( DependencyMap& depMap,
-                         const cmStdString& lib,
+                         const LibraryID& lib,
-                         const cmStdString& dep );
+                         const LibraryID& dep);
 
   /*
    * Deletes \a dep from the dependency list of \a lib.
    */
   void DeleteDependency( DependencyMap& depMap,
-                         const cmStdString& lib,
+                         const LibraryID& lib,
-                         const cmStdString& dep );
+                         const LibraryID& dep);
 
   /**
    * Emits the library \a lib and all its dependencies into link_line.
@@ -289,18 +285,19 @@ private:
    * link_line is in reverse order, in that the dependencies of a
    * library are listed before the library itself.
    */
-  void Emit( const std::string& lib,
+  void Emit( const LibraryID lib,
              const DependencyMap& dep_map,
-             std::set<cmStdString>& emitted,
+             std::set<LibraryID>& emitted,
-             std::set<cmStdString>& visited,
+             std::set<LibraryID>& visited,
-             std::vector<std::string>& link_line );
+             DependencyList& link_line);
 
   /**
    * Finds the dependencies for \a lib and inserts them into \a
    * dep_map.
    */
-  void GatherDependencies( const cmMakefile& mf, const std::string& lib,
+  void GatherDependencies( const cmMakefile& mf, 
-                           DependencyMap& dep_map ); 
+                           const LibraryID& lib,
+                           DependencyMap& dep_map); 
 
   const char* GetSuffixVariableInternal(TargetType type, bool implib);
   const char* GetPrefixVariableInternal(TargetType type, bool implib);

Source/cmTargetLinkLibrariesCommand.cxx

@@ -62,10 +62,43 @@ bool cmTargetLinkLibrariesCommand::InitialPass(std::vector<std::string>
       this->Makefile->AddLinkLibraryForTarget(args[0].c_str(),i->c_str(),
                                  cmTarget::OPTIMIZED);
       }
+    else if (*i == "general")
+      {
+      ++i;
+      if(i == args.end())
+        {
+        this->SetError(
+          "The \"general\" argument must be followed by a library");
+        return false;
+        }
+      this->Makefile->AddLinkLibraryForTarget(args[0].c_str(),i->c_str(),
+                                 cmTarget::GENERAL);
+      }
     else
       {
-      this->Makefile->AddLinkLibraryForTarget(args[0].c_str(),i->c_str(),
+      // make sure the type is correct if it is currently general.  So if you
-                                          cmTarget::GENERAL);  
+      // do a target_link_libraries(foo optimized bar) it will stay optimized
+      // and not use the lookup.  As there maybe the case where someone has
+      // specifed that a library is both debug and optimized.  (this check is
+      // only there for backwards compatibility when mixing projects built
+      // with old versions of CMake and new)
+      cmTarget::LinkLibraryType llt = cmTarget::GENERAL;
+      std::string linkType = args[0];
+      linkType += "_LINK_TYPE";
+      const char* linkTypeString = 
+        this->Makefile->GetDefinition( linkType.c_str() );
+      if(linkTypeString)
+        {
+        if(strcmp(linkTypeString, "debug") == 0)
+          {
+          llt = cmTarget::DEBUG;
+          }
+        if(strcmp(linkTypeString, "optimized") == 0)
+          {
+          llt = cmTarget::OPTIMIZED;
+          }
+        }
+      this->Makefile->AddLinkLibraryForTarget(args[0].c_str(),i->c_str(),llt);
       }
     } 
   return true;

Source/cmTargetLinkLibrariesCommand.h

@@ -64,12 +64,13 @@ public:
     {
     return
       "  TARGET_LINK_LIBRARIES(target library1\n"
-      "                        <debug | optimized> library2\n"
+      "                        <debug | optimized | general> library2\n"
       "                        ...)\n"
       "Specify a list of libraries to be linked into the specified target.  "
       "The debug and optimized strings may be used to indicate that "
       "the next library listed is to be used only for that specific "
-      "type of build";
+      "type of build. general indicates it is used for all build types "
+      "and is assumed if not specified.";
     }
   
   cmTypeMacro(cmTargetLinkLibrariesCommand, cmCommand);