Merge topic 'find_package-empty-roots'

adfb0623cb find_package: Fix assertion failure on empty sysroots
1c6cecdce9 cmFindPackageCommand: Fix searching a root path as a prefix

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: buildbot <buildbot@kitware.com>
Merge-request: !10113

Source/cmFindCommon.cxx

@@ -248,19 +248,19 @@ void cmFindCommon::RerootPaths(std::vector<std::string>& 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) {

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<CallbackFn&&>(filesCollector)(fullPath + '/', type);
+  assert(!fullPath.empty() && fullPath.back() == '/');
+  return std::forward<CallbackFn&&>(filesCollector)(fullPath, type);
 }
 
 template <typename CallbackFn, typename Generator, typename... Rest>
@@ -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);

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
   {

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()

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)