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

[CI] Push circt python wheel to pypi nightly #5412

Merged
merged 2 commits into from
Jun 16, 2023

Conversation

rsetaluri
Copy link
Contributor

No description provided.

@rsetaluri rsetaluri force-pushed the ci-add-nightly-circt-python-wheel branch from afed1d7 to a7b372c Compare June 15, 2023 19:33
with:
path: ./wheelhouse/*.whl
retention-days: 7

- name: Upload wheels (Tag)
- name: Upload wheels (Non-Tag and Nightly)
uses: pypa/gh-action-pypi-publish@release/v1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mikeurbach @teqdruid how do we version this to be made into a dev release on pypi?

Copy link
Contributor

Choose a reason for hiding this comment

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

If the versioning works like the pycde versioning scheme, breaking it out isn't necessary. The versioning lib automatically detects the most recent tag in the commit's history. If the commit is the tag, it uses that. If not, it appends .dev<numberOfCommitsSinceTag>.

Copy link
Contributor

Choose a reason for hiding this comment

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

So you really just need the schedule addition above. Am I correct @mikeurbach ?

Copy link
Contributor

Choose a reason for hiding this comment

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

We do use setuptools_scm, so the versioning should work as expected. But I think it makes sense to have three steps:

  • one for when there is a tag (which is run on the release event trigger) and uploads the tag version to PyPI
  • one for when the event is the cron, which uploads the dev version to PyPI
  • one for anything else (useful for testing the workflow), which uploads artifacts to the job for inspection

I think it would make sense to use:

- name: Upload wheels (Tag)
   if: github.ref_type == 'tag'

- name: Upload wheels (Nightly)
   if: github.event_name == 'cron'

- name: Upload wheels (Workflow Dispatch)
   if: github.ref_type != 'tag' && github.event_name != 'cron'

What do you think @teqdruid @rsetaluri ?

Copy link
Contributor

Choose a reason for hiding this comment

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

What would be the actions? My point is, how would they differ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see, yes we can combine the steps that push to pypi, something like:

- name: Upload wheels (Tag or Nightly)
   if: github.ref_type == 'tag' || github.event_name == 'cron'

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah. I was assuming that you'd also want to push to pypi on a workflow_dispatch, but perhaps not...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think we should only push to pypi on automated builds. If someone is testing the workflow itself or something, I don't think we need to push new versions.

Copy link
Contributor Author

@rsetaluri rsetaluri Jun 16, 2023

Choose a reason for hiding this comment

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

agree with the two modes. In the case of !(tag || cron), we can just download artifacts from github actions

@rsetaluri rsetaluri merged commit 4d9c7e6 into llvm:main Jun 16, 2023
@rsetaluri rsetaluri deleted the ci-add-nightly-circt-python-wheel branch June 16, 2023 21:02
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

Successfully merging this pull request may close these issues.

3 participants