pitrou commented on code in PR #48539: URL: https://github.com/apache/arrow/pull/48539#discussion_r2683112701
########## ci/scripts/python_wheel_windows_build.bat: ########## @@ -127,10 +198,21 @@ set PYARROW_WITH_PARQUET=%ARROW_PARQUET% set PYARROW_WITH_PARQUET_ENCRYPTION=%PARQUET_REQUIRE_ENCRYPTION% set PYARROW_WITH_SUBSTRAIT=%ARROW_SUBSTRAIT% set PYARROW_WITH_S3=%ARROW_S3% -set ARROW_HOME=C:\arrow-dist -set CMAKE_PREFIX_PATH=C:\arrow-dist +set ARROW_HOME=%ARROW_DIST% +set CMAKE_PREFIX_PATH=%ARROW_DIST% + +if "%arch%"=="ARM64" ( + set Arrow_DIR=%ARROW_DIST%\lib\cmake\arrow +) -pushd C:\arrow\python +pushd %ARROW_SRC%\python + +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 +) Review Comment: Why not install these in a dedicated step in the GitHub Actions workflow? ########## dev/tasks/python-wheels/github.windows.arm64.yml: ########## @@ -0,0 +1,73 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +{% import 'macros.jinja' as macros with context %} + +{{ macros.github_header() }} + +jobs: + build: + name: "Build wheel for Windows ARM64" + runs-on: "{{ runs_on }}" + timeout-minutes: 240 + env: + PYTHON_CMD: "py -{{ python_version }}-arm64" + CMAKE_BUILD_TYPE: release + steps: + {{ macros.github_checkout_arrow()|indent }} + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: "{{ python_version }}" + architecture: "arm64" + + - name: Cache vcpkg dependencies + 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 %}" Review Comment: Why `arrow/ci/vcpkg/arm64-windows-*.cmake`? ########## ci/scripts/python_wheel_windows_build.bat: ########## @@ -145,8 +227,14 @@ pushd C:\arrow\python for /f %%i in ('dir dist\pyarrow-*.whl /B') do (set WHEEL_NAME=%cd%\dist\%%i) || exit /B 1 echo "Wheel name: %WHEEL_NAME%" -%PYTHON_CMD% -m delvewheel repair -vv ^ - --ignore-existing --with-mangle ^ - -w repaired_wheels %WHEEL_NAME% || exit /B 1 +if "%arch%"=="ARM64" ( + %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 Review Comment: Why are the `--add-path` options needed? Can you add a comment? ########## ci/scripts/python_wheel_windows_build.bat: ########## @@ -127,10 +198,21 @@ set PYARROW_WITH_PARQUET=%ARROW_PARQUET% set PYARROW_WITH_PARQUET_ENCRYPTION=%PARQUET_REQUIRE_ENCRYPTION% set PYARROW_WITH_SUBSTRAIT=%ARROW_SUBSTRAIT% set PYARROW_WITH_S3=%ARROW_S3% -set ARROW_HOME=C:\arrow-dist -set CMAKE_PREFIX_PATH=C:\arrow-dist +set ARROW_HOME=%ARROW_DIST% +set CMAKE_PREFIX_PATH=%ARROW_DIST% + +if "%arch%"=="ARM64" ( + set Arrow_DIR=%ARROW_DIST%\lib\cmake\arrow +) Review Comment: Why? ########## ci/scripts/python_wheel_windows_build.bat: ########## @@ -24,16 +24,35 @@ py -0p %PYTHON_CMD% -m sysconfig || exit /B 1 -call "C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Auxiliary\Build\vcvars64.bat" +@REM Detect architecture if not set +if "%arch%"=="" set arch=x64 + +@REM Set architecture-specific options +if "%arch%"=="ARM64" ( + set CMAKE_PLATFORM=ARM64 Review Comment: Can you make sure `CMAKE_PLATFORM` is always set? ########## ci/scripts/python_wheel_windows_build.bat: ########## @@ -46,67 +65,119 @@ set ARROW_ORC=ON set ARROW_PARQUET=ON set PARQUET_REQUIRE_ENCRYPTION=ON set ARROW_MIMALLOC=ON -set ARROW_SUBSTRAIT=ON set ARROW_S3=ON -set ARROW_TENSORFLOW=ON -set ARROW_WITH_BROTLI=ON -set ARROW_WITH_BZ2=ON -set ARROW_WITH_LZ4=ON -set ARROW_WITH_SNAPPY=ON -set ARROW_WITH_ZLIB=ON -set ARROW_WITH_ZSTD=ON -set CMAKE_INTERPROCEDURAL_OPTIMIZATION=ON -set CMAKE_UNITY_BUILD=ON set CMAKE_GENERATOR=Visual Studio 17 2022 -set CMAKE_PLATFORM=x64 set VCPKG_ROOT=C:\vcpkg set VCPKG_FEATURE_FLAGS=-manifests -set VCPKG_TARGET_TRIPLET=amd64-windows-static-md-%CMAKE_BUILD_TYPE% + +if "%arch%"=="ARM64" ( + set ARROW_SUBSTRAIT=OFF + set ARROW_TENSORFLOW=OFF +) else ( + set ARROW_SUBSTRAIT=ON + set ARROW_TENSORFLOW=ON + set ARROW_WITH_BROTLI=ON + set ARROW_WITH_BZ2=ON + set ARROW_WITH_LZ4=ON + set ARROW_WITH_SNAPPY=ON + set ARROW_WITH_ZLIB=ON + set ARROW_WITH_ZSTD=ON + set CMAKE_INTERPROCEDURAL_OPTIMIZATION=ON + set CMAKE_UNITY_BUILD=ON +) mkdir C:\arrow-build pushd C:\arrow-build -cmake ^ - -DARROW_ACERO=%ARROW_ACERO% ^ - -DARROW_BUILD_SHARED=ON ^ - -DARROW_BUILD_STATIC=OFF ^ - -DARROW_BUILD_TESTS=OFF ^ - -DARROW_COMPUTE=ON ^ - -DARROW_CSV=ON ^ - -DARROW_CXXFLAGS="/MP" ^ - -DARROW_DATASET=%ARROW_DATASET% ^ - -DARROW_DEPENDENCY_SOURCE=VCPKG ^ - -DARROW_DEPENDENCY_USE_SHARED=OFF ^ - -DARROW_FILESYSTEM=ON ^ - -DARROW_FLIGHT=%ARROW_FLIGHT% ^ - -DARROW_GANDIVA=%ARROW_GANDIVA% ^ - -DARROW_GCS=%ARROW_GCS% ^ - -DARROW_HDFS=%ARROW_HDFS% ^ - -DARROW_JSON=ON ^ - -DARROW_MIMALLOC=%ARROW_MIMALLOC% ^ - -DARROW_ORC=%ARROW_ORC% ^ - -DARROW_PACKAGE_KIND="python-wheel-windows" ^ - -DARROW_PARQUET=%ARROW_PARQUET% ^ - -DARROW_S3=%ARROW_S3% ^ - -DARROW_SUBSTRAIT=%ARROW_SUBSTRAIT% ^ - -DARROW_TENSORFLOW=%ARROW_TENSORFLOW% ^ - -DARROW_WITH_BROTLI=%ARROW_WITH_BROTLI% ^ - -DARROW_WITH_BZ2=%ARROW_WITH_BZ2% ^ - -DARROW_WITH_LZ4=%ARROW_WITH_LZ4% ^ - -DARROW_WITH_SNAPPY=%ARROW_WITH_SNAPPY% ^ - -DARROW_WITH_ZLIB=%ARROW_WITH_ZLIB% ^ - -DARROW_WITH_ZSTD=%ARROW_WITH_ZSTD% ^ - -DCMAKE_BUILD_TYPE=%CMAKE_BUILD_TYPE% ^ - -DCMAKE_INSTALL_PREFIX=C:\arrow-dist ^ - -DCMAKE_INTERPROCEDURAL_OPTIMIZATION=%CMAKE_INTERPROCEDURAL_OPTIMIZATION% ^ - -DCMAKE_UNITY_BUILD=%CMAKE_UNITY_BUILD% ^ - -DMSVC_LINK_VERBOSE=ON ^ - -DPARQUET_REQUIRE_ENCRYPTION=%PARQUET_REQUIRE_ENCRYPTION% ^ - -DVCPKG_MANIFEST_MODE=OFF ^ - -DVCPKG_TARGET_TRIPLET=%VCPKG_TARGET_TRIPLET% ^ - -Dxsimd_SOURCE=BUNDLED ^ - -G "%CMAKE_GENERATOR%" ^ - -A "%CMAKE_PLATFORM%" ^ - C:\arrow\cpp || exit /B 1 + +if "%arch%"=="ARM64" ( Review Comment: Why the different code paths? Please let's reconcile them and make sure this scripts remains easily maintainable. ########## ci/scripts/python_wheel_windows_build.bat: ########## @@ -46,67 +65,119 @@ set ARROW_ORC=ON set ARROW_PARQUET=ON set PARQUET_REQUIRE_ENCRYPTION=ON set ARROW_MIMALLOC=ON -set ARROW_SUBSTRAIT=ON set ARROW_S3=ON -set ARROW_TENSORFLOW=ON -set ARROW_WITH_BROTLI=ON -set ARROW_WITH_BZ2=ON -set ARROW_WITH_LZ4=ON -set ARROW_WITH_SNAPPY=ON -set ARROW_WITH_ZLIB=ON -set ARROW_WITH_ZSTD=ON -set CMAKE_INTERPROCEDURAL_OPTIMIZATION=ON -set CMAKE_UNITY_BUILD=ON set CMAKE_GENERATOR=Visual Studio 17 2022 -set CMAKE_PLATFORM=x64 set VCPKG_ROOT=C:\vcpkg set VCPKG_FEATURE_FLAGS=-manifests -set VCPKG_TARGET_TRIPLET=amd64-windows-static-md-%CMAKE_BUILD_TYPE% + +if "%arch%"=="ARM64" ( + set ARROW_SUBSTRAIT=OFF + set ARROW_TENSORFLOW=OFF +) else ( + set ARROW_SUBSTRAIT=ON + set ARROW_TENSORFLOW=ON + set ARROW_WITH_BROTLI=ON Review Comment: Why wouldn't these be enabled on ARM64? ########## dev/tasks/python-wheels/github.windows.arm64.yml: ########## @@ -0,0 +1,73 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +{% import 'macros.jinja' as macros with context %} + +{{ macros.github_header() }} + +jobs: + build: + name: "Build wheel for Windows ARM64" + runs-on: "{{ runs_on }}" + timeout-minutes: 240 + env: + PYTHON_CMD: "py -{{ python_version }}-arm64" + CMAKE_BUILD_TYPE: release + steps: + {{ macros.github_checkout_arrow()|indent }} + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: "{{ python_version }}" + architecture: "arm64" + + - name: Cache vcpkg dependencies + 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 %}" + restore-keys: vcpkg-installed-windows-arm64- + + - name: Install vcpkg dependencies + shell: cmd + run: | + cd arrow\ci\vcpkg + C:\vcpkg\vcpkg.exe install ^ + --binarysource="clear;x-gha,readwrite" ^ + --x-install-root=C:\vcpkg\installed ^ + --x-manifest-root=. ^ + --x-feature=flight ^ + --x-feature=json ^ + --x-feature=gcs ^ + --x-feature=orc ^ + --x-feature=parquet ^ + --x-feature=s3 + - name: Build Arrow C++ and PyArrow wheel + shell: cmd + env: + arch: "ARM64" Review Comment: Can we keep environment variables uppercase? ########## ci/scripts/python_wheel_windows_build.bat: ########## @@ -24,16 +24,35 @@ py -0p %PYTHON_CMD% -m sysconfig || exit /B 1 -call "C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Auxiliary\Build\vcvars64.bat" +@REM Detect architecture if not set +if "%arch%"=="" set arch=x64 + +@REM Set architecture-specific options +if "%arch%"=="ARM64" ( + set CMAKE_PLATFORM=ARM64 + set VCVARS_BAT=C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsarm64.bat + set VCPKG_TARGET_TRIPLET=arm64-windows + set ARROW_SRC=%GITHUB_WORKSPACE%\arrow + set ARROW_DIST=%GITHUB_WORKSPACE%\arrow-dist +) else ( + set VCVARS_BAT=C:\Program Files ^(x86^)\Microsoft Visual Studio\2022\BuildTools\VC\Auxiliary\Build\vcvars64.bat + set VCPKG_TARGET_TRIPLET=amd64-windows-static-md-%CMAKE_BUILD_TYPE% + set ARROW_SRC=C:\arrow + set ARROW_DIST=C:\arrow-dist +) + +call "%VCVARS_BAT%" @echo on -echo "=== (%PYTHON%) Clear output directories and leftovers ===" -del /s /q C:\arrow-build -del /s /q C:\arrow-dist -del /s /q C:\arrow\python\dist -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" ( Review Comment: Why this condition? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
