pypa / cibuildwheel

🎡 Build Python wheels for all the platforms with minimal configuration.
https://cibuildwheel.pypa.io
Other
1.78k stars 227 forks source link

Issues building wheels for MacOS M2 Pro #1896

Open mgrover1 opened 6 days ago

mgrover1 commented 6 days ago

Description

After a recent release of the Python ARM Radar Toolkit (Py-ART), users are having issues accessing the arm-64 wheel, seeing claims of the following: https://github.com/ARM-DOE/pyart/issues/1602#issuecomment-2188946360

The CI passed upon building the wheels, and the related conda-forge build passed https://github.com/conda-forge/arm_pyart-feedstock/pull/95

We are seeing the error:

ERROR: arm-pyart has an invalid wheel, arm-pyart has an invalid wheel, could not read 'arm_pyart-1.18.5.dist-info/WHEEL' file: BadZipFile("Bad CRC-32 for file 'arm_pyart-1.18.5.dist-info/WHEEL'")

See https://github.com/ARM-DOE/pyart/issues/1602

Build log

https://github.com/yt-project/yt/actions/runs/9659702514/job/26644579993

CI config

https://github.com/ARM-DOE/pyart/blob/main/.github/workflows/wheels.yml

henryiii commented 5 days ago

I see https://github.com/ARM-DOE/pyart/blob/a54e9547138b1c5dc5d80c36dcab0c479bac32f1/.github/workflows/wheels.yml#L95-L100

Are you building the same wheel in multiple jobs? This looks like GHA bug https://github.com/actions/download-artifact/issues/298

henryiii commented 5 days ago

Yes, you are; please remove this line:

https://github.com/ARM-DOE/pyart/blob/a54e9547138b1c5dc5d80c36dcab0c479bac32f1/.github/workflows/wheels.yml#L55

Then you'll only build Intel on Intel and Apple Silicon on Apple Silicon. Currently you are building both on both and GHA corrupts the files if you download multiple artifacts with the same name.

mgrover1 commented 5 days ago

Ahhh that makes sense. Thank you!

henryiii commented 5 days ago

Also, this will break you when you start building for 3.13:

https://github.com/ARM-DOE/pyart/blob/a54e9547138b1c5dc5d80c36dcab0c479bac32f1/.github/workflows/wheels.yml#L56C11-L56C35

3.13 requires 10.13+. cibuildwheel will do this for you if you don't force 10.9.

I'd make this auto64, since GHA is adding ARM runners: https://github.com/ARM-DOE/pyart/blob/a54e9547138b1c5dc5d80c36dcab0c479bac32f1/.github/workflows/wheels.yml#L54

And I'd also get rid of this, it's not true anymore, NumPy does ship musllinux wheels: https://github.com/ARM-DOE/pyart/blob/a54e9547138b1c5dc5d80c36dcab0c479bac32f1/.github/workflows/wheels.yml#L53

And finally, I'd move most of it to pyproject.toml so you can also run it locally and/or switch CI's more easily, or even just make a new job more easily. Also you don't need the whole list https://github.com/ARM-DOE/pyart/blob/a54e9547138b1c5dc5d80c36dcab0c479bac32f1/.github/workflows/wheels.yml#L52 if you set requires-python somewhere reasonably easy to read (pyproject.toml or setup.cfg, for example).

mgrover1 commented 5 days ago

Sounds good - we will make these changes as well!

We appreciate your insight and help here!!

henryiii commented 5 days ago

No problem, close if/when it works!

mgrover1 commented 5 days ago

All good now! The wheels built properly and have been tested. Thanks again for your help here @henryiii