-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
cpython: Add native support for CMake's builtin FindPython(3) #23394
cpython: Add native support for CMake's builtin FindPython(3) #23394
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit 579268bcpython/3.10.0@#c132fd9a55da10c5f36f739a6acaaca1
|
@valgur Since you're reviewing, what would you think about having an option to opt-out of this behavior? We could install the wrapper module regardless, and then the option would just affect the package info, and not change the ID. |
@Ahajha That's not a bad idea, but I can't think of a reason why someone would include the cpython package in requires, but would not want it to be picked up by |
@valgur My only concern is that perhaps it would break someone's workflow. But it likely won't, so yea I think we can leave it for now. |
Want to see if I can get |
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit 5ac5a41cpython/3.8.12@#68e2fda52ca261db5b82dcf4852c42b0
|
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit defcf60cpython/3.8.12@#f14b179f66e0b599224e5d5d20d75876
cpython/3.10.0@#847d16b5c081218315afb9185f98bfdc
cpython/3.9.7@#f98d90a3cd2926a45c790762dc314235
|
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.
Thanks!
test_package looks so much saner as well now. :)
@valgur Seems that there's a bug in FindPython regarding SOABI and Windows debug builds, I've reported it here: https://gitlab.kitware.com/cmake/cmake/-/issues/25874. It's possible the difficulty in auto-detection is related, I might make a separate issue for that. Just going to fix the lint warning and then this should be go (aside from waiting for the other PR to be merged). |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Conan v1 pipeline ✔️All green in build 13 (
Conan v2 pipeline ✔️
All green in build 13 (
|
Specify library name and version: cpython/all
With these new changes, consumers should be able to use
find_package(Python)
orfind_package(Python3)
without any additional setup. See notes about CMake and FindPython here: #23151I've removed the tests for the old
find_package(PythonInterp)
andfind_package(PythonLibs)
. Nothing's preventing them from working, but I figure most people at this point would be interested in the new versions, so this is just to clean up the test package (these are still tested in the test_v1_package, along with the "manual"find_package(Python3)
integration, since I haven't added support for the v1 CMake genererators).Going to wait for the new versions to be merged before taking this out of draft.