Fix finding headers when cross compiling#145
Merged
Conversation
When cross-compiling third-party extensions, get_python_inc() may be called to return the path to Python's headers. However, it uses the sys.prefix or sys.exec_prefix of the build Python, which returns paths pointing to build system headers when instead we really want the host system headers. To fix this, we use the INCLUDEPY and CONFINCLUDEPY conf variables, which can be configured to point at host Python by setting _PYTHON_SYSCONFIGDATA_NAME. The existing behavior is maintained on non-POSIX platforms or if a prefix is manually specified.
mweinelt
reviewed
Jun 4, 2022
Member
|
I note that |
… treatment diverges even more.
…Use value algebra to simplify the logic.
Contributor
Author
|
Thanks! |
Contributor
|
I can't believe this is broken again: the fix didn't even live up to the next release. 😞 |
Contributor
|
I can fix it with this change Cross and non-cross compilation work, but I don't know if this has unintended consequences. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a resubmission of #35, rebased and reworked a bit. I've included the same notes I posted in #35 describing the use case this patch enables:
We use the following techniques to make cross-compilation work:
_sysconfigdatamodule for the host platform Python installation toPYTHONPATH, so that it can be imported from build platform Python._PYTHON_HOST_PLATFORMto the correct value for the host platform (e.g.linux-armv7l)_PYTHON_SYSCONFIGDATA_NAMEto the appropriate value for the host platform (e.g._sysconfigdata__linux_arm-linux-gnueabihf).setup.pyis executed using build platform Python (by necessity, since the build machine can't normally run host platform binaries), but thesysconfigmodule returns data from the host platform Python installation, due to_PYTHON_SYSCONFIGDATA_NAME.If the package contains extension modules, eventually
distutils.sysconfig.get_python_inc()will be called to get the location of the Python headers. Without this patch, the build platform headers will be returned, sinceget_python_inc()returns paths relative tosys.base_prefixorsys.base_exec_prefix. This causes problems when the build platform word length doesn't match the host platform, resulting in errors like the following:This patch avoids this problem by relying on sysconfig to provide the include paths. With
_PYTHON_SYSCONFIGDATA_NAMEset appropriately, the include directories will be returned from host platform Python.The latest version of the patch will always use sysconfig on posix if the relevant config variables are defined, otherwise it will fall back to the current behavior.
I have been having trouble coming up with an effective and simple automated test due to the amount of infrastructure required. I think you would need multiple Python installations in order to demonstrate how
_PYTHON_SYSCONFIGDATA_NAMEcan affect the return value ofget_python_inc(). IMO, the most important thing is ensuring that existing (non-cross) use cases are not affected by this change.