Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't build the full compiler before running unit tests #95445

Merged
merged 1 commit into from
Mar 31, 2022

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Mar 29, 2022

This has been present since builder.ensure was first added in #43059.
It's unclear to me why it was added then - I tested these changes locally
with x test compiler/rustc_data_structures --stage 0 and they worked fine.

Fixes #51748.

This has been present since `builder.ensure` was first added in rust-lang#43059.
It's unclear to me why it was added then - I tested these changes locally
with `x test compiler/rustc_data_structures --stage 0` and they worked fine.

Fixes rust-lang#51748.
@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 29, 2022
@Dylan-DPC
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 31, 2022

📌 Commit ae12597 has been approved by Dylan-DPC

@bors bors 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 Mar 31, 2022
@jyn514
Copy link
Member Author

jyn514 commented Mar 31, 2022

@Dylan-DPC please don't rollup sensitive bootstrap changes like this; it's not unheard of for them to only fail in a full bors test, and I don't really have a way of replicating all those tests locally.

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 31, 2022
Rollup of 5 pull requests

Successful merges:

 - rust-lang#95445 (Don't build the full compiler before running unit tests)
 - rust-lang#95470 (Fix last rustdoc-gui spurious test)
 - rust-lang#95478 (Add note to the move size diagnostic)
 - rust-lang#95495 (Remove unneeded `to_string` call)
 - rust-lang#95505 (Fix library/std compilation on openbsd.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@Mark-Simulacrum
Copy link
Member

In general the likely intent here was that if some of our unit tests (as seems not unlikely, though perhaps wrong and fixable) are making use of extern crate foo where foo is loaded from the sysroot and not communicated to Cargo via dev-dependencies or something like that, then this will fail. I think CI should catch that, though there may be some amount of stage-dependence (running tests in stage 2 may not notice this, nor stage 0, though I'm not 100% sure on that).

@jyn514
Copy link
Member Author

jyn514 commented Mar 31, 2022

if some of our unit tests (as seems not unlikely, though perhaps wrong and fixable) are making use of extern crate foo where foo is loaded from the sysroot and not communicated to Cargo via dev-dependencies or something like that, then this will fail.

Ah, interesting, that sounds possible. But we should be able to fix it by adding to dev-dependencies, right? Unless we have a situation where e.g. a test for rustc_lexer depends on rustc_driver ... but that seems kind of cursed and at that point it should probably just be a UI test.

Anyway, I'll run --stage 1 tests of all the compiler crates to make sure this doesn't break them :)

@Mark-Simulacrum
Copy link
Member

Well, in theory there's nothing wrong with rustc_lexer depending on rustc_driver as a dev-dep (beyond "why???") from Cargo's perspective I think.

But yes, I think it should be fixable unless I'm missing something, just that as-is this may break things. I wish we had a better way of reviewing and testing these patches, but as-is I am tempted to r- this sort of thing (or I guess we can wait till someone reports breakage...).

@jyn514
Copy link
Member Author

jyn514 commented Mar 31, 2022

Ok, I ran x test --stage 1 compiler/* (manually removing codegen_gcc and codegen_cranelift, which apparently aren't supported right now) and it passed fine. I'll leave it up to you whether to r- or not.

@Mark-Simulacrum
Copy link
Member

That seems likely good enough for now.

@bors bors merged commit 14d8bfd into rust-lang:master Mar 31, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 31, 2022
@jyn514 jyn514 deleted the rustc-unit-tests branch March 31, 2022 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run compiler unit tests without stage 1 build
6 participants