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

x test core fails when download-rustc is enabled #110352

Closed
jyn514 opened this issue Apr 15, 2023 · 3 comments · Fixed by #110701
Closed

x test core fails when download-rustc is enabled #110352

jyn514 opened this issue Apr 15, 2023 · 3 comments · Fixed by #110701
Labels
A-download-rustc Area: The `rust.download-rustc` build option. A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jyn514
Copy link
Member

jyn514 commented Apr 15, 2023

I tried this code: ./configure --set download-rustc && x test core

I expected to see this happen: All tests pass

Instead, this happened:

Testing {core} stage2 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
   Compiling core v0.0.0 (/home/jyn/src/rust/library/core)
error[E0773]: attempted to define built-in macro more than once
    --> /home/jyn/src/rust/library/core/src/macros/mod.rs:1310:5
     |
1310 |     macro_rules! cfg {
     |     ^^^^^^^^^^^^^^^^
     |
note: previously defined here
    --> /rustc/ce1073ba9d894b2e351b2a85fcd39f2c99b78974/library/core/src/macros/mod.rs:1310:5

error[E0152]: duplicate lang item in crate `core` (which `corebenches` depends on): `sized`.
  |
  = note: the lang item is first defined in crate `core` (which `std` depends on)
  = note: first definition in `core` loaded from /home/jyn/src/rust/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-05898138a596088a.rlib
  = note: second definition in `core` loaded from /home/jyn/src/rust/build/x86_64-unknown-linux-gnu/stage2-std/x86_64-unknown-linux-gnu/release/deps/libcore-83f7e02d8b0410a3.rlib

Some errors have detailed explanations: E0152, E0277, E0307, E0773.
error: could not compile `core` (test "coretests") due to 372 previous errors; 10 warnings emitted

Meta

HEAD is branched from ce1073b

@jyn514 jyn514 added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) C-bug Category: This is a bug. A-download-rustc Area: The `rust.download-rustc` build option. A-testsuite Area: The testsuite used to check the correctness of rustc labels Apr 15, 2023
@jyn514
Copy link
Member Author

jyn514 commented Apr 21, 2023

Here is where std is loaded from in stage 0:

INFO rustc_metadata::locator lib candidate: /home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/stage0-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-7217c7a623948b1f.rlib
INFO rustc_metadata::locator lib candidate: /home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/stage0-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-7217c7a623948b1f.so
INFO rustc_metadata::locator rlib reading metadata from: /home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/stage0-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-7217c7a623948b1f.rlib

which requires loading core from the sysroot:

INFO rustc_metadata::creader resolving dep crate core hash: `34298ac765912203` extra filename: `-999232a2b2133b8a`
INFO rustc_metadata::creader resolving crate `core`
INFO rustc_metadata::creader falling back to a load
INFO rustc_metadata::locator lib candidate: /home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/libcore-999232a2b2133b8a.rmeta
INFO rustc_metadata::locator lib candidate: /home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/stage0-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-999232a2b2133b8a.rlib
INFO rustc_metadata::locator rmeta reading metadata from: /home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/libcore-999232a2b2133b8a.rmeta
INFO rustc_metadata::creader register crate `core` (cnum = 2. private_dep = false)

and in stage 2:

INFO rustc_metadata::creader resolving crate `std`
INFO rustc_metadata::creader falling back to a load
INFO rustc_metadata::locator lib candidate: /home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-cd20bc9d771091ea.so
INFO rustc_metadata::locator lib candidate: /home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-cd20bc9d771091ea.rlib
INFO rustc_metadata::locator rlib reading metadata from: /home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-cd20bc9d771091ea.rlib
INFO rustc_metadata::creader resolving dep crate core hash: `ca783ee9d7dc024f` extra filename: `-05898138a596088a`
INFO rustc_metadata::creader falling back to a load
INFO rustc_metadata::locator lib candidate: /home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-05898138a596088a.rlib
INFO rustc_metadata::locator rlib reading metadata from: /home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-05898138a596088a.rlib
INFO rustc_metadata::creader register crate `core` (cnum = 2. private_dep = false)

So far so good, core/std are both loaded from the sysroot at first. But then, core gets loaded again in stage 2:

INFO rustc_metadata::creader resolving crate `rustc_std_workspace_core`
INFO rustc_metadata::creader resolving dep crate core hash: `ca783ee9d7dc024f` extra filename: `-05898138a596088a`
INFO rustc_metadata::creader resolving dep crate unicode_width hash: `1ee9c73903764d5e` extra filename: `-e2c026a3fbc51a2a`
INFO rustc_metadata::creader resolving crate `unicode_width`
INFO rustc_metadata::creader resolving crate `core`
INFO rustc_metadata::creader resolving crate `core`
INFO rustc_metadata::creader resolving dep crate compiler_builtins hash: `8adb4d62f5b608ec` extra filename: `-35b8a4bd2de4e62e`
INFO rustc_metadata::creader falling back to a load
INFO rustc_metadata::creader resolving crate `compiler_builtins`
INFO rustc_metadata::creader resolving dep crate core hash: `ca783ee9d7dc024f` extra filename: `-05898138a596088a`
INFO rustc_metadata::creader resolving crate `core`
INFO rustc_metadata::creader falling back to a load
INFO rustc_metadata::locator rlib reading metadata from: /home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/stage2-std/x86_64-unknown-linux-gnu/release/deps/libcore-ac23cab5970c56fe.rlib

I think what's going wrong here is that rustc-std-workspace-core explicitly has #![no_core] and a path dependency, so it uses the version in the workspace instead of from the sysroot. In stage0 this works ok, because libcore.rlib is exactly the same in stage0-std and stage0-sysroot. But in stage2, the two differ, and resolve gives a duplicate macro error.

@jyn514
Copy link
Member Author

jyn514 commented Apr 21, 2023

I had the idea to not download rust-std at all when download-rustc is enabled, only rustc, and unconditionally build std from source. Unfortunately that doesn't work because Sysroot returns the same thing for both stages 0 and 1:

rust/src/bootstrap/compile.rs

Lines 1247 to 1268 in da48140

impl Step for Sysroot {
type Output = Interned<PathBuf>;
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.never()
}
/// Returns the sysroot for the `compiler` specified that *this build system
/// generates*.
///
/// That is, the sysroot for the stage0 compiler is not what the compiler
/// thinks it is by default, but it's the same as the default for stages
/// 1-3.
fn run(self, builder: &Builder<'_>) -> Interned<PathBuf> {
let compiler = self.compiler;
let host_dir = builder.out.join(&compiler.host.triple);
let sysroot_dir = |stage| {
if stage == 0 {
host_dir.join("stage0-sysroot")
} else if builder.download_rustc() && compiler.stage != builder.top_stage {
host_dir.join("ci-rustc-sysroot")

so we end up copying two different stages of std into the same ci-rustc-sysroot, which gives an error:

error[E0464]: multiple candidates for `rlib` dependency `core` found
  |
  = note: candidate #1: /home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/li
bcore-05898138a596088a.rlib
  = note: candidate #2: /home/jyn/src/rust3/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/li
bcore-ac23cab5970c56fe.rlib

I think the right fix looks something like "in impl Step for Std, build std from source instead of using the download-rustc artifacts BUT only if compiler.stage == builder.top_stage.

anyway, I'm out of time for tonight.

@jyn514
Copy link
Member Author

jyn514 commented Apr 22, 2023

I think the right fix looks something like "in impl Step for Std, build std from source instead of using the download-rustc artifacts BUT only if compiler.stage == builder.top_stage.

For this to work, we need to avoid copying rust-std into stage2 in the Sysroot step unless it's asked for, just how we avoided it for rustc-dev in #110229.

bors added a commit to rust-lang-ci/rust that referenced this issue May 13, 2023
…apped, r=cjgillot

Apply simulate-remapped-rust-src-base even if remap-debuginfo is set in config.toml

This is really a mess. Here is the situation before this change:

- UI tests depend on not having `rust-src` available. In particular, <https://github.com/rust-lang/rust/blob/3f374128ee3924514aacadf96479e17fee8f9903/tests/ui/tuple/wrong_argument_ice.stderr#L7-L8> is depending on the `note` being a single line and not showing the source code.
- When `download-rustc` is disabled, we pass `-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX` `-Ztranslate-remapped-path-to-local-path=no`, which changes the diagnostic to something like `  --> /rustc/FAKE_PREFIX/library/alloc/src/collections/vec_deque/mod.rs:1657:12`
- When `download-rustc` is enabled, we still pass those flags, but they no longer have an effect. Instead rustc emits diagnostic paths like this: `  --> /rustc/39c6804b92aa202369e402525cee329556bc1db0/library/alloc/src/collections/vec_deque/mod.rs:1657:12`. Notice how there's a real commit and not `FAKE_PREFIX`. This happens because we set `CFG_VIRTUAL_RUST_SOURCE_BASE_DIR` during bootstrapping for CI artifacts, and rustc previously didn't allow for `simulate-remapped` to affect paths that had already been remapped.
- Pietro noticed this and decided the right thing was to normalize `/rustc/<commit>` to `$SRC_DIR` in compiletest: rust-lang@470423c
- After my change to `x test core`, which rebuilds stage 2 std from source so `build/stage2-std` and `build/stage2` use the same `.rlib` metadata, the compiler suddenly notices it has sources for `std` available and prints those in the diagnostic, causing the test to fail.

This changes `simulate-remapped-rust-src-base` to support remapping paths that have already been remapped, unblocking download-rustc.

Unfortunately, although this fixes the specific problem for
download-rustc, it doesn't seem to affect all the compiler's
diagnostics. In particular, various `mir-opt` tests are failing to
respect `simulate-remapped-path-prefix` (I looked into fixing this but
it seems non-trivial). As a result, we can't remove the normalization in
compiletest that maps `/rustc/<commit>` to `$SRC_DIR`, so this change is
currently untested anywhere except locally.

You can test this locally yourself by setting `rust.remap-debuginfo = true`, running any UI test with `ERROR` annotations, then rerunning the test manually with a dev toolchain to verify it prints `/rustc/FAKE_PREFIX`, not `/rustc/1.71.0`.

Helps with rust-lang#110352.
RalfJung pushed a commit to RalfJung/miri that referenced this issue May 16, 2023
…cjgillot

Apply simulate-remapped-rust-src-base even if remap-debuginfo is set in config.toml

This is really a mess. Here is the situation before this change:

- UI tests depend on not having `rust-src` available. In particular, <https://github.com/rust-lang/rust/blob/3f374128ee3924514aacadf96479e17fee8f9903/tests/ui/tuple/wrong_argument_ice.stderr#L7-L8> is depending on the `note` being a single line and not showing the source code.
- When `download-rustc` is disabled, we pass `-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX` `-Ztranslate-remapped-path-to-local-path=no`, which changes the diagnostic to something like `  --> /rustc/FAKE_PREFIX/library/alloc/src/collections/vec_deque/mod.rs:1657:12`
- When `download-rustc` is enabled, we still pass those flags, but they no longer have an effect. Instead rustc emits diagnostic paths like this: `  --> /rustc/39c6804b92aa202369e402525cee329556bc1db0/library/alloc/src/collections/vec_deque/mod.rs:1657:12`. Notice how there's a real commit and not `FAKE_PREFIX`. This happens because we set `CFG_VIRTUAL_RUST_SOURCE_BASE_DIR` during bootstrapping for CI artifacts, and rustc previously didn't allow for `simulate-remapped` to affect paths that had already been remapped.
- Pietro noticed this and decided the right thing was to normalize `/rustc/<commit>` to `$SRC_DIR` in compiletest: rust-lang/rust@470423c
- After my change to `x test core`, which rebuilds stage 2 std from source so `build/stage2-std` and `build/stage2` use the same `.rlib` metadata, the compiler suddenly notices it has sources for `std` available and prints those in the diagnostic, causing the test to fail.

This changes `simulate-remapped-rust-src-base` to support remapping paths that have already been remapped, unblocking download-rustc.

Unfortunately, although this fixes the specific problem for
download-rustc, it doesn't seem to affect all the compiler's
diagnostics. In particular, various `mir-opt` tests are failing to
respect `simulate-remapped-path-prefix` (I looked into fixing this but
it seems non-trivial). As a result, we can't remove the normalization in
compiletest that maps `/rustc/<commit>` to `$SRC_DIR`, so this change is
currently untested anywhere except locally.

You can test this locally yourself by setting `rust.remap-debuginfo = true`, running any UI test with `ERROR` annotations, then rerunning the test manually with a dev toolchain to verify it prints `/rustc/FAKE_PREFIX`, not `/rustc/1.71.0`.

Helps with rust-lang/rust#110352.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue 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 bors closed this as completed in f2f0e6c Jun 4, 2023
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 20, 2024
…cjgillot

Apply simulate-remapped-rust-src-base even if remap-debuginfo is set in config.toml

This is really a mess. Here is the situation before this change:

- UI tests depend on not having `rust-src` available. In particular, <https://github.com/rust-lang/rust/blob/3f374128ee3924514aacadf96479e17fee8f9903/tests/ui/tuple/wrong_argument_ice.stderr#L7-L8> is depending on the `note` being a single line and not showing the source code.
- When `download-rustc` is disabled, we pass `-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX` `-Ztranslate-remapped-path-to-local-path=no`, which changes the diagnostic to something like `  --> /rustc/FAKE_PREFIX/library/alloc/src/collections/vec_deque/mod.rs:1657:12`
- When `download-rustc` is enabled, we still pass those flags, but they no longer have an effect. Instead rustc emits diagnostic paths like this: `  --> /rustc/39c6804b92aa202369e402525cee329556bc1db0/library/alloc/src/collections/vec_deque/mod.rs:1657:12`. Notice how there's a real commit and not `FAKE_PREFIX`. This happens because we set `CFG_VIRTUAL_RUST_SOURCE_BASE_DIR` during bootstrapping for CI artifacts, and rustc previously didn't allow for `simulate-remapped` to affect paths that had already been remapped.
- Pietro noticed this and decided the right thing was to normalize `/rustc/<commit>` to `$SRC_DIR` in compiletest: rust-lang/rust@470423c
- After my change to `x test core`, which rebuilds stage 2 std from source so `build/stage2-std` and `build/stage2` use the same `.rlib` metadata, the compiler suddenly notices it has sources for `std` available and prints those in the diagnostic, causing the test to fail.

This changes `simulate-remapped-rust-src-base` to support remapping paths that have already been remapped, unblocking download-rustc.

Unfortunately, although this fixes the specific problem for
download-rustc, it doesn't seem to affect all the compiler's
diagnostics. In particular, various `mir-opt` tests are failing to
respect `simulate-remapped-path-prefix` (I looked into fixing this but
it seems non-trivial). As a result, we can't remove the normalization in
compiletest that maps `/rustc/<commit>` to `$SRC_DIR`, so this change is
currently untested anywhere except locally.

You can test this locally yourself by setting `rust.remap-debuginfo = true`, running any UI test with `ERROR` annotations, then rerunning the test manually with a dev toolchain to verify it prints `/rustc/FAKE_PREFIX`, not `/rustc/1.71.0`.

Helps with rust-lang/rust#110352.
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
…cjgillot

Apply simulate-remapped-rust-src-base even if remap-debuginfo is set in config.toml

This is really a mess. Here is the situation before this change:

- UI tests depend on not having `rust-src` available. In particular, <https://github.com/rust-lang/rust/blob/3f374128ee3924514aacadf96479e17fee8f9903/tests/ui/tuple/wrong_argument_ice.stderr#L7-L8> is depending on the `note` being a single line and not showing the source code.
- When `download-rustc` is disabled, we pass `-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX` `-Ztranslate-remapped-path-to-local-path=no`, which changes the diagnostic to something like `  --> /rustc/FAKE_PREFIX/library/alloc/src/collections/vec_deque/mod.rs:1657:12`
- When `download-rustc` is enabled, we still pass those flags, but they no longer have an effect. Instead rustc emits diagnostic paths like this: `  --> /rustc/39c6804b92aa202369e402525cee329556bc1db0/library/alloc/src/collections/vec_deque/mod.rs:1657:12`. Notice how there's a real commit and not `FAKE_PREFIX`. This happens because we set `CFG_VIRTUAL_RUST_SOURCE_BASE_DIR` during bootstrapping for CI artifacts, and rustc previously didn't allow for `simulate-remapped` to affect paths that had already been remapped.
- Pietro noticed this and decided the right thing was to normalize `/rustc/<commit>` to `$SRC_DIR` in compiletest: rust-lang/rust@470423c
- After my change to `x test core`, which rebuilds stage 2 std from source so `build/stage2-std` and `build/stage2` use the same `.rlib` metadata, the compiler suddenly notices it has sources for `std` available and prints those in the diagnostic, causing the test to fail.

This changes `simulate-remapped-rust-src-base` to support remapping paths that have already been remapped, unblocking download-rustc.

Unfortunately, although this fixes the specific problem for
download-rustc, it doesn't seem to affect all the compiler's
diagnostics. In particular, various `mir-opt` tests are failing to
respect `simulate-remapped-path-prefix` (I looked into fixing this but
it seems non-trivial). As a result, we can't remove the normalization in
compiletest that maps `/rustc/<commit>` to `$SRC_DIR`, so this change is
currently untested anywhere except locally.

You can test this locally yourself by setting `rust.remap-debuginfo = true`, running any UI test with `ERROR` annotations, then rerunning the test manually with a dev toolchain to verify it prints `/rustc/FAKE_PREFIX`, not `/rustc/1.71.0`.

Helps with rust-lang/rust#110352.
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. A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant