Skip to content

chore(infra): remove build-macos-x86_64 #1325

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.

Sign up for GitHub

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

Open
kevinjqliu wants to merge 2 commits into
base: main
Choose a base branch
from
Open

chore(infra): remove build-macos-x86_64 #1325

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3b412e1
split windows and mac build
kevinjqliu Oct 16, 2025
63ed1bc
windows-x86_64
kevinjqliu Oct 16, 2025
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
24 changes: 9 additions & 15 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,14 @@ jobs:
name: python-wheel-license
path: LICENSE.txt

build-python-mac-win:
build-windows:
Copy link
Copy Markdown
Contributor

@milenkovicm milenkovicm Oct 16, 2025

Choose a reason for hiding this comment

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

I'm not sure why you removed mac from here? is there other job which tests this python on mac

Copy link
Copy Markdown
Contributor Author

@kevinjqliu kevinjqliu Oct 22, 2025

Choose a reason for hiding this comment

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

yes, build-macos-arm64.

build-python-mac-win used to run for windows and macos. but since build-macos-arm64 already runs for macos, im chaning this to just run windows.

Copy link
Copy Markdown
Contributor

@milenkovicm milenkovicm Oct 24, 2025

Choose a reason for hiding this comment

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

build-python-mac-win builds ballista/python/ extension, on the other hand build-macos-arm64 builds core ballista/ or I'm missing something

Copy link
Copy Markdown
Contributor Author

@kevinjqliu kevinjqliu Oct 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Screenshot 2025-10-24 at 5 17 46 PM

heres a side by side diff of build-python-mac-win and build-macos-arm64 taken from latest main

datafusion-ballista/.github/workflows/build.yml

Lines 108 to 213 in 400f818

build-python-mac-win:
needs: [generate-license]
name: Mac/Win
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
python-version: ["3.10"]
os: [macos-latest, windows-latest]
steps:
- uses: actions/checkout@v5
- uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
- uses: dtolnay/rust-toolchain@stable
- run: rm LICENSE.txt
- name: Download LICENSE.txt
uses: actions/download-artifact@v4
with:
name: python-wheel-license
path: .
- name: Install Protoc
uses: arduino/setup-protoc@v3
with:
version: "27.4"
repo-token: ${{ secrets.GITHUB_TOKEN }}
- uses: astral-sh/setup-uv@v6
with:
enable-cache: true
- name: Build Python package
run: |
cd python
uv sync --dev --no-install-package ballista
uv run --no-project maturin build --release --strip
- name: List Windows wheels
if: matrix.os == 'windows-latest'
run: dir python\target\wheels\
# since the runner is dynamic shellcheck (from actionlint) can't infer this is powershell
# so we specify it explicitly
shell: powershell
- name: List Mac wheels
if: matrix.os != 'windows-latest'
run: find python/target/wheels/
- name: Archive wheels
uses: actions/upload-artifact@v4
with:
name: dist-${{ matrix.os }}
path: python/target/wheels/*
build-macos-x86_64:
needs: [generate-license]
name: Mac x86_64
runs-on: macos-latest
strategy:
fail-fast: false
matrix:
python-version: ["3.10"]
steps:
- uses: actions/checkout@v5
- uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
- uses: dtolnay/rust-toolchain@stable
- run: rm LICENSE.txt
- name: Download LICENSE.txt
uses: actions/download-artifact@v4
with:
name: python-wheel-license
path: .
- name: Install Protoc
uses: arduino/setup-protoc@v3
with:
version: "27.4"
repo-token: ${{ secrets.GITHUB_TOKEN }}
- uses: astral-sh/setup-uv@v6
with:
enable-cache: true
- name: Build Python package
run: |
cd python
uv sync --dev --no-install-package ballista
uv run --no-project maturin build --release --strip
- name: List Mac wheels
run: find python/target/wheels/
- name: Archive wheels
uses: actions/upload-artifact@v4
with:
name: dist-macos-aarch64
path: python/target/wheels/*

the only difference is which os is running. the refactor here is to get rid of the matrix.os in build-python-mac-win since build-macos-x86_64 is already running for macos-latest

needs: [generate-license]
name: Mac/Win
runs-on: ${{ matrix.os }}
name: Windows x86_64
runs-on: windows-latest
strategy:
fail-fast: false
matrix:
python-version: ["3.10"]
os: [macos-latest, windows-latest]
steps:
- uses: actions/checkout@v5

Expand Down Expand Up @@ -147,25 +146,20 @@ jobs:
uv run --no-project maturin build --release --strip

- name: List Windows wheels
if: matrix.os == 'windows-latest'
run: dir python\target\wheels\
# since the runner is dynamic shellcheck (from actionlint) can't infer this is powershell
# so we specify it explicitly
shell: powershell

- name: List Mac wheels
if: matrix.os != 'windows-latest'
run: find python/target/wheels/

- name: Archive wheels
uses: actions/upload-artifact@v4
with:
name: dist-${{ matrix.os }}
name: dist-windows-x86_64
path: python/target/wheels/*

build-macos-x86_64:
build-macos-arm64:
Copy link
Copy Markdown
Contributor

@milenkovicm milenkovicm Oct 16, 2025

Choose a reason for hiding this comment

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

this is what i was asking, as name changed after you push update to use arm64 👍🏻

needs: [generate-license]
name: Mac x86_64
name: Mac arm64
runs-on: macos-latest
strategy:
fail-fast: false
Expand Down Expand Up @@ -209,7 +203,7 @@ jobs:
- name: Archive wheels
uses: actions/upload-artifact@v4
with:
name: dist-macos-aarch64
name: dist-macos-arm64
path: python/target/wheels/*

build-manylinux-x86_64:
Expand Down Expand Up @@ -339,8 +333,8 @@ jobs:
merge-build-artifacts:
runs-on: ubuntu-latest
needs:
- build-python-mac-win
- build-macos-x86_64
- build-windows
- build-macos-arm64
- build-manylinux-x86_64
- build-manylinux-aarch64
- build-sdist
Expand Down