-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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] Improve support for embedding and add tools #13556
Conversation
Thanks for the PR! @ras0219-msft, could you help further review? |
@@ -15,7 +15,7 @@ get_filename_component(PYTHON2_DIR ${PYTHON2} DIRECTORY) | |||
vcpkg_configure_cmake( | |||
SOURCE_PATH ${SOURCE_PATH} | |||
PREFER_NINJA | |||
OPTIONS -DCMAKE_PROGRAM_PATH=${PYTHON2_DIR} -DUSE_WINDOWS6_API=ON | |||
OPTIONS -DPYTHON:FILEPATH=${PYTHON2} -DCMAKE_PROGRAM_PATH=${PYTHON2_DIR} -DUSE_WINDOWS6_API=ON |
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 like this writing. it's tiny and beautiful!
Thanks for this PR! Python is a very tricky subject in vcpkg, because there are three different use cases:
Framed by the above three circumstances, could you re-explain what this PR is intended to accomplish? It's important to keep these situations separate in order to enable cross compilation. |
Agreed, Python is definitely very tricky. Given the three use case you pointed out, this pull request was designed to address issues I've experienced in projects I contribute to around use case 2 and, to some extent, use case 1. When using vcpkg's python with any of cmake's FindPython modules on Windows, they first go to the registry. The find modules introduced in CMake 3.12 (FindPython, FindPython2, FindPython3) add components such as Development and Interpreter. When a downstream user of vcpkg python is both embedding and using python as part of their build process eg by Additionally, specifying the vcpkg artifacts is tricky with these finders, especially when using a multi-config generator (eg Visual Studio). A common complaint from our users is that the debug library is not properly found, leading to either surprise link errors with python38_d.lib not being found or with FindPython deciding to use some other non-vcpkg python. Overall, this leads to an unacceptable level of surprises and GOTCHAS, IMO. This pull request fixes that by building the python interpreter and ensuring artifacts are properly specified for all five cmake find python variants: FindPythonLibs, FindPythonInterp, FindPython, FindPython2, and FindPython3. In the case of embedding, having python without its standard library is largely pointless. In fact, in Python 3, python cannot be correctly embedded without the standard library. It imports the I built a project to test what I expect to work on Windows, macOS, and Linux here. (I disabled Windows CI in that repository in favor of local testing.) Several ports fall into multiple use cases in that they use python3 in their build process and either embed or extend it. In those cases, we find fragile artifact specifications for the python development libraries in the portfile.cmake and a call to download the python interpreter. With a properly functioning python.exe built by vcpkg, these use cases can be simplified fairly significantly... Unless the upstream library rolls its own python finder (eg pybind11). If, on the other hand, the port only needs python as part of its build process, use case 1 remains unaffected. I did address one issue related to case 3 while working on the PR. Python is ABI stable at the minor release level (eg, 3.8.3 is compatible with 3.8.5 but not with 3.9.0) unless using the |
This allows overriding the name of the install target, allowing specification of something other than `make install`, eg `make altinstall`.
- Many deprecated functions were removed. - A working python interpreter is now installed under tools. - The import library is now properly frozen into the interpreter.
This allows us to prevent CMake going to the registry or otherwise exploding when using: - FindPython - FindPython3 - FindPythonInterp - FindPythonLibs
The upstream port supports python3 now, but a new release is not yet available.
It seems pybind11 has its own python find module. The current upstream head allows using CMake's new FindPython module with the definition `-DPYBIND11_FINDPYTHON`, but there has been no upstream release with this feature. In the meantime, we correctly find the vcpkg python3 interpreter and libraries.
This fixes an observed build failure on x86-windows when the vcpkg expat port is installed before the python3 port is built.
This has been rebased to merge cleanly with master. |
blocked by #13722. |
I am unable to reproduce the CI failures with irrilicht and qt5-location locally. |
blocked by #13765. |
osg regression will be fixed in #13942. |
After re-reading the comment by @ras0219-msft, I have reworked some of the commits in this pull request to properly support use case 1 by not assuming that the python interpreter build by vcpkg may be used on the host machine. |
Before I take the time to resolve the merge conflicts, would it help the review process if this were split into multiple pull requests? It's been a little over a month since any substantial input was given. |
@Hoikas yes, that's better. |
This pull request improves the downstream experience when working with the python3 port by building a functioning interpreter with support for pip and improving support when cmake's FindPython3 finder is used. Previously, when using FindPython3, if both the
DEVELOPMENT
andINTERPRETER
components were requested, you would have to have a local python install matching the vcpkg python. Further, on Windows, this module preferred registry python over any slightly misconfigured python artifacts. Fixing these issues allows us to simplify ports whose cmake build systems need the python3 development libraries.Anyone who intends to do further work on the python3 port after this might consider fixing
vcpkg_find_acquire_program(PYTHON3)
to use the interpreter installed by this port.Note that on Windows a patch is applied in static python interpreters to disable extension modules-due to the way DLLs function, it is not possible to load them without crashing the interpreter. Further, support for the static CRT has been removed both to avoid surprises in that regard and due to the nature of
PyObject
s.What does your PR fix?
This should address [python3] cmake FindPython3 Development component #9026 and [python3] Install python executable from port #10903.
Which triplets are supported/not supported? Have you updated the CI baseline?
Support for x64-windows-static has been dropped.
Does your PR follow the maintainer guide?
I think so 😄