Cargo: 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 (#7664)
• examples of other actions, like documentation generation, publishing to crates.io, publishing binaries (like GitHub releases), doing more advanced testing, etc. (#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:

• https://github.com/actix/actix/issues/184
• https://github.com/actix/actix/issues/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