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

Add support for windows arm64 #1037

Merged
merged 3 commits into from
Aug 17, 2024
Merged

Conversation

Nagico2
Copy link
Contributor

@Nagico2 Nagico2 commented Jul 26, 2024

Why to modify

Recently, I attempted to install sentencepiece using pip on Windows on ARM, but due to python/setup.py not taking into account arm64 in the judgment of Windows arch, cmake mistakenly built the lib for the AMD64 architecture.

cmake_arch = 'Win32'
if arch == 'amd64':
cmake_arch = 'x64'
subprocess.check_call([
'cmake',
'sentencepiece',
'-A',
cmake_arch,
'-B',
'build',
'-DSPM_ENABLE_SHARED=OFF',
'-DCMAKE_INSTALL_PREFIX=build\\root',
])

What has been modified

1. For building from source or pip install from source

I added a judgment for arm64 through platform.machine(), without modifying the judgment statements for x86 and x64, to ensure that it does not have any side effects on the original architecture building.

At the same time, I have added a section in the Python build documentation that explains how to build from source code on Windows for the convenience of Windows users.

2. For building wheels for win-arm64 automatically

I've also modified the .github/workflows/wheel.yml to build wheels for win-arm64 automatically. But there's a bug in cibuildwheel. pypa/cibuildwheel#1942 (comment)

Simply put, for the win arm64 arch in cibuildwheel, platform.machine() will retrieve AMD64 instead of ARM64. This will cause issues with the judgment of arch in setup.py, resulting in selecting the wrong lib path during build. So I add a special check for win-arm under ciwheelbuild using env PYTHON_ARCH.

Here's the Github Actions building results on my own repo. https://github.com/Nagico2/sentencepiece/actions/runs/10105917616

The outcomes

Now windows on arm users can build and install python wrapper from source. And after the modification of python/setup.py have been published to PyPI, windows on arm users can use pip install sentencepiece to build and install.

After the wheels for win-arm published to PyPI, indows on arm users can use pip install sentencepiece to install the pre-built wheels.

Others

I've also tried to add win-arm build in cmake.yml. But there's another bug in setup-python (actions/setup-python#915). So I haven't made any modifications to this yet.


Modifying the build code may lead to serious consequences, please conduct comprehensive and full testing.

Thank you all for building the Windows on ARM ecosystem. :)

Copy link

google-cla bot commented Jul 26, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@taku910 taku910 merged commit e963f99 into google:master Aug 17, 2024
16 of 22 checks passed
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.

2 participants