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

Upload dev wheels to Anaconda.org + revamp wheels publishing workflow #714

Conversation

agriyakhetarpal
Copy link
Collaborator

@agriyakhetarpal agriyakhetarpal commented Mar 12, 2024

Description

This PR introduces the following changes:

  1. It adds a deploy_anaconda job where the wheels can be uploaded to https://anaconda.org/scientific-python-nightly-wheels/PyWavelets/ using the scientific-python/upload-nightly-action GitHub Action
  2. A workflow_dispatch trigger to push the nightly wheels, and a CRON schedule that matches the one in Upload nightly wheels for PyWavelets to the Scientific Python Nightly Wheels index on Anaconda #710
  3. Replaces the cibuildwheel installation with its upstream GitHub Action, in order to get updates from Dependabot (see Keep GitHub Actions up to date with GitHub's Dependabot #708)
  4. Bumps up versions for the checkout actions and bumps the major version for download-artifact and upload-artifact
  5. Ensures that the necessary job runs or is skipped, based on the other wheel builds that may pass or fail

Footnotes

This PR is related to the changes requested on #712.

@agriyakhetarpal
Copy link
Collaborator Author

@rgommers, this is ready for your review whenever you have the time for it. I thought that the changes weren't much!

@agriyakhetarpal
Copy link
Collaborator Author

With these changes, the PyPI job will run on:

  1. Tags

and the Anaconda PyPI index job will run on:

  1. Pushes to master/v1.XX,
  2. On a schedule, same as the WASM upload job (should we alter the CRON statement to have a 5-minute gap in case too many jobs start?)
  3. Manually

Should we allow the PyPI job to be triggered manually as well?

@rgommers
Copy link
Member

Should we allow the PyPI job to be triggered manually as well?

Yes, that would be useful to do.

@rgommers rgommers added the CI Continuous integration label Mar 12, 2024
@agriyakhetarpal
Copy link
Collaborator Author

I guess we should add another input to the workflow_dispatch: event to gauge where to upload the wheels (there might be a situation where we would want to publish to just Anaconda, or just PyPI, or both).

@rgommers
Copy link
Member

I guess we should add another input to the workflow_dispatch: event to gauge where to upload the wheels (there might be a situation where we would want to publish to just Anaconda, or just PyPI, or both).

You are right. Actually, on second thought, this is a little dangerous. It should be a little hard to deploy to PyPI. Let's drop that last change. We do like 1-2 releases a year, so in case there's a problem with building from the tag, let's just make sure a maintainer fixes the problem and then moves the tag to do a release.

@agriyakhetarpal
Copy link
Collaborator Author

You are right. Actually, on second thought, this is a little dangerous. It should be a little hard to deploy to PyPI. Let's drop that last change. We do like 1-2 releases a year, so in case there's a problem with building from the tag, let's just make sure a maintainer fixes the problem and then moves the tag to do a release.

Thanks for the clarification! Yes, PyPI is permanent, so we do not want to trigger a broken release (even if the input is false by default). I have reverted the change in e8e4d86.

@rgommers
Copy link
Member

I tried this on my fork, and the uploading is broken: https://github.com/rgommers/pywt/actions/runs/8252417966. The problem is the name: field of upload-artifact, it is not specific enough. Maybe it used to work and the action got more strict.

On other projects I see that there is one wheel per zip file with the Python interpreter included in the upload name, e.g.: https://github.com/numpy/numpy/actions/runs/8238976297. I'm not sure if that is the optimal solution. Could you investigate, and then test from the master branch of your own fork?

@agriyakhetarpal
Copy link
Collaborator Author

agriyakhetarpal commented Mar 12, 2024

Yes, it used to be more lenient with v3. I checked the workflow and the reason is that we are restricting the Python versions being built using cibw_python in the matrix – this produces parallel jobs per Python version (and therefore the name of the artifact must be different and the matrix variable must be present).

I had not used the CIBW_BUILD environment variable before and therefore did not face this problem (the difference is that it would build Python 3.9–3.12 in serial, one after the other, in the same job).

I have pushed 90ec5e4 which should fix this, here's a workflow run I have triggered just now – let's see if it passes: https://github.com/agriyakhetarpal/pywt/actions/runs/8252934295

@rgommers
Copy link
Member

Looks like it doesn't like the * in the name.

@agriyakhetarpal
Copy link
Collaborator Author

agriyakhetarpal commented Mar 12, 2024

Yes, https://github.com/agriyakhetarpal/pywt/actions/runs/8253018855 works and all wheel builds plus their uploads are passing.

@agriyakhetarpal
Copy link
Collaborator Author

agriyakhetarpal commented Mar 12, 2024

Now that both cibuildwheel and meson-python correctly set MACOSX_DEPLOYMENT_TARGET, we should look to build macOS arm64 wheels and test them natively before cutting 1.6.0. Do you mind opening a separate issue and PR for that or can I make that change here?

Edit: based on our Slack conversation, I shall do this and MUSL wheels in a follow-up PR.

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

That all looks good now, so let's give it a go!

I noticed that the Linux aarch64 jobs were taking forever; gh-716 should take care of that. I'll merge that first, so that the wheel builds triggered on merging this PR will be much faster.

@rgommers rgommers added this to the v1.6.0 milestone Mar 12, 2024
@rgommers rgommers merged commit b078b7d into PyWavelets:master Mar 12, 2024
15 checks passed
@agriyakhetarpal agriyakhetarpal deleted the upload-nightly-wheels-for-all-platforms branch March 12, 2024 18:25
@rgommers
Copy link
Member

The upload to anaconda.org didn't actually work: https://github.com/PyWavelets/pywt/actions/runs/8253753639/job/22576823450. Could you have a look at that?

@rgommers
Copy link
Member

Ah never mind, you're already on it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants