Skip to content

Consistently use --locked for cargo commands - #7040

Open
jieyouxu wants to merge 7 commits into
rust-lang:mainfrom
jieyouxu:jieyouxu/ci/lock
Open

Consistently use --locked for cargo commands#7040
jieyouxu wants to merge 7 commits into
rust-lang:mainfrom
jieyouxu:jieyouxu/ci/lock

Conversation

@jieyouxu

@jieyouxu jieyouxu commented Aug 21, 2026

Copy link
Copy Markdown
Member

Summary

Helps with #7037 but does not completely adddress it:

  • We still download, build and run the ecosystem integration test repositories without --locked.

For cases modulo the ecosystem integration tests, use --locked consistently -- in CI, bootstrap, docs, test harness, so that we use the exact dependencies checked in via Cargo.lock lockfiles instead of floating resolution, where we risk being exposed to fresh supply-chain attacks1.

Reminded me to double-check after yesterday's supply-chain attack.

Footnotes

  1. Obviously this won't help with long-term supply-chain attacks that get into the lockfile, but defense-in-depth etc.

@jieyouxu jieyouxu added S-waiting-on-review Status: awaiting review from the assignee but also interested parties. A-CI Area: CI A-test-suite Area: test suites A-docs Area: documentation labels Aug 21, 2026
@jieyouxu
jieyouxu marked this pull request as draft August 21, 2026 10:01
@rustbot rustbot added S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: awaiting review from the assignee but also interested parties. labels Aug 21, 2026
@jieyouxu

jieyouxu commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

Oh that's interesting, right, we don't check in lockfiles for the integration crates (since we just clone the repos)

@ytmimi

ytmimi commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

We should also add --locked when building from source in the Diff-Check 😁

info!("Building rustfmt from source");
let Ok(_) = Command::new("cargo")
.current_dir(dir)
.args(["build", "-q", "--release", "--bin", "rustfmt"])
.output()
else {
return Err(CheckDiffError::FailedSourceBuild(
"Error building rustfmt from source",
));
};

Comment thread ci/src/build_and_test.rs

@jieyouxu jieyouxu Aug 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussion: I considered if we can use --locked for the ecosystem integration tests. My assessment is that we can't in the general case:

  • Some of these, like bitflags, are library crates that do not check in a lockfile. We can technically patch in a lockfile ourselves but that's a maintenance PITA.
  • We could consider using --locked iff the repo has a lockfile, but cf. no-lockfile case above.

View changes since the review

@jieyouxu

jieyouxu commented Aug 24, 2026

Copy link
Copy Markdown
Member Author

Changes since last review:

The ecosystem crate case feels a bit iffy to me, because that's a very large surface area versus rustfmt's own dependency graph. I think that will be helped once cargo stabilizes -Zmin-publish-age which might help us out here...

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author. labels Aug 24, 2026
@jieyouxu
jieyouxu marked this pull request as ready for review August 24, 2026 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-CI Area: CI A-docs Area: documentation A-test-suite Area: test suites S-waiting-on-review Status: awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants