Skip to content

cargotest: add lockfiles - #162265

Open
jnkel wants to merge 1 commit into
rust-lang:mainfrom
jnkel:cargotest-lockfile
Open

cargotest: add lockfiles#162265
jnkel wants to merge 1 commit into
rust-lang:mainfrom
jnkel:cargotest-lockfile

Conversation

@jnkel

@jnkel jnkel commented Sep 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

The cargotest suite clones (specific commit hashes of) some repos from GitHub and builds them. One such repo (iron) had no lockfile, and one of its dependencies (tinyvec) pushed an update which failes to compile.

Add a lockfile for this test to downgrade tinyvec, unblock CI, and prevent this from happening in the future. Also add lockfiles for other tests that were missing them (diesel and stylo), as not having them risks both broken builds and supply-chain attacks on CI.

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Sep 3, 2026
@rustbot

rustbot commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Fallback group: @Mark-Simulacrum, @jieyouxu
  • @Mark-Simulacrum, @jieyouxu expanded to Mark-Simulacrum, jieyouxu

@jnkel
jnkel marked this pull request as draft September 3, 2026 23:01
@rustbot rustbot added S-waiting-on-author Status: This is 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 Sep 3, 2026
@jnkel
jnkel force-pushed the cargotest-lockfile branch from e76720f to 16d7f60 Compare September 3, 2026 23:14
@jnkel
jnkel marked this pull request as ready for review September 3, 2026 23:14
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 3, 2026
@jnkel jnkel changed the title cargotest: add lockfile for iron cargotest: add lockfiles Sep 3, 2026
This was referenced Sep 3, 2026
Merged
Merged
@ChrisDenton

ChrisDenton commented Sep 4, 2026

Copy link
Copy Markdown
Member

I'm not entirely sure which team cargotest comes under but r=me if you want to get things moving again.

@jnkel

jnkel commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

A fixed release of tinyvec has been published so the tree is no longer blocked. The PR is still worth merging IMO, but there's no rush on it.

@JonathanBrouwer

JonathanBrouwer commented Sep 4, 2026

Copy link
Copy Markdown
Member

@bors try jobs=x86_64-gnu-aux

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors Bot pushed a commit that referenced this pull request Sep 4, 2026
@bors
Auto merge of #162265 - jnkel:cargotest-lockfile, r=<try>
43d03dc 
cargotest: add lockfiles


try-job: x86_64-gnu-aux
@jieyouxu

jieyouxu commented Sep 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

I'm not entirely sure which team cargotest comes under but r=me if you want to get things moving again.

AFAIK cargotest is more of a historic relic that exercises some basic ecosystem crate for "nothing is obviously entirely borked". I would say somewhere in between compiler/bootstrap/cargo but in practice, no one is actually actively maintaining this.

Looks like this was introduced way back in #32348. Alex did indeed very astutely note #32348 (comment)

Looks good to me, although I do think that we'll want to check in a lock file to ensure that tests don't break to unintended consequences.

jieyouxu
jieyouxu approved these changes Sep 4, 2026
View reviewed changes

@jieyouxu jieyouxu left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, incrementally covering more of these w/ lockfiles is strictly better than the previous status quo.

You can r= Chris and me with these two nits

@rustbot author

View changes since this review

Comment thread src/tools/cargotest/main.rs Outdated
Comment thread src/tools/cargotest/main.rs Outdated
@jieyouxu jieyouxu assigned jieyouxu and unassigned Mark-Simulacrum Sep 4, 2026
@jieyouxu

jieyouxu commented Sep 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Apparently the infra team was maintaining cargotest. I did not know that. Looking back at #70659...

Well it turns out I did know. I just forgot: #t-infra > Who's maintaining cargotest?

jieyouxu
jieyouxu reviewed Sep 4, 2026
View reviewed changes
Comment thread src/tools/cargotest/lockfiles/diesel.lock
@rust-bors

rust-bors Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 43d03dc (43d03dcca17e79764ee54e2e4605782d28548477)
Base parent: 71238e2 (71238e21fc55e73ab3aad8c9f79fed7a47a179e1)

@rustbot rustbot added S-waiting-on-author Status: This is 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 Sep 4, 2026
@rustbot

rustbot commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@jnkel
jnkel force-pushed the cargotest-lockfile branch from 16d7f60 to 70e0d6d Compare September 4, 2026 14:28
@jnkel

jnkel commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

@rustbot ready

(I don't think I have permission to r=Chris and you)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 4, 2026
jieyouxu
jieyouxu approved these changes Sep 5, 2026
View reviewed changes

@jieyouxu jieyouxu left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks
@bors r+ rollup

View changes since this review

@rust-bors

rust-bors Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 70e0d6d has been approved by jieyouxu

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 10. This pull request will be tested once the tree is reopened.

Reason for tree closure: Linux x64 EC2 instances are not starting

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 5, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 5, 2026
@JonathanBrouwer
Rollup merge of rust-lang#162265 - jnkel:cargotest-lockfile, r=jieyouxu
480d197 
cargotest: add lockfiles

The `cargotest` suite clones (specific commit hashes of) some repos from GitHub and builds them. One such repo ([iron](https://github.com/iron/iron)) had no lockfile, and one of its dependencies (`tinyvec`) [pushed an update which failes to compile](Lokathor/tinyvec#225).

Add a lockfile for this test to downgrade tinyvec, unblock CI, and prevent this from happening in the future. Also add lockfiles for other tests that were missing them (`diesel` and `stylo`), as not having them risks both broken builds and supply-chain attacks on CI.
@JonathanBrouwer JonathanBrouwer mentioned this pull request Sep 5, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jieyouxu jieyouxu jieyouxu approved these changes

Assignees

@jieyouxu jieyouxu

Labels

A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jnkel @rustbot @ChrisDenton @JonathanBrouwer @jieyouxu @Mark-Simulacrum