diff mbox series

cmake: Ensure find_package() uses deterministic search order by default

Message ID 20250620075818.3320026-1-Moritz.Haase@bmw.de
State New
Headers show
Series cmake: Ensure find_package() uses deterministic search order by default | expand

Commit Message

Moritz Haase June 20, 2025, 7:58 a.m. UTC
Historically, find_package() does not guarantee the order in which directories
matching a search path containing a glob expression are processed in - the
"first valid package" will be selected if there are multiple candidates. In such
cases, which package is chosen is completely random and can change, potentially
leading to build failures and reproducibility issues. This is rather unexpected
and confusing for developers.

Changing the default sort order and direction to natural sorting with a
descending order will result in the newest version of a library being picked in
case there are multiple ones available.

This commit backports a corresponding fix from upstream, which was accepted for
CMake 4.2 (see [0]). Documentation changes have been dropped, as have been
changes to tests that do not exist on CMake 3.31.6.

Signed-off-by: Moritz Haase <Moritz.Haase@bmw.de>
---
 meta/recipes-devtools/cmake/cmake.inc         |  1 +
 ...-deterministic-search-order-by-defau.patch | 74 +++++++++++++++++++
 2 files changed, 75 insertions(+)
 create mode 100644 meta/recipes-devtools/cmake/cmake/0001-find_package-Use-deterministic-search-order-by-defau.patch

Comments

Gyorgy Sarvari June 20, 2025, 8:14 a.m. UTC | #1
On 6/20/25 09:58, Moritz Haase via lists.openembedded.org wrote:
> This commit backports a corresponding fix from upstream, which was accepted for
> CMake 4.2 (see [0]). Documentation changes have been dropped, as have been
> changes to tests that do not exist on CMake 3.31.6.
>
> Signed-off-by: Moritz Haase <Moritz.Haase@bmw.de>
>
[0] seems to be missing from the message.
Alexander Kanavin June 20, 2025, 8:18 a.m. UTC | #2
Can you work on a cmake update to 4.x rather please? There's been
another similar 'cmake backport to fix my particular issue' patch
recently, and I'd rather see efforts to keep up to date with upstream.


Alex

On Fri, 20 Jun 2025 at 09:58, Moritz Haase via lists.openembedded.org
<Moritz.Haase=bmw.de@lists.openembedded.org> wrote:
>
> Historically, find_package() does not guarantee the order in which directories
> matching a search path containing a glob expression are processed in - the
> "first valid package" will be selected if there are multiple candidates. In such
> cases, which package is chosen is completely random and can change, potentially
> leading to build failures and reproducibility issues. This is rather unexpected
> and confusing for developers.
>
> Changing the default sort order and direction to natural sorting with a
> descending order will result in the newest version of a library being picked in
> case there are multiple ones available.
>
> This commit backports a corresponding fix from upstream, which was accepted for
> CMake 4.2 (see [0]). Documentation changes have been dropped, as have been
> changes to tests that do not exist on CMake 3.31.6.
>
> Signed-off-by: Moritz Haase <Moritz.Haase@bmw.de>
> ---
>  meta/recipes-devtools/cmake/cmake.inc         |  1 +
>  ...-deterministic-search-order-by-defau.patch | 74 +++++++++++++++++++
>  2 files changed, 75 insertions(+)
>  create mode 100644 meta/recipes-devtools/cmake/cmake/0001-find_package-Use-deterministic-search-order-by-defau.patch
>
> diff --git a/meta/recipes-devtools/cmake/cmake.inc b/meta/recipes-devtools/cmake/cmake.inc
> index 9b1898f22f..a18a95233d 100644
> --- a/meta/recipes-devtools/cmake/cmake.inc
> +++ b/meta/recipes-devtools/cmake/cmake.inc
> @@ -19,6 +19,7 @@ CMAKE_MAJOR_VERSION = "${@'.'.join(d.getVar('PV').split('.')[0:2])}"
>  SRC_URI = "https://cmake.org/files/v${CMAKE_MAJOR_VERSION}/cmake-${PV}.tar.gz \
>             file://0001-CMakeDetermineCompilerABI-Strip-pipe-from-compile-fl.patch \
>             file://0001-cmCurl-Avoid-using-undocumented-type-for-CURLOPT_NET.patch \
> +           file://0001-find_package-Use-deterministic-search-order-by-defau.patch \
>             "
>
>  SRC_URI[sha256sum] = "653427f0f5014750aafff22727fb2aa60c6c732ca91808cfb78ce22ddd9e55f0"
> diff --git a/meta/recipes-devtools/cmake/cmake/0001-find_package-Use-deterministic-search-order-by-defau.patch b/meta/recipes-devtools/cmake/cmake/0001-find_package-Use-deterministic-search-order-by-defau.patch
> new file mode 100644
> index 0000000000..164f5c6428
> --- /dev/null
> +++ b/meta/recipes-devtools/cmake/cmake/0001-find_package-Use-deterministic-search-order-by-defau.patch
> @@ -0,0 +1,74 @@
> +From fa3c5c6d914abfe7c8d3335eeddedd53518946be Mon Sep 17 00:00:00 2001
> +From: Moritz Haase <Moritz.Haase@bmw.de>
> +Date: Fri, 13 Jun 2025 13:41:50 +0200
> +Subject: [PATCH] find_package: Use deterministic search order by default
> +
> +Historically, find_package() does not guarantee the order in which directories
> +matching a search path containing a glob expression are processed in - the
> +"first valid package" will be selected if there are multiple candidates. In such
> +cases, which package is chosen is completely random and can change, potentially
> +leading to build failures and reproducibility issues. This is rather unexpected
> +and confusing for developers.
> +
> +Now that CMake has bumped its major version, it's a good time to change default
> +sort order and direction to natural sorting with a descending order. That will
> +result in the newest version of a library being picked in case there are
> +multiple ones available.
> +
> +Upstream-Status: Backport [4.2.0 (planned), 61d8fae11655d3a2c4b8fd80af7a0d1d5f3e2f9f]
> +Signed-off-by: Moritz Haase <Moritz.Haase@bmw.de>
> +---
> + Source/cmFindPackageCommand.h        |  8 ++++----
> + Tests/FindPackageTest/CMakeLists.txt | 13 ++++++++++++-
> + 2 files changed, 16 insertions(+), 5 deletions(-)
> +
> +diff --git a/Source/cmFindPackageCommand.h b/Source/cmFindPackageCommand.h
> +index 653e15f51faf84406e8c962207303852d1c8ad3e..478a0c6ff96d08afb01d5f056bc86372bdeae149 100644
> +--- a/Source/cmFindPackageCommand.h
> ++++ b/Source/cmFindPackageCommand.h
> +@@ -220,10 +220,10 @@ private:
> +
> +   class FlushDebugBufferOnExit;
> +
> +-  /*! the selected sortOrder (None by default)*/
> +-  SortOrderType SortOrder = None;
> +-  /*! the selected sortDirection (Asc by default)*/
> +-  SortDirectionType SortDirection = Asc;
> ++  /*! the selected sortOrder (Natural by default)*/
> ++  SortOrderType SortOrder = Natural;
> ++  /*! the selected sortDirection (Dec by default)*/
> ++  SortDirectionType SortDirection = Dec;
> +
> +   struct ConfigFileInfo
> +   {
> +diff --git a/Tests/FindPackageTest/CMakeLists.txt b/Tests/FindPackageTest/CMakeLists.txt
> +index 73d3fb4832203b4d4d10c9a4956546a1ad8be7fd..0a2186df562d302eb44d2e4046c42c96ed524f09 100644
> +--- a/Tests/FindPackageTest/CMakeLists.txt
> ++++ b/Tests/FindPackageTest/CMakeLists.txt
> +@@ -116,7 +116,7 @@ foreach(p ${PACKAGES})
> + endforeach()
> +
> + # Enable framework and bundle searching.  Make sure bundles are found
> +-# before unix-syle packages.
> ++# before unix-style packages.
> + set(CMAKE_FIND_FRAMEWORK LAST)
> + set(CMAKE_FIND_APPBUNDLE FIRST)
> +
> +@@ -579,6 +579,17 @@ IF (NOT "${SortLib_VERSION}" STREQUAL "4.0.0")
> + endif()
> + unset(SortLib_VERSION)
> +
> ++
> ++set(SortLib_DIR "" CACHE FILEPATH "Wipe out find results for testing." FORCE)
> ++# Expected to default to 'NATURAL' and 'DEC'
> ++unset(CMAKE_FIND_PACKAGE_SORT_ORDER)
> ++unset(CMAKE_FIND_PACKAGE_SORT_DIRECTION)
> ++FIND_PACKAGE(SortLib CONFIG)
> ++IF (NOT "${SortLib_VERSION}" STREQUAL "3.10.1")
> ++  message(SEND_ERROR "FIND_PACKAGE_SORT_ORDER Default! ${SortLib_VERSION}")
> ++endif()
> ++unset(SortLib_VERSION)
> ++
> + unset(CMAKE_FIND_PACKAGE_SORT_ORDER)
> + unset(CMAKE_FIND_PACKAGE_SORT_DIRECTION)
> + set(CMAKE_PREFIX_PATH )
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#219104): https://lists.openembedded.org/g/openembedded-core/message/219104
> Mute This Topic: https://lists.openembedded.org/mt/113739065/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Moritz Haase June 20, 2025, 9:18 a.m. UTC | #3
Yes, you're right, thanks. Here is the missing URL:

https://gitlab.kitware.com/cmake/cmake/-/merge_requests/10882

Moritz
Moritz Haase June 20, 2025, 9:20 a.m. UTC | #4
Sure, that's fair enough. I'll give it a try.

Moritz
diff mbox series

Patch

diff --git a/meta/recipes-devtools/cmake/cmake.inc b/meta/recipes-devtools/cmake/cmake.inc
index 9b1898f22f..a18a95233d 100644
--- a/meta/recipes-devtools/cmake/cmake.inc
+++ b/meta/recipes-devtools/cmake/cmake.inc
@@ -19,6 +19,7 @@  CMAKE_MAJOR_VERSION = "${@'.'.join(d.getVar('PV').split('.')[0:2])}"
 SRC_URI = "https://cmake.org/files/v${CMAKE_MAJOR_VERSION}/cmake-${PV}.tar.gz \
            file://0001-CMakeDetermineCompilerABI-Strip-pipe-from-compile-fl.patch \
            file://0001-cmCurl-Avoid-using-undocumented-type-for-CURLOPT_NET.patch \
+           file://0001-find_package-Use-deterministic-search-order-by-defau.patch \
            "
 
 SRC_URI[sha256sum] = "653427f0f5014750aafff22727fb2aa60c6c732ca91808cfb78ce22ddd9e55f0"
diff --git a/meta/recipes-devtools/cmake/cmake/0001-find_package-Use-deterministic-search-order-by-defau.patch b/meta/recipes-devtools/cmake/cmake/0001-find_package-Use-deterministic-search-order-by-defau.patch
new file mode 100644
index 0000000000..164f5c6428
--- /dev/null
+++ b/meta/recipes-devtools/cmake/cmake/0001-find_package-Use-deterministic-search-order-by-defau.patch
@@ -0,0 +1,74 @@ 
+From fa3c5c6d914abfe7c8d3335eeddedd53518946be Mon Sep 17 00:00:00 2001
+From: Moritz Haase <Moritz.Haase@bmw.de>
+Date: Fri, 13 Jun 2025 13:41:50 +0200
+Subject: [PATCH] find_package: Use deterministic search order by default
+
+Historically, find_package() does not guarantee the order in which directories
+matching a search path containing a glob expression are processed in - the
+"first valid package" will be selected if there are multiple candidates. In such
+cases, which package is chosen is completely random and can change, potentially
+leading to build failures and reproducibility issues. This is rather unexpected
+and confusing for developers.
+
+Now that CMake has bumped its major version, it's a good time to change default
+sort order and direction to natural sorting with a descending order. That will
+result in the newest version of a library being picked in case there are
+multiple ones available.
+
+Upstream-Status: Backport [4.2.0 (planned), 61d8fae11655d3a2c4b8fd80af7a0d1d5f3e2f9f]
+Signed-off-by: Moritz Haase <Moritz.Haase@bmw.de>
+---
+ Source/cmFindPackageCommand.h        |  8 ++++----
+ Tests/FindPackageTest/CMakeLists.txt | 13 ++++++++++++-
+ 2 files changed, 16 insertions(+), 5 deletions(-)
+
+diff --git a/Source/cmFindPackageCommand.h b/Source/cmFindPackageCommand.h
+index 653e15f51faf84406e8c962207303852d1c8ad3e..478a0c6ff96d08afb01d5f056bc86372bdeae149 100644
+--- a/Source/cmFindPackageCommand.h
++++ b/Source/cmFindPackageCommand.h
+@@ -220,10 +220,10 @@ private:
+ 
+   class FlushDebugBufferOnExit;
+ 
+-  /*! the selected sortOrder (None by default)*/
+-  SortOrderType SortOrder = None;
+-  /*! the selected sortDirection (Asc by default)*/
+-  SortDirectionType SortDirection = Asc;
++  /*! the selected sortOrder (Natural by default)*/
++  SortOrderType SortOrder = Natural;
++  /*! the selected sortDirection (Dec by default)*/
++  SortDirectionType SortDirection = Dec;
+ 
+   struct ConfigFileInfo
+   {
+diff --git a/Tests/FindPackageTest/CMakeLists.txt b/Tests/FindPackageTest/CMakeLists.txt
+index 73d3fb4832203b4d4d10c9a4956546a1ad8be7fd..0a2186df562d302eb44d2e4046c42c96ed524f09 100644
+--- a/Tests/FindPackageTest/CMakeLists.txt
++++ b/Tests/FindPackageTest/CMakeLists.txt
+@@ -116,7 +116,7 @@ foreach(p ${PACKAGES})
+ endforeach()
+ 
+ # Enable framework and bundle searching.  Make sure bundles are found
+-# before unix-syle packages.
++# before unix-style packages.
+ set(CMAKE_FIND_FRAMEWORK LAST)
+ set(CMAKE_FIND_APPBUNDLE FIRST)
+ 
+@@ -579,6 +579,17 @@ IF (NOT "${SortLib_VERSION}" STREQUAL "4.0.0")
+ endif()
+ unset(SortLib_VERSION)
+ 
++
++set(SortLib_DIR "" CACHE FILEPATH "Wipe out find results for testing." FORCE)
++# Expected to default to 'NATURAL' and 'DEC'
++unset(CMAKE_FIND_PACKAGE_SORT_ORDER)
++unset(CMAKE_FIND_PACKAGE_SORT_DIRECTION)
++FIND_PACKAGE(SortLib CONFIG)
++IF (NOT "${SortLib_VERSION}" STREQUAL "3.10.1")
++  message(SEND_ERROR "FIND_PACKAGE_SORT_ORDER Default! ${SortLib_VERSION}")
++endif()
++unset(SortLib_VERSION)
++
+ unset(CMAKE_FIND_PACKAGE_SORT_ORDER)
+ unset(CMAKE_FIND_PACKAGE_SORT_DIRECTION)
+ set(CMAKE_PREFIX_PATH )