-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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.
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
[BUG]: Warning when using CMake 3.27, CMP0148 policy is ignored #4785
Comments
TL;DR: I'd say please go ahead and send a PR that resolves the warnings you're seeing. I don't know a lot about cmake, we'll have to wait for @henryiii for a conclusive answer, but to me this looks like one of those little things that are prone to slip through the cracks. It's also something that seems very difficult to cover with tests, and since it's just a warning, not critical enough to warrant a big effort to cover all possible scenarios. My conclusion: Let's fix the obvious and go from there. |
Here you go: #4786. I have bumped up all CMake 3.26 references that I found in both the main code, tests, and documentation to 3.27, thus raising the maximum CMake version supported. I haven't messed with any of the CI build scripts as those were already updated as part of #4719. All tests pass on my local build, but of course we'll have to wait for the CI pipelines to finish to make sure no regressions were introduced. |
No. It should only switch if a user specifies 3.27 as their minimum or maximum CMake version, thereby acknowledging they are aware of the change. If you don't do that, then it shouldn't change behavior, which would break existing packages. CMake updates do not break existing packages. 99% percent of users should be specifying
It should not auto-switch if one is not set. Users need to respond to this warning, it's there to help them. However, if it is set, yes, it needs to inherit it, that's a bug. |
Sorry, I don't think I was very precise with my language when I wrote that. What I meant is that if the current CMake minimum/maximum version is 3.27, i.e.:
then pybind11 should inherit the CMP0148 policy (which would be set to Due to the current bug where pybind11 is not inheriting that setting from the root project, that is not happening either, and you are right that that is what the bug actually is. |
* Upgrade maximum supported CMake version to 3.27 to fix warning with CMP0148 policy (#4785) * Update `macos_brew_install_llvm` pipeline to use expected Python installation * Fix `Python_EXECUTABLE` Cmake variable typo * Apply suggestions from code review * fix: use FindPython for CMake 3.18+ by default for pybind11's tests Signed-off-by: Henry Schreiner <[email protected]> * tests: fix issues with finding Python Signed-off-by: Henry Schreiner <[email protected]> * tests: also set executable on subdir tests Signed-off-by: Henry Schreiner <[email protected]> * fix(cmake): correct logic for FindPython Signed-off-by: Henry Schreiner <[email protected]> * Update ci.yml * Revert "Update ci.yml" This reverts commit 33798ad. --------- Signed-off-by: Henry Schreiner <[email protected]> Co-authored-by: Henry Schreiner <[email protected]>
Required prerequisites
What version (or hash if on master) of pybind11 are you using?
v2.11.1
Problem description
The latest version of pybind11 (v2.11.1) results in a warning when configuring a project (that includes pybind11) with CMake 3.27 that reads:
This issue occurs in
pybind11Tools.cmake
, which is called frompybind11Common.cmake
when it determines that it should be using the "classic" CMake mode that relies on the deprecatedFindPythonInterp
andFindPythonLibs
modules as detailed here.With the changes from #4719, CMake 3.27 should by default switch to the "new"
pybind11ToolsNew.cmake
, which relies on the more modernFindPython
CMake modules.However, this is not happening automatically. Furthermore, if one sets either of the following policies on the root project:
or
the behavior is exactly the same, defaulting to the old
pybind11Tools.cmake
and printing the same warning about the CMP0148 policy.Of course, looking at the logic in the code, one can easily figure out that setting
on the root project will get around this issue, and it indeed does, so this is not a critical issue as there is a fairly simple workaround.
Having said that, pybind11 should be able to inherit the CMP0148 policy from the root project and, if one is not provided and the current CMake version is 3.27, it should automatically switch to the
pybind11ToolsNew.cmake
logic.As far as I can tell, the reason why this is not happening automatically is that in the pybind11 root
CMakeLists.txt
, the CMake policy is being set as:for CMake versions >= 3.26.
Thus, when CMake tries to get the current policy value in
pybind11Common.cmake
with:the check always returns an empty string as policy CMP0148 is not defined if the CMake policy version is 3.26 (was added in 3.27).
This can easily be resolved by updating the root
CMakeLists.txt
(and possibly otherCMakeLists.txt
files in the repo) to:for CMake versions >= 3.27.
I can confirm that making the above change forces CMake 3.27 in my project to use the new logic from
pybind11ToolsNew.cmake
and the warning goes away, exactly the same way as if thePYBIND11_FINDPYTHON
flag had been enabled.I can make a PR with the changes, but I wanted to hear from @henryiii or @rwgk (from the original #4719 PR) if there was any reason why this change was not implemented at that time.
Thanks!
Reproducible example code
No response
Is this a regression? Put the last known working version here if it is.
Not a regression
The text was updated successfully, but these errors were encountered: