peggyjs / peggy

Peggy: Parser generator for JavaScript
https://peggyjs.org/
MIT License
964 stars 65 forks source link

Publish from GitHub Actions #552

Closed hildjj closed 2 months ago

hildjj commented 2 months ago

The goals are:

  1. Make publishing a new release not take lots of manual steps (e.g. #461)
  2. Make it possible for someone other than hildjj to do a release
  3. Get provenance attestations on npm.js

Suggested approach:

  1. Switch doc publishing to happen from branch being published.
  2. Leave gh-pages workflow in place for now, in case we need it for manual fix-ups, but modify it to use main as the default branch.
  3. Remove stable branch. It's main use was to have a stable reference point for documentation publishes, but if goal 1 is met, we don't need that anymore.
  4. Remove this list of files from git tracking:
    • docs/vendor/peggy/*
    • docs/js/*-bundle.js
  5. The above files will get generated on doc publish with the correct version information in them. Having them in git causes unnecessary churn.
  6. For now, version information will still be updated in a single commit on the machine of the person doing the release. Those version updates now happen automatcially when doing npm version however.
  7. A publish workflow will run when a new release is created on GitHub.
    • Build
    • Strip devDependencies and scripts from package.json (this reduces the package size and ensures that we don't get security dings for dev-only dependencies)
    • ~Re-generate package-lock.json. pnpm-lock.yaml is excluded from being published by .npmignore. package-lock.json is currently not being published, but I can't figure out why. Be safe in case it starts being published again, or if it's published when on GHA or something.~ (neither of these files are included in the published package)
    • Publish docs. This requires write access to the repo, as current workflow, using GITHUB_TOKEN.
    • Publish new version to NPM. This will require a NPM_TOKEN secret to be added to the repo. For now, this will be a secret from hildjj's account, but can be changed by any project admin in case hildjj gets hit by a bus.
on:
  release:
    types: [published]

Comments welcome.

Mingun commented 2 months ago

What about if you want to test that everything works normally? It would be possible to go through all the steps except actual publishing?

hildjj commented 2 months ago

I do all of that locally, and also on the CI run from merging the last PR before changing the version.

Mingun commented 2 months ago

I mean, imagine, I have to make a new release. How can I be sure that I'm not missing anything or creating a broken release? If CI will find some problem that I did not catch locally, it will be possible to break release process, make force push with fixes and repeat?

hildjj commented 2 months ago

I'm going to write the release issue for 4.1 that assumes that we have made the change suggested by this issue, so it can be compared with #461. I think it will be easier to see then what steps I've missed above, and what other checks might be needed.

In other projects, when the publish has failed, it has been one of these things:

For some of the issues above, it's been possible to fix the issue, then re-run the failed job in GitHub Actions. For others, it has been better to create a new patch version and abandon the version number from the failed release. I don't like force-pushing to main, and would rather just not use a given version number than have to do that.

I haven't had the issue of a build failure at this point, since I'm careful to wait until CI for the merge containing the new version passes before creating the release in GitHub.

Mingun commented 2 months ago

This is what I'm using to create a quick-xml releases: simple and fail-safe. CI first run on my own fork and when it is green I just push to the upstream. That ensures that published release will match tag on GitHub and at the same time I can fix problems before the push to registry (crates.io in that case) will be made and without clogging the official history by technical commits. When the release in the registry, the only thing that may get wrong is synchronization with official master branch which is very unlikely and even if that happens I can repeat later.

Release checklist (minimal list of actions for cutting a release):

  1. $env:RUSTDOCFLAGS="--cfg docsrs"; cargo +nightly doc --all-features and check generated documentation for missing / unclear things
  2. Update version in Cargo.toml
  3. Update Changelog.md with date of release, add new empty Unreleased headings
  4. Commit changes with message "Release x.y.z"
  5. cargo package for final check
  6. Push master to my fork, wait while CI pass. Repeat with force pushs if necessary
  7. cargo publish
  8. Create and push tag vx.y.z and push master to upstream
  9. Create a Release on GitHub (in GitHub UI)

If I understand correctly, the proposed workflow will check everything in PR. If it will build some artefacts which somehow depends from git commit (commit hash in the meta-information for registry, for example) and then push it to the registry, that may be not very good.

hildjj commented 2 months ago

One of the things I'd like to achieve is a signed provenance statement on npmjs.com. For an example, see the bottom of https://www.npmjs.com/package/abnf where you can see links to the commit and workflow that were used to publish that release.

hildjj commented 2 months ago

package-lock.json is always ignored on publish, according to the docs, so there has been no need to keep it up to date or use it in any way. I'll remove it from the repo, and since I always use pnpm, it will never get recreated again. Editing the above to cross out the rebuild-lockfile steps.

Mingun commented 2 months ago

Ok, the proposed workflow seems support the ability I try to describe -- if you miss something when preparing release (for example, lint is unhappy), the release will not be published and you will have chance to fix problems and try again.

hildjj commented 2 months ago

I'm preparing a PR that implements this. I'll wait for your review before landing it.