Merge topic 'find-libarch-not-symlink'

6f5aede7 find_library: Skip 'lib => lib<arch>' searches if one symlinks the other
e67963ed cmFindLibraryCommand: Refactor AddArchitecturePath logic

4 files changed, 77 insertions, 16 deletions

diff --git a/Source/cmFindLibraryCommand.cxx b/Source/cmFindLibraryCommand.cxx
index 2feedf3..c86f9c1 100644
--- a/Source/cmFindLibraryCommand.cxx
+++ b/Source/cmFindLibraryCommand.cxx
@@ -84,36 +84,68 @@ 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)
 {
   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<suffix>".
-    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<suffix>" and use it first.
+    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;
     }
 
-    // Follow "lib".
-    if (cmSystemTools::FileIsDirectory(cur_dir)) {
+    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);
+    }
+
+    if (use_lib) {
       this->AddArchitecturePath(dir, pos + 3 + 1, suffix, false);
     }
   }
+
   if (fresh) {
-    // Check for <dir><suffix>/.
-    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 <dir><suffix>/ and use it first.
+    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);
     }
 
-    // Now add the original unchanged path
-    if (cmSystemTools::FileIsDirectory(dir)) {
+    if (use_dir) {
       this->SearchPaths.push_back(dir);
     }
   }
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()