chore: update rust toolchain and rework ci workflows #322
Conversation
* Replace unmaintained actions-rs steps with manual steps * Rely on rust-toolchain.toml file for installing required toolchain, targets and components. This also cleans up Makefile.toml a bit * Improve the way we handle build caches using Swatinem/rust-cache. See below for details. * Restructure jobs to run lint/formatter checks and tests in parallel, but introduce some ordering in those groups to improve cache coherency and reuse, and in some cases, to avoid running expensive actions which are very likely to fail anyway. The main difference here is that amongst the test group, the unit test job is run first, followed by the midenc and cargo-miden integration test jobs. Previously only the cargo-miden integration tests were split out into a separate job. By breaking apart unit and integration tests, we can fail fast when unit tests are broken, and avoid running the expensive integration tests when they are very likely to fail if the unit tests are broken. * Update the `book.yml` workflow to publish our new mkdocs-based documentation, rather than the old mdbook-based docs. This was causing the workflow to fail, it should now start working again. RE: caching in CI. See https://github.com/Swatinem/rust-cache for details on how the caching step works, particularly _what_ is cached, how it is keyed, and _when_ it is cached. The changes in this commit cache the results of a successful run _only_ when a push to the 'next' branch occurs, i.e. when a PR is merged. Thus, all PR runs will use the cache associated with the last successful build of `next` as a base, and build off of that. Furthermore, there are three different caches: * The cache shared by non-test jobs in the `ci.yml` workflow * The cache shared by test jobs in the `ci.yml` workflow * The cache used by the release job in the `release.yml` workflow Previous to this commit, each job had its own cache, and the cache would be updated on every successful build, causing a fair amount of cache instability, and resulting in very little reuse. This commit is more conservative about updating the cache, but shares it more broadly across jobs, and the more conservative baseline means that the cache is hit more frequently across all PRs. The choice to not share the cache between non-test and test jobs in the `ci.yml` workflow is because we want to maximize the number of jobs that run in parallel, but to reuse the cache, we must ensure that only one job is responsible for maintaining it. If we maximized cache reuse, we'd run all jobs in sequence, but this would be slow. Instead, we break up the workflow into non-test checks, and tests, and pick a baseline job in both groups to handle maintaining the cache for that group. These groups can then be run in parallel, and after the job that maintains the cache is run, all remaining jobs in that group can be run in parallel.
|
Correction: looks like the workflow updates are actually run as part of the PR, so that's great, I can work out the kinks here before we merge |
wrap_comments does not interact well with complex Markdown lists, of which we have multiple. In general it is still an unstable feature and not totally reliable, so disabling for now
|
@bobbinth If you have a moment, and can skim this and review, I'd like to try and get this merged so I can test the interaction with our other PRs and in particular the caching behavior, and then I can get #319 merged as well, and get things unblocked for Denys and I. Not a big deal if you aren't available, but figured its worth a ping 😅 |
|
Thanks @bobbinth!
We're using |
This PR updates our Rust toolchain to a recent nightly (same date as the recent 1.81.0 stable release, 08-06), updates dependencies, and takes the opportunity to address some outstanding improvements/fixes to our GitHub Actions workflows.
See the commit description for the commit that modifies the workflows for details on what the changes are and the rationale behind them.
This closes out #256, and makes
rust-toolchain.tomlthe canonical source for the Rust toolchain that will be used both locally and in CI. If we need to add targets or components to the "base" installation of our toolchain, they should be added there, rather than performed manually. For things like Cargo extensions that we use,cargo-makehandles installing them for us, so we don't need manually-defined tasks for those items, except when we need to do something special. For example, we currently install a pinned version ofcargo-component, which requires us to define a task to control that.Since the workflow changes won't take effect until this is merged (IIRC), we may have to iterate on them a bit to fix any issues that come up. Let's prioritize addressing those when they come up, so we can rely on CI being stable.