Use recent lock file when checking API files

[tcharding]

Recently we added --locked to all the cargo invocations in the script to check API text files. But if one has an updated lock file (newer than recent) then this achieves nothing. I.e., if one has run cargo without --locked then the local lock file will have deps that are too new (cough serde I'm looking at you).

Copy the recent lock file into place when the script is run locally but preserve the lock file state once the script is finished running.

[apoelstra]

Should we also remove the cp Cargo-recent.lock Cargo.lock line in .github/workflows/rust.yml then?

We should at least warn the user before clobbering their lockfile. I would prefer we be non-destructive and just bail out if the wrong lockfile is loaded.

Or we could use cargo --lockfile-path $REPO_DIR/Cargo-recent.lock which would avoid any of this.

[tcharding]

Should we also remove the cp Cargo-recent.lock Cargo.lock line in .github/workflows/rust.yml then?

We should at least warn the user before clobbering their lockfile. I would prefer we be non-destructive and just bail out if the wrong lockfile is loaded.

Yeah I actually played around with being non-destructive, if that is you gut reaction also then we should definitely do so. I'll re-spin.

Or we could use cargo --lockfile-path $REPO_DIR/Cargo-recent.lock which would avoid any of this.

I like that.

[apoelstra]

In 992a151:

👀

(This entire tempfile should be deleted, but it's funny that you clearly manually edited the "do not manually edit this" line in a way that corrupted the TOML)

[tcharding]

Ah heck, how'd that get in there. I just munged it during testing to make sure my Cargo.lock wasn't being overwritten. No clue how I managed to commit the tmp file. I gotta run jj st more often ...

[apoelstra]

Curious why you use this tmpfile solution instead of --lockfile-path. As written this will have different behavior on CI and locally, which will be frustrating for users.

[tcharding]

hmm, not sure what is going on with me at the moment. I keep getting some sort of tunnel vision on what needs doing while reading review suggestions then doggedly following that path, ignoring/forgetting later and better review suggestions. Here I even acknowledged that I liked the --lockfile-path solution but still did not commit it to memory.

Thanks for being so gentle with you reminder, I appreciated it.

[tcharding]

Or we could use cargo --lockfile-path $REPO_DIR/Cargo-recent.lock which would avoid any of this.

lolz, after all that I think you owe me a whiskey for this one: rust-lang/cargo#14421

[apoelstra]

Gah! It's there but only in nightly. So stupid.

[apoelstra]

I'm gonna go ahead and merge this for the sake of fixing our CI -- but we'll have to decide what to do the next time we get somebody with an API CI failure but their local check-api-files.sh is clean.

[apoelstra]

ACK e8c42a7; successfully ran local tests

[tcharding]

More lolz, I didn't realize it was there in nightly. But when I went to use it I hit

cargo +nightly update -Z unstable-options --lockfile-path Cargo-recent.lock
error: the lockfile-path must be a path to a Cargo.lock file (please rename your lock file to Cargo.lock)

Face palm.

[apoelstra]

Lol, wtf.