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

build: Add windows-aarch64 to python binaries #19966

Merged
merged 1 commit into from
Nov 25, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions .github/workflows/release-python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,13 @@ jobs:
fail-fast: false
matrix:
package: [polars, polars-lts-cpu, polars-u64-idx]
os: [ubuntu-latest, macos-13, windows-32gb-ram]
os: [ubuntu-latest, macos-13, windows-latest, windows-arm64-16gb]
architecture: [x86-64, aarch64]
exclude:
- os: windows-32gb-ram
- os: windows-latest
Copy link
Member Author

Choose a reason for hiding this comment

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

Public windows runners now have 16GB RAM. Seems enough.

architecture: aarch64
- os: windows-arm64-16gb
architecture: x86-64
Comment on lines 93 to +101
Copy link
Member

@stinodego stinodego Nov 25, 2024

Choose a reason for hiding this comment

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

The following matrix is equivalent but a bit more readable (using include rather than exclude):

      matrix:
        package: [polars, polars-lts-cpu, polars-u64-idx]
        os: [ubuntu-latest, macos-13]
        architecture: [x86-64, aarch64]
        include:
          - os: windows-latest
            architecture: x86-64
          - os: windows-arm64-16gb
            architecture: aarch64

Copy link
Member Author

@ritchie46 ritchie46 Nov 25, 2024

Choose a reason for hiding this comment

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

Don't we need to include all variants then? (E.g. is the exclude set not smaller than the include set?)

Copy link
Member

@stinodego stinodego Nov 25, 2024

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, I think you're right. I also do need to think harder with the include set. 😆


env:
SED_INPLACE: ${{ matrix.os == 'macos-13' && '-i ''''' || '-i'}}
Expand Down Expand Up @@ -180,7 +182,15 @@ jobs:
if: matrix.architecture == 'aarch64'
id: target
run: |
TARGET=${{ matrix.os == 'macos-13' && 'aarch64-apple-darwin' || 'aarch64-unknown-linux-gnu'}}
TARGET=$(
if [[ "${{ matrix.os }}" == "macos-13" ]]; then
echo "aarch64-apple-darwin";
elif [[ "${{ matrix.os }}" == "windows-arm64-16gb" ]]; then
echo "aarch64-pc-windows-msvc";
else
echo "aarch64-unknown-linux-gnu";
fi
)
echo "target=$TARGET" >> $GITHUB_OUTPUT

- name: Set jemalloc for aarch64 Linux
Expand Down