Automate various parts of the release process
Description
Most tasks around releasing crates in our monorepo are currently performed manually:
- Adding changelog entries (happens in PRs usually)
- Pull through version numbers of workspace dependencies (also happens in PRs)
- Actually releasing crates to crates.io (done by @mxinden)
Motivation
Repetitive tasks consume unnecessary time and should be automated.
Requirements
- I'd like to have the pulling through of version bumps somehow automated, i.e. if I bump
libp2p-coreby a minor version, all dependent crates should update their dependency automatically and bump their on version as required. - Adding a changelog entry about the above would be useful.
It would be nice to trigger these operations through a bot / CI so contributors don't have to learn how to do it.
Open questions
Are you planning to do it yourself in a pull request?
Maybe.
I would suggest we make use of the cargo-xtask pattern: https://github.com/matklad/cargo-xtask/
It's great to see this issue.
I think the first step here is to document today what we do. I don't best (automation) to be the enemy of better (document what we do). Basically I view the first step is "repeatability". Then we can move to automating more and more of it. We need to make sure this isn't trible knowledge that only lives in a limited number of peoples' heads.
@mxinden : in https://github.com/libp2p/test-plans/pull/41#issuecomment-1249508288, you raised the question on the priority of this. I'd like to prevent this from being an "either or" conversation. I think it should be a "both and". Something is better than nothing on this front.
A couple of options for moving forward:
- the next time you do a release, have someone else shepherd you the whole time. Maybe even do a screen recording. It's their responsibility to then write up the steps if you can't do it. The next time you reverse shadow to see where the gaps are.
- (better I think) You take first stab at writing the steps. For the next release, you have someone else attempt to follow them. You are shadowing the process in case they get stuck. The release is then responsible for fixing any issues in the steps that see.
I think option 2 is better because it gets us validating the correctness/completeness of the release process quicker, but I defer to the team.
Other ideas:
- @p-shahi may be able to help, but I think it's also great if rust-libp2p maintainers move the ball forward independently as well.
- Maybe this also plays well in the Little Bear Labs camp of them having Rust expertise and wanting to work towards sharing more of the maintenance of the project.
I've played around with Google's release-please and it works quite well. Here is an example of a release PR: https://github.com/thomaseizinger/rust-libp2p/pull/5
- It builds on the conventional commit spec. This should be easy for us because we squash commits, so as the maintainers, we can ensure those conventions are properly enforces as we merge a PR.
- It supports[^1] cargo workspaces and will automatically bump dependent crates accordingly. I think that it only patch-bumps those so we'd need to check if we can somehow enforce a minor/major bump for the crates where we know that a dependency bump is a breaking change. This is the case for dependencies such as
libp2p-ping->libp2p-swarmfor example.
[^1]: There is an issue at the moment with our specific setup but I think that should be able to get resolved.
I took a look at cargo-release. As far as I can tell from a small dry-run cargo-release could simplify the last step of a release, namely the publishing of the many crates in dependency-then-dependent order. It would automate the cargo pubish and git tag steps.
I took a look at cargo-release. As far as I can tell from a small dry-run cargo-release could simplify the last step of a release, namely the publishing of the many crates in dependency-then-dependent order. It would automate the
cargo pubishandgit tagsteps.
Well that is already something!
If we manage to get release-please working then that would cover everything I think?
For the record, I did the v0.49.0 release with cargo release. https://github.com/libp2p/rust-libp2p/pull/3028 updates our release documentation accordingly.
This tool makes my life A LOT easier. Wonderful.
For the record, I did the
v0.49.0release withcargo release. #3028 updates our release documentation accordingly.This tool makes my life A LOT easier. Wonderful.
That is great! What do you think of making a workflow that does what you did every time a release/x.y.z branch is merged into master?
This should be helpful, regardless of how we create that release branch, i.e. manually or with "release-please" or another tool.
I am in favor of fully automating this. Do we know of any projects that do this today? (I am asking as I don't want rust-libp2p to be special here.)
I don't have the capacity to work on this any time soon. In the meantime, I am fine continuing to do the releases.
I haven't been following along in all the conversation, but I think it's good hygiene for a project with multiple maintainers to have a document that is clear about "how we do a release". If that document consolidates down to "run this script and you're done", that's great. I don't want us to put off expressing what we do today while we wait for the automated solution.
The reasons for this ask:
- It fits within our tenets of taking security, stability, performance, and specs seriously. It doesn't directly map to one of those, but it fits in the "aura". We want consumers walking away with a sense of "wow, this project takes the quality of their work very seriously."
- I want to make sure we have the place to hook additional steps. For example, if we need to have a step for "run this big interoperability test suite to confirm no regressions before releasing because it doesn't run automatically in CI" we want to have a place to inject and show that we did that step. Similarly, if there are marketing-related pieces that we do for a release (e.g., forum post, twitter post, blog entry) that aren't going to be automated in the short term, we want a place to add those.
- it also gives visibility into how much work is involved with doing a release and how much potential return on investment for simplifying/automating parts. This document doesn't have to be much, but it should be accurate to what we're doing today.
I think the document we have is up to date with our release process.
I do think we should change it though. All the manual steps like changelog entries and version bumps need to go away. It slows down my own work, esp. right after a release when all the versions are still untouched. It is also an inconvenience for contributors as we need to babysit them to make the correct changelog entries and version bumps.
I am gonna work towards adopting release-please. For that, we need to fix our circular dependencies which conveniently should also improve incremental compile-times.
Anyone against adopting conventional commits? That would be required to automate the changelogs and detect the correct version bumps.
Thanks @thomaseizinger . I'm certainly supportive of automating and making things simpler.
My bad (and it may just be me since I'm not making rust-libp2p commits) but I only became aware that we have a written document when you alluded to it. If anyone else is following along, the release process document is https://github.com/libp2p/rust-libp2p/blob/master/docs/release.md
Concerning conventional commits, you may want to set a deadline by when folks need to respond by if they are against the idea, or otherwise move forward (i.e., no response means support).
I assume this is known, but I didn't see it mentioned here: js-libp2p is using release-please I believe.
@mxinden : I may be out of the loop, but doesn't our release process need a checklist item for confirming libp2p/test-plan's "big interoperability test" passes before release?
I assume this is known, but I didn't see it mentioned here: js-libp2p is using release-please I believe.
I did not know this! Thanks for mentioning it. @achingbrain @MarcoPolo Can you comment on this?
Concerning conventional commits, you may want to set a deadline by when folks need to respond by if they are against the idea, or otherwise move forward (i.e., no response means support).
Thanks for the encouragement. Whilst I think it is a good idea, I believe we should properly challenge it first. I'll open a separate issue to discuss: https://github.com/libp2p/rust-libp2p/issues/3054
@mxinden : I may be out of the loop, but doesn't our release process need a checklist item for confirming libp2p/test-plan's "big interoperability test" passes before release?
We run all interoperability tests on each pull request and (typically) only merge when they are green: https://github.com/libp2p/rust-libp2p/actions/workflows/interop-test.yml
Thanks @thomaseizinger . Concenring interoperability tests, my understanding from a month ago was that what we run in CI is a subset of the full interoperability testsuite (because the full suite takes too long).
@laurentsenta : I'm sorry to pull you in here. I believe this has been discussed before. I looked in https://github.com/libp2p/test-plans/issues for a discussion on this, but didn't find one. Please correct me too if there is no longer a "big tests" that should be run manually for a release.
Correct. We currently only run the go-rust-interop-latest composition test instead of the go-rust-interop composition test.
https://github.com/libp2p/rust-libp2p/blob/b42f28630e2a7a04642c95aa8e7f4deea823f901/.github/workflows/interop-test.yml#L26-L32
https://github.com/libp2p/test-plans/blob/master/ping/_compositions/go-rust-interop.toml
@laurentsenta do you know off the top of your head how much longer the full test suite would take? If it is not too much time, my preference would be to simply run the full one on rust-libp2p's pull request CI instead of adding a new step to the release process.
@mxinden the all test takes approx. 30 minutes at the moment.
https://github.com/libp2p/test-plans/actions/workflows/ping-interop.yml
Status update:
We now enforce that each PR uses conventional commits: https://github.com/libp2p/rust-libp2p/pull/3204 This should allow us to at least stop writing changelog entries in every PR and instead generate one at the time of release.
We also enforce that every PR makes the according semver bump with its API changes: https://github.com/libp2p/rust-libp2p/pull/2647. This has the benefit that we detect breaking changes in our APIs very early. This however has the downside that we need to do the version bump within the PR that makes the breaking change which is one of the things I've been trying to move away from.
Tools like cargo release however make it fairly easy to perform version bumps across the repository. For example, the following command bumps the version of libp2p-core and fixes all dependencies:
cargo release version --package libp2p-core minor --execute --no-confirm
I opened https://github.com/crate-ci/cargo-release/issues/628 which would be exactly what we need. In case that feature request gets declined, I am thinking of scripting something together based on cargo metadata and a hand-rolled configuration file. We can check that into the repository and have users run that to make CI green.
Input welcome. I am sure this process can be further improved.
We discussed this in our open maintainers call and @rkuhn suggested that instead of failing CI on detected breaking changes via cargo semver-checks, we could just flag the PR with a label to make us aware that we are making breaking changes. This check would always have to run with the base branch as reference. @jxs mentioned that this could be achieved by passing the baseline rustdoc directly into cargo semver-checks.
Only applying the label would make us aware that the a PR breaks the API but we can then defer the actual bump of versions to a release pull request. This would allow us to consider a tool like release-please to automate all of that.
That said, it wouldn't be too difficult to do it manually with some repeated invocations of cargo release version.
Perhaps a good combination of automation would be to:
- Use
release-pleaseto open the release PRs and have it generate the changelog. - Run
cargo semver-checksas we currently run it to ensure we only merge a release with correct semver bumps. - Upon release time, inspect the failures of
cargo semver-checksand issue an accordingcargo release versioncommand untilcargo semver-checksis happy.
(3) could potentially be automated with a script.
Heads-up that development has started on a v2 of the cargo-semver-checks GitHub Action (https://github.com/obi1kenobi/cargo-semver-checks-action/pull/21). A new feature we're considering for v2 is the ability to apply labels to PRs corresponding to their semver-required minimum version bump (e.g. semver:major / semver:minor / semver:patch), as a separate mode compared to the current behavior of just exiting non-zero if the currently-specified version doesn't support the changes made in the PR.
Would that be useful here? If so, would you be interested in being an early adopter and sharing your feedback?
Of course, everyone working on cargo-semver-checks is an unpaid volunteer at the moment, so I can't promise a specific timeline for when this new feature might be available.
Heads-up that development has started on a v2 of the cargo-semver-checks GitHub Action (obi1kenobi/cargo-semver-checks-action#21). A new feature we're considering for v2 is the ability to apply labels to PRs corresponding to their semver-required minimum version bump (e.g.
semver:major / semver:minor / semver:patch), as a separate mode compared to the current behavior of just exiting non-zero if the currently-specified version doesn't support the changes made in the PR.Would that be useful here? If so, would you be interested in being an early adopter and sharing your feedback?
It might be useful yes!
Bear in mind that we are still figuring things out here ourselves too so take any feedback with a grain of salt :)
One aspect I want to highlight again is that such a "labeling" action would have to always use master as the baseline in order to evaluate each PR in isolation, otherwise all subsequent PRs to one that introduced breaking changes will be flagged as having breaking changes which isn't very useful.
The perfect process
In a perfect world, I think this would be a good process:
- Use
cargo semver-checksat release time to pick the appropriate version bump for each crate - Use
cargo release versionto pull the version bumps through accordingly - Use some external tool to generate the changelogs based on our commit messages (probably not
release-pleaseas it tries to do versions + changelogs at the same time which is proving to be difficult)
Limitations
This has some limitations though:
cargo semver-checksis not perfect and may miss certain breaking changes- One PR often touches multiple crates and thus it may be unclear, which changelog should be amended for a commit (remember that we squash PRs into one commit) that touches multiple crates.
(1) warrants some kind of human override, ideally at the time we write the PR. (2) may require us to continue writing manual changelog entries.
Human override for next version bump
One solution for (1) could be to have a configuration file in our repository that looks something like:
{
"libp2p-swarm": "minor",
"libp2p-gossipsub": "major",
}
We would maintain this file between releases, i.e. it gets cleared every time we release a version. This file would be used by a script at release time that:
- Runs
cargo semver-checksto get an initial idea of what the version bump should be. - Checks the configuration file for more input on the version bump.
- Picks the higher one of the two
- Executes the version bump with
cargo release version
This would allow us to fairly easily control the next version bump of a crate during the PR process.
Manual changelog entries
Instead of amending the actual changelog file (which often causes merge conflicts), we could employ a strategy where the changelog notes for a particular PR are added as separate files. I've seen projects that have a .changelog directory where the collect the notes and later use a tool / script to pull them all together into the final changelog.
This would be a much more automation friendly approach and would cause less conflicts. Additionally, it would be easy to write a lint that ensures we add a changelog entry for a crate as soon as any source-code in that crate is touched.
Conclusion
We've set out this journey with the goal of "let's not reinvent the wheel" and I think it is a novel goal. However, it seems that our particular combination of squash merging PRs, big workspaces and the requirement of "brain-less" releases doesn't fit any of the existing tools. I think they are all good requirements so I am reluctant to change any of them for the sake of fitting our process into a tool, hence I am not strictly opposed to rolling our own solution especially when it is mostly plumbing work.
Had a chat with @mxinden today about this.
We definitely want to keep the PR squash-merging strategy. Unfortunately this means that pretty much every tool / strategy that extracts changelog or version information from commits is out the window because it is just not granular enough after we squash merge the commit history of a PR.
We also definitely want to retain the "ease" of releases in terms of figuring out the next version number. Currently, we achieve this by bumping the version as part of the PR where we have the most context on whether or not the change is breaking. In an ideal world, we would only bump the version at release time, meaning we need to somehow retain the information whether the PR is breaking.
Regardless of when we bump the version, what we definitely need is a script that:
- Bumps the major / minor / patch version of a particular crate
- Updates all workspace crates to point to the new version
- Repeats this process for each crate that just had one of its dependencies bumped [^1]
(1) and (2) can be handled by cargo release version. (3) can be scripted out of a combination of cargo metadata and cargo release version.
The first step in moving forward with this issue is to write a script that does the above 3. I created an issue that tracks the creation of this script here: https://github.com/libp2p/rust-libp2p/issues/3348
[^1]: Technically, this is only necessary if the dependency is part of the public API. In our workspace, that is the case for all crates so we can approach this with such a simple rule.
In a 2nd step, I'd like to move away from bumping versions as part of the pull request and instead only capture what bump is necessary given the changes since the last release.
In https://github.com/libp2p/rust-libp2p/issues/2902#issuecomment-1381213313, I suggested to use an external configuration file but I think we can do much better! Cargo has support for arbitrary config values as part of the package.metadata table, meaning we can store something like:
[package.metadata.semver]
next-bump-level = "minor"
This has several advantages:
- This value can be set at coding & reviewing time, where we have the most knowledge whether a PR is a breaking change or not
- We can create a CI check that fails if
cargo semver-checksrequires a major bump but we only specify a minor bump in here, i.e. we can temporarily bump the version by the specified level (usingcargo release version) and then executecargo semver-checks, expecting it to succeed. - It is easy to create a script (or tool) that:
- reads this value
- calls
cargo release versionwith it - clears the value
- It does not clutter the repository unnecessarily (i.e. no extra configuration file)
- It can easily be extracted with
cargo metadata
Changelog entries would be added as they are currently with the only difference that the top heading would always say "unreleased" until we actually release a version.
What do you think about this idea @libp2p/rust-libp2p-maintainers? I think it is quite lean and would allow for a very clean automation of the entire version bumping requirement.
The story of https://github.com/libp2p/rust-libp2p/pull/3312#issuecomment-1444604719 is another data point towards not bumping versions in PRs. It hides from us, which changes would be breaking in lieu of what is already merged.
If you decide to go the metadata route, you can use the value of the next-bump-level field to populate the --release-type flag when invoking cargo-semver-checks. This would avoid having to temporarily alter the version number with cargo release version.
If you decide to go the metadata route, you can use the value of the
next-bump-levelfield to populate the--release-typeflag when invokingcargo-semver-checks. This would avoid having to temporarily alter the version number withcargo release version.
Nice! This synergy really makes me want to go for this solution :)
Do I understand correctly that --release-type validates whether the current API diff can be released as the specified level?
This makes me think we might want to go back to the idea of structured changelogs:
- Collect changelog entries as individual files with a structured format, something like:
# file: changelogs/3254.changelog
semver-bump = "minor"
changelog-text = '''
This is my changelog text.
'''
- A CI job can then lint, whether the specified semver-bump is indeed correct for the given changeset.
- At release time, we can then produce the changelog from these files.
- At release time, we simply grab the highest bump out of all files added since the last release and amend the changelog file.
Do I understand correctly that
--release-typevalidates whether the current API diff can be released as the specified level?
Yes, exactly. It says "assume this is a <X> release" and semver-checks it as such. tokio uses it to disable the handful of semver-minor lints (deprecations, #[must_use] added, etc.) we have, since they don't care about those.
semver-bump = "minor"
Actually, what we should be tracking is breaking: true/false and infer the next bump from the current version + that.
Once crates have published a 1.0 version, this might not be enough — assuming you care about "deprecations/#[must_use] are semver-minor" rules, that is.
semver-bump = "minor" to me means "the next bump is minor at least, based on changes made so far." Its default value is semver-bump = "patch" and can be raised to minor/major based on the changes made. It would go from patch -> minor on deprecations, for example.