WIP: Use CMake's own FindPythonLibs instead of vendored FindPythonLibsNew.

[codypiersall]

This enables easy cross-compilation and pybind11 doesn't have to carry
around an old version of FindPythonLibs.

[codypiersall]

I think this PR could use some feedback -- I am not a CMake expert, and I worry the law of Chesterton's Fence might apply here. I'm sure there is a reason that the FindPythonLibsNew file was added, but I'm not sure what it is.

I tested that this enables cross compilation using an SDK generated with the Yocto project and still allows native compilation, but I haven't vetted this thoroughly yet.

[codypiersall]

One thing to note is that this would likely bump the minimum CMake version required. I have only tested it on CMake 3.10.3.

This addresses #2139 directly, and is the fix proposed in that issue. Can you think of drawbacks @ahesford?

Also related to #1718 and #1330

[wjakob]

Simply getting rid of the old approach to find Python definitely will not fly, since this library must continue to function on legacy systems with old versions of CMake. What might be an acceptable compromise is to decide on a CMake version where we deem FindPythonLibs sufficiently non-broken to automatically switch over the CMake-provided script.

[codypiersall]

Makes sense -- I should be able to udpate this PR soon. If CMake version 3.10 or greater, I'll use the system FindPythonLibs, and fall back to FindPythonLibsNew on older CMake.

[codypiersall]

I force-pushed the change--now on newer CMake versions the version of PythonLibs shipped with CMake will be used, and the version of PythonLibsNew has not been touched.

[wjakob]

Rather than just trying on one version and using that, think this will need a bit more investigative work on your part (or somebody else who is willing to help) -- understanding what was broken in the old version of the CMake-supplied scripts, and when it was fixed.

[codypiersall]

Sounds like it's time to do a little spelunking for me. Unfortunately the original commit message that adds the version to this repository is

Include a more robust FindPythonLibs module for CMake

I'll go through CMake's source too to see if I can figure out why this was done, and what about it was more robust.

[codypiersall]

From a bit of surfing the net, it looks like this is a pretty common library to be included in CMake-based projects. For example, it's in pyarrow, a random repo from 6 years ago, and other hits as well. Looks like something that's been cop/pasted around for a while now. Still trying to figure out where it actually came from and what problem it was solving that FindPythonLibs.cmake failed to solve.

This PR from years ago on a different project gives the only solid info I have found:

*The reason for FindPythonLibsNew.cmake is that cmake's builtin FindPythonLibs pretty much never did what I wanted, and usually found the wrong libraries, had trouble finding them, etc. This version gets the libraries for the python interpreter it found instead of doing an independent search, so you control which Python you want by configuring FindPythonInterp, and then FindPythonLibsNew will always set the libraries for that particular python. This might make cross-compiling slightly more difficult, and it doesn't handle 100% of cases with debug python builds, but it works way better for me.

(Emphasis mine)

The PR has a dead link to the original.

@dean0x7d, if you've got the time, do you remember why the original FindPythonLibsNew.cmake was added? I saw that you were the one who originally added it.

[codypiersall]

Also just want to point out that the version I used in this PR as it is right now has been deprecated since 3.12

[ahesford]

Honestly, I don't know the right path here. I also am not a CMake expert. The FindPython{,2,3} modules suggested as a replacement do not seem to accomplish what's going on in the custom module as far as discovering properties of the Python interpreter. It's not clear to me how to define some of these properties without either hard-cording them for each target system or executing the target Python interpreter at build time.

I'm coming at this from the perspective of a Void Linux packaging, where packages can be cross-compiled for any supported architecture using the xbps-src build system. All official packages for all architectures are built on x86 host systems, with 32-bit packages being built from an i686 system image and 64-bit packages being built from an x86_64 system. The value of PYTHON_SIZEOF_VOID_P should be correct in these instances, but the shared-object extension is guaranteed to be wrong for non-native packages.

Hard-coding the values is probably feasible (but undesirable) for supported architectures in Void, but I wonder if it's worthwhile. The CMake modules provide some convenience to dependent projects, but is otherwise expendible. I'll probably sidestep this issue by forgoing CMake modules in the package.

[codypiersall]

@ahesford thanks for the comment. I think one important thing I was missing was that the Python architecture does not have to match the host architecture. When I say "host" here, I mean in terms of cross-compilation, so the host is not the machine doing the build necessariliy, but the machine where the executables will run. In my case my build machine is x86-64 but the host is 32-bit ARM.

It's very common in Windows to run a 32-bit Python executable in a 64-bit OS, and I suspect that reasons like that is why FindPythonLibsNew was made, possibly? I'm really not sure. And I'm unable to test it at the moment.

---

Another strategy possibly worth considering is to only call FindPythonLibs instead of FindPythonLibsNew when we're cross-compiling, because in that situation FindPythonLibsNew can't possibly work correctly without doing some workarounds.

[ahesford]

It doesn't look like FindPythonLibs offers everything FindPythonLibsNew defines, but the now-preferred FindPython seems to offer most things with different names. I think one of the biggest things missing would be PYTHON_SIZEOF_VOID_P, which is used to trigger a fatal error when bitness of the Python runtime and C++ compiler disagree. It's not clear to me that this check is critical at the CMake level. It's convenient to know during build whether there is a discrepancy that will produce broken extensions, but the failure mode is so obvious that any reasonable testing regimen would catch this issue.

It seems to me a reasonable course of action would be:

1. Use FindPython where possible as the preferred default,
2. Modify FindPythonLibsNew to:

• Stop trying to discover variables that are not offered by FindPython, and
• Set variable names that agree with those in FindPython

This makes FindPythonLibsNew nothing but a fallback for old CMake versions that can't support proper built-in discovery.

If there is a lot of momentum behind the existing variable names in FindPythonLibsNew, those names could be retained and a thin wrapper developed to prefer FindPython where possible but map its variables to those set by FindPythonLibsNew. (It's not clear how the "momentum" would be gauged, since their use depends on individual dependants.) I still think the pointer-size validator has to go to make cross-compilation work.

[codypiersall]

@ahesford,

ack your last comment.

The biggest problem with FindPythonLibsNew is that the way it finds out about the Python interpreter is by running it, which it certainly won't be able to do in a cross-compilation environment. Regardless of anything else we do, when cross-compiling we can't do the check that is being done here:

pybind11/tools/FindPythonLibsNew.cmake

Lines 77 to 92 in a54eab9

	execute_process(COMMAND "${PYTHON_EXECUTABLE}" "-c"
	"from distutils import sysconfig as s;import sys;import struct;
	print('.'.join(str(v) for v in sys.version_info));
	print(sys.prefix);
	print(s.get_python_inc(plat_specific=True));
	print(s.get_python_lib(plat_specific=True));
	print(s.get_config_var('SO'));
	print(hasattr(sys, 'gettotalrefcount')+0);
	print(struct.calcsize('@P'));
	print(s.get_config_var('LDVERSION') or s.get_config_var('VERSION'));
	print(s.get_config_var('LIBDIR') or '');
	print(s.get_config_var('MULTIARCH') or '');
	"
	RESULT_VARIABLE _PYTHON_SUCCESS
	OUTPUT_VARIABLE _PYTHON_VALUES
	ERROR_VARIABLE _PYTHON_ERROR_VALUE)

Your suggestions makes sense to me.

[codypiersall]

Also want to mention that my PR is still totally broken still for the cross-compile case: it allows for pybind11 itself to build, but actually building modules fails still. i.e. from the pybind repository, running mkdir build && cd build && cmake -DPYBIND_TEST=OFF works, but integrating pybind into a cross-compilation project still fails. Change upcoming for that...

[codypiersall]

My naive implementation here keeps failing in new and exciting ways; added WIP to the title to reflect this.

[wjakob]

One thing to keep in mind is that cross-compilation is an incredibly fringe use case. Running the Python interpreter to query it for its properties is the thing that one should do in 99.9% of cases. When this project was started, FindPythonLibs didn't do that, causing a huge amount of pain, eventually motivating our custom solution here. I am not sure what it does now. We should be very careful to adapt any cmake-provided feature if it relies on other more fragile ways of finding the Python interpreter and associated include paths. Another point is that any changes to this component must continue to work for Python 2.7.

[codypiersall]

I wouldn't call cross-compilation "incredibly fringe". For embedded systems it's very normal, and "embedded" systems include dual core 1GHz ARM chips these days :-).

But I definitely take your point that it is much less common, and that native compilation must continue working as well as it does today for everyone, and that if anything done in this PR messes up that the PR will not be accepted.

Running the Python interpreter to query it for its properties is the thing that one should do in 99.9% of cases

Ack. I need to dive in to CMake's source and figure out how they do things and see what the mismatch is.

[codypiersall]

I won't be able to look into much for a couple weeks (in a sprint at work) but here's a list of stuff that I (or someone) need to do still:

1. Determine exactly in what scenarios the implementation of FindPythonLibsNew works better than the Cmake-provided FindPythonLibs or FindPython. e.g. find a scenario where FindPythonLibs fails and FindPythonLibsNew does The Right Thing™. I suspect this may have to do with things like automatically finding out if the Python being linked against is a debug build; this probably cannot be done simply without actually running the Python executable.
2. Consider using FindPython instead of FindPythonLibs. FindPythonLibs has been deprecated since 3.12: https://cmake.org/cmake/help/v3.12/module/FindPythonLibs.html. Or maybe just pick the appropriate one based on the running CMake version.
3. Ensure that the variables that need to get set are set correctly. In particular, this PR fails to (at least) set PYTHON_MODULE_EXTENSION, which causes extensions to be built without exensions.
4. This PR still calls FindPythonLibsNew unconditionally in pybind11Config.cmake.in: https://github.com/pybind/pybind11/blob/master/tools/pybind11Config.cmake.in#L86. I overlooked that initially.
5. Possibly consider using FindPythonLibs instead of FindPythonLibsNew only in the case of cross-compilation.

Number 1. on that list is probably the most important.

---

I'm using pybind11 in a Yocto-based project, and I wrote a Bitbake recipe that solves these problems for my use case, but it is not robust to be included upstream—for example, it hardcodes ".so" as the PYTHON_MODULE_EXTENSION

[codypiersall]

I'm still interested in this in theory, but unfortunately I'm unable to work on this any time soon, and rather than just leaving it open I'm going to close it. I've solved it locally for the project I'm working on (patched pybind11 in a bitbake recipe for my Yocto setup), but as I mentioned it's not a robust fix.

Thanks everyone who put time and energy into this issue!

[ahesford]

For now, I'm packaging pybind11 for Void without the cmake modules. That gets around the cross-compilation issue for our use case, at the expense of breaking any downstream packages that might assume the pybind11 modules exist. For now, the only dependent in the Void repo is pyopencl, which I also maintain and builds just fine without cmake stuff.

If somebody ever comes complaining to me about missing cmake modules, I'll come poking around here to see if the issue has been revived or resolved. In the meantime, this doesn't seem to be a major blocker for anything I'm doing.