From e67963ed7316768bf0ebfbe42137d028c0d1d2a4 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 28 Feb 2017 11:30:14 -0500 Subject: cmFindLibraryCommand: Refactor AddArchitecturePath logic Use boolean variables to save results and rename variables to more closely represent their roles. --- Source/cmFindLibraryCommand.cxx | 42 +++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/Source/cmFindLibraryCommand.cxx b/Source/cmFindLibraryCommand.cxx index 082350f..58d92aa 100644 --- a/Source/cmFindLibraryCommand.cxx +++ b/Source/cmFindLibraryCommand.cxx @@ -77,31 +77,41 @@ void cmFindLibraryCommand::AddArchitecturePath( bool fresh) { std::string::size_type pos = dir.find("lib/", start_pos); + if (pos != std::string::npos) { - std::string cur_dir = dir.substr(0, pos + 3); - - // Follow "lib". - std::string next_dir = cur_dir + suffix; - if (cmSystemTools::FileIsDirectory(next_dir)) { - next_dir += dir.substr(pos + 3); - std::string::size_type next_pos = pos + 3 + strlen(suffix) + 1; - this->AddArchitecturePath(next_dir, next_pos, suffix); + // Check for "lib". + std::string lib = dir.substr(0, pos + 3); + bool use_lib = cmSystemTools::FileIsDirectory(lib); + + // Check for "lib" and use it first. + std::string libX = lib + suffix; + bool use_libX = cmSystemTools::FileIsDirectory(libX); + + if (use_libX) { + libX += dir.substr(pos + 3); + std::string::size_type libX_pos = pos + 3 + strlen(suffix) + 1; + this->AddArchitecturePath(libX, libX_pos, suffix); } - // Follow "lib". - if (cmSystemTools::FileIsDirectory(cur_dir)) { + if (use_lib) { this->AddArchitecturePath(dir, pos + 3 + 1, suffix, false); } } + if (fresh) { - // Check for /. - std::string cur_dir = dir + suffix + "/"; - if (cmSystemTools::FileIsDirectory(cur_dir)) { - this->SearchPaths.push_back(cur_dir); + // Check for the original unchanged path. + bool use_dir = cmSystemTools::FileIsDirectory(dir); + + // Check for / and use it first. + std::string dirX = dir + suffix; + bool use_dirX = cmSystemTools::FileIsDirectory(dirX); + + if (use_dirX) { + dirX += "/"; + this->SearchPaths.push_back(dirX); } - // Now add the original unchanged path - if (cmSystemTools::FileIsDirectory(dir)) { + if (use_dir) { this->SearchPaths.push_back(dir); } } -- cgit v0.12 From 6f5aede71642b05a351d5cf92e0c884ed9b4fd3c Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 28 Feb 2017 13:51:35 -0500 Subject: find_library: Skip 'lib => lib' searches if one symlinks the other The `FIND_LIBRARY_USE_LIB_PATHS` global properties ask `find_library` to look in `lib` directories automatically before corresponding `lib` directories. However, if `lib` is just a symlink to `lib` (or vice-versa) then we should skip adding the `lib` path. Such symlinks typically only exist to satisfy software that expects the `lib` path to be available. Fixes: #16687 --- Source/cmFindLibraryCommand.cxx | 22 ++++++++++++++++++++ Tests/RunCMake/find_library/LibArchLink-stderr.txt | 2 ++ Tests/RunCMake/find_library/LibArchLink.cmake | 24 ++++++++++++++++++++++ Tests/RunCMake/find_library/RunCMakeTest.cmake | 3 +++ 4 files changed, 51 insertions(+) create mode 100644 Tests/RunCMake/find_library/LibArchLink-stderr.txt create mode 100644 Tests/RunCMake/find_library/LibArchLink.cmake diff --git a/Source/cmFindLibraryCommand.cxx b/Source/cmFindLibraryCommand.cxx index 58d92aa..412d573 100644 --- a/Source/cmFindLibraryCommand.cxx +++ b/Source/cmFindLibraryCommand.cxx @@ -72,6 +72,18 @@ void cmFindLibraryCommand::AddArchitecturePaths(const char* suffix) } } +static bool cmLibDirsLinked(std::string const& l, std::string const& r) +{ + // Compare the real paths of the two directories. + // Since our caller only changed the trailing component of each + // directory, the real paths can be the same only if at least one of + // the trailing components is a symlink. Use this as an optimization + // to avoid excessive realpath calls. + return (cmSystemTools::FileIsSymlink(l) || + cmSystemTools::FileIsSymlink(r)) && + cmSystemTools::GetRealPath(l) == cmSystemTools::GetRealPath(r); +} + void cmFindLibraryCommand::AddArchitecturePath( std::string const& dir, std::string::size_type start_pos, const char* suffix, bool fresh) @@ -87,6 +99,11 @@ void cmFindLibraryCommand::AddArchitecturePath( std::string libX = lib + suffix; bool use_libX = cmSystemTools::FileIsDirectory(libX); + // Avoid copies of the same directory due to symlinks. + if (use_libX && use_lib && cmLibDirsLinked(libX, lib)) { + use_libX = false; + } + if (use_libX) { libX += dir.substr(pos + 3); std::string::size_type libX_pos = pos + 3 + strlen(suffix) + 1; @@ -106,6 +123,11 @@ void cmFindLibraryCommand::AddArchitecturePath( std::string dirX = dir + suffix; bool use_dirX = cmSystemTools::FileIsDirectory(dirX); + // Avoid copies of the same directory due to symlinks. + if (use_dirX && use_dir && cmLibDirsLinked(dirX, dir)) { + use_dirX = false; + } + if (use_dirX) { dirX += "/"; this->SearchPaths.push_back(dirX); diff --git a/Tests/RunCMake/find_library/LibArchLink-stderr.txt b/Tests/RunCMake/find_library/LibArchLink-stderr.txt new file mode 100644 index 0000000..139e077 --- /dev/null +++ b/Tests/RunCMake/find_library/LibArchLink-stderr.txt @@ -0,0 +1,2 @@ +TOP_LIBRARY='[^']*/Tests/RunCMake/find_library/LibArchLink-build/lib/libtop.a' +SUB_LIBRARY='[^']*/Tests/RunCMake/find_library/LibArchLink-build/lib/sub/libsub.a' diff --git a/Tests/RunCMake/find_library/LibArchLink.cmake b/Tests/RunCMake/find_library/LibArchLink.cmake new file mode 100644 index 0000000..c91381d --- /dev/null +++ b/Tests/RunCMake/find_library/LibArchLink.cmake @@ -0,0 +1,24 @@ +set(CMAKE_SIZEOF_VOID_P 4) +set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB32_PATHS ON) +list(APPEND CMAKE_FIND_LIBRARY_PREFIXES lib) +list(APPEND CMAKE_FIND_LIBRARY_SUFFIXES .a) + +file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/lib) +execute_process(COMMAND ${CMAKE_COMMAND} -E create_symlink lib ${CMAKE_CURRENT_BINARY_DIR}/lib32) +file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/lib/libtop.a" "top") +find_library(TOP_LIBRARY + NAMES top + PATHS ${CMAKE_CURRENT_BINARY_DIR}/lib + NO_DEFAULT_PATH + ) +message("TOP_LIBRARY='${TOP_LIBRARY}'") + +file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/lib/sub) +execute_process(COMMAND ${CMAKE_COMMAND} -E create_symlink . ${CMAKE_CURRENT_BINARY_DIR}/lib/sub/32) +file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/lib/sub/libsub.a" "sub") +find_library(SUB_LIBRARY + NAMES sub + PATHS ${CMAKE_CURRENT_BINARY_DIR}/lib/sub + NO_DEFAULT_PATH + ) +message("SUB_LIBRARY='${SUB_LIBRARY}'") diff --git a/Tests/RunCMake/find_library/RunCMakeTest.cmake b/Tests/RunCMake/find_library/RunCMakeTest.cmake index 5733965..e7e8db3 100644 --- a/Tests/RunCMake/find_library/RunCMakeTest.cmake +++ b/Tests/RunCMake/find_library/RunCMakeTest.cmake @@ -1,6 +1,9 @@ include(RunCMake) run_cmake(Created) +if(CMAKE_HOST_UNIX) + run_cmake(LibArchLink) +endif() if(WIN32 OR CYGWIN) run_cmake(PrefixInPATH) endif() -- cgit v0.12