pypa / cibuildwheel

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

Support `python-version` in github action #2001

Open guywilsonjr opened 1 week ago

guywilsonjr commented 1 week ago

Description

I don't believe there's a way to specify python-version or cpython when using the github action called from a matrix.

Currently I use something like

steps:
    - name: Build wheels
      uses: pypa/cibuildwheel@v2.20.0
      env:
         CIBW_BUILD: "*3${{fromJson('{ \"3.10\": \"10\", \"3.11\": \"11\",\"3.12\": \"12\" }')[matrix.python-version]}}*"

To achieve:

steps:
    - name: Build wheels
      uses: pypa/cibuildwheel@v2.20.0
      env:
         CIBW_BUILD: "*310*"

What I'd love to see:

steps:
    - name: Build wheels
      uses: pypa/cibuildwheel@v2.20.0
      with:
          python-version: ${{ matrix.python-version }}

Is there any way support could be added for running cibuildwheel on specific python versions?

I like to use cibuildwheel locally and have it run all the builds, however when it's split into a matrix I like to run it by python version before uploading to pypi

Build log

No response

CI config

No response

henryiii commented 1 week ago

Why can't you use CIBW_BUILD: "cp${{ matrix.python-version}}*" but set the python version to 310 instead of 3.10? Or maybe CIBW_BUILD: "cp3${{ matrix.minor-version}}*"? It seems it is much easier to control your matrix than to add a feature here; and there are a lot of other potential requests (like splitting up 32 and 64 bit builds, etc) that can be handled via the string already. Also see https://iscinumpy.dev/post/cibuildwheel-2-10-0#only.

Also, you don't need to use setup-python with cibuildwheel with setup-python, which is a possible reason someone might think they want this. ;)

guywilsonjr commented 1 week ago

Why can't you use CIBW_BUILD: "cp${{ matrix.python-version}}*" but set the python version to 310 instead of 3.10? Or maybe CIBW_BUILD: "cp3${{ matrix.minor-version}}*"? It seems it is much easier to control your matrix than to add a feature here

I'd have to define 2 variables if I want to use python-version in other workflows that use major.minor notation

henryiii commented 5 days ago

You can't share a matrix across workflows. Or even jobs. I can't think of reason to have a usage of python-version in the same job matrix as cibuildwheel, and even if there is an occasional one, I've shown ways to make it work. You can define python-version for those workflows, and build = ["cp38*", ...] for the cibuildwheel one. CIBW_BUILD is more powerful than python-version, especially now that cp313t-* and cp313-* exist.

https://github.com/scikit-hep/boost-histogram/blob/d2ac027c346999db6ea6fb03593642c85dcf3c89/.github/workflows/wheels.yml#L43-L73 is an example of a workflow using cibuildwheel. It doesn't need python-version. And just below it, https://github.com/scikit-hep/boost-histogram/blob/d2ac027c346999db6ea6fb03593642c85dcf3c89/.github/workflows/wheels.yml#L75-L126 is parametrizing over different things. This doesn't affect https://github.com/scikit-hep/boost-histogram/blob/d2ac027c346999db6ea6fb03593642c85dcf3c89/.github/workflows/tests.yml#L63 where python-version is at all, and there's not any reason or way to share those.

guywilsonjr commented 2 days ago

I can't think of reason to have a usage of python-version

Ex) A composite action that includes cibuildwheel, but has another step that uses python-version. Ex) build wheel then later use a third-party action that requires python version

I've shown ways to make it work

True.

It can be worked around and it's really just a convenience thing. If gh actions had string replacement I could simply remove the .. It's definitely more on that side where I'd like the solution to ideally lie.

Anyways I ended up figuring out a decent solution in my use case. It still uses 2 variables, but gets the job done with little effort

name: Use python-version then buildwheel
inputs:
  python-major-version:
    required: true
    description: 'Python major version '
  python-minor-version:
    required: true
    description: 'Python minor version'
runs:
  using: composite
  steps:
    - name: uses python version
      uses: reusable/action/uses/python-version@v1.0
      with:
         python-version: format('{0}.{1}', inputs.python-major-version, inputs.python-minor-version)

    - name: Build wheels
      uses: pypa/cibuildwheel@v2.21.0
      with:
        only: "cp3${{format('{0}{1}', inputs.python-major-version, inputs.python-minor-version)}}-manylinux_x86_64"
guywilsonjr commented 2 days ago

Thanks for engaging with me on this quickly. This can be closed

henryiii commented 1 day ago

FYI, your example looks almost exactly why I don't like making this too easy, you do not need to activate Python before cibuildwheel. Cibuildwheel won't use it. That's why I said:

Also, you don't need to use setup-python with cibuildwheel with setup-python, which is a possible reason someone might think they want this. ;)

You description had the correct order, though, building a wheel then activating Python, probably to do something with a wheel (other than normal cibuildwheel testing). Though you could also do this via two separate jobs matrices that communicate via upload/download artifact, which would make sense if you also want to use the wheels later, upload them, etc.

You know you can just use a scripting language like bash (or python!) in actions, though?

name: Use python-version then buildwheel
inputs:
  python-version:
    required: true
    description: 'Python version'

runs:
  using: composite
  steps:
    - name: uses python version
      uses: reusable/action/uses/python-version@v1.0
      with:
         python-version: ${{inputs.python-version}}

    - name: Compute cibuildwheel version
      shell: bash
      id: pyver
      run: echo "joinedversion=${{inputs.python-version}}" | tr -d . >> ${GITHUB_OUTPUT}

    - name: Build wheels
      uses: pypa/cibuildwheel@v2.21.0
      with:
        only: "cp${{ steps.pyver.outputs.joinedversion )}}-manylinux_x86_64"

(shell: bash writes to a file a bit easier than shell: python, so used that above)

guywilsonjr commented 1 day ago

Thanks for the nice example.

you do not need to activate Python before cibuildwheel. Cibuildwheel won't use it. That's why I said

I completely agree! My gripe was that I need the pyver step as opposed to an inline variable substitution.

It is definitely a first world problem