ci: Make wheel building workflow reusable

[matthewfeickert]

Resolves #3014

Seperate out the wheel building from .github/workflows/deploy-cpp.yml into its own workflow in .github/workflows/build-wheels.yml that uses the workflow_call trigger. This allows for .github/workflows/deploy-cpp.yml to be able to simply call it with

  build-wheels:
    uses: ./.github/workflows/build-wheels.yml

but also makes the wheel building independent so that it can run on a schedule to build nightly wheels for .github/workflows/upload-nightly-wheels.yml to upload.

Given that .github/workflows/deploy.yml builds a pure-Python sdist and wheel there isn't a strong motivating factor to bother with finding the sdist and wheel built in the deploy-cpp stage that will have been run just before it, and so it is left alone to simply rebuild everything.

To match the existing search patterns used in

awkward/.github/workflows/upload_nightly_wheels.yml

Line 34 in b7eb8b9

ARTIFACT_PATTERN="awkward*wheel*" # awkward-wheel and awkward-cpp-wheels-*

.github/workflows/build-wheels.yml also uses patterns of awkward-cpp-wheels-* and awkward-wheel as artifact names.

I developed this on my fork before pushing it here (with some cleanup of names), so here's an example of this workflow running there:

Instead of actually uploading I had a ls -lhtra dist/ which gave

$ ls -lhtra dist/
total 36M
drwxr-xr-x 3 runner docker 4.0K Feb  9 20:50 ..
-rw-r--r-- 1 runner docker 643K Feb  9 20:50 awkward_cpp-29-cp39-cp39-manylinux_2_17_aarch64.manylinux2014_aarch64.whl
-rw-r--r-- 1 runner docker 643K Feb  9 20:50 awkward_cpp-29-cp38-cp38-manylinux_2_17_aarch64.manylinux2014_aarch64.whl
-rw-r--r-- 1 runner docker 643K Feb  9 20:50 awkward_cpp-29-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl
-rw-r--r-- 1 runner docker 690K Feb  9 20:50 awkward_cpp-29-pp310-pypy310_pp73-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
-rw-r--r-- 1 runner docker 487K Feb  9 20:50 awkward_cpp-29-cp38-cp38-win32.whl
-rw-r--r-- 1 runner docker 525K Feb  9 20:50 awkward_cpp-29-cp310-cp310-win_amd64.whl
-rw-r--r-- 1 runner docker 643K Feb  9 20:50 awkward_cpp-29-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl
-rw-r--r-- 1 runner docker 690K Feb  9 20:50 awkward_cpp-29-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
-rw-r--r-- 1 runner docker 640K Feb  9 20:50 awkward_cpp-29-cp310-cp310-macosx_10_9_x86_64.whl
-rw-r--r-- 1 runner docker 642K Feb  9 20:50 awkward_cpp-29-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl
-rw-r--r-- 1 runner docker 641K Feb  9 20:50 awkward_cpp-29-pp310-pypy310_pp73-macosx_10_9_x86_64.whl
-rw-r--r-- 1 runner docker 487K Feb  9 20:50 awkward_cpp-29-cp39-cp39-win32.whl
-rw-r--r-- 1 runner docker 526K Feb  9 20:50 awkward_cpp-29-cp311-cp311-win_amd64.whl
-rw-r--r-- 1 runner docker 690K Feb  9 20:50 awkward_cpp-29-pp38-pypy38_pp73-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
-rw-r--r-- 1 runner docker 1.2M Feb  9 20:50 awkward_cpp-29-cp310-cp310-macosx_10_9_universal2.whl
-rw-r--r-- 1 runner docker 642K Feb  9 20:50 awkward_cpp-29-cp311-cp311-macosx_10_9_x86_64.whl
-rw-r--r-- 1 runner docker 641K Feb  9 20:50 awkward_cpp-29-pp38-pypy38_pp73-macosx_10_9_x86_64.whl
-rw-r--r-- 1 runner docker 1.2M Feb  9 20:50 awkward_cpp-29-cp39-cp39-musllinux_1_1_aarch64.whl
-rw-r--r-- 1 runner docker 1.2M Feb  9 20:50 awkward_cpp-29-cp38-cp38-musllinux_1_1_aarch64.whl
-rw-r--r-- 1 runner docker 1.2M Feb  9 20:50 awkward_cpp-29-cp310-cp310-musllinux_1_1_aarch64.whl
-rw-r--r-- 1 runner docker 526K Feb  9 20:50 awkward_cpp-29-cp312-cp312-win_amd64.whl
-rw-r--r-- 1 runner docker 1.6M Feb  9 20:50 awkward-cpp-29.tar.gz
-rw-r--r-- 1 runner docker 690K Feb  9 20:50 awkward_cpp-29-pp39-pypy39_pp73-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
-rw-r--r-- 1 runner docker 1.2M Feb  9 20:50 awkward_cpp-29-cp311-cp311-musllinux_1_1_aarch64.whl
-rw-r--r-- 1 runner docker 1.2M Feb  9 20:50 awkward_cpp-29-cp310-cp310-musllinux_1_1_x86_64.whl
-rw-r--r-- 1 runner docker 1.2M Feb  9 20:50 awkward_cpp-29-cp312-cp312-musllinux_1_1_aarch64.whl
-rw-r--r-- 1 runner docker 644K Feb  9 20:50 awkward_cpp-29-cp312-cp312-macosx_10_9_x86_64.whl
-rw-r--r-- 1 runner docker 641K Feb  9 20:50 awkward_cpp-29-pp39-pypy39_pp73-macosx_10_9_x86_64.whl
-rw-r--r-- 1 runner docker 525K Feb  9 20:50 awkward_cpp-29-cp38-cp38-win_amd64.whl
-rw-r--r-- 1 runner docker 1.2M Feb  9 20:50 awkward_cpp-29-cp311-cp311-macosx_10_9_universal2.whl
-rw-r--r-- 1 runner docker 640K Feb  9 20:50 awkward_cpp-29-cp38-cp38-macosx_10_9_x86_64.whl
-rw-r--r-- 1 runner docker 521K Feb  9 20:50 awkward_cpp-29-cp39-cp39-win_amd64.whl
-rw-r--r-- 1 runner docker 691K Feb  9 20:50 awkward_cpp-29-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
-rw-r--r-- 1 runner docker 640K Feb  9 20:50 awkward_cpp-29-cp39-cp39-macosx_10_9_x86_64.whl
-rw-r--r-- 1 runner docker 1.2M Feb  9 20:50 awkward_cpp-29-cp312-cp312-macosx_10_9_universal2.whl
-rw-r--r-- 1 runner docker 1.2M Feb  9 20:50 awkward_cpp-29-cp311-cp311-musllinux_1_1_x86_64.whl
-rw-r--r-- 1 runner docker 692K Feb  9 20:50 awkward_cpp-29-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
-rw-r--r-- 1 runner docker 1.2M Feb  9 20:50 awkward_cpp-29-cp38-cp38-macosx_10_9_universal2.whl
-rw-r--r-- 1 runner docker 1.2M Feb  9 20:50 awkward_cpp-29-cp312-cp312-musllinux_1_1_x86_64.whl
-rw-r--r-- 1 runner docker 1.2M Feb  9 20:50 awkward_cpp-29-cp39-cp39-macosx_10_9_universal2.whl
-rw-r--r-- 1 runner docker 690K Feb  9 20:50 awkward_cpp-29-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
-rw-r--r-- 1 runner docker 1.2M Feb  9 20:50 awkward_cpp-29-cp38-cp38-musllinux_1_1_x86_64.whl
-rw-r--r-- 1 runner docker 690K Feb  9 20:50 awkward_cpp-29-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
drwxr-xr-x 2 runner docker 4.0K Feb  9 20:50 .
-rw-r--r-- 1 runner docker 1.2M Feb  9 20:50 awkward_cpp-29-cp39-cp39-musllinux_1_1_x86_64.whl

As the .github/workflows/build-wheels.yml now exists, then the CRON job added to .github/workflows/packaging-test.yml in PR #3012

awkward/.github/workflows/packaging-test.yml

Lines 5 to 7 in b7eb8b9

	# Run daily at 1:23 UTC
	schedule:
	- cron: '23 1 * * *'

can be reverted

[matthewfeickert]

@jpivarski @agoose77 I'd like to clean up the commit history on this a bit after this get reviewed, so once we're happy with the state of this I'll do some interactive rebasing to make thing logically easier to read later when looking at the commit messages squash.

[matthewfeickert]

These are just comments to help guide the reviewer on actions I took.

[jpivarski]

I'm confused. I would have thought that having the building and publishing in the same workflow would be better because the publishing step would only run if the building is successful. As two workflows, the building step might fail and the publishing step would start anyway. There could also be a race condition between them. (I don't see them both having cron-job times, though...)

What's the overall picture here? What gets tested, built, published, and deployed, and when?

(Going through the set of possible Actions, I already have to distinguish between Tests and Tests,

because a different YAML file supplies a workflow of the same name. One of them hasn't run in months; that's how I can tell them apart...)

[matthewfeickert]

I'm confused. I would have thought that having the building and publishing in the same workflow would be better because the publishing step would only run if the building is successful. As two workflows, the building step might fail and the publishing step would start anyway.

Ah, sorry no. Look at the graph screenshot that I've included in the PR body. Nothing about the publishing workflow has changed. You can see that the publishing step still requires the wheels to build successfully. The only thing different is that this logic now lives in another file and can be called indepdently of publishing.

What's the overall picture here? What gets tested, built, published, and deployed, and when?

• With this PR all the awkward-cpp wheels can now be built on demand at anytime and on a nightly basis via CRON job. The wheels are uploaded as GitHub Action artifacts.
• These GitHub Action artifacts from the nightly cron job are queried and downloaded by the nightly wheels workflow and then uploaded to Anaconda Cloud.
• The wheel building workflow is now a step that gets called inside of the publish-cpp workflow, reusing the exact same logic (as if it was copied and pasted into the workflow).

because a different YAML file supplies a workflow of the same name. One of them hasn't run in months; that's how I can tell them apart...)

You can select the old one and delete it.

[jpivarski]

Oh, is one YAML file acting as an include file for the other? (I didn't know you could do that.)

(I also looked for a "delete workflow" on Actions and couldn't find it. But now I know that it exists and will look further into GitHub documentation.)

[matthewfeickert]

Oh, is one YAML file acting as an include file for the other? (I didn't know you could do that.)

Yup. That's what

  build-wheels:
    uses: ./.github/workflows/build-wheels.yml

or

  build-wheels:
    uses: scikit-hep/awkward/.github/workflows/build-wheels.yml@main

does. This is covered in the GitHub Actions docs here: https://docs.github.com/en/actions/using-workflows/reusing-workflows#using-inputs-and-secrets-in-a-reusable-workflow

(I think it is less than a year old at this point. Maybe a bit more?)

(I also looked for a "delete workflow" on Actions and couldn't find it. But now I know that it exists and will look further into GitHub documentation.)

Ah yeah you have to do it manually for each run until they eventually clean them up. :(

[matthewfeickert]

(This rebase was just to clean up the PR into more cohrerent commits that should make the logic easier to review in chunks. Nothing was changed from when I last pushed.)

[agoose77]

@matthewfeickert thanks for authoring this!

Do you have any feelings on the motivations to use a re-usable workflow vs just adding a new job that is opt-in? Our deploy-cpp workflow now only has a single step that is skipped unless the workflow_dispatch input parameter is set.

As I see it, there are two ways of working here:

1. Use separate workflows, such that deploying the C++ is a different workflow to building the wheels
2. Use a workflow input parameter to chose between build and build + deploy

Right now we have the latter, and I'm not sure whether I'm missing something RE splitting the workflow (is it just preference)?

[matthewfeickert]

Do you have any feelings on the motivations to use a re-usable workflow vs just adding a new job that is opt-in?

@agoose77 In my mind the advantage is simple (and here I mean simple in a (good) boring way), but two-fold:

1. Avoid very verbose duplication of YAML. You only need to maintain one ~200 line file instead of two that are nearly the same.
2. You're testing with the same wheel building workflow that you deploy with, so you again don't have to maintain seperate files and if the wheels are building as expected in the nightly (or whatever frequency) CI then you should have a high confidence that you're not going to get hit at wheel building time with some strange bug that has crept into the GitHub Actions your workflow depends on since the last release.

(This screenshot from the PR body is of a (test)deploy workflow that is using the wheel building workflow as the first 2 columns of the workflow run

so those first 2 columns get rigourously tested with use and the only deployment step logic is simply the publishing with the PyPI publish action.)

[agoose77]

@matthewfeickert I'm a little under the weather, so this might be going straight over my head.

AFAICT we don't need duplicate logic; the singular build-wheels workflow could have all of cron, release, and workflow_dispatch triggers, with a "push to pypi" job that only runs on release or workflow_dispatch input. Am I missing something?

[matthewfeickert]

AFAICT we don't need duplicate logic; the singular build-wheels workflow could have all of cron, release, and workflow_dispatch triggers, with a "push to pypi" job that only runs on release or workflow_dispatch input. Am I missing something?

Hm, let me try to walk through this as I'm also pretty low on sleep and so might be missing the obvious and making things more complex. This is going to be long and maybe just makes your point, so bear with me. Though I think this will mainly come down to if you want a long and complicated single workflow file, or if you want shorter workflow files that you compose workflows with (both are fine for me btw). Also as I'm low on sleep if things don't make sense let me know and I'll revise it.

I think the main thing that differs here is that the deployment of awkward to PyPI first has a validation step that awkward-cpp wheels are already up on PyPI.

awkward/.github/workflows/deploy.yml

Lines 45 to 64 in 26e5e65

	check-cpp-on-pypi:
	name: "Check awkward-cpp dependency on PyPI"
	runs-on: ubuntu-latest
	needs: [determine-source-date-epoch]
	env:
	SOURCE_DATE_EPOCH: ${{ needs.determine-source-date-epoch.outputs.source-date-epoch }}
	steps:
	- uses: actions/checkout@v4
	with:
	submodules: true
	- name: Prepare build files
	run: pipx run nox -s prepare
	- name: Build awkward-cpp sdist
	run: pipx run build --sdist awkward-cpp
	- name: Check sdist matches PyPI
	run: pipx run nox -s check_cpp_sdist_released -- awkward-cpp/dist/awkward-cpp*.tar.gz

Deploying awkward-cpp to PyPI and then having a job that needs that job to have finished before starting these checks before deployment of awkward could be done in a single workflow, but the current deploy.yml is already decently long and you'd need to have similar logic. I assume as this is something that has been kept separate in the past that is a design choice that would want to be kept(?).

I think the other differences here that might make more sense for separate workflows is that the current deploy-cpp.yml workflow only builds and uploads the awkward-cpp sdist and wheels, where the build-wheels.yml workflow this PR introduces builds sdists and wheels for both awkward-cpp and awkward and uploads both to GitHub as workflow artifacts.

The upload-nightly-wheels.yml workflow queries for these upload artifacts and then downloads both awkward and awkward-cpp wheels (and only the wheels) to upload both to the scikit-hep-nightly-wheels Anaconda Cloud org

awkward/.github/workflows/upload_nightly_wheels.yml

Line 34 in 26e5e65

ARTIFACT_PATTERN="awkward*wheel*" # awkward-wheel and awkward-cpp-wheels-*

The main takeaway here is that this works really nicely if all of the wheels were uploaded from the same GitHub Actions workflow, and would require multiple queries if it came from multiple uploads.

The current deploy-cpp.yml workflow is only building the awkward-cpp sdist and wheels so that it can have those get up on PyPI and pass the validation step before anything awkward release related gets kicked off. This could all be done as conditions in the build-wheels.yml workflow, but that would require adding all the validation step logic of deploy.yml as jobs into build-wheels.yml. That feels a bit large to me, but if that seems much more comprehensible to you then I'm fine with changing it so that the Awkward team can maintain all components of the workflow easier.

2. Use a workflow input parameter to chose between build and build + deploy

Right now we have the latter, and I'm not sure whether I'm missing something RE splitting the workflow (is it just preference)?

Okay, yeah the longer I think about this, I think it does just come down to preference on workflow atomicity.

[matthewfeickert]

@agoose77 @jpivarski While this isn't critical (aka, gentle nudge that you should ignore for other pressing work), can we revisit this PR this week?

[jpivarski]

This is my (partial) understanding of what's happening here:

So, three workflows all want to build wheels, deploy-cpp.yml, packaging-test.yml, and upload-nightly-wheels.yml, and previously, upload-nightly-wheels.yml called packaging-test.yml to do that.

Now you've introduced build-wheels.yml, which isn't intended to be run by itself; it's a step that both deploy-cpp.yml and upload-nightly-wheels.yml use. This moved a lot of YAML code out of deploy-cpp.yml and into build-wheels.yml. I don't know why packaging-test.yml didn't have to change (apart from no longer needing a cron schedule).

In the end, is it true that the YAML files that are used by other YAML files don't get run directly? In other words, is the current arrangement maintaining a separation between libraries that are called by applications and applications that are directly run?

[matthewfeickert]

So, three workflows all want to build wheels, deploy-cpp.yml, packaging-test.yml, and upload-nightly-wheels.yml, and previously, upload-nightly-wheels.yml called packaging-test.yml to do that.

Only one workflows wants to build all the wheels: deploy-cpp.yml. packaging-test.yml intentionally only builds a subset for speed (I assume?). upload-nightly-wheels.yml is designed to download a target collection of wheels from other workflows to then upload to a target package index.

Now you've introduced build-wheels.yml, which isn't intended to be run by itself; it's a step that both deploy-cpp.yml and upload-nightly-wheels.yml use.

No, it is designed to run by itself to do the wheel building

on:
  # Run daily at 1:23 UTC
  schedule:
    - cron: '23 1 * * *'
  # Run on demand with workflow dispatch
  workflow_dispatch:
...

but it can also be called by other workflows

...
  # Use from other workflows
  workflow_call:

upload-nightly-wheels.yml doesn't trigger any build jobs. It downloads the build artifacts from the last build job run on main which is scheduled to happen in advance of it running.

This moved a lot of YAML code out of deploy-cpp.yml and into build-wheels.yml.

Correct. This is to avoid duplicating the wheel building logic across multiple files or to avoid making the logic of the run conditions on one file very complicated. Having wheel building be totally factored out into a seperate workflow from deployment also means that any other workflow can safely depend on it and building wheels can be done mentally cheaply without ever having to worry about if the correct environment settings have been selected or not (i.e. removes concerns about accidental deployments — though to be fair that is already quite low given how the current deployment system is setup).

I don't know why packaging-test.yml didn't have to change (apart from no longer needing a cron schedule).

As Awkward didn't have a standalone job that would build all the wheels (it currently only builds them on deployment) packaging-test.yml was standing in for the nightly upload as a job that would build something until a PR like this one could build all the wheels. c.f. Issue #3014.

In the end, is it true that the YAML files that are used by other YAML files don't get run directly? In other words, is the current arrangement maintaining a separation between libraries that are called by applications and applications that are directly run?

No. What

...
  # Use from other workflows
  workflow_call:

is doing is allowing other workflows to basically directly insert the workflow into the calling workflow, which is how the .github/workflows/deploy-cpp.yml demo run presents as a single task graph in #3016 (comment).

(c.f. https://docs.github.com/en/actions/using-workflows/reusing-workflows#calling-a-reusable-workflow)

I'm not sure if this helps, but think of this as reusing steps in a workflow language project.

[matthewfeickert]

Self note:

• This PR needs to be rebased once PR ci: Update scientific-python/upload-nightly-action from 0.3.0 to 0.4.0 #3031 and ci: Upload nightly wheels to scientific-python-nightly-wheels org #3032 goes in.

[matthewfeickert]

@jpivarski @agoose77 gentle ping on this so that the full suite of awkard-cpp and awkward nightly wheels can start getting uploaded to https://anaconda.org/scientific-python-nightly-wheels .

[jpivarski]

This refactoring is a good one. Thanks!

[agoose77]

@matthewfeickert thanks for pushing this through, and thanks @jpivarski for merging. It's highly appreciated, and a good change ❤️

[matthewfeickert]

Thanks for your reviews @agoose77 and @jpivarski! If there are questions in the future I am more than happy to help address them or to help fix anything. 👍