Skip to content

[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

Merged
merged 2 commits into from
Apr 23, 2020

Conversation

edymtt
Copy link
Contributor

@edymtt edymtt commented Apr 23, 2020

This is to support workflows that do not rely on reading the main
CMakeLists.txt.

Based upon #30992, #31020

Addresses rdar://62245784

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
@edymtt
Copy link
Contributor Author

edymtt commented Apr 23, 2020

@swift-ci please smoke test

Copy link
Member

@compnerd compnerd left a 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.

@edymtt edymtt changed the title [build] refactor Python detection code [build] copy Python detection code into StandaloneOverlays.cmake Apr 23, 2020
@edymtt edymtt requested a review from compnerd April 23, 2020 15:28
@edymtt
Copy link
Contributor Author

edymtt commented Apr 23, 2020

@swift-ci please smoke test

@edymtt
Copy link
Contributor Author

edymtt commented Apr 23, 2020

@swift-ci please test Windows

@compnerd
Copy link
Member

@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.

@edymtt
Copy link
Contributor Author

edymtt commented Apr 23, 2020

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.

@edymtt edymtt merged commit 39ddc67 into swiftlang:master Apr 23, 2020
@edymtt edymtt deleted the refactor_python_detection_logic branch April 23, 2020 18:53
edymtt added a commit that referenced this pull request Apr 23, 2020
…#31235)

This is to support workflows that do not rely on reading the main
CMakeLists.txt.

Builds upon #30992, #31020

Cherry pick of #31234 (39ddc67)

Addresses rdar://62245784
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants