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

[WIP] Don't require building a stage1 compiler before running cargotest #84780

Closed
wants to merge 2 commits into from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented May 1, 2021

  • Always use a bootstrap compiler for cargo and cargo-credential helpers

    This seems wrong; I think cargo is intentionally built with stage 2
    because it's distributed with other parts of the release. However, I
    don't know how to avoid it - just changing CargoTest to build it with
    stage 0 unconditionally still builds rustc because it uses a ToolRustc
    mode.

  • Don't call builder.ensure(Rustc). It's unnecessary.

  • Don't use the same compiler for building cargo as for testing.

Builds on #84779 to avoid merge conflicts.

This allows running a single test without having to wait for all tests
to complete.
@jyn514 jyn514 added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself labels May 1, 2021
@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 May 1, 2021
@jyn514 jyn514 force-pushed the cargotest-stage0 branch from b7f67aa to 1d0674e Compare May 1, 2021 05:38
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

- Always use a bootstrap compiler for cargo and cargo-credential helpers

  This seems wrong; I think cargo is intentionally built with stage 2
  because it's distributed with other parts of the release. However, I
  don't know how to avoid it - just changing CargoTest to build it with
  stage 0 unconditionally still builds rustc because it uses a `ToolRustc`
  mode.

- Don't call `builder.ensure(Rustc)`. It's unnecessary.
- Don't use the same compiler for building cargo as for testing.
@jyn514 jyn514 force-pushed the cargotest-stage0 branch from 1d0674e to 2266746 Compare May 1, 2021 06:04
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-10 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
.................................................................................................... 9400/11812
.................................................................................................... 9500/11812
.................................................................................................... 9600/11812
..................................i......i.......................................................... 9700/11812
................................................................................iiiiiii..iiiiii.i... 9800/11812
.................................................................................................... 10000/11812
.................................................................................................... 10100/11812
.................................................................................................... 10200/11812
.................................................................................................... 10300/11812
---
Suite("src/test/assembly") not skipped for "bootstrap::test::Assembly" -- not in ["src/tools/tidy"]
Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 33 tests
iiiiiiiiiiiiiiiiiiiiiiiiiiiii....

 finished in 0.175 seconds
Suite("src/test/incremental") not skipped for "bootstrap::test::Incremental" -- not in ["src/tools/tidy"]
Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
 finished in 11.008 seconds
Check compiletest suite=debuginfo mode=debuginfo (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 116 tests
iiiiiiiiii.i.i..i..i..ii....i.i....ii..........iiii.........i.....i...i.......ii.i.ii.....iiii.....i 100/116
test result: ok. 78 passed; 0 failed; 38 ignored; 0 measured; 0 filtered out; finished in 2.41s

 finished in 2.475 seconds
Suite("src/test/ui-fulldeps") not skipped for "bootstrap::test::UiFullDeps" -- not in ["src/tools/tidy"]
---
test builder::tests::dist::test_with_no_doc_stage0 ... ok

failures:

---- builder::tests::dist::dist_only_cross_host stdout ----
thread 'main' panicked at 'assertion failed: !use_snapshot || stage == 0 || self.local_rebuild', src/bootstrap/builder.rs:1024:9


failures:
    builder::tests::dist::dist_only_cross_host

@Mark-Simulacrum
Copy link
Member

Some of Cargo's tests require rustc to be built, so we'll want to make sure rustc is available there. Regardless though I expect almost no one to run cargo tests in-tree and I imagine the download-rustc support is pretty likely sufficient for those who do? I may be missing the motivation for this. I think we could move Cargo itself to a Std tool, rather than a Rustc tool, but I think your PR description is accurate to say Bootstrap tool is wrong.

tests/testsuite/cross_compile.rs
466:                #![feature(plugin_registrar, rustc_private)]

tests/testsuite/proc_macro.rs
230:                #![feature(plugin_registrar, rustc_private)]

tests/testsuite/plugins.rs
70:                #![feature(plugin_registrar, rustc_private)]
194:                #![feature(plugin_registrar, rustc_private)]
341:        // requires rustc_private
377:                #![feature(rustc_private)]
389:        // requires rustc_private
429:                #![feature(rustc_private)]

@Mark-Simulacrum Mark-Simulacrum 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 May 6, 2021
@jyn514
Copy link
Member Author

jyn514 commented May 9, 2021

Yeah, I don't think this is super useful and it seems complicated.

@jyn514 jyn514 closed this May 9, 2021
@jyn514 jyn514 deleted the cargotest-stage0 branch May 9, 2021 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants