Require Cargo.lock matches Cargo.toml in CI

[huonw]

This passes --locked to the first interesting cargo invocation, to ensure that the Cargo.lock file is up to date with Cargo.toml. Without this, if there's been changes to Cargo.toml and the user hasn't run a cargo command locally afterwards, the invocation might change the Cargo.lock file (e.g. bump versions, add packages) and thus CI runs with an unexpected configuration, that's not recorded in the repo.

(This came up in pantsbuild/scie-pants#290 and I thought to check other Rust repos that are tangentially related.)

[kaos]

Cool. So I guess it will fail if the lock is out of date?

[jsirois]

What workflow leads to this state? It's fine to guard against of course, but I've also not encountered the state ever on scie-pants, jump or ptex and it's been a while and alot of commits.

[jsirois]

Yeah, I'd like to understand what this solves a bit better. Here I update Cargo.toml over in ptex without doing anything that might regen the lock and get no error:

jsirois@Gill-Windows:~/dev/a-scie/ptex (CI/ensure-lock *) $ git diff
diff --git a/Cargo.toml b/Cargo.toml
index a6a4bee..32ac308 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -20,7 +20,7 @@ codegen-units = 1

 [dependencies]
 anyhow = "1.0"
-indicatif = "0.17"
+indicatif = "0.17.1"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
 url = "2.4"
jsirois@Gill-Windows:~/dev/a-scie/ptex (CI/ensure-lock *) $ cargo check --all --locked
    Checking package v0.2.0 (/home/jsirois/dev/a-scie/ptex/package)
    Checking ptex v0.7.0 (/home/jsirois/dev/a-scie/ptex)
    Finished dev [unoptimized + debuginfo] target(s) in 0.48s
jsirois@Gill-Windows:~/dev/a-scie/ptex (CI/ensure-lock *) $ echo $?
0
$ git grep -A1 indicatif Cargo.lock
Cargo.lock:name = "indicatif"
Cargo.lock-version = "0.17.6"
--
Cargo.lock: "indicatif",
Cargo.lock- "serde",

So it appears all --locked does is run using the lock, it does not update the lock based on Cargo.toml having changed. That, I'd think, is what you really want to check. That people don't just edit Cargo.toml deps, commit and push without ever actually testing anything via cargo {check,test,build,...} which updates the lock as-needed.

[jsirois]

Ok, maybe cargo is just buggy or I don't understand version numbers. Although my example above fails to fail - which seems wrong to me, simpler examples, like adding a new dep to Cargo.toml, do fail when using --locked. I'm ambivalent, some automation just lets you seem to go faster when you should just slow down.

[huonw]

That people don't just edit Cargo.toml deps, commit and push without ever actually testing anything via cargo {check,test,build,...} which updates the lock as-needed.

Yeah, this mistaken workflow is what it's catching. It happened in practice for me in pantsbuild/scie-pants#290 with bumping the package version (yes, I know scie-pants the release procedure says do a cargo run ..., but humans make mistakes all the time).

Ok, maybe cargo is just buggy or I don't understand version numbers

I believe cargo only checks the lockfile versions match the input ranges, and this is all that it can do, given what's in the lockfile.

In this case, 0.17.6 is compatible with both 0.17 and 0.17.1: in Cargo, unqualified versions are equivalent to having the ^ semver-compatible qualifier, e.g. ^0.17.1. Changing the requirement to an exact specifier like =0.17.1 indeed results in a change to Cargo.lock without --locked, and the the lock file .../ptex/Cargo.lock needs to be updated but --locked was passed to prevent this error with --locked.

(The lockfiles are moderately carefully optimised to reduce merge conflicts (rust-lang/cargo#7070), and in particular try not to have a central listing of info. E.g. they don't record the input requirements, only the resolved result.)

[jsirois]

Hrm, OK. I think if you have a situation where 2 humans make a mistake and it still gets by, that's again a reason to seriously pause. Both you and the reviewer missed this. That, in my mind, puts alot in question that the automation will just paper over.

SGTM though, but be careful out there in scie-pants land!