From 1c6cecdce98c3f2f6481686630d5cbda826b064e Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 19 Dec 2024 09:18:10 -0500 Subject: cmFindPackageCommand: Fix searching a root path as a prefix A root path like `/` or `c:/` needs to end in a slash. Revise our prefix search logic to maintain a trailing slash instead of removing one just to add it again. --- Source/cmFindPackageCommand.cxx | 53 ++++++++++++++--------------------------- Source/cmFindPackageCommand.h | 6 ++--- 2 files changed, 21 insertions(+), 38 deletions(-) diff --git a/Source/cmFindPackageCommand.cxx b/Source/cmFindPackageCommand.cxx index c0e215c..37b7a0a 100644 --- a/Source/cmFindPackageCommand.cxx +++ b/Source/cmFindPackageCommand.cxx @@ -130,7 +130,7 @@ public: return {}; } this->NeedReset = true; - return cmStrCat(parent, '/', this->DirName); + return cmStrCat(parent, this->DirName, '/'); } void Reset() { this->NeedReset = false; } @@ -152,7 +152,7 @@ public: std::string GetNextCandidate(const std::string& parent) { if (this->Current != this->Names.get().cend()) { - return cmStrCat(parent, '/', *this->Current++); + return cmStrCat(parent, *this->Current++, '/'); } return {}; } @@ -189,7 +189,7 @@ public: continue; } if (cmsysString_strcasecmp(fname, this->DirName.data()) == 0) { - auto candidate = cmStrCat(parent, '/', fname); + auto candidate = cmStrCat(parent, fname, '/'); if (cmSystemTools::FileIsDirectory(candidate)) { return candidate; } @@ -271,7 +271,7 @@ public: } if (this->Current != this->Matches.cend()) { - auto candidate = cmStrCat(parent, '/', *this->Current++); + auto candidate = cmStrCat(parent, *this->Current++, '/'); return candidate; } @@ -386,8 +386,8 @@ bool TryGeneratedPaths(CallbackFn&& filesCollector, cmFindPackageCommand::PackageDescriptionType type, const std::string& fullPath) { - assert(!fullPath.empty() && fullPath.back() != '/'); - return std::forward(filesCollector)(fullPath + '/', type); + assert(!fullPath.empty() && fullPath.back() == '/'); + return std::forward(filesCollector)(fullPath, type); } template @@ -2744,17 +2744,17 @@ void cmFindPackageCommand::StoreVersionFound() } } -bool cmFindPackageCommand::SearchPrefix(std::string const& prefix_in) +bool cmFindPackageCommand::SearchPrefix(std::string const& prefix) { - assert(!prefix_in.empty() && prefix_in.back() == '/'); + assert(!prefix.empty() && prefix.back() == '/'); // Skip this if the prefix does not exist. - if (!cmSystemTools::FileIsDirectory(prefix_in)) { + if (!cmSystemTools::FileIsDirectory(prefix)) { return false; } // Skip this if it's in ignored paths. - std::string prefixWithoutSlash = prefix_in; + std::string prefixWithoutSlash = prefix; if (prefixWithoutSlash != "/" && prefixWithoutSlash.back() == '/') { prefixWithoutSlash.erase(prefixWithoutSlash.length() - 1); } @@ -2763,10 +2763,6 @@ bool cmFindPackageCommand::SearchPrefix(std::string const& prefix_in) return false; } - // Strip the trailing slash because the path generator is about to - // add one. - std::string const prefix = prefix_in.substr(0, prefix_in.size() - 1); - auto searchFn = [this](const std::string& fullPath, PackageDescriptionType type) -> bool { return this->SearchDirectory(fullPath, type); @@ -2811,7 +2807,7 @@ bool cmFindPackageCommand::SearchPrefix(std::string const& prefix_in) } // PREFIX/ (useful on windows or in build trees) - if (this->SearchDirectory(prefix_in, pdt::CMake)) { + if (this->SearchDirectory(prefix, pdt::CMake)) { return true; } @@ -2916,13 +2912,9 @@ bool cmFindPackageCommand::SearchPrefix(std::string const& prefix_in) commonGen, secondPkgDirGen, iCMakeGen); } -bool cmFindPackageCommand::SearchFrameworkPrefix(std::string const& prefix_in) +bool cmFindPackageCommand::SearchFrameworkPrefix(std::string const& prefix) { - assert(!prefix_in.empty() && prefix_in.back() == '/'); - - // Strip the trailing slash because the path generator is about to - // add one. - std::string const prefix = prefix_in.substr(0, prefix_in.size() - 1); + assert(!prefix.empty() && prefix.back() == '/'); auto searchFn = [this](const std::string& fullPath, PackageDescriptionType type) -> bool { @@ -2971,13 +2963,9 @@ bool cmFindPackageCommand::SearchFrameworkPrefix(std::string const& prefix_in) rGen, iCMakeGen); } -bool cmFindPackageCommand::SearchAppBundlePrefix(std::string const& prefix_in) +bool cmFindPackageCommand::SearchAppBundlePrefix(std::string const& prefix) { - assert(!prefix_in.empty() && prefix_in.back() == '/'); - - // Strip the trailing slash because the path generator is about to - // add one. - std::string const prefix = prefix_in.substr(0, prefix_in.size() - 1); + assert(!prefix.empty() && prefix.back() == '/'); auto searchFn = [this](const std::string& fullPath, PackageDescriptionType type) -> bool { @@ -3004,20 +2992,15 @@ bool cmFindPackageCommand::SearchAppBundlePrefix(std::string const& prefix_in) cmCaseInsensitiveDirectoryListGenerator{ "cmake"_s }); } -bool cmFindPackageCommand::SearchEnvironmentPrefix( - std::string const& prefix_in) +bool cmFindPackageCommand::SearchEnvironmentPrefix(std::string const& prefix) { - assert(!prefix_in.empty() && prefix_in.back() == '/'); + assert(!prefix.empty() && prefix.back() == '/'); // Skip this if the prefix does not exist. - if (!cmSystemTools::FileIsDirectory(prefix_in)) { + if (!cmSystemTools::FileIsDirectory(prefix)) { return false; } - // Strip the trailing slash because the path generator is about to - // add one. - std::string const prefix = prefix_in.substr(0, prefix_in.size() - 1); - auto searchFn = [this](const std::string& fullPath, PackageDescriptionType type) -> bool { return this->SearchDirectory(fullPath, type); diff --git a/Source/cmFindPackageCommand.h b/Source/cmFindPackageCommand.h index 7631c2c..10a5026 100644 --- a/Source/cmFindPackageCommand.h +++ b/Source/cmFindPackageCommand.h @@ -173,9 +173,9 @@ private: bool CheckVersionFile(std::string const& version_file, std::string& result_version); bool SearchPrefix(std::string const& prefix); - bool SearchFrameworkPrefix(std::string const& prefix_in); - bool SearchAppBundlePrefix(std::string const& prefix_in); - bool SearchEnvironmentPrefix(std::string const& prefix_in); + bool SearchFrameworkPrefix(std::string const& prefix); + bool SearchAppBundlePrefix(std::string const& prefix); + bool SearchEnvironmentPrefix(std::string const& prefix); struct OriginalDef { -- cgit v0.12 From adfb0623cb68e59a398816f34fa43386adbc46e9 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 18 Dec 2024 09:37:59 -0500 Subject: find_package: Fix assertion failure on empty sysroots Previously we crashed if at least one root variable was set to empty and at least one to non-empty. Fixes: #26538 --- Source/cmFindCommon.cxx | 8 ++++---- Tests/RunCMake/find_package/EmptyRoots.cmake | 10 ++++++++++ Tests/RunCMake/find_package/RunCMakeTest.cmake | 1 + 3 files changed, 15 insertions(+), 4 deletions(-) create mode 100644 Tests/RunCMake/find_package/EmptyRoots.cmake diff --git a/Source/cmFindCommon.cxx b/Source/cmFindCommon.cxx index 01c87c2..f57564e 100644 --- a/Source/cmFindCommon.cxx +++ b/Source/cmFindCommon.cxx @@ -248,19 +248,19 @@ void cmFindCommon::RerootPaths(std::vector& paths, // Construct the list of path roots with no trailing slashes. cmList roots; debugRoot("CMAKE_FIND_ROOT_PATH", rootPath); - if (rootPath) { + if (cmNonempty(rootPath)) { roots.assign(*rootPath); } debugRoot("CMAKE_SYSROOT_COMPILE", sysrootCompile); - if (sysrootCompile) { + if (cmNonempty(sysrootCompile)) { roots.emplace_back(*sysrootCompile); } debugRoot("CMAKE_SYSROOT_LINK", sysrootLink); - if (sysrootLink) { + if (cmNonempty(sysrootLink)) { roots.emplace_back(*sysrootLink); } debugRoot("CMAKE_SYSROOT", sysroot); - if (sysroot) { + if (cmNonempty(sysroot)) { roots.emplace_back(*sysroot); } for (auto& r : roots) { diff --git a/Tests/RunCMake/find_package/EmptyRoots.cmake b/Tests/RunCMake/find_package/EmptyRoots.cmake new file mode 100644 index 0000000..22d4e16 --- /dev/null +++ b/Tests/RunCMake/find_package/EmptyRoots.cmake @@ -0,0 +1,10 @@ +set(vars CMAKE_SYSROOT CMAKE_SYSROOT_COMPILE CMAKE_SYSROOT_LINK CMAKE_FIND_ROOT_PATH) +foreach(v IN LISTS vars) + set("${v}" "") +endforeach() +foreach(v IN LISTS vars) + block() + set("${v}" "/dummy") + find_package(dummy CONFIG NO_DEFAULT_PATH PATHS "/") + endblock() +endforeach() diff --git a/Tests/RunCMake/find_package/RunCMakeTest.cmake b/Tests/RunCMake/find_package/RunCMakeTest.cmake index 1995ac4..0552535 100644 --- a/Tests/RunCMake/find_package/RunCMakeTest.cmake +++ b/Tests/RunCMake/find_package/RunCMakeTest.cmake @@ -3,6 +3,7 @@ include(RunCMake) run_cmake(CMP0074-WARN) run_cmake(CMP0074-OLD) run_cmake(ComponentRequiredAndOptional) +run_cmake(EmptyRoots) run_cmake(FromPATHEnv) run_cmake_with_options(FromPATHEnvDebugPkg --debug-find-pkg=Resolved) run_cmake(FromPrefixPath) -- cgit v0.12