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

Updated Linux and macOS wheels matrix variable name #7691

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

radarhere
Copy link
Member

Minor suggestion.

Renames archs in Linux and macOS

build:
name: ${{ matrix.name }}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
include:
- name: "macOS x86_64"
os: macos-latest
archs: x86_64
macosx_deployment_target: "10.10"
- name: "macOS arm64"
os: macos-latest
archs: arm64

to cibw_arch, to better match the variable name used in Windows
windows:
name: Windows ${{ matrix.arch }}
runs-on: windows-latest
strategy:
fail-fast: false
matrix:
include:
- arch: x86
cibw_arch: x86
- arch: x64
cibw_arch: AMD64

@hugovk hugovk merged commit 865a23a into python-pillow:main Jan 4, 2024
16 checks passed
@hugovk
Copy link
Member

hugovk commented Jan 4, 2024

@nulano What do you think about removing matrix.arch from the Windows job so it only has matrix.cibw_arch, and refactoring build_prepare.py to use matrix.cibw_arch values?

More effort than it's worth, or a nice cleanup?

@radarhere radarhere deleted the wheels branch January 4, 2024 12:19
@nulano
Copy link
Contributor

nulano commented Jan 4, 2024

I have considered it. It would make sense, but I didn't feel like doing it 😄.
The current arch definition matches the use by webp, so a new key would need to be added to ARCHITECTURES.

...is what I was going to write, but looking at it now, webp in fact uses a different value for arm.
It might also make sense to build webp with CMake (which doesn't need the arch at all) instead of NMake; I'll take a look.

Edit: #7693

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.

3 participants