Skip to content
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

[python3] Build interpreter #14891

Merged
merged 8 commits into from
Dec 18, 2020
Merged

Conversation

Hoikas
Copy link
Contributor

@Hoikas Hoikas commented Dec 2, 2020

This change allows the python3 port to install the interpreter as a tool. This is useful because, when embedding python3 in a program, the python standard library must be available in the final application (read: bundled). CMake provides the location to the standard library on the build machine, but the interpreter itself is required for this functionality to work. More critically, CMake 3.12+'s python find modules require that the python executable and development libraries match exactly, or it will either reject the supplied (read: vcpkg) libraries and either fail or silently use the system libraries, causing an undocumented gotcha dependency on the system python's major.minor version matching vcpkg's exactly.

Internally, this changeset removes the usage of custom calls to configure and make on unix-like platforms and replaces them with vcpkg's vcpkg_configure_make and vcpkg_install_make. This was extracted from #13556 and represents the minimal "first half" of said extraction. The second half of the extraction will be the vcpkg-cmake-wrapper, which will further improve the story around consumers of the python3 port.

@Hoikas Hoikas force-pushed the py3_interpreter branch 3 times, most recently from 41a5962 to 5bbbc8e Compare December 2, 2020 02:14
This allows overriding the name of the install target, allowing
specification of something other than `make install`, eg `make
altinstall`.
@JonLiu1993 JonLiu1993 requested a review from JackBoosY December 2, 2020 05:45
@JonLiu1993 JonLiu1993 removed the request for review from JackBoosY December 2, 2020 05:46
@JackBoosY JackBoosY added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly labels Dec 2, 2020
@Hoikas
Copy link
Contributor Author

Hoikas commented Dec 2, 2020

Blocked by #14884 and #14913

@JackBoosY JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Dec 3, 2020
@Hoikas Hoikas marked this pull request as ready for review December 4, 2020 09:08
@Hoikas
Copy link
Contributor Author

Hoikas commented Dec 7, 2020

I should have mentioned a few days ago... This is ready for review now that the baseline issues have been fixed.

@JackBoosY
Copy link
Contributor

Oh sorry, I forgot this PR.

@JackBoosY
Copy link
Contributor

/azp run

@JackBoosY JackBoosY removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Dec 8, 2020
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY JackBoosY requested a review from BillyONeal December 8, 2020 09:12
@JackBoosY JackBoosY added requires:author-response info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Dec 8, 2020
@Hoikas
Copy link
Contributor Author

Hoikas commented Dec 12, 2020

After re-reading the previous messages, it seems like there is a quite a bit of work to do to properly "fix" every issue with static Python. All of which is outside the scope of this pull request, which is to simply build the interpreter itself. So, I restored the original static CRT functionality. The interpreter built with a static CRT works in my testing, so I think that should address @ras0219-msft's concern in that python3 will not cause any cascading skips on the x64-windows-static triplet.

Note that the original patch for static CRT support is now handled by a props file.

@Hoikas
Copy link
Contributor Author

Hoikas commented Dec 13, 2020

It looks like the macOS builder died before it even got started. Could someone kindly trigger a rebuild on this? 😅

@BillyONeal
Copy link
Member

It looks like the macOS builder died before it even got started. Could someone kindly trigger a rebuild on this? 😅

It says passed to me?

@Hoikas
Copy link
Contributor Author

Hoikas commented Dec 16, 2020

Heh, I swear when I originally posted that, AZP had "stopped hearing from" the macOS machine. Maybe that wasn't a hallucination 😄

BillyONeal added a commit to BillyONeal/vcpkg that referenced this pull request Dec 16, 2020
In microsoft#14891 @Hoikas points out that the port depends on Python but the portfile.cmake explicitly disables it with `-DPYILMBASE_ENABLE=FALSE`.
@BillyONeal
Copy link
Member

It looks like hyperscan was already fixed.

@JackBoosY
Copy link
Contributor

I restarted the pipeline test and the regression is gone.

BillyONeal added a commit that referenced this pull request Dec 16, 2020
In #14891 @Hoikas points out that the port depends on Python but the portfile.cmake explicitly disables it with `-DPYILMBASE_ENABLE=FALSE`.
@BillyONeal BillyONeal added the info:reviewed Pull Request changes follow basic guidelines label Dec 17, 2020
@BillyONeal BillyONeal merged commit bdb225b into microsoft:master Dec 18, 2020
@BillyONeal
Copy link
Member

Thanks for your contribution!

@Hoikas Hoikas deleted the py3_interpreter branch December 24, 2020 02:05
ryukw7 pushed a commit to ryukw7/vcpkg that referenced this pull request Dec 24, 2020
In microsoft#14891 @Hoikas points out that the port depends on Python but the portfile.cmake explicitly disables it with `-DPYILMBASE_ENABLE=FALSE`.
ryukw7 pushed a commit to ryukw7/vcpkg that referenced this pull request Dec 24, 2020
Sungeun0318 pushed a commit to Sungeun0318/-vcpkg that referenced this pull request May 15, 2024
In microsoft/vcpkg#14891 @Hoikas points out that the port depends on Python but the portfile.cmake explicitly disables it with `-DPYILMBASE_ENABLE=FALSE`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[python3] Install python executable from port
4 participants