Skip to content
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

Fix finding headers when cross compiling #145

Merged
merged 7 commits into from Jun 7, 2022

Conversation

lopsided98
Copy link
Contributor

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:

  • Apply this patch to either stdlib distutils (before setuptools 60) or the bundled distutils in setuptools.
  • Add the directory containing the _sysconfigdata module for the host platform Python installation to PYTHONPATH, so that it can be imported from build platform Python.
  • Set _PYTHON_HOST_PLATFORM to the correct value for the host platform (e.g. linux-armv7l)
  • Set _PYTHON_SYSCONFIGDATA_NAME to the appropriate value for the host platform (e.g. _sysconfigdata__linux_arm-linux-gnueabihf).

setup.py is executed using build platform Python (by necessity, since the build machine can't normally run host platform binaries), but the sysconfig module 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, since get_python_inc() returns paths relative to sys.base_prefix or sys.base_exec_prefix. This causes problems when the build platform word length doesn't match the host platform, resulting in errors like the following:

In file included from /nix/store/01kia41csjia67pry1rv828i9pvnnqfq-python3-3.9.12/include/python3.9/Python.h:50,
                 from btrfsutilpy.h:27,
                 from error.c:20:
/nix/store/01kia41csjia67pry1rv828i9pvnnqfq-python3-3.9.12/include/python3.9/pyport.h:741:2: error: #error "LONG_BIT definition appears wrong for platform (bad gcc/glibc config?)."
  741 | #error "LONG_BIT definition appears wrong for platform (bad gcc/glibc config?)."
      |  ^~~~~

This patch avoids this problem by relying on sysconfig to provide the include paths. With _PYTHON_SYSCONFIGDATA_NAME set 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_NAME can affect the return value of get_python_inc(). IMO, the most important thing is ensuring that existing (non-cross) use cases are not affected by this change.

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.
distutils/sysconfig.py Outdated Show resolved Hide resolved
@jaraco
Copy link
Member

jaraco commented Jun 7, 2022

I note that get_python_inc is unique to distutils.sysconfig. There's no equivalent in stdlib:sysconfig. So at least there's no issue there with alignment.

@jaraco jaraco merged commit 75ed79d into pypa:main Jun 7, 2022
@lopsided98 lopsided98 deleted the python-inc-cross branch June 9, 2022 21:57
@lopsided98
Copy link
Contributor Author

Thanks!

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Aug 21, 2022

I can't believe this is broken again: the fix didn't even live up to the next release. 😞
b05a823 is bad, the cross compilation fails exactly in the same way as before.

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Aug 21, 2022

I can fix it with this change

diff --git a/distutils/sysconfig.py b/distutils/sysconfig.py
index aae9c1b..4b72204 100644
--- a/distutils/sysconfig.py
+++ b/distutils/sysconfig.py
@@ -165,9 +165,8 @@ def _get_python_inc_from_config(plat_specific, spec_prefix):
     platform Python installation, while the current Python
     executable is from the build platform installation.
     """
-    if not spec_prefix:
-        return
-    return get_config_var('CONF' * plat_specific + 'INCLUDEPY')
+    if spec_prefix is None:
+        return get_config_var('CONF' * plat_specific + 'INCLUDEPY')
 
 
 def _get_python_inc_posix_prefix(prefix):

Cross and non-cross compilation work, but I don't know if this has unintended consequences.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants