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.

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

Jump to bottom

Full CI support for public builds + switch to use cibuildwheel #267

Merged
merged 65 commits into from
Dec 6, 2024
Merged

Full CI support for public builds + switch to use cibuildwheel #267

merged 65 commits into from
Dec 6, 2024

Conversation

leofang
Copy link
Member

@leofang leofang commented Dec 5, 2024 •
edited
Loading

Close #227. Part of #81.

The earlier build pipeline was deficient because

  1. it only covered a single point in our large support matrix (ex: no aarch64/win-64 coverage)
  2. it used a miniforge container to build wheels, which does not guarantee compatibility with the manylinux/glibc requirement
  3. since a container is used, it is impossible to reuse many widely used/validated, OSS GHA workflows, and everything has to be homegrown scripts, which is an unnecessary burden to both developer and devops teams
    • one addition downside of this is it's really hard to understand the overall CI design, since codes are scattered into many files that are hard to follow
  4. the old CI had too many unnecessary pieces not suitable for this project (ex: CMake, Legate, ...)

This PR switches to use public GHA workflows. Most steps are meant to be easily reusable (after some tweaking/refactoring) by other Python/CUDA projects, either first-party or third-party. For example, cibuildwheels has been used to offer guarantee to manylinux. It also lays the ground work for later bring-up of the CI test stage. (This PR only covers the build stage.)

As of commit 0d554f0, we can build all needed wheels using GHA now! 🎉

  • cuda.bindings
  • cuda.core
  • Python: 3.9-3.12
  • OS: linux-64, linux-aarch64, win-64
  • CUDA 12.6 (we can evaluate backporting to the cuda-11 branch)

The artifacts are available here for inspection: https://github.com/NVIDIA/cuda-python/actions/runs/12192916020

@copy-pr-bot copy-pr-bot
Copy link

copy-pr-bot bot commented Dec 5, 2024

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@leofang leofang self-assigned this Dec 5, 2024
@leofang leofang added CI/CD CI/CD infrastructure P0 High priority - Must do! labels Dec 5, 2024
@leofang
Copy link
Member Author

leofang commented Dec 5, 2024

/ok to test

@leofang
Copy link
Member Author

leofang commented Dec 5, 2024

/ok to test

@leofang
Copy link
Member Author

leofang commented Dec 5, 2024

/ok to test

@leofang
Copy link
Member Author

leofang commented Dec 5, 2024

/ok to test

@leofang
Copy link
Member Author

leofang commented Dec 5, 2024

/ok to test

@leofang
Copy link
Member Author

leofang commented Dec 5, 2024

/ok to test

@leofang
Copy link
Member Author

leofang commented Dec 5, 2024

/ok to test

@leofang
Copy link
Member Author

leofang commented Dec 5, 2024

/ok to test

@leofang
Copy link
Member Author

leofang commented Dec 5, 2024

/ok to test

@leofang
Copy link
Member Author

leofang commented Dec 5, 2024

/ok to test

@leofang
Copy link
Member Author

leofang commented Dec 5, 2024

/ok to test

@leofang
Copy link
Member Author

leofang commented Dec 5, 2024

/ok to test

@leofang
Copy link
Member Author

leofang commented Dec 5, 2024

/ok to test

@leofang
Copy link
Member Author

leofang commented Dec 6, 2024

(updated the PR description)

@leofang leofang marked this pull request as ready for review December 6, 2024 05:19
@leofang leofang added cuda.bindings Everything related to the cuda.bindings module cuda.core Everything related to the cuda.core module labels Dec 6, 2024
@leofang leofang mentioned this pull request Dec 6, 2024
Open
leofang
leofang commented Dec 6, 2024
View reviewed changes
.github/actions/build/action.yml
PARALLEL_LEVEL=${{ env.PARALLEL_LEVEL }}
CIBW_ENVIRONMENT_WINDOWS: >
CUDA_HOME="$(cygpath -w $(realpath ./cuda_toolkit))"
# PARALLEL_LEVEL=${{ env.PARALLEL_LEVEL }}
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: This was commented out because of #271.

@leofang leofang mentioned this pull request Dec 6, 2024
Merged
leofang
leofang commented Dec 6, 2024
View reviewed changes
.github/actions/setup/action.yml
Comment on lines +32 to +39
# WAR: setup-python is not relocatable...
# see https://github.com/actions/setup-python/issues/871
- name: Set up Python ${{ inputs.python-version }}
if: ${{ startsWith(inputs.host-platform, 'linux') }}
id: setup-python
uses: actions/setup-python@v5
with:
python-version: "3.12"
Copy link
Member Author

Choose a reason for hiding this comment

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

Technically we don't need this step. However, because actions/setup-python is not relocatable (its RPATH is hard-wired, see actions/setup-python#871), on a self-hosted runner cibuildwheels cannot properly load libpython3.12. This is a WAR by forcing the same Python version installed on the host (and mounted into the container).

@ksimpson-work
Copy link
Contributor

Last comment I think: Perhaps we should squash these commits. Not hellbent on this

@leofang
Copy link
Member Author

leofang commented Dec 6, 2024

/ok to test

@leofang
Copy link
Member Author

leofang commented Dec 6, 2024

Perhaps we should squash these commits.

Yes when we merge there's a drop-down menu to select merge or squash.

@ksimpson-work ksimpson-work self-requested a review December 6, 2024 21:27
@ksimpson-work
Copy link
Contributor

ahh, that option is grayed out for me on this project

@leofang
Copy link
Member Author

leofang commented Dec 6, 2024

@ksimpson-work is it still grey after you approved it?

@ksimpson-work
Copy link
Contributor

No, I noticed it when I went to merge my own PRs. I'll take a screenshot next time, unless I was hallucinating

@leofang
Copy link
Member Author

leofang commented Dec 6, 2024

Let's merge! Thanks for quick reviews @ksimpson-work!

@leofang leofang merged commit 1c86afa into NVIDIA:main Dec 6, 2024
12 checks passed
@leofang leofang deleted the more_ci_tweak branch December 6, 2024 21:43
This was referenced Dec 7, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ksimpson-work ksimpson-work ksimpson-work approved these changes

@vzhurba01 vzhurba01 Awaiting requested review from vzhurba01

Assignees

@leofang leofang

Labels
CI/CD CI/CD infrastructure cuda.bindings Everything related to the cuda.bindings module cuda.core Everything related to the cuda.core module P0 High priority - Must do!
Projects
None yet
Milestone
cuda.core beta 2
Development

Successfully merging this pull request may close these issues.

Validate wheel build in CI/CD
2 participants
@leofang @ksimpson-work