Since CMake version 3.12, CMake has included improved detection for python installations for builds. Updating and using this capability allows the custom find-python Linux script to be deprecated/removed. In theory this also allows pulling the package find into the main CMakeLists.txt, but due to the way the libdir is discovered/set it currently isn't able to find the bundled dependency.
In addition, by updating to 3.18 and using separate components for embed vs module, it simplifies and improves the process of building blender as a python wheel.
Details
Details
Diff Detail
Diff Detail
Event Timeline
Comment Actions
Unfortunately, I don't currently have a way to test the windows and macos changes (especially the macos ones related to the framework).
Comment Actions
With the use of scikit-build and this patch, building blender as a wheel becomes rather trivial. Note that while you can build/publish wheels without the use of this patch, it's more manual, and scikit-build fails altogether because the process attempts to link for embedding even when building as a module.
Comment Actions
Thanks for looking into this, although as Python is one of the more complicated libraries to detect (with spesific version requirements and sub-packages - numpy, requests... etc), I'd want to be sure this change isn't too disruptive for people building Blender.
My main concern with this change is FindPython3 doesn't provide a way for us to specify a minimum version, or a spesific version which wasn't released when CMake's FindPython module was written.
- If the system Python is old/unsupported, an older Python will be found and we wont be able to build.
- If we depend on a recent version of Python, developers using older CMake versions wont be able to detect Python (even if they have it built locally).
Currently our own find module supports setting the root and the version so you can easily test new releases of Python (even beta / unrelated versions) with a command like this:
cmake -UPYTHON -DPYTHON_VERSION=3.11 -DPYTHON_ROOT_DIR=/opt/py311 ../blender
.. which un-defines all Python variables, sets the root and version which all other variables are based on. I couldn't see a way to do this using CMake's finder.
It may be some limitations are acceptable, when testing a newer Python than CMake has support for - explicitly setting the paths should be possible.
Other notes:
- Bumping to 3.18 seems OK (recent Ubuntu / Debian stable releases use this version or newer).
- Older systems will most likely use the pre-built Python version, although we would still want to support pointing to their own Python version either built manually or via build_files/build_environment/install_deps.sh
- While it's generally good to use CMake's Find modules, our own is currently working quite well so there isn't much motivation to switch away from it, interested in what platform maintainers think about this.
Comment Actions
given it makes changes to my platform adding myself, given the mac codepath is changed as well adding the mac maintainer as well
| build_files/cmake/platform/platform_win32.cmake | ||
|---|---|---|
| 465 | find_package is optional on windows, by default hardcoded paths are used (we could look into getting away from this, this diff is not the place to do so) windows_find_package is to be used here and the old hardcoded codepath restored. you can cheat off WITH_IMAGE_TIFF and WITH_IMAGE_WEBP sections | |
Comment Actions
Bumping CMake requirement to 3.18 is rather disruptive for both our infra and people who are building Blender on the VFX platform compatible machine: CentOS 7 only has CMake 3.17. Such bump is definitely not something what should be sneaked into a "lets use CMake's find_package module" change.
Comment Actions
In that case it's still worth checking if we would include this once CMake 3.18 is supported on all platforms, or - if we really wanted we could bundle the finder for older CMake versions.
Comment Actions
This link seems dead:
https://github.com/Clockwork-Muse/bpy/tree/scikit-build
It's not clear to me exactly what changes between Development and Development.Module, maybe that's easy to add to our own FindPythonLibsUnix.cmake rather than making a big change like this.
| build_files/build_environment/patches/cmakelists_tbb.txt | ||
|---|---|---|
| 1 | Don't change this patch, we don't build TBB with Python support and the patch will be removed anyway the next time we upgrade TBB. | |
| build_files/cmake/platform/platform_apple.cmake | ||
| 120–124 | To match the previous logic this would need to be: if(WITH_PYTHON_MODULE OR WITH_PYTHON_FRAMEWORK) set(Python3_FIND_FRAMEWORK ONLY) else() set(Python3_FIND_FRAMEWORK NEVER) endif() | |
Comment Actions
My main concern with this change is FindPython3 doesn't provide a way for us to specify a minimum version, or a spesific version which wasn't released when CMake's FindPython module was written.
- If the system Python is old/unsupported, an older Python will be found and we wont be able to build.
... what? Of course you can specify a version number: find_package(Python3 ${PYTHON_VERSION} REQUIRED ${PYTHON_COMPONENTS})
Note that it searches the relevant ${LIBDIR} paths first (as well?), so system python may not even be considered.
Currently our own find module supports setting the root and the version so you can easily test new releases of Python (even beta / unrelated versions) with a command like this: `cmake -UPYTHON -DPYTHON_VERSION=3.11 -DPYTHON_ROOT_DIR=/opt/py311 ../blender` .. which un-defines all Python variables, sets the root and version which all other variables are based on. I couldn't see a way to do this using CMake's finder.
I specifically included workarounds in the build process to retain this behavior (only for linux, since that appears to be the only build that supported it).
The documentation for FindPython/FindPython3 specifically call out ways to supply hints for this. Are you thinking of the old FindPythonLibs?
Comment Actions
Sorry, was previously set to private.
It's not clear to me exactly what changes between Development and Development.Module, maybe that's easy to add to our own FindPythonLibsUnix.cmake rather than making a big change like this.
What/whether it's linked. For building distributable python wheels you can normally only use Development.Module, or the compiler tries to link against <libpython>.so/<libpython>.a/etc, making the wheel not work with multiple python versions/multiple different OSes (meaning CentOS vs Ubuntu, not Linux vs Windows).
If we were going to try to reproduce this behavior for WITH_PYTHON_MODULE you'd have to do it for each platform, Linux/Windows/Mac, not just for the Linux/Unix find.
Comment Actions
Ah, sorry. I've been doing this on manylinux images, which have 3.22+. That would be a valid concern.
I'm not seeing CMake listed on the VFX reference platform page, though, only libraries. yum install cmake on a CentOS 7 docker image yields 2.8.12. It looks like the EPEL repository (and some third-parties) has 3.17 (that's the highest version I'm seeing), is that what you're thinking of?
Comment Actions
My mistake, reading the version number handling in /usr/share/cmake/Modules/FindPython/Support.cmake made it seem like the known versions would be used.
I specifically included workarounds in the build process to retain this behavior (only for linux, since that appears to be the only build that supported it).
The documentation for FindPython/FindPython3 specifically call out ways to supply hints for this. Are you thinking of the old FindPythonLibs?
In this case the change seems less disruptive than I thought.
CMake's finder draws a distinction between major versions in a way that isn't so important for Blender. If/when Python release v4, Blender's finder should be able to upgrade to that without much trouble, CMake's looks like it's a bit more work to support.
We could keep the non-versioned names PYTHON_EXECUTABLE, PYTHON_LIBRARIES etc.. so a major version bump isn't as inconvenient to test while v3 is in use.
| build_files/cmake/platform/platform_unix.cmake | ||
|---|---|---|
| 109–113 | Numpy dependency should check: if((WITH_PYTHON_INSTALL AND WITH_PYTHON_INSTALL_NUMPY) OR WITH_PYTHON_NUMPY) | |
Comment Actions
If this is a concern, I can switch everything from FindPython3 to FindPython, and so result variables are of the form Python_ (instead of Python3_). Currently it searches both 2 and 3, but presumably it would be updated (or would as-is?) handle searching for any potential Python 4.
I debated making this change, but didn't initially consider a new version of Python to be terribly likely....
Comment Actions
If we pass in the version number then it shouldn't look for Python2.x right? A developer could set PYTHON_VERSION to 2.x but this isn't likely and we raise an error for unsupported Python versions already.
Currently it searches both 2 and 3, but presumably it would be updated (or would as-is?) handle searching for any potential Python 4.
I debated making this change, but didn't initially consider a new version of Python to be terribly likely....
While it's debatable, I'd prefer not to include the major version in names as it doesn't have significant down-sides (as far as I can see) - while it does complicate migrating to a new major version.
Comment Actions
If it's just a matter of leaving PYTHON_LIBRARIES empty for WITH_PYTHON_MODULE, that seems simple even if it's done separately for every platform. macOS is doing this already.
It's generally better to use native CMake modules, but this is looking like it has so many side effects it might not be worth it now.
Comment Actions
FYI An apple-only change was proposed in D10372: macOS: use find_package for finding Python