Skip to content

build: support Eigen 5 #6036

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
e-kwsm wants to merge 1 commit into
base: master
Choose a base branch
from
+4 −1
Open

build: support Eigen 5 #6036

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,11 +299,14 @@ if(PYBIND11_TEST_FILES_EIGEN_I GREATER -1)
set(EIGEN3_VERSION "${PYBIND11_EIGEN_VERSION_STRING}")

else()
find_package(Eigen3 3.2.7 QUIET CONFIG)
find_package(Eigen3 3.2.7...5 QUIET CONFIG)
Copy link
Copy Markdown
Collaborator

@rwgk rwgk Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With a lot of help from Cursor GPT-5.4 Extra High Fast:

One small suggestion: I think this would be a bit clearer and more robust as a two-step CONFIG probe instead of 3.2.7...5.

I checked this locally, and 3.2.7...5 is not a wildcard for “any 5.x” in standard CMake version semantics. It is a bounded range whose upper endpoint is effectively 5.0.0. The reason this still passed with Eigen 5.0.1 in CI seems to be that the particular package version file accepted it, but that is package-specific behavior rather than something I would want to rely on.

Something like this seems safer and more explicit:

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@
-    find_package(Eigen3 3.2.7...5 QUIET CONFIG)
-    set(EIGEN3_FOUND ${Eigen3_FOUND})
-    set(EIGEN3_VERSION ${Eigen3_VERSION})
+    find_package(Eigen3 3.2.7 QUIET CONFIG)
+    if(NOT Eigen3_FOUND)
+      find_package(Eigen3 5 QUIET CONFIG)
+    endif()
+    set(EIGEN3_FOUND ${Eigen3_FOUND})
+    set(EIGEN3_VERSION ${Eigen3_VERSION})
 
     if(NOT EIGEN3_FOUND)
       # Couldn't load via target, so fall back to allowing module mode finding, which will pick up
       # tools/FindEigen3.cmake
       # XXX: MODULE mode does not work with Eigen 5
       find_package(Eigen3 3.2.7 QUIET)

Also, please add something like the following to the PR description:

This intentionally fixes the CONFIG-mode path and normalizes the legacy EIGEN3_* variables after the CONFIG probe. It does not touch the old tools/FindEigen3.cmake logic, which is more invasive and easier to get wrong. In other words, this is a targeted compatibility fix for environments that provide an Eigen3Config.cmake, while leaving the legacy MODULE-mode finder as a fallback for older setups.

I think that extra explanation would help reviewers understand why this “band-aid” approach is actually a good first step here.

set(EIGEN3_FOUND ${Eigen3_FOUND})
set(EIGEN3_VERSION ${Eigen3_VERSION})

if(NOT EIGEN3_FOUND)
# Couldn't load via target, so fall back to allowing module mode finding, which will pick up
# tools/FindEigen3.cmake
# XXX: MODULE mode does not work with Eigen 5
find_package(Eigen3 3.2.7 QUIET)
endif()
endif()
Expand Down
Loading