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

Fix x test core when download-rustc is enabled #110701

Merged
merged 2 commits into from
Jun 4, 2023
Merged

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Apr 22, 2023

Fix x test --stage 2 core when download-rustc is enabled

This works by building std from source instead of downloading it, for library tests only.

This was somewhat complicated because of the following requirements:

  1. Unconditionally downloading libstd breaks x test core, because coretests requires the std loaded from the sysroot to match the std that's currently being tested.
  2. Unconditionally rebuilding libstd breaks x test ui-fulldeps librustdoc, because anything loading rustc_private needs to use the same libstd that rustc was built with.

Break the knot by introducing a new stage2-test-sysroot, used only for testing std itself. This
holds a freshly compiled std, while stage2 and ci-rustc-sysroot still hold the downloaded std.

This also extends the existing cp_filtered in Sysroot to apply to the rust-std component, not just the rustc-dev component, to avoid having both versions of std in stage2-test-sysroot.

Fixes #110352.

@jyn514 jyn514 added the A-download-rustc Area: The `rust.download-rustc` build option. label Apr 22, 2023
@rustbot
Copy link
Collaborator

rustbot commented Apr 22, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Apr 22, 2023
@jyn514 jyn514 added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 22, 2023
@jyn514 jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels May 18, 2023
@jyn514
Copy link
Member Author

jyn514 commented May 18, 2023

Update on this: since #110699 merged, x test ui is fixed, but x test ui-fulldeps rustdoc are both broken with errors similar to

error[E0460]: found possibly newer version of crate `std` which `thin_vec` depends on
  --> src/librustdoc/lib.rs:24:1
   |
24 | extern crate thin_vec;
   | ^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: perhaps that crate needs to be recompiled?
   = note: the following crate versions were found:
           crate `std`: /home/jyn/src/rust/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-9768f62f7b152226.rlib
           crate `std`: /home/jyn/src/rust/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-9768f62f7b152226.so
           crate `thin_vec`: /home/jyn/src/rust/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/libthin_vec-812d4d34d6de684d.rmeta

I'm too tired tonight to figure out if these are fixable or not; they're similar to the errors fixed in #110121 so maybe it's possible to do something clever like run x test core against the stage1 sysroot but all rustc and tool tests against the stage2 sysroot, and rely on using ci-rustc being the same in both stages to avoid issues? But that's really hacky and I worry it will make --stage even more confusing ... maybe we should disallow --stage altogether when download-rustc is enabled?

Anyway, I'm not going to dig into it more tonight.

@jyn514
Copy link
Member Author

jyn514 commented May 18, 2023

maybe it's possible to do something clever like run x test core against the stage1 sysroot but all rustc and tool tests against the stage2 sysroot, and rely on using ci-rustc being the same in both stages to avoid issues

(actually even now i think this is a terrible idea, if i'm going to do something like it at all i'd introduce a new stage2-std-sysroot directory or something)

@jyn514
Copy link
Member Author

jyn514 commented May 18, 2023

actually hold on - the only reason I added the downloaded std to the sysroot in the first place was so ui-fulldeps tests would pass (since they need the std loaded at runtime to match the one the downloaded rustc was linked against). I think if we disable those tests when download-rustc is enabled we can avoid these issues altogether. That seems ok to me; ui-fulldeps tests are only testing the compiler itself, not std, and if download-rustc is enabled that means you haven't modified the compiler.

@jyn514
Copy link
Member Author

jyn514 commented May 20, 2023

the only reason I added the downloaded std to the sysroot in the first place was so ui-fulldeps tests would pass

this is not actually true - the version of std used for the downloaded rustc also needs to match the version used for the freshly built rustdoc:

error[E0460]: found possibly newer version of crate `std` which `thin_vec` depends on
  --> src/librustdoc/lib.rs:24:1
   |
24 | extern crate thin_vec;
   | ^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: perhaps that crate needs to be recompiled?
   = note: the following crate versions were found:
           crate `std`: /home/jyn/src/rust/build/x86_64-unknown-linux-gnu/ci-rustc-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-9768f62f7b152226.rlib
           crate `std`: /home/jyn/src/rust/build/x86_64-unknown-linux-gnu/ci-rustc-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-9768f62f7b152226.so
           crate `thin_vec`: /home/jyn/src/rust/build/x86_64-unknown-linux-gnu/ci-rustc-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/lib/libthin_vec-812d4d34d6de684d.rmeta

Previously, this would print a message for each doctest, which was quite
verbose:
```
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/exploit-mitigations.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/instrument-coverage.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/json.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/linker-plugin-lto.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/lints/groups.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/lints/index.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/lints/levels.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/aarch64-apple-ios-sim.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/aarch64-nintendo-switch-freestanding.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/apple-watchos.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/armeb-unknown-linux-gnueabi.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/armv4t-none-eabi.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/armv5te-none-eabi.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/armv6k-nintendo-3ds.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/armv7-sony-vita-newlibeabihf.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/armv7-unknown-linux-uclibceabi.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/armv7-unknown-linux-uclibceabihf.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/esp-idf.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/fuchsia.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/kmc-solid.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/loongarch-linux.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/m68k-unknown-linux-gnu.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/mipsel-sony-psx.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/nto-qnx.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/openbsd.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/openharmony.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/riscv32imac-unknown-xous-elf.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/unknown-uefi.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/wasm64-unknown-unknown.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/x86_64-fortanix-unknown-sgx.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/platform-support/x86_64-unknown-none.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/profile-guided-optimization.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/target-tier-policy.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/targets/custom.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/targets/index.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/tests/index.md
doc tests for: /home/jyn/src/rust/src/doc/rustc/src/what-is-rustc.md
```
@jyn514 jyn514 changed the title [wip] fix x test core when download-rustc is enabled Fix x test core when download-rustc is enabled May 31, 2023
@jyn514 jyn514 marked this pull request as ready for review May 31, 2023 04:13
@jyn514 jyn514 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 May 31, 2023
@rust-log-analyzer

This comment has been minimized.

This works by building std from source unconditionally instead of downloading it, for library tests only.

This was somewhat complicated because of the following requirements:
1. Unconditionally downloading libstd breaks `x test std`, because `coretests` requires the std loaded from the sysroot to match the std that's currently being tested.
2. Unconditionally rebuilding libstd breaks `x test ui-fulldeps librustdoc`, because anything loading `rustc_private` needs to use the same libstd that rustc was built with.

Break the knot by introducing a new `stage2-test-sysroot`, used only for testing `std` itself. This
holds a freshly compiled std, while `stage2` and `ci-rustc-sysroot` still hold the downloaded std.

This also extends the existing `cp_filtered` in Sysroot to apply to the `rust-std` component, not just the `rustc-dev` component.
@Mark-Simulacrum
Copy link
Member

Sad that all this complexity is needed, but seems OK -- I don't see obvious ways to get around it.

@bors r+

@bors
Copy link
Contributor

bors commented Jun 3, 2023

📌 Commit a80d69a has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@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 Jun 3, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 4, 2023
Fix `x test core` when download-rustc is enabled

Fix `x test --stage 2 core` when download-rustc is enabled

This works by building std from source instead of downloading it, for library tests only.

This was somewhat complicated because of the following requirements:
1. Unconditionally downloading libstd breaks `x test core`, because `coretests` requires the std loaded from the sysroot to match the std that's currently being tested.
2. Unconditionally rebuilding libstd breaks `x test ui-fulldeps librustdoc`, because anything loading `rustc_private` needs to use the same libstd that rustc was built with.

Break the knot by introducing a new `stage2-test-sysroot`, used only for testing `std` itself. This
holds a freshly compiled std, while `stage2` and `ci-rustc-sysroot` still hold the downloaded std.

This also extends the existing `cp_filtered` in Sysroot to apply to the `rust-std` component, not just the `rustc-dev` component, to avoid having both versions of std in `stage2-test-sysroot`.

Fixes rust-lang#110352.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 4, 2023
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#109093 (add `#[doc(alias="flatmap")]` to `Option::and_then`)
 - rust-lang#110701 (Fix `x test core` when download-rustc is enabled)
 - rust-lang#111982 (Revert "Enable incremental independent of stage")
 - rust-lang#112158 (Add portable-simd mention)
 - rust-lang#112172 (doc: improve explanation)
 - rust-lang#112178 (Fix bug where private item with intermediate doc hidden re-export was not inlined)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f2f0e6c into rust-lang:master Jun 4, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jun 4, 2023
@jyn514 jyn514 deleted the test-core branch June 7, 2023 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-download-rustc Area: The `rust.download-rustc` build option. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

x test core fails when download-rustc is enabled
5 participants