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

Allow cross-compiling for Windows ARM64 #343

Merged
merged 4 commits into from May 17, 2022
Merged

Conversation

zooba
Copy link
Contributor

@zooba zooba commented Apr 15, 2022

When cross-compiling for Windows ARM64 (the most likely scenario), the VSCMD_ARG_TGT_ARCH variable is set to arm64. In this case, we still want to grab the ARM64 binaries instead of those matching the host (build) platform.

This variable is the same one that is used inside distutils/setuptools to tell when cross-compiling, so everything else works fine. It is set by default in a Visual Studio cross-compilation environment, but can also be set manually and setuptools will pick up the right settings by itself.

This allow proper handling of cross-compilation added to setuptools but not to [deprecated] distutils.
buildlibxml.py Show resolved Hide resolved
setupinfo.py Outdated
from distutils.command.build_ext import build_ext as _build_ext
from setuptools.command.build_ext import build_ext as _build_ext
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably ok to change this since we're importing setuptools anyway. What was the reason why this was necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setuptools has an additional override in its build_ext.get_ext_filename to set the right suffix when compiling (in this case, .cp310-win_arm64.pyd) that distutils does not. Under normal circumstances, the presence of setuptools is enough to get the right command, so it's transparent, but when you explicitly import it from distutils (even with the setuptools bundled version of distutils) and subclass, then you override it.

Eventually setuptools will merge the two, and I think they consider imports directly from distutils to be deprecated (at least, I hope they do), so those will eventually stop working. But for the transition, they're keeping them working.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I think we should then import setuptools before from distutils.core import Extension, to mark the dependency. Otherwise, this would stop working if the import order in setup.py changed.

Co-authored-by: scoder <stefan_ml@behnel.de>
setupinfo.py Outdated
from distutils.command.build_ext import build_ext as _build_ext
from setuptools.command.build_ext import build_ext as _build_ext
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I think we should then import setuptools before from distutils.core import Extension, to mark the dependency. Otherwise, this would stop working if the import order in setup.py changed.

setupinfo.py Show resolved Hide resolved
setupinfo.py Outdated Show resolved Hide resolved
@zooba
Copy link
Contributor Author

zooba commented May 17, 2022

LGTM 👍 Thanks for keeping on this one

@scoder scoder merged commit dcab105 into lxml:master May 17, 2022
@scoder
Copy link
Member

scoder commented May 17, 2022

Thanks

@zooba zooba deleted the arm64-cross branch May 17, 2022 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants