Revert "Copy bin/* and lib/*.dylib files to stage0-sysroot"

[albertlarsan68]

This reverts commit 6990ab9 (PR #107956).
Hopefully this reverts the right thing, first time I revert something.
r? @jyn514

[Kobzol]

@bors try @rust-timer queue

Testing if this fixes bootstrap measurement.

[bors]

⌛ Trying commit c55177e with merge 3f4f43fc7c52a1365acb650bde97b39a5ed1cf59...

[bors]

☀️ Try build successful - checks-actions
Build commit: 3f4f43fc7c52a1365acb650bde97b39a5ed1cf59 (3f4f43fc7c52a1365acb650bde97b39a5ed1cf59)

[jyn514]

apparently this wasn't even the problem :(

       thread 'main' panicked at 'self.symlink_file(&rustfmt_path, &legacy_rustfmt) failed with File exists (os error 17)', download.rs:346:17

I'm suspecting #107834; cc @zephaniahong

[albertlarsan68]

This PR reverts the PR that caused the problem to appear and break perf, but I think there should be a fix.
Let's see if perf is fixed by this revert, and if so, we can land the revert while the correct fix is done.

[albertlarsan68]

But I think it is the combination of those two PRs that is the problem, each PR in isolation is good I think.

[rust-timer]

Finished benchmarking commit (3f4f43fc7c52a1365acb650bde97b39a5ed1cf59): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.2%	[-1.2%, -1.2%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.3%	[-2.6%, -2.1%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.3%	[-2.6%, -2.1%]	2

Cycles

This benchmark run did not return any relevant results for this metric.

[zephaniahong]

Any actions required by me?

[Kobzol]

It seems like this didn't help, as there are no bootstrap timings. The failure is probably not shown, because it also failed before this PR I think.

[albertlarsan68]

This likely didn't fix it, because the rustc step was less than a minute (I was monitoring the status page)

Does the runner gets a fully new env each time, or is some FS data kept across executions?

[Kobzol]

The rust directory with the downloaded compiler is maybe cached? But it shouldn't affect anything I hope. I tried to revert the other PR in #108302, let's see if that helps. If not, we can also try to revert both of them at once.

[albertlarsan68]

This may be the problem, because it complains that the file already exists.
The fix would be to silently ignore this specific failure, or to make perf clean the FS across invocations.

[Kobzol]

Where is this file located? I tried to clean the build directory of the downloaded rust directory locally, and the error persisted:

thread 'main' panicked at 'self.symlink_file(&rustfmt_path, &legacy_rustfmt) failed with File exists (os error 17)', download.rs:346:17
stack backtrace:
   0: rust_begin_unwind
             at /rustc/9aa5c24b7d763fb98d998819571128ff2eb8a3ca/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/9aa5c24b7d763fb98d998819571128ff2eb8a3ca/library/core/src/panicking.rs:64:14
   2: bootstrap::download::<impl bootstrap::config::Config>::maybe_download_rustfmt
             at ./src/bootstrap/download.rs:346:17
   3: bootstrap::config::Config::initial_rustfmt
             at ./src/bootstrap/config.rs:1562:28
   4: bootstrap::Build::initial_rustfmt
             at ./src/bootstrap/lib.rs:313:17
   5: bootstrap::Build::build
             at ./src/bootstrap/lib.rs:658:18
   6: bootstrap::main
             at ./src/bootstrap/bin/main.rs:55:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/9aa5c24b7d763fb98d998819571128ff2eb8a3ca/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

(This might be the second error though, it's a bit confusing).

[zephaniahong]

@Kobzol The symlink file should be in ./build/host/rustfmt/bin

[Kobzol]

Aha, the problem is in another directory. From rustc-perf:

$ rm -rf rust/build
$ RUST_BACKTRACE=1 cargo run --bin collector bench_local --id dc89a803d64fb6172c8406996831353bee18c3a7 /home/kobzol/.rustup/toolchains/nightly-2023-02-18-x86_64-unknown-linux-gnu/bin/rustc --bench-rustc --iterations 1 --cargo /home/kobzol/.rustup/toolchains/nightly-2023-02-18-x86_64-unknown-linux-gnu/bin/cargo --include await

(examined with strace)
[pid 40260] symlink("/projects/personal/rust/rustc-perf/rust/build/x86_64-unknown-linux-gnu/rustfmt/bin/rustfmt", "/projects/personal/rust/rustc-perf/target/debug/rustfmt") = -1 EEXIST (File exists)

So the problem is that rustfmt already exists in target/debug. Why is it trying to write something into target/debug of rustc-perf though? 🤔

[albertlarsan68]

Does this has to do with (maybe) the fake Rustc shims?

[KittyBorgX]

Aha, the problem is in another directory. From rustc-perf:

$ rm -rf rust/build
$ RUST_BACKTRACE=1 cargo run --bin collector bench_local --id dc89a803d64fb6172c8406996831353bee18c3a7 /home/kobzol/.rustup/toolchains/nightly-2023-02-18-x86_64-unknown-linux-gnu/bin/rustc --bench-rustc --iterations 1 --cargo /home/kobzol/.rustup/toolchains/nightly-2023-02-18-x86_64-unknown-linux-gnu/bin/cargo --include await

(examined with strace)
[pid 40260] symlink("/projects/personal/rust/rustc-perf/rust/build/x86_64-unknown-linux-gnu/rustfmt/bin/rustfmt", "/projects/personal/rust/rustc-perf/target/debug/rustfmt") = -1 EEXIST (File exists)

So the problem is that rustfmt already exists in target/debug. Why is it trying to write something into target/debug of rustc-perf though? 🤔

From my knowledge, It should ideally write to build/host/stage0/bin/rustfmt
So the symlink should be:

build/host/rustfmt/bin/rustfmt -> build/host/stage0/bin/rustfmt

Would bisecting it and checking help in any way?

[lqd]

Does this has to do with (maybe) the fake Rustc shims?

I'd think so.

Why is it trying to write something into target/debug of rustc-perf though?

build.rustc is bootstrap-rustc in the rustc benchmark.

It's a local binary path, that makes bootstrap think initial_rustc is in ./target/{debug,release} when trying to symlink, and legacy_rustfmt is then thought to be in that local directory.

[jyn514]

#108302 fixed perf, right? Does that mean we can close this revert?

[Kobzol]

Yes, this can be closed now.