Merge topic 'improve-package-search'

6f3dc1161a find_package: Also sort Framework matches
e90f60f864 find_package: Don't glob certain macOS paths

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

Help/command/find_package.rst

@@ -507,13 +507,12 @@ Paths are searched in the order described above.  The first viable package
 configuration file found is used, even if a newer version of the package
 resides later in the list of search paths.
 
-For search paths which contain ``<name>*``, the order among matching paths
-is unspecified unless the :variable:`CMAKE_FIND_PACKAGE_SORT_ORDER` variable
-is set.  This variable, along with the
-:variable:`CMAKE_FIND_PACKAGE_SORT_DIRECTION` variable, determines the order
-in which CMake considers paths that match a single search path containing
-``<name>*``.  For example, if the file system contains the package
-configuration files
+For search paths which contain glob expressions (``*``), the order in which
+directories matching the glob are searched is unspecified unless the
+:variable:`CMAKE_FIND_PACKAGE_SORT_ORDER` variable is set.  This variable,
+along with the :variable:`CMAKE_FIND_PACKAGE_SORT_DIRECTION` variable,
+determines the order in which CMake considers glob matches.  For example, if
+the file system contains the package configuration files
 
 ::
 
@@ -543,6 +542,14 @@ before calling ``find_package``.
    Added the ``CMAKE_FIND_USE_<CATEGORY>`` variables to globally disable
    various search locations.
 
+.. versionchanged:: 3.32
+   The variables :variable:`CMAKE_FIND_PACKAGE_SORT_ORDER` and
+   :variable:`CMAKE_FIND_PACKAGE_SORT_DIRECTION` now also control the order
+   in which ``find_package`` searches directories matching the glob expression
+   in the search paths ``<prefix>/<name>.framework/Versions/*/Resources/``
+   and ``<prefix>/<name>.framework/Versions/*/Resources/CMake``.  In previous
+   versions of CMake, this order was unspecified.
+
 .. include:: FIND_XXX_ROOT.txt
 .. include:: FIND_XXX_ORDER.txt
 

Source/cmFindPackageCommand.cxx

@@ -16,7 +16,6 @@
 
 #include "cmsys/Directory.hxx"
 #include "cmsys/FStream.hxx"
-#include "cmsys/Glob.hxx"
 #include "cmsys/RegularExpression.hxx"
 #include "cmsys/String.h"
 
@@ -206,8 +205,10 @@ private:
 class cmDirectoryListGenerator
 {
 public:
-  cmDirectoryListGenerator(std::vector<std::string> const& names)
+  cmDirectoryListGenerator(std::vector<std::string> const& names,
+                           bool exactMatch)
     : Names{ names }
+    , ExactMatch{ exactMatch }
     , Current{ this->Matches.cbegin() }
   {
   }
@@ -231,20 +232,28 @@ public:
       // `isDirentryToIgnore(i)` condition to check.
       for (auto i = 0ul; i < directoryLister.GetNumberOfFiles(); ++i) {
         const char* const fname = directoryLister.GetFile(i);
-        if (isDirentryToIgnore(fname)) {
+        // Skip entries to ignore or that aren't directories.
+        if (isDirentryToIgnore(fname) || !directoryLister.FileIsDirectory(i)) {
           continue;
         }
 
-        for (const auto& n : this->Names.get()) {
-          // NOTE Customization point for `cmMacProjectDirectoryListGenerator`
-          const auto name = this->TransformNameBeforeCmp(n);
-          // Skip entries that don't match and non-directories.
-          // ATTENTION BTW, original code also didn't check if it's a symlink
-          // to a directory!
-          const auto equal =
-            (cmsysString_strncasecmp(fname, name.c_str(), name.length()) == 0);
-          if (equal && directoryLister.FileIsDirectory(i)) {
-            this->Matches.emplace_back(fname);
+        if (!this->ExactMatch && this->Names.get().empty()) {
+          this->Matches.emplace_back(fname);
+        } else {
+          for (const auto& n : this->Names.get()) {
+            // NOTE Customization point for
+            // `cmMacProjectDirectoryListGenerator`
+            const auto name = this->TransformNameBeforeCmp(n);
+            // Skip entries that don't match.
+            const auto equal =
+              ((this->ExactMatch
+                  ? cmsysString_strcasecmp(fname, name.c_str())
+                  : cmsysString_strncasecmp(fname, name.c_str(),
+                                            name.length())) == 0);
+            if (equal) {
+              this->Matches.emplace_back(fname);
+              break;
+            }
           }
         }
       }
@@ -273,6 +282,7 @@ protected:
   virtual std::string TransformNameBeforeCmp(std::string same) { return same; }
 
   std::reference_wrapper<const std::vector<std::string>> Names;
+  bool const ExactMatch;
   std::vector<std::string> Matches;
   std::vector<std::string>::const_iterator Current;
 };
@@ -282,8 +292,9 @@ class cmProjectDirectoryListGenerator : public cmDirectoryListGenerator
 public:
   cmProjectDirectoryListGenerator(std::vector<std::string> const& names,
                                   cmFindPackageCommand::SortOrderType so,
-                                  cmFindPackageCommand::SortDirectionType sd)
-    : cmDirectoryListGenerator{ names }
+                                  cmFindPackageCommand::SortDirectionType sd,
+                                  bool exactMatch)
+    : cmDirectoryListGenerator{ names, exactMatch }
     , SortOrder{ so }
     , SortDirection{ sd }
   {
@@ -310,7 +321,7 @@ class cmMacProjectDirectoryListGenerator : public cmDirectoryListGenerator
 public:
   cmMacProjectDirectoryListGenerator(const std::vector<std::string>& names,
                                      cm::string_view ext)
-    : cmDirectoryListGenerator{ names }
+    : cmDirectoryListGenerator{ names, true }
     , Extension{ ext }
   {
   }
@@ -325,47 +336,18 @@ private:
   const cm::string_view Extension;
 };
 
-class cmFileListGeneratorGlob
+class cmAnyDirectoryListGenerator : public cmProjectDirectoryListGenerator
 {
 public:
-  cmFileListGeneratorGlob(cm::string_view pattern)
-    : Pattern(pattern)
+  cmAnyDirectoryListGenerator(cmFindPackageCommand::SortOrderType so,
+                              cmFindPackageCommand::SortDirectionType sd)
+    : cmProjectDirectoryListGenerator(this->EmptyNamesList, so, sd, false)
   {
   }
 
-  std::string GetNextCandidate(const std::string& parent)
-  {
-    if (this->Files.empty()) {
-      // Glob the set of matching files.
-      std::string expr = cmStrCat(parent, this->Pattern);
-      cmsys::Glob g;
-      if (!g.FindFiles(expr)) {
-        return {};
-      }
-      this->Files = g.GetFiles();
-      this->Current = this->Files.cbegin();
-    }
-
-    // Skip non-directories
-    for (; this->Current != this->Files.cend() &&
-         !cmSystemTools::FileIsDirectory(*this->Current);
-         ++this->Current) {
-    }
-
-    return (this->Current != this->Files.cend()) ? *this->Current++
-                                                 : std::string{};
-  }
-
-  void Reset()
-  {
-    this->Files.clear();
-    this->Current = this->Files.cbegin();
-  }
-
 private:
-  cm::string_view Pattern;
-  std::vector<std::string> Files;
-  std::vector<std::string>::const_iterator Current;
+  // NOTE `cmDirectoryListGenerator` needs to hold a reference to this
+  std::vector<std::string> EmptyNamesList;
 };
 
 #if defined(__LCC__)
@@ -2666,7 +2648,7 @@ bool cmFindPackageCommand::SearchPrefix(std::string const& prefix_in)
   auto iCMakeGen = cmCaseInsensitiveDirectoryListGenerator{ "cmake"_s };
   auto firstPkgDirGen =
     cmProjectDirectoryListGenerator{ this->Names, this->SortOrder,
-                                     this->SortDirection };
+                                     this->SortDirection, false };
 
   // PREFIX/(cmake|CMake)/ (useful on windows or in build trees)
   if (TryGeneratedPaths(searchFn, prefix, iCMakeGen)) {
@@ -2685,7 +2667,7 @@ bool cmFindPackageCommand::SearchPrefix(std::string const& prefix_in)
 
   auto secondPkgDirGen =
     cmProjectDirectoryListGenerator{ this->Names, this->SortOrder,
-                                     this->SortDirection };
+                                     this->SortDirection, false };
 
   // PREFIX/(Foo|foo|FOO).*/(cmake|CMake)/(Foo|foo|FOO).*/
   if (TryGeneratedPaths(searchFn, prefix, firstPkgDirGen, iCMakeGen,
@@ -2764,7 +2746,8 @@ bool cmFindPackageCommand::SearchFrameworkPrefix(std::string const& prefix_in)
     cmMacProjectDirectoryListGenerator{ this->Names, ".framework"_s };
   auto rGen = cmAppendPathSegmentGenerator{ "Resources"_s };
   auto vGen = cmAppendPathSegmentGenerator{ "Versions"_s };
-  auto grGen = cmFileListGeneratorGlob{ "/*/Resources"_s };
+  auto anyGen =
+    cmAnyDirectoryListGenerator{ this->SortOrder, this->SortDirection };
 
   // <prefix>/Foo.framework/Resources/
   if (TryGeneratedPaths(searchFn, prefix, fwGen, rGen)) {
@@ -2777,12 +2760,13 @@ bool cmFindPackageCommand::SearchFrameworkPrefix(std::string const& prefix_in)
   }
 
   // <prefix>/Foo.framework/Versions/*/Resources/
-  if (TryGeneratedPaths(searchFn, prefix, fwGen, vGen, grGen)) {
+  if (TryGeneratedPaths(searchFn, prefix, fwGen, vGen, anyGen, rGen)) {
     return true;
   }
 
   // <prefix>/Foo.framework/Versions/*/Resources/CMake/
-  return TryGeneratedPaths(searchFn, prefix, fwGen, vGen, grGen, iCMakeGen);
+  return TryGeneratedPaths(searchFn, prefix, fwGen, vGen, anyGen, rGen,
+                           iCMakeGen);
 }
 
 bool cmFindPackageCommand::SearchAppBundlePrefix(std::string const& prefix_in)

Tests/FindPackageTest/CMakeLists.txt

@@ -550,10 +550,10 @@ endif()
 ############################################################################
 ##Test FIND_PACKAGE using sorting
 set(CMAKE_PREFIX_PATH ${CMAKE_CURRENT_SOURCE_DIR})
-SET(CMAKE_FIND_PACKAGE_SORT_ORDER NAME)
-SET(CMAKE_FIND_PACKAGE_SORT_DIRECTION ASC)
 
 set(SortLib_DIR "" CACHE FILEPATH "Wipe out find results for testing." FORCE)
+SET(CMAKE_FIND_PACKAGE_SORT_ORDER NAME)
+SET(CMAKE_FIND_PACKAGE_SORT_DIRECTION ASC)
 FIND_PACKAGE(SortLib CONFIG)
 IF (NOT "${SortLib_VERSION}" STREQUAL "3.1.1")
   message(SEND_ERROR "FIND_PACKAGE_SORT_ORDER Name Asc! ${SortLib_VERSION}")
@@ -579,6 +579,28 @@ IF (NOT "${SortLib_VERSION}" STREQUAL "4.0.0")
 endif()
 unset(SortLib_VERSION)
 
+
+set(SortFramework_DIR "" CACHE FILEPATH "Wipe out find results for testing." FORCE)
+SET(CMAKE_FIND_PACKAGE_SORT_ORDER NAME)
+SET(CMAKE_FIND_PACKAGE_SORT_DIRECTION ASC)
+FIND_PACKAGE(SortFramework CONFIG)
+IF (NOT "${SortFramework_VERSION}" STREQUAL "3.1.1")
+  message(SEND_ERROR "FIND_PACKAGE_SORT_ORDER Framework Name Asc! ${SortFramework_VERSION}")
+endif()
+set(SortLib_DIR "" CACHE FILEPATH "Wipe out find results for testing." FORCE)
+unset(SortFramework_VERSION)
+
+
+set(SortFramework_DIR "" CACHE FILEPATH "Wipe out find results for testing." FORCE)
+SET(CMAKE_FIND_PACKAGE_SORT_ORDER NATURAL)
+SET(CMAKE_FIND_PACKAGE_SORT_DIRECTION DEC)
+FIND_PACKAGE(SortFramework CONFIG)
+IF (NOT "${SortFramework_VERSION}" STREQUAL "3.10.1")
+  message(SEND_ERROR "FIND_PACKAGE_SORT_ORDER Framework Natural! Dec ${SortFramework_VERSION}")
+endif()
+set(SortLib_DIR "" CACHE FILEPATH "Wipe out find results for testing." FORCE)
+unset(SortFramework_VERSION)
+
 unset(CMAKE_FIND_PACKAGE_SORT_ORDER)
 unset(CMAKE_FIND_PACKAGE_SORT_DIRECTION)
 set(CMAKE_PREFIX_PATH )

Tests/FindPackageTest/SortFramework.framework/Versions/3.1.1/Resources/CMake/SortFrameworkConfig.cmake

@@ -0,0 +1,2 @@
+set(SORT_FRAMEWORK_VERSION 3.1.1)
+message("SortFramework 3.1.1 config reached")

Tests/FindPackageTest/SortFramework.framework/Versions/3.1.1/Resources/CMake/SortFrameworkConfigVersion.cmake

@@ -0,0 +1,9 @@
+set(PACKAGE_VERSION 3.1.1)
+if(PACKAGE_FIND_VERSION_MAJOR EQUAL 3)
+  if(PACKAGE_FIND_VERSION_MINOR EQUAL 1)
+    set(PACKAGE_VERSION_COMPATIBLE 1)
+    if(PACKAGE_FIND_VERSION_PATCH EQUAL 1)
+      set(PACKAGE_VERSION_EXACT 1)
+    endif()
+  endif()
+endif()

Tests/FindPackageTest/SortFramework.framework/Versions/3.10.1/Resources/CMake/SortFrameworkConfig.cmake

@@ -0,0 +1,2 @@
+set(SORT_FRAMEWORK_VERSION 3.10.1)
+message("SortFramework 3.10.1 config reached")

Tests/FindPackageTest/SortFramework.framework/Versions/3.10.1/Resources/CMake/SortFrameworkConfigVersion.cmake

@@ -0,0 +1,9 @@
+set(PACKAGE_VERSION 3.10.1)
+if(PACKAGE_FIND_VERSION_MAJOR EQUAL 3)
+  if(PACKAGE_FIND_VERSION_MINOR EQUAL 10)
+    set(PACKAGE_VERSION_COMPATIBLE 1)
+    if(PACKAGE_FIND_VERSION_PATCH EQUAL 1)
+      set(PACKAGE_VERSION_EXACT 1)
+    endif()
+  endif()
+endif()