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

Macos wheel generated/tagged for wrong macos version #1804

Open
LecrisUT opened this issue Apr 11, 2024 · 4 comments
Open

Macos wheel generated/tagged for wrong macos version #1804

LecrisUT opened this issue Apr 11, 2024 · 4 comments

Comments

@LecrisUT
Copy link

Description

For example on macOS-13 runner it generated:

5 wheels produced in 8 minutes:
  spglib-2.4.0-cp310-cp310-macosx_10_9_x86_64.whl   771 kB
  spglib-2.4.0-cp311-cp311-macosx_10_9_x86_64.whl   771 kB
  spglib-2.4.0-cp312-cp312-macosx_10_9_x86_64.whl   771 kB
  spglib-2.4.0-cp38-cp38-macosx_10_9_x86_64.whl     771 kB
  spglib-2.4.0-cp39-cp39-macosx_10_9_x86_64.whl     771 kB
GH Workflow
jobs:
  build_wheels:
    name: 🖥️ ${{ matrix.os }}
    runs-on: ${{ matrix.os }}
    strategy:
      fail-fast: false
      matrix:
        os: [ ubuntu-latest, windows-latest, macOS-11, macOS-13, macOS-14 ]

    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0
        # CIBW requires emulation to build for arm linux
        # https://cibuildwheel.readthedocs.io/en/stable/faq/#emulation
      - name: Set up QEMU for Linux-arm
        if: runner.os == 'Linux'
        uses: docker/setup-qemu-action@v2
        with:
          platforms: all
      - name: Build wheels
        uses: pypa/cibuildwheel@v2.17
        env:
          CIBW_BUILD: ${{ inputs.cibw_build }}
          CIBW_ARCHS_LINUX: auto aarch64
      - uses: actions/upload-artifact@v4
        with:
          name: cibw-wheels-${{ matrix.os }}-${{ strategy.job-index }}
          path: ./wheelhouse/*.whl

Build log

https://github.com/spglib/spglib/actions/runs/8647336672/job/23708827446

CI config

https://github.com/spglib/spglib/blob/e6bdd0bafcb60ff26c7ce9ff95d13c43e56d995f/pyproject.toml#L97-L111

@henryiii
Copy link
Contributor

henryiii commented Apr 11, 2024

This is correct. If you don't set MACOSX_DEPLOYMENT_VERSION it will match CPython; we carefully download and use the official installers which target 10.9. (This is more complex on ARM, which is 11 when targeting ARM, 10.9 for CPython 3.9+ targeting Intel, and 11.0 again for CPython 3.8 targeting Intel because they built that one as pure 11.0).

@LecrisUT
Copy link
Author

So then we don't need to run the macos builder for multiple versions? It would still be ABI compatible on both C and C++ side? I've had an issue where the results were different for macos-11/13 and macos-12, granted it was on numerical error issue, but it was constantly reproducible with failing and passing results on those machines.

@henryiii
Copy link
Contributor

Yes, Apple designed this so that you can target any older macOS version from a newer one, to encourage everyone to upgrade and not keep older macOS around (I guess). You set a target version, and they provide guards around any non-header-only functionality. If you want to use C++17, for example, you usually need to set 10.12 (partial support) - 10.14 (full support), unless of course you use only templated, header only additions. The official CPython builds are compiled targeting 10.9, so that's the earliest you can target.

Were the tests failing in cibuildwheel, or in normal testing? Your normal tests are being built with the default value probably and are probably not using the official CPython installers either. We set MACOSX_DEPLOYMENT_VERSION if it's not set already to match our CPython installers.

@LecrisUT
Copy link
Author

cibuildwheel, or in normal testing

Both, I first noticed in cibuildwheel runner, but than I replicated in the GH hosted runner as well

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

No branches or pull requests

2 participants