-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[build] copy Python detection code into StandaloneOverlays.cmake #31234
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
[build] copy Python detection code into StandaloneOverlays.cmake #31234
Conversation
Move detection of Python code into dedicate CMake module, so it can be reused for workflows that do not rely on reading the main `CMakeLists.txt`. Builds upon swiftlang#30992, swiftlang#31020 Addresses rdar://62245784
@swift-ci please smoke test |
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.
Please do not call this FindPython
. Find*
have very specific requirements (see the CMake documentation).
Personally, I think it is better to duplicate the code rather than create a module for this. I don't think its valuable for:
find_package(Python2 COMPONENTS Interpreter REQUIRED)
find_package(Python3 COMPONENTS Interpreter REQUIRED)
to be extracted into a module (just as if you have 2 function calls, it doesn't make sense to extract that into a function).
The workaround is a temporary thing until Apple's builders are caught up and have python3 available. Making that more obvious is better IMO and indicative that we should get the builders updated so that python3 can be detected properly.
Furthermore, realize that this will eventually become:
find_package(Python3 COMPONENTS Interpreter REQUIRED)
as more of the tools are weened off of python2 in favour of python3.
As per review feedback.
@swift-ci please smoke test |
@swift-ci please test Windows |
@edymtt any idea if the overlays use anything that requires python2? I think if we can avoid spreading the use of python2, it will be much more helpful in weening the project off of python2. |
To the best of my knowledge, building the overlays does not have any part that requires Python 2 -- the main issue here was that we needed to detect Python properly when building some configurations that do not rely on the main CMakeLists.txt, we needed to piggyback the workaround as well since we are still figuring out some detection issues around Python 3 in our environment. |
This is to support workflows that do not rely on reading the main
CMakeLists.txt
.Based upon #30992, #31020
Addresses rdar://62245784