Merge topic 'optimize-macos-runtime-dependencies'

93c5864aa1 cmBinUtilsMacOSMachOLinker: improve performance by memoizing otool calls
fc92d6640b cmFileCommand: improve error message

Acked-by: Kitware Robot <[email protected]>
Merge-request: !6616

Source/cmBinUtilsMacOSMachOLinker.cxx

@@ -5,6 +5,8 @@
 
 #include <sstream>
 #include <string>
+#include <type_traits>
+#include <utility>
 #include <vector>
 
 #include <cm/memory>
@@ -52,6 +54,26 @@ bool cmBinUtilsMacOSMachOLinker::Prepare()
   return true;
 }
 
+auto cmBinUtilsMacOSMachOLinker::GetFileInfo(std::string const& file)
+  -> const FileInfo*
+{
+  // Memoize processed rpaths and library dependencies to reduce the number
+  // of calls to otool, especially in the case of heavily recursive libraries
+  auto iter = ScannedFileInfo.find(file);
+  if (iter != ScannedFileInfo.end()) {
+    return &iter->second;
+  }
+
+  FileInfo file_info;
+  if (!this->Tool->GetFileInfo(file, file_info.libs, file_info.rpaths)) {
+    // Call to otool failed
+    return nullptr;
+  }
+
+  auto iter_inserted = ScannedFileInfo.insert({ file, std::move(file_info) });
+  return &iter_inserted.first->second;
+}
+
 bool cmBinUtilsMacOSMachOLinker::ScanDependencies(
   std::string const& file, cmStateEnums::TargetType type)
 {
@@ -65,12 +87,12 @@ bool cmBinUtilsMacOSMachOLinker::ScanDependencies(
   if (!executableFile.empty()) {
     executablePath = cmSystemTools::GetFilenamePath(executableFile);
   }
-  std::vector<std::string> libs;
-  std::vector<std::string> rpaths;
-  if (!this->Tool->GetFileInfo(file, libs, rpaths)) {
+  const FileInfo* file_info = this->GetFileInfo(file);
+  if (file_info == nullptr) {
     return false;
   }
-  return this->ScanDependencies(file, libs, rpaths, executablePath);
+  return this->ScanDependencies(file, file_info->libs, file_info->rpaths,
+                                executablePath);
 }
 
 bool cmBinUtilsMacOSMachOLinker::ScanDependencies(
@@ -98,14 +120,16 @@ bool cmBinUtilsMacOSMachOLinker::GetFileDependencies(
             !IsMissingSystemDylib(path)) {
           auto filename = cmSystemTools::GetFilenameName(path);
           bool unique;
-          std::vector<std::string> libs;
-          std::vector<std::string> depRpaths;
-          if (!this->Tool->GetFileInfo(path, libs, depRpaths)) {
+          const FileInfo* dep_file_info = this->GetFileInfo(path);
+          if (dep_file_info == nullptr) {
             return false;
           }
-          this->Archive->AddResolvedPath(filename, path, unique, depRpaths);
+
+          this->Archive->AddResolvedPath(filename, path, unique,
+                                         dep_file_info->rpaths);
           if (unique &&
-              !this->ScanDependencies(path, libs, depRpaths, executablePath)) {
+              !this->ScanDependencies(path, dep_file_info->libs,
+                                      dep_file_info->rpaths, executablePath)) {
             return false;
           }
         }

Source/cmBinUtilsMacOSMachOLinker.h

@@ -5,6 +5,7 @@
 
 #include <memory>
 #include <string>
+#include <unordered_map>
 #include <vector>
 
 #include "cmBinUtilsLinker.h"
@@ -24,7 +25,16 @@ public:
                         cmStateEnums::TargetType type) override;
 
 private:
+  struct FileInfo
+  {
+    std::vector<std::string> libs;
+    std::vector<std::string> rpaths;
+  };
+
   std::unique_ptr<cmBinUtilsMacOSMachOGetRuntimeDependenciesTool> Tool;
+  std::unordered_map<std::string, FileInfo> ScannedFileInfo;
+
+  const FileInfo* GetFileInfo(std::string const& file);
 
   bool ScanDependencies(std::string const& file,
                         std::vector<std::string> const& libs,

Source/cmFileCommand.cxx

@@ -3170,9 +3170,12 @@ bool HandleGetRuntimeDependenciesCommand(std::vector<std::string> const& args,
                             archive.GetUnresolvedPaths().begin(),
                             archive.GetUnresolvedPaths().end());
     } else {
-      auto it = archive.GetUnresolvedPaths().begin();
-      assert(it != archive.GetUnresolvedPaths().end());
-      status.SetError(cmStrCat("Could not resolve file ", *it));
+      std::ostringstream e;
+      e << "Could not resolve runtime dependencies:";
+      for (auto const& path : archive.GetUnresolvedPaths()) {
+        e << "\n  " << path;
+      }
+      status.SetError(e.str());
       cmSystemTools::SetFatalErrorOccured();
       return false;
     }

Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/linux-notfile-all-stderr.txt

@@ -1,2 +1,4 @@
 ^CMake Error at cmake_install\.cmake:[0-9]+ \(file\):
-  file Could not resolve file libtest\.so$
+  file Could not resolve runtime dependencies:
+
+    libtest\.so$

Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/linux-unresolved-all-stderr.txt

@@ -1,2 +1,4 @@
 ^CMake Error at cmake_install\.cmake:[0-9]+ \(file\):
-  file Could not resolve file libunresolved\.so$
+  file Could not resolve runtime dependencies:
+
+    libunresolved\.so$

Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/macos-unresolved-all-stderr.txt

@@ -1,2 +1,4 @@
 ^CMake Error at cmake_install\.cmake:[0-9]+ \(file\):
-  file Could not resolve file @rpath/libunresolved\.dylib$
+  file Could not resolve runtime dependencies:
+
+    @rpath/libunresolved\.dylib$

Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/windows-unresolved-all-stderr.txt

@@ -1,2 +1,4 @@
 ^CMake Error at cmake_install\.cmake:[0-9]+ \(file\):
-  file Could not resolve file (lib)?unresolved\.dll$
+  file Could not resolve runtime dependencies:
+
+    (lib)?unresolved\.dll$