Expand "CI best practicies" section to the guide

[matklad]

Things to talk about:

• what directories should be cached (probably ~/.cargo, ./target, but keep ~/.cargo/credentials in mind)
• Rust channels (testing on current stable, beta and the oldest supported rustc)
• using --minimal-versions to verify dependencies in Cargo.toml
• setting-up rustfmt/clippy
• examples for popular CI services like GitHub Actions (Suggestion for cargo book Continuous Integration section to include GitHub Actions #7664)
• examples of other actions, like documentation generation, publishing to crates.io, publishing binaries (like GitHub releases), doing more advanced testing, etc. (Suggestion for cargo book Continuous Integration section to include more examples #7665)

Ideally, the section should contain copy-pastable config files for various CI providers.

[matklad]

A blog post about this feature: https://blog.illicitonion.com/2018/06/rust-minimum-versions-semver-is-lie.html

[ehuss]

I assume this means to expand https://github.com/rust-lang/cargo/blob/master/src/doc/src/guide/continuous-integration.md? Should it maybe leverage or at least point to japaric/trust?

[matklad]

Ah, I haven't realized that https://github.com/rust-lang/cargo/blob/master/src/doc/src/guide/continuous-integration.md exists.

Should it maybe leverage or at least point to japaric/trust?

Yep, we should definitely mention it!

[Eh2406]

@ehuss That is a good place for it to go, thanks for the reminder!

The impetus for this is that when we stabilize --minimal-versions we want to craft a message about how to use it. Specifically as a small part of a thorough CI setup. Witch will just lead everyone to ask, "What showed the big parts of the CI setup be?" So we thought we should write up our opinion before we stabilize --minimal-versions.

[withoutboats]

Relevant: rust-lang/rfcs#2483

[aturon]

I've just posted a blog post that talks about version selection in detail, and touches on the questions here as well.

[Eh2406]

That is an excellent read. Thank you for writing this up so articulately. I especially liked the paragraph of:

In the long run, it could even make sense to combine the two approaches, allowing crates to state their toolchain requirements (and have that influence resolution), but encourage core crates to state “LTS” as their requirement.

That sounds like the tradeoff bending I know and love from the rust ethic. I.E. a careful and pedantic system to ensure you are correct but with a large pit of success to make it usable. It also involves the most design work, so it may not be feasible.

In our earlier discussion today you suggested that "equality constraints are rare in the ecosystem" and I resisted being pedantic and pointing out lock files. So I was glad to see them addressed in your blog post. But I do have one nit with:

Similarly, when dependencies are subsequently adjusted, Cargo will “unlock” the affected dependencies and again choose the maximum version.

Currently that adjustment is very conservative, if the version of the sub dependency that appears in the lock file satisfies the requirement of the new dependency then it is not unlocked. Meaning that I am often testing a combination not in the well tested set. So if the new dependency does not have lower-bound precision and my lockfile is old enough then I will brake.

Thanks for the thought provoking guidance of this community.

[Nemo157]

There's also the https://crate-ci.github.io/ project (cc @epage). Maybe the Cargo Guide could have a small CI best practices section and link out to a larger exploration of the area like this?

[withoutboats]

In our earlier discussion today you suggested that "equality constraints are rare in the ecosystem" and I resisted being pedantic and pointing out lock files.

The important difference with lock files is that you once you have one you should have a successful build, and because it was generated using max versions, it will not run into the too low min version problem. Ceiling'd constraints are a problem because they influence version resolution, whereas the lockfile is after version resolution.

You do identify exactly where a problem can arise, though: cargo update -p. It might be worth considering changing the behavior of cargo update -p to update the entire subgraph under the crate you're updating.

[dwijnand]

More blog posts: https://levans.fr/rust_travis_cache.html (https://www.reddit.com/r/rust/comments/9d7sax/beware_the_rust_cache_on_travis_or_why_you_should/)

[Turbo87]

Currently that adjustment is very conservative, if the version of the sub dependency that appears in the lock file satisfies the requirement of the new dependency then it is not unlocked. Meaning that I am often testing a combination not in the well tested set. So if the new dependency does not have lower-bound precision and my lockfile is old enough then I will brake.

I've run into this problem quite a few times lately. I'm using dependabot on some of my projects to automatically update dependencies, and the updates regularly break CI because the lockfile has an older transitive dependency, but some other dependency relies on it being the most recent release.

Examples:

• the trait bound rand_hc::Hc128Rng: rand_core::SeedableRng is not satisfied actix/actix#184
• trust-dns-proto fails to compile actix/actix#177

It’s possible that we will eventually have workflows that depend on the accuracy of lower bounds in Cargo.toml. At the moment, however, this is purely speculative; the Cargo team does not have any ready examples.

Hopefully the examples above are helpful in that regard

[mathstuf]

I've been working to improve CI times on crates I work on here. There's gitlab-ci examples there (minimum version, stable, nightly, with/without feature flags, -Z minimal-versions, and, because it affects us, testing against git master). It handles caching and artifacting between steps. The most comprehensive one I have is for git-checks.

For Cirrus CI, I have keyutils. Though it runs the -Z minimal-versions under nightly rather than the minimum compiler version.

Basic strategy for "optimal" builds (AFAICT):

• run cargo generate-lockfile, cargo fetch --frozen, cargo vendor; cache the downloaded crates, artifact the vendored crates. This is important to avoid downloads, but also not inflating the build step with stale dependencies
• run the build for each version (clippy goes here too); add the target/ directory to the artifact set
• run the test suite for each version (this is split out because one of them gets run with git master in the PATH). Also good for testing against different DBs or other external dependencies without duplicating the build step.

[trevordmiller]

I recently added CI / CD for a Rust project and after a good amount of research this is what I ended up with (specific to GitHub Actions), in case it is helpful for this issue:

Run CI when pushing to branches for pull requeests

name: Pull request
on:
  push:
    branches-ignore:
      - master
jobs:
  verify:
    runs-on: ubuntu-latest
    steps:
    - name: Checkout
      uses: actions/checkout@v1
    - name: Check
      run: cargo check
    - name: Test
      run: cargo test
    - name: Lint
      run: cargo clippy --all-targets -- -D warnings
    - name: Format
      run: cargo fmt -- --check
    - name: Publish
      run: cargo publish --dry-run

Run CI and CD when merging pull requests to master

name: Merge
on:
  push:
    branches:
      - master
jobs:
  verify:
    runs-on: ubuntu-latest
    steps:
    - name: Checkout
      uses: actions/checkout@v1
    - name: Check
      run: cargo check
    - name: Test
      run: cargo test
    - name: Lint
      run: cargo clippy --all-targets -- -D warnings
    - name: Format
      run: cargo fmt -- --check
  executable:
    runs-on: ubuntu-latest
    needs: [verify]
    steps:
    - name: Checkout
      uses: actions/checkout@v1
    - name: Login
      run: cargo login ${{ secrets.CRATE_REGISTRY_PAT }}
    - name: Publish
      if: success()
      run: cargo publish

[mathstuf]

So my feedback on this:

• linting and formatting should be done in separate jobs (so that they fail fast)
• cargo check should also be separate (building will have to do it anyways, so if you want the faster results, I'd just do that)
• passing --tests and --examples to cargo check would be useful
• if you're publishing to crates.io please push a tag to the repo as well
• no guidelines for --features-based testing (probably out-of-scope, but a template/best practices should at least mention it)
• acknowledgement of workspace repositories

[4ydan]

To reduce our pipeline time I have come up with following testing strategy:
parallel cargo doc, build, test and check
does that seem reasonable to you?

cargo check should also be separate (building will have to do it anyways, so if you want the faster results, I'd just do that)

When building is already doing cargo check can we just skip it or did I get that wrong?

[epage]

imo cargo check and cargo build are mutually exclusive unless you want the artifact.

Similarly, cargo check and cargo test` may be mutually exclusive, depending on your needs.

My CI tends to be broken down into

• parallel jobs of cargo test against different target platforms
• an MSRV cargo check job
• a cargo doc job
• a cargo clippy job (sometimes this can supersede a cargo check job)
• a cargo fmt job
• a cargo deny job

[epage]

#12382 documented a way to verify latest dependencies.

#13056 documents a way to verify MSRV

Holes I still see

• cargo doc (--no-deps, RUSTDOCFLAGS=-Dwarnings)
• cargo clippy and warning best practices (don't deny in source)
  • If we can include reporting issues in the workflow, like with SARIF, then that'd be great
• cargo fmt
• Caching best practices
• Optimization practices from https://matklad.github.io/2021/09/04/fast-rust-builds.html#CI-Workflow
• cargo test
  • Matrix of OS / rust version (for myself, I split alt rust versions to another pipeline that runs on a schedule)
  • cargo test --no-run, see https://matklad.github.io/2021/09/04/fast-rust-builds.html#CI-Workflow
  • cargo test for benches
• bypassing feature unification with cargo hack
• cargo deny? Ideally we slowly pull this functionality into cargo

[briansmith]

IMO, this project (cargo) isn't the place to document CI best practices for Rust projects, even ones that use Cargo. I do think that such guides need to exist but why burden the cargo project with maintaining these docs?

[epage]

Because there isn't a better place at this time?

• Guides need to exist somewhere
• Ideally those guides would be evergreen, rather than blog posts
  • For myself, when I started my first projects, I pored over blog posts and had to infer from their contradictions and timestamps what was still valid and right
• The guide needs to be someplace respectable that people will read and update so it stays relevant and not forgotten
• The guide needs to be some place the cargo team can cross-link to and trusts to cross-link to as we have cases to refer to them (see the linked PRs)

[rursprung]

thanks for working on this!

would it be an option to provide a (set of) GitHub Workflows which contain these best practices (where possible)?
then users of other CIs can essentially copy whatever is being done there (with the GH Workflows being the canonical reference for the best practices at that time).

as workflows are versioned (semver) and come with documentation that might be a good way of rolling out new patterns over time (and if users have dependabot enabled for the workflows then they'll also notice if a new version comes around)

[mathstuf]

Things I do in my Rust CI (though on GitLab-CI; the steps might be useful at least) that aren't mentioned here:

• do generate-lockfile once per resolution ("standard", lock-file, -Zminimal-versions, etc.)
• pull the crates and cache them
• everything else does --frozen --locked to avoid skew between jobs if an index update comes in during the pipeline run
• cargo audit each dependency resolution
• cargo tarpaulin for coverage
• cargo semver-checks
• rather than doing "artifact cache"-friendly things like no incremental and no debuginfo, use sccache instead (though probably not better with cloud-based CI; we have our CI all on local machines with a Redis server we use for compile caching)

[epage]

would it be an option to provide a (set of) GitHub Workflows which contain these best practices (where possible)?

For what we've included so far, we list a lot of trade offs so there isn't a single right solution.

Personally, I also am unsure of the value of providing an Action over people just calling the commands directly. The exception is I have a specific workflow in my own projects for clippy that leverages SARIF reporting in github.

• do generate-lockfile once per resolution ("standard", lock-file, -Zminimal-versions, etc.)
• pull the crates and cache them
• everything else does --frozen --locked to avoid skew between jobs if an index update comes in during the pipeline run
• rather than doing "artifact cache"-friendly things like no incremental and no debuginfo, use sccache instead (though probably not better with cloud-based CI; we have our CI all on local machines with a Redis server we use for compile caching)

Some of this gets in the question of how much we talk about principles vs CI specific practices (caching for that one CI provider).

• cargo audit each dependency resolution

One question would be cargo deny vs cargo audit and whether we feel rustsec is GA enough for us to advertise

• cargo tarpaulin for coverage

There are multiple solutions. We'd need to at least reference the main ones even if we point to one

• cargo semver-checks

imo this isn't GA enough for us to include yet.