GH-47195: [Python][CI] Add support for building PyArrow library on Windows ARM64 #48539
Conversation
MugundanMCW
requested review from
assignUser,
jonkeane,
kou and
raulcd
as code owners
amoeba
changed the title
|
|
github-actions
bot
added
awaiting changes
github-actions
bot
added
awaiting change review
|
@github-actions crossbow submit wheel-windows-*arm64 |
|
Revision: bc3020d Submitted crossbow builds: ursacomputing/crossbow @ actions-15d135b85a
|
Can you open report the issue on the xsimd issue tracker? Better yet if you can help them fix the issue :)
Is there a reason to use a separate script? It seems quite similar to the one for x86 Windows wheels. |
|
Hi @pitrou
|
Yes, please do so. |
|
Hi @pitrou |
|
Happy new year! Looking forward to see it merged 😊 |
|
|
||
| @REM Set architecture-specific options | ||
| if "%arch%"=="ARM64" ( | ||
| set CMAKE_PLATFORM=ARM64 |
There was a problem hiding this comment.
Can you make sure CMAKE_PLATFORM is always set?
There was a problem hiding this comment.
I have modified CMAKE_PLATFORM to set for both x64 and ARM64 as per the build configurations.
| del /s /q C:\arrow\python\build | ||
| del /s /q C:\arrow\python\pyarrow\*.so | ||
| del /s /q C:\arrow\python\pyarrow\*.so.* | ||
| if "%arch%"=="x64" ( |
There was a problem hiding this comment.
Previously, this condition was needed because .so files are not generated on Windows ARM64, so these cleanup steps would fail there. However, I have now made this block common for both ARM64 and x64 since running these commands does not cause any issues on ARM64 either.
| ) else ( | ||
| set ARROW_SUBSTRAIT=ON | ||
| set ARROW_TENSORFLOW=ON | ||
| set ARROW_WITH_BROTLI=ON |
There was a problem hiding this comment.
Why wouldn't these be enabled on ARM64?
There was a problem hiding this comment.
Initially, these options are disabled to produce a minimal PyArrow build on Windows ARM64. The current patch enables all features except xsimd support.
| -A "%CMAKE_PLATFORM%" ^ | ||
| C:\arrow\cpp || exit /B 1 | ||
|
|
||
| if "%arch%"=="ARM64" ( |
There was a problem hiding this comment.
Why the different code paths? Please let's reconcile them and make sure this scripts remains easily maintainable.
There was a problem hiding this comment.
@pitrou I’ve updated the source to share common code paths between x64 and ARM64.
| if "%arch%"=="ARM64" ( | ||
| set Arrow_DIR=%ARROW_DIST%\lib\cmake\arrow | ||
| ) |
There was a problem hiding this comment.
This condition has been removed and Arrow_DIR env variable is not required to get the successful build.
| if "%arch%"=="ARM64" ( | ||
| @REM Install Python dependencies for Win-ARM64 | ||
| echo "=== Installing Python dependencies ===" | ||
| %PYTHON_CMD% -m pip install --upgrade pip || exit /B 1 | ||
| %PYTHON_CMD% -m pip install cython numpy setuptools_scm setuptools wheel || exit /B 1 | ||
| ) |
There was a problem hiding this comment.
Why not install these in a dedicated step in the GitHub Actions workflow?
There was a problem hiding this comment.
@pitrou I have moved these steps as part of Windows ARM64 workflow.
| %PYTHON_CMD% -m delvewheel repair -vv --add-path "%ARROW_DIST%\bin" ^ | ||
| --add-path "C:\vcpkg\installed\arm64-windows\bin" --ignore-existing ^ | ||
| --with-mangle -w repaired_wheels %WHEEL_NAME% || exit /B 1 |
There was a problem hiding this comment.
Why are the --add-path options needed? Can you add a comment?
There was a problem hiding this comment.
The --add-path options are used to help delvewheel locate and bundle native ARM64 DLL dependencies (from vcpkg and pyarrow) that are not in standard system locations.
| uses: actions/cache@v4 | ||
| with: | ||
| path: C:\vcpkg\installed | ||
| key: "{% raw %}vcpkg-installed-windows-arm64-${{ hashFiles('arrow/ci/vcpkg/vcpkg.json', 'arrow/ci/vcpkg/arm64-windows-*.cmake') }}{% endraw %}" |
There was a problem hiding this comment.
Why arrow/ci/vcpkg/arm64-windows-*.cmake?
There was a problem hiding this comment.
The line "arrow/ci/vcpkg/arm64-windows-*.cmake" is not required and it can be removed from the key.
| - name: Build Arrow C++ and PyArrow wheel | ||
| shell: cmd | ||
| env: | ||
| arch: "ARM64" |
There was a problem hiding this comment.
Can we keep environment variables uppercase?
|
@github-actions crossbow submit wheel-windows-cp314-*arm64 |
|
Revision: 28731a7 Submitted crossbow builds: ursacomputing/crossbow @ actions-8da1bbedc9
|
|
Hi @pitrou
Thanks |
4 participants
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?
Github Issue: #47195