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

Implement hynek/build-and-inspect-python-package in python-package-create.yml #137

Merged
merged 3 commits into from
May 30, 2024

Conversation

rmartin16
Copy link
Member

@rmartin16 rmartin16 commented May 24, 2024

Changes

  • Outsource's package building to hynek/build-and-inspect-python-package
  • It looks like Hynek is fostering some decent oversight on this workflow and has the necessary community participation to add existing and upcoming attestation and provenance metadata for packages

Implementations

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@rmartin16 rmartin16 force-pushed the hynek-build branch 13 times, most recently from 0739c7b to b86cec9 Compare May 26, 2024 17:10
@rmartin16 rmartin16 changed the title Experiment with hynek/build-and-inspect-python-package Implement hynek/build-and-inspect-python-package in python-package-create.yml May 26, 2024
@rmartin16
Copy link
Member Author

rmartin16 commented May 26, 2024

This change replaces the current package building with hynek/build-and-inspect-python-package; it is quite similar to the current process:

  • python -m build to create the sdist and wheel
  • python -m twine check verification
  • check-wheel-contents verification (new)
  • GitHub attestation (new)

So, along with our current checks, we gain check-wheel-contents verification and attestation. See existing examples of attestation in practice at structlog or pytest-mock.

I'm also expecting this Action to add support for the upcoming support within PyPI for declaring provenance....which we should be able to get more or less for free.

Along with beeware/briefcase#1839 and beeware/beeware#371, I'll need to update the repos below to properly use this new version of python-package-create.yml as well as update their tox.ini to run check-wheel-contents for tox -e package.

  • beeware
  • briefcase
  • gbulb
  • rubicon-objc
  • toga
  • toga-chart
  • travertino

I'll pause here, though, to ensure there aren't general objections to using Hynek's GitHub Action for this purpose.

@rmartin16
Copy link
Member Author

Also, here's an example of the attestation working for BeeWare: https://github.com/beeware/.github/attestations

This PR (as well as any PR from a fork) cannot create attestations; so, I created these initial ones using #138. This means we can't enable attestations across the board; instead, the release.yml workflows in each repo will need to pass a flag to ci.yml to tell python-package-create.yml to create an attestation.

@freakboy3742
Copy link
Member

tl;dr - no objection to adopting this. As you've described - Hynek is well known in the Python ecosystem, and he sweats the little details when it comes to issues like CI and packaging; so if we can leverage his work, all the better.

@rmartin16 rmartin16 force-pushed the hynek-build branch 8 times, most recently from 190d584 to ab3504b Compare May 27, 2024 18:19
@rmartin16 rmartin16 force-pushed the hynek-build branch 18 times, most recently from a570e8b to f9dbf70 Compare May 28, 2024 21:27
@rmartin16
Copy link
Member Author

rmartin16 commented May 28, 2024

I created #139 to introduce a GitHub Action to run install_requirement.py in arbitrary repos' CI. I'll rebase this once that is merged.

@freakboy3742
Copy link
Member

I think I could go either way; this is a bit similar to pre-commit insofar as CI doesn't use Tox for pre-commit either....although, pre-commit in Tox is a definitely more useful than packaging is. I think the likelihood for significant divergence is probably low....and the workflow will be useful if packaging issues arise....although, running the commands manually isn't a big deal. So...I'm fine getting rid of it if you see the balance going more that way.,

I think the big difference is that pre-commit is something that everyone needs to run, but packaging is something that, ideally, nobody should be running. Manual packaging is a definite edge case; if we end up in that hole, I'd suggest we'd be better served re-running the workflow and trashing a release than trying to fix it manually.

So - since we're doing all this re-org across all the projects, my inclination is to trash the tox configs while we're at it.

  1. Now that install_requirement.py is available as a resource in .github, do we still need the copy in the briefcase repo? Can we rationalise that into the single version in .github?

This is something I've been wanting to reconcile. The primary issue is Briefcase is using this script in tox.ini.

Ah - I forgot that tox is using this one as well.

maybe by turning beeware/.github into an installable Python package....

... I mean ... install_requirements is a pretty well constrained tool... it wouldn't be the worst thing to turn into a standalone package...

@rmartin16
Copy link
Member Author

All right...

  • removed install_requirement.py from beeware/briefcase
  • deferred creating an installable package for install_requirement.py but that might be reasonable later...
  • removed tox -e package from the repos

Now that install_requirement.py is a GitHub Action, it probably makes sense to update pre-commit-run.yml and towncrier-run.yml to use it. Of course, though, that will require another round of updates for all repos using them.

@rmartin16
Copy link
Member Author

Now that install_requirement.py is a GitHub Action, it probably makes sense to update pre-commit-run.yml and towncrier-run.yml to use it. Of course, though, that will require another round of updates for all repos using them.

I looked at bit at these today; while they may benefit from implementing the install-requirement action, they will need to maintain the existing functionality since many repos don't have a pyproject.toml.

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

👍

I presume the process from here is that this needs to be landed, then one last CI pass on all the downstream repos for confirmation, then land those repos?

@rmartin16
Copy link
Member Author

I presume the process from here is that this needs to be landed, then one last CI pass on all the downstream repos for confirmation, then land those repos?

Yep; this one first and then the others. It may be sufficient to just re-run the failed CI run....but with how GitHub caches, I may need to re-push the commit.

@freakboy3742 freakboy3742 merged commit 04c7038 into beeware:main May 30, 2024
64 checks passed
@rmartin16 rmartin16 deleted the hynek-build branch May 30, 2024 01:53
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.

2 participants