From fc92d6640b09def31d129f3e347b98af1aa1e098 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Sun, 10 Oct 2021 06:31:42 -0400 Subject: [PATCH 1/2] cmFileCommand: improve error message Print full list of unresolved dependencies and a phrase that indicates what about the file is unresolved. --- Source/cmFileCommand.cxx | 9 ++++++--- .../linux-notfile-all-stderr.txt | 4 +++- .../linux-unresolved-all-stderr.txt | 4 +++- .../macos-unresolved-all-stderr.txt | 4 +++- .../windows-unresolved-all-stderr.txt | 4 +++- 5 files changed, 18 insertions(+), 7 deletions(-) diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx index d2341c5bcd..fd0595d88e 100644 --- a/Source/cmFileCommand.cxx +++ b/Source/cmFileCommand.cxx @@ -3170,9 +3170,12 @@ bool HandleGetRuntimeDependenciesCommand(std::vector 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; } diff --git a/Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/linux-notfile-all-stderr.txt b/Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/linux-notfile-all-stderr.txt index 5b863220ab..9ed17286f6 100644 --- a/Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/linux-notfile-all-stderr.txt +++ b/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$ diff --git a/Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/linux-unresolved-all-stderr.txt b/Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/linux-unresolved-all-stderr.txt index eaca5121b8..51010a19d1 100644 --- a/Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/linux-unresolved-all-stderr.txt +++ b/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$ diff --git a/Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/macos-unresolved-all-stderr.txt b/Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/macos-unresolved-all-stderr.txt index 01762b45f5..5743d27f4d 100644 --- a/Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/macos-unresolved-all-stderr.txt +++ b/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$ diff --git a/Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/windows-unresolved-all-stderr.txt b/Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/windows-unresolved-all-stderr.txt index a20654c5aa..0efcb57d45 100644 --- a/Tests/RunCMake/file-GET_RUNTIME_DEPENDENCIES/windows-unresolved-all-stderr.txt +++ b/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$ From 93c5864aa19a9af058910172939db177bd5a3b9f Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Sun, 10 Oct 2021 06:33:33 -0400 Subject: [PATCH 2/2] cmBinUtilsMacOSMachOLinker: improve performance by memoizing otool calls Libraries with many repeated transitive dependencies (e.g. Trilinos) can result in very long runtime dependency call times, especially if system calls are made more expensive by antivirus software. This change caches the results of the calls to otool for efficiency. --- Source/cmBinUtilsMacOSMachOLinker.cxx | 42 +++++++++++++++++++++------ Source/cmBinUtilsMacOSMachOLinker.h | 10 +++++++ 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/Source/cmBinUtilsMacOSMachOLinker.cxx b/Source/cmBinUtilsMacOSMachOLinker.cxx index 47f77d8b99..c0643776cd 100644 --- a/Source/cmBinUtilsMacOSMachOLinker.cxx +++ b/Source/cmBinUtilsMacOSMachOLinker.cxx @@ -5,6 +5,8 @@ #include #include +#include +#include #include #include @@ -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 libs; - std::vector 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 libs; - std::vector 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; } } diff --git a/Source/cmBinUtilsMacOSMachOLinker.h b/Source/cmBinUtilsMacOSMachOLinker.h index eae23cc88b..ac1177bb57 100644 --- a/Source/cmBinUtilsMacOSMachOLinker.h +++ b/Source/cmBinUtilsMacOSMachOLinker.h @@ -5,6 +5,7 @@ #include #include +#include #include #include "cmBinUtilsLinker.h" @@ -24,7 +25,16 @@ public: cmStateEnums::TargetType type) override; private: + struct FileInfo + { + std::vector libs; + std::vector rpaths; + }; + std::unique_ptr Tool; + std::unordered_map ScannedFileInfo; + + const FileInfo* GetFileInfo(std::string const& file); bool ScanDependencies(std::string const& file, std::vector const& libs,