Do not provide python libraries for linking if building python module.
AcceptedPublic
Actions

Authored by Stephen Imhoff (Clockwork-Muse) on May 23 2022, 2:06 AM.

Details

Reviewers

Campbell Barton (campbellbarton)
Ray Molenkamp (LazyDodo)
Sergey Sharybin (sergey)
Arnd Marijnissen (Arnd)
Sybren A. Stüvel (sybren)

Summary

When building blender as a python module, such as for inclusion in a wheel, it is not permitted to link against python libraries.
This diff does so by simply unsetting the library when building blender as a python module, instead of the more heavyweight solution of switching to the cmake FindPython module.

Campbell Barton (campbellbarton)
Ray molenkamp (LazyDodo)
Sergey Sharybin (sergey)
Arnd Marijnissen (Arnd)
Sybren A. Stüvel (sybren)

Diff Detail

Repository: rB Blender

Event Timeline

Stephen Imhoff (Clockwork-Muse) requested review of this revision.May 23 2022, 2:06 AM

Stephen Imhoff (Clockwork-Muse) created this revision.

Way out of my comfort zone, can't say i understand, if we're not allowed to link against python, how does it get the python symbols we rely on?

As in D14954 , this enables trivial wheel builds, but without much of the other baggage in that other change. It would still be good to look at accepting that (or similar) change in the future, but isn't required for my desired results.

In D15012#403925, @Ray Molenkamp (LazyDodo) wrote:

Way out of my comfort zone, can't say i understand, if we're not allowed to link against python, how does it get the python symbols we rely on?

... In all honesty, I'm not sure myself. I'm a little fuzzy on some of the more arcane sides of C++.
Python_LIBRARIES, which is the equivalent result variable from the FindPython module, is blank if Development/Development.Embed is not set (that is, only Development.Module is set). This is reinforced when you consider the manylinux wheels, which don't have any python .a/.so files to link against.
For that matter, notice I didn't mess with the macos build, where this behavior is already present.

If neither of us is sure, I'd say go find out. blender HAS to link with something to get the symbols it needs. So either this diff will cause a build error, or your premise you can't link the python binary that provides these symbols is false

In D15012#403939, @Ray Molenkamp (LazyDodo) wrote:

If neither of us is sure, I'd say go find out. blender HAS to link with something to get the symbols it needs. So either this diff will cause a build error, or your premise you can't link the python binary that provides these symbols is false

Linux module builds succeed with this change, and the linked wheel project fails to build without this change.

The official manylinux PEP explicitly mentions that libpython is explicitly disallowed for linking (and is mentioned as being mostly unnecessary).

From the look of things, Windows builds do require linking against a .lib , which would likely mean that the change is incorrect for the windows build. I haven't had a way to test that yet, though.

Campbell Barton (campbellbarton) accepted this revision.May 23 2022, 10:52 AM

This revision is now accepted and ready to land.May 23 2022, 10:52 AM

Accepting but only for the Linux side, I won't apply in case other reviewers wanted to reply.

Ray Molenkamp (LazyDodo) requested changes to this revision.May 24 2022, 1:53 AM

Ray Molenkamp (LazyDodo) added inline comments.

build_files/cmake/platform/platform_win32.cmake
513	Ok managed to test this, the result was unexpected, I expected a flat out linker error since nothing is providing the symbols we rely on, what I got was a slightly different linker error :) LINK : fatal error LNK1104: cannot open file 'python310.lib' wait....what? who asked it to link that?! turns out even though you removed the libraries here, the python headers add the implicit link dependency right back in over here however since we do not supply a search folder, it cannot find a copy of `python310.lib` and craps out with a linker error. Windows really needs to link the python libs, this part of the patch has to be reverted.