-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[cmake] Use new mode to find python for pybind11 #22012
[cmake] Use new mode to find python for pybind11 #22012
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Experimental packaging (to use to test d-e-e): https://drake-jenkins.csail.mit.edu/view/Packaging/job/mac-arm-sonoma-unprovisioned-clang-cmake-experimental-packaging/3/
Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @BetsyMcPhail)
f30350d
to
cb19a39
Compare
Update pybind11-config.cmake to follow the same logic as upstream pybind11Common so that the new (not deprecated) method of finding Python is used.
cb19a39
to
3f6e83a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the output from the failing Mac d-e-e job (https://github.com/RobotLocomotion/drake-external-examples/actions/runs/11273554281/job/31350829452) we can see a deprecation warning:
CMake Warning (dev) at /opt/drake/lib/cmake/pybind11/FindPythonLibsNew.cmake:101 (message):
Policy CMP0148 is not set: The FindPythonInterp and FindPythonLibs modules
are removed. Run "cmake --help-policy CMP0148" for policy details. Use
the cmake_policy command to set the policy and suppress this warning, or
preferably upgrade to using FindPython, either by calling it explicitly
before pybind11, or by setting PYBIND11_FINDPYTHON ON before pybind11.
Also, the python that pybind11 is finding using the old, deprecated method is Python 3.9 rather than 3.12, as expected:
-- Found PythonInterp: /usr/bin/python3 (found suitable version "3.9.6", minimum required is "3.6")
-- Found PythonLibs: /Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.9/lib/libpython3.9.dylib
With the changes in this PR, we should no longer see a warning and the mac d-e-e job should pass.
To test, I created an experimental package: https://drake-jenkins.csail.mit.edu/view/Packaging/job/mac-arm-sonoma-unprovisioned-clang-cmake-experimental-packaging/5/
And temporarily updated d-e-e to use that package: RobotLocomotion/drake-external-examples#324.
Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @BetsyMcPhail)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+@sammy-tri for platform review per schedule please. I'm not sure if we also need a feature reviewer for this one?
Reviewable status: LGTM missing from assignee sammy-tri(platform), needs at least two assigned reviewers, missing label for release notes (waiting on @BetsyMcPhail)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can just take this one to unload -@sammy-tri.
both +(status: single reviewer ok) +(release notes: fix).
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: needs platform reviewer assigned, needs at least one assigned reviewer (waiting on @BetsyMcPhail)
Update
pybind11-config.cmake
to follow the same logic as upstreampybind11Common
so that the new (not deprecated) method of finding Python is used.Towards #22009
This change is