From 4bff9704134608bbce373e64d62441180e98b67a Mon Sep 17 00:00:00 2001 From: Amitha Perera Date: Fri, 3 May 2002 00:27:34 -0400 Subject: [PATCH] - bug fix where paths weren't being output when LIB_OUT_PATH *isn't* used - test case for above mentioned bug - more comments. Comments are good. --- Source/CMakeLists.txt | 13 ++++-- Source/cmTarget.cxx | 80 ++++++++++++++++++++++++++------- Source/cmTarget.h | 10 +++-- Tests/Dependency/CMakeLists.txt | 34 +++++++------- 4 files changed, 98 insertions(+), 39 deletions(-) diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt index 589c52df7..56713daa0 100644 --- a/Source/CMakeLists.txt +++ b/Source/CMakeLists.txt @@ -163,11 +163,18 @@ IF(BUILD_TESTING) ${CMake_BINARY_DIR}/Tests/Wrapping/bin TestDriverTest subdir/test3) - ADD_TEST(dependency ${CMake_BINARY_DIR}/Source/cmaketest + ADD_TEST(dependency_w_libout ${CMake_BINARY_DIR}/Source/cmaketest ${CMake_SOURCE_DIR}/Tests/Dependency - ${CMake_BINARY_DIR}/Tests/Dependency + ${CMake_BINARY_DIR}/Tests/Dependency/WithLibOut exec - ${CMake_BINARY_DIR}/Tests/Dependency/Exec + ${CMake_BINARY_DIR}/Tests/Dependency/WithLibOut/Exec + Dependency CMAKE_ARGS -DLIBRARY_OUTPUT_PATH:PATH=${CMake_BINARY_DIR}/Tests/Dependency/WithLibOut/Lib) + + ADD_TEST(dependency_wo_lib_out ${CMake_BINARY_DIR}/Source/cmaketest + ${CMake_SOURCE_DIR}/Tests/Dependency + ${CMake_BINARY_DIR}/Tests/Dependency/WOLibOut + exec + ${CMake_BINARY_DIR}/Tests/Dependency/WOLibOut/Exec Dependency) ENDIF (DART_ROOT) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index d3f272649..9142f1f9a 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -139,6 +139,32 @@ bool cmTarget::HasCxx() const void cmTarget::AnalyzeLibDependencies( const cmMakefile& mf ) { + // There are two key parts of the dependency analysis: (1) + // determining the libraries in the link line, and (2) constructing + // the dependency graph for those libraries. + // + // The latter is done using the cache entries that record the + // dependencies of each library. + // + // The former is a more thorny issue, since it is not clear how to + // determine if two libraries listed on the link line refer to the a + // single library or not. For example, consider the link "libraries" + // /usr/lib/libtiff.so -ltiff + // Is this one library or two? The solution implemented here is the + // simplest (and probably the only practical) one: two libraries are + // the same if their "link strings" are identical. Thus, the two + // libraries above are considered distinct. This also means that for + // dependency analysis to be effective, the CMake user must specify + // libraries build by his project without using any linker flags or + // file extensions. That is, + // LINK_LIBRARIES( One Two ) + // instead of + // LINK_LIBRARIES( -lOne ${binarypath}/libTwo.a ) + // The former is probably what most users would do, but it never + // hurts to document the assumptions. :-) Therefore, in the analysis + // code, the "canonical name" of a library is simply its name as + // given to a LINK_LIBRARIES command. + typedef std::vector< std::string > LinkLine; // Maps the canonical names to the full objects of m_LinkLibraries. @@ -146,7 +172,8 @@ cmTarget::AnalyzeLibDependencies( const cmMakefile& mf ) // The unique list of libraries on the orginal link line. They // correspond to lib_map keys. However, lib_map will also get - // further populated by the dependency analysis. + // further populated by the dependency analysis, while this will be + // unchanged. LinkLine orig_libs; // The list canonical names in the order they were orginally @@ -156,6 +183,10 @@ cmTarget::AnalyzeLibDependencies( const cmMakefile& mf ) // The dependency maps. DependencyMap dep_map, dep_map_implicit; + + // 1. Determine the list of unique libraries in the original link + // line. + LinkLibraries::iterator lib; for(lib = m_LinkLibraries.begin(); lib != m_LinkLibraries.end(); ++lib) { @@ -163,7 +194,7 @@ cmTarget::AnalyzeLibDependencies( const cmMakefile& mf ) // if a variable expands to nothing. if (lib->first.size() == 0) continue; - std::string cname = lib->first; + const std::string& cname = lib->first; lib_order.push_back( cname ); if( lib_map.end() == lib_map.find( cname ) ) { @@ -172,11 +203,18 @@ cmTarget::AnalyzeLibDependencies( const cmMakefile& mf ) } } + // 2. Gather the dependencies. + + // If LIBRARY_OUTPUT_PATH is not set, then we must add search paths + // for all the new libraries added by the dependency analysis. + const char* libOutPath = mf.GetDefinition("LIBRARY_OUTPUT_PATH"); + bool addLibDirs = (libOutPath==0 || strcmp(libOutPath,"")==0); + // First, get the explicit dependencies for those libraries that - // have specified them + // have specified them. for( LinkLine::iterator i = orig_libs.begin(); i != orig_libs.end(); ++i ) { - this->GatherDependencies( mf, *i, dep_map, lib_map ); + this->GatherDependencies( mf, *i, dep_map, lib_map, addLibDirs ); } // For the rest, get implicit dependencies. A library x depends @@ -216,7 +254,10 @@ cmTarget::AnalyzeLibDependencies( const cmMakefile& mf ) dep_map[ i->first ] = i->second; } - // Create a new link line order. + // 3. Create a new link line, trying as much as possible to keep the + // original link line order. + + // Get the link line as canonical names. std::set done, visited; std::vector link_line; for( LinkLine::iterator i = orig_libs.begin(); i != orig_libs.end(); ++i ) @@ -224,19 +265,14 @@ cmTarget::AnalyzeLibDependencies( const cmMakefile& mf ) Emit( *i, dep_map, done, visited, link_line ); } - - // If LIBRARY_OUTPUT_PATH is not set, then we must add search paths - // for all the new libraries added by the dependency analysis. - const char* libOutPath = mf.GetDefinition("LIBRARY_OUTPUT_PATH"); - bool addLibDirs = (libOutPath==0 || strcmp(libOutPath,"")==0); + // Translate the canonical names into internal objects. m_LinkLibraries.clear(); for( std::vector::reverse_iterator i = link_line.rbegin(); i != link_line.rend(); ++i ) { // Some of the libraries in the new link line may not have been in // the orginal link line, but were added by the dependency - // analysis. For these libraries, we assume the GENERAL type and - // add the name of the library. + // analysis. if( lib_map.find(*i) == lib_map.end() ) { if( addLibDirs ) @@ -317,7 +353,8 @@ void cmTarget::Emit( const std::string& lib, void cmTarget::GatherDependencies( const cmMakefile& mf, const std::string& lib, DependencyMap& dep_map, - LibTypeMap& lib_map ) const + LibTypeMap& lib_map, + bool addLibDirs ) { // If the library is already in the dependency map, then it has // already been fully processed. @@ -343,7 +380,20 @@ void cmTarget::GatherDependencies( const cmMakefile& mf, std::string l = depline.substr( start, end-start ); if( l.size() != 0 ) { - const std::string cname = l; + if( addLibDirs ) + { + const char* libpath = mf.GetDefinition( l.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); + } + } + } + const std::string& cname = l; std::string linkType = l; linkType += "_LINK_TYPE"; cmTarget::LinkLibraryType llt = cmTarget::GENERAL; @@ -361,7 +411,7 @@ void cmTarget::GatherDependencies( const cmMakefile& mf, } lib_map[ cname ] = std::make_pair(l,llt); dep_map[ lib ].insert( cname ); - GatherDependencies( mf, cname, dep_map, lib_map ); + GatherDependencies( mf, cname, dep_map, lib_map, addLibDirs ); } start = end+1; // skip the ; end = depline.find( ";", start ); diff --git a/Source/cmTarget.h b/Source/cmTarget.h index 516d16b7b..0fcf35356 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -121,7 +121,7 @@ private: typedef std::map< std::string, std::set< std::string > > DependencyMap; /** - * Maps a canonical library name to it's proper type + * Maps a library name to its internal structure */ typedef std::map< std::string, std::pair > LibTypeMap; @@ -141,11 +141,15 @@ private: /** * Finds the explicit dependencies for \param lib, if they have been - * specified, and inserts them into \param dep_map. + * specified, and inserts them into \param dep_map. It also adds the + * maps from the library names to internal structures for any + * libraries introduced by the analysis. \param addLibDirs is true + * if paths to newly found libraries should be added to the search + * path. */ void GatherDependencies( const cmMakefile& mf, const std::string& lib, DependencyMap& dep_map, - LibTypeMap& lib_map ) const; + LibTypeMap& lib_map, bool addLibDirs ); /** * Returns true if lib1 depends on lib2 according to \param diff --git a/Tests/Dependency/CMakeLists.txt b/Tests/Dependency/CMakeLists.txt index 3f2bee8ca..b7debc683 100644 --- a/Tests/Dependency/CMakeLists.txt +++ b/Tests/Dependency/CMakeLists.txt @@ -1,27 +1,25 @@ PROJECT( Dependency ) -SET( LIBRARY_OUTPUT_PATH ${Dependency_BINARY_DIR}/Lib ) -SET( CMAKE_ANALYZE_LIB_DEPENDS "ON" ) - # There is one executable that depends on eight libraries. The # system has the following dependency graph: # -# +----------- NoDepC <---- EXECUTABLE -# | | | -# V | | -# | | -# NoDepA <----- NoDepB <-------+ | -# | -# ^ | +# +----------- NoDepC <---- EXECUTABLE ---+ +# | | | | +# V | | | +# | | | +# NoDepA <----- NoDepB <-------+ | | +# | | +# ^ | V # | | -# One <------ Four -----> Two <----- Five <---+ -# | -# ^ ^ ^ | ^ ^ ^ -# | | +-----+ | | | +----+ -# | | | | | | | -# +--------- Three <------+ +------- SixA SixB -# | | -# +-----------------------+ +# One <------ Four -----> Two <----- Five <---|----- SixB +# | | | +# ^ ^ ^ | ^ ^ | | +# | | +-----+ | \ | | | +# | | | | \ | | | +# +--------- Three <------+ --- SixA <----+ | +# | | +# | | +# +---------------------------------+ # NoDepA: # NoDepB: NoDepA # NoDepC: NoDepA