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

Bootstrap regression in 1.71.0 #114338

Open
lovesegfault opened this issue Aug 1, 2023 · 3 comments
Open

Bootstrap regression in 1.71.0 #114338

lovesegfault opened this issue Aug 1, 2023 · 3 comments
Assignees
Labels
C-bug Category: This is a bug. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@lovesegfault
Copy link
Contributor

If I look at the bins that are part of rustc's tarball in 1.70.0 I see this:

$ tar tvf old-release/rustc-1.70.0-aarch64-unknown-linux-gnu.tar.xz | rg 'rustc/bin/'
-rwxr-xr-x 12569/100       980 2023-07-22 14:15 rustc-1.70.0-aarch64-unknown-linux-gnu/rustc/bin/rust-gdb
-rwxr-xr-x 12569/100      1072 2023-07-22 14:15 rustc-1.70.0-aarch64-unknown-linux-gnu/rustc/bin/rust-lldb
-rwxr-xr-x 12569/100  13517832 2023-07-22 14:15 rustc-1.70.0-aarch64-unknown-linux-gnu/rustc/bin/rustdoc
-rwxr-xr-x 12569/100   2919232 2023-07-22 14:15 rustc-1.70.0-aarch64-unknown-linux-gnu/rustc/bin/rustc
-rwxr-xr-x 12569/100      2164 2023-07-22 14:15 rustc-1.70.0-aarch64-unknown-linux-gnu/rustc/bin/rust-gdbgui

But when I built Rust again in 1.71.0, and inspected the produced artifacts, I see:

$ tar tvf pre-patch/rustc-1.71.0-aarch64-unknown-linux-gnu.tar.xz | rg 'rustc/bin/'
-rwxr-xr-x 22314791/100    980 2023-07-31 20:34 rustc-1.71.0-aarch64-unknown-linux-gnu/rustc/bin/rust-gdb
-rwxr-xr-x 22314791/100   1072 2023-07-31 20:34 rustc-1.71.0-aarch64-unknown-linux-gnu/rustc/bin/rust-lldb
-rwxr-xr-x 22314791/100 13628504 2023-07-31 20:34 rustc-1.71.0-aarch64-unknown-linux-gnu/rustc/bin/rustdoc
-rwxr-xr-x 22314791/100  2919256 2023-07-31 20:34 rustc-1.71.0-aarch64-unknown-linux-gnu/rustc/bin/rustc
-rwxr-xr-x 22314791/100     2164 2023-07-31 20:34 rustc-1.71.0-aarch64-unknown-linux-gnu/rustc/bin/rust-gdbgui
-rwxr-xr-x 22314791/100  12758400 2023-07-31 20:34 rustc-1.71.0-aarch64-unknown-linux-gnu/rustc/bin/clippy-driver
-rwxr-xr-x 22314791/100   5902856 2023-07-31 20:34 rustc-1.71.0-aarch64-unknown-linux-gnu/rustc/bin/cargo-fmt
-rwxr-xr-x 22314791/100   6089640 2023-07-31 20:34 rustc-1.71.0-aarch64-unknown-linux-gnu/rustc/bin/rustfmt
-rwxr-xr-x 22314791/100   1815536 2023-07-31 20:34 rustc-1.71.0-aarch64-unknown-linux-gnu/rustc/bin/cargo-clippy

The difference being that cargo-fmt, rustfmt, cargo-clippy, and clippy-driver are now included.

I tracked this change down to this commit: 6d99d6a, which was a part of #110365 by @ozkanonur.

I validated my claim by applying this patch to Rust's source:

diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs
index b3791efaf58..70633f5b41c 100644
--- a/src/bootstrap/tool.rs
+++ b/src/bootstrap/tool.rs
@@ -790,7 +790,7 @@ fn run(mut $sel, $builder: &Builder<'_>) -> Option<PathBuf> {
                     allow_features: concat!($($allow_features)*),
                 })?;
 
-                if (false $(|| !$add_bins_to_sysroot.is_empty())?) && $sel.compiler.stage > 0 {
+                if false {
                     let bindir = $builder.sysroot($sel.compiler).join("bin");
                     t!(fs::create_dir_all(&bindir));

And upon re-building, inspecting the tarball again:

$ tar tvf post-patch/rustc-1.71.0-x86_64-unknown-linux-gnu.tar.xz | rg 'rustc/bin'
drwxr-xr-x 22314791/100      0 2023-08-01 00:16 rustc-1.71.0-x86_64-unknown-linux-gnu/rustc/bin
-rwxr-xr-x 22314791/100    980 2023-08-01 00:16 rustc-1.71.0-x86_64-unknown-linux-gnu/rustc/bin/rust-gdb
-rwxr-xr-x 22314791/100   1072 2023-08-01 00:16 rustc-1.71.0-x86_64-unknown-linux-gnu/rustc/bin/rust-lldb
-rwxr-xr-x 22314791/100 13462808 2023-08-01 00:16 rustc-1.71.0-x86_64-unknown-linux-gnu/rustc/bin/rustdoc
-rwxr-xr-x 22314791/100  2777920 2023-08-01 00:16 rustc-1.71.0-x86_64-unknown-linux-gnu/rustc/bin/rustc
-rwxr-xr-x 22314791/100     2164 2023-08-01 00:16 rustc-1.71.0-x86_64-unknown-linux-gnu/rustc/bin/rust-gdbgui

I spoke with @jyn514, who said:

ah. hmm. that's likely a real bug. dist::Rustc needs to only package rustc/rustdoc, not blindly copy everything currently in the sysroot

One thing that isn't clear to me is why do the officially produced artifacts served through static.rust-lang.org not suffer from this:

$ curl -sSLN https://static.rust-lang.org/dist/2023-07-13/rustc-1.71.0-x86_64-unknown-linux-gnu.tar.xz | tar tvJ | rg 'rustc/bin/'
-rwxr-xr-x 1000/1000       980 2023-07-12 00:58 rustc-1.71.0-x86_64-unknown-linux-gnu/rustc/bin/rust-gdb
-rwxr-xr-x 1000/1000      1072 2023-07-12 00:58 rustc-1.71.0-x86_64-unknown-linux-gnu/rustc/bin/rust-lldb
-rwxr-xr-x 1000/1000  13481400 2023-07-12 00:58 rustc-1.71.0-x86_64-unknown-linux-gnu/rustc/bin/rustdoc
-rwxr-xr-x 1000/1000   2821488 2023-07-12 00:58 rustc-1.71.0-x86_64-unknown-linux-gnu/rustc/bin/rustc
-rwxr-xr-x 1000/1000      2164 2023-07-12 00:58 rustc-1.71.0-x86_64-unknown-linux-gnu/rustc/bin/rust-gdbgui
@lovesegfault lovesegfault added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Aug 1, 2023
@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Aug 1, 2023
@lovesegfault
Copy link
Contributor Author

This is a stripped down version of the build script in use, which was unchanged between 1.70.0 and 1.71.0:

#!/bin/bash

# Fail when commands exit unsuccesfully.
set -o errexit

# Fail when using an undefined variable.
set -o nounset

# Fail if commands fail as part of a pipeline.
set -o pipefail

echo "Discovering build and source directory..."
RUST_CONFIGURE_ARGS=(
    # https://github.com/rust-lang/rust/blob/934624fe5f66ce3fb8abf0597a6deb079783335f/src/ci/run.sh#L50
    --disable-manage-submodules
    --enable-locked-deps
    --enable-cargo-native-static
    --set rust.codegen-units-std=1
    --set rust.remap-debuginfo
    --debuginfo-level-std=1
    # don't spend time _also_ building .gz files
    --dist-compression-formats=xz
    # avoid taking a runtime dependency on libstdc++
    --enable-llvm-static-stdcpp
    # disallow nightly features
    --release-channel=stable
    # don't access the internet to get dependencies
    --enable-vendor
    --local-rust-root "$BOOTSTRAP_INSTALL_DIR"
    # we're building with the same version of the compiler, so speed up a bit
    --enable-local-rebuild
    # https://github.com/rust-lang/rust/blob/673d0db5e393e9c64897005b470bfeb6d5aec61b/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile#L94
    # note that full-tools implies --extended
    --enable-full-tools
    --enable-sanitizers
    --enable-profiler
    # don't spend time generating docs, since those won't be patched
    --disable-docs
    --set llvm.thin-lto=true
    --set rust.jemalloc
    # limit the number of concurrent link jobs to avoid build host OOMs
    --set llvm.link-jobs=3
    # install to build/
    --prefix "$INSTALL_DIR"
    # and stick configuration under there too (note the relative path)
    --sysconfdir "etc"
    # we also need to make sure `--remap-path-prefix` is passed to rustc and
    # llvm builds to avoid the absolute builder path from ending up in the
    # final artifact. If we don't do this, re-builds of the exact same Rust
    # version generates different binaries entirely unnecessarily (i.e., when
    # there are no source-level differences). We also do it so that the paths
    # in the debug symbols in our binaries match those in the upstream release
    # artifacts, namely /rustc/<git commit of release>, which other tooling
    # might depend on.
    --set rust.remap-debuginfo=true
    --tools "cargo,clippy,rustfmt,llvm-tools,src,rustdoc,rust-analyzer-proc-macro-srv"
    --build "$targetTriple"
    --target "$targetTriple"
)
./configure "${RUST_CONFIGURE_ARGS[@]}"

echo "Patching out unneeded build targets..."
sed -i.bkp \
    -e '/dist::RustDev,/d' \
    -e '/dist::Extended,/d' \
    src/bootstrap/builder.rs

echo "Make build scripts respect LDFLAGS during bootstrap..."
mkdir -p .cargo
cat > .cargo/config <<EOF
target-applies-to-host = false

[host]
rustflags=["-Clink-args=$LDFLAGS"]

# include the vendor source replacement that x.py would normally generate with
# ./configure --enable-vendor:
[source.crates-io]
replace-with = 'vendored-sources'
registry = 'https://example.com'

[source.vendored-sources]
directory = '$PWD/vendor'
EOF

echo "Running rust build..."
python x.py dist

echo "Installing build artifacts..."
python x.py install

echo "Exporting rustup artifacts..."
ls -l build/dist/
mkdir -p "$BUILD_DIR"/rustup
cp -r build/dist/* "$BUILD_DIR"/rustup

@onur-ozkan
Copy link
Member

ah. hmm. that's likely a real bug. dist::Rustc needs to only package rustc/rustdoc, not blindly copy everything currently in the sysroot

What is bug? We are not blindly copying the entire sysroot.

I'm not sure if I understand the problem. Are we supposed to find these binaries in tarballs too, or is it something else?

@oli-obk oli-obk added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) and removed regression-untriaged Untriaged performance or correctness regression. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 2, 2023
@oli-obk oli-obk self-assigned this Aug 3, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Aug 8, 2023

Did some investigating. Just running x dist in a clean checkout does not contain the tools, but that is expected, as we are only building tools with build.extended=true.

> tar tvf rustc-1.73.0-dev-x86_64-unknown-linux-gnu.tar.gz | grep rustc/bin
drwxrwxr-x 1000/1000         0 2023-08-08 11:11 rustc-1.73.0-dev-x86_64-unknown-linux-gnu/rustc/bin
-rwxr-xr-x 1000/1000       980 2023-08-08 11:11 rustc-1.73.0-dev-x86_64-unknown-linux-gnu/rustc/bin/rust-gdb
-rwxr-xr-x 1000/1000      1072 2023-08-08 11:11 rustc-1.73.0-dev-x86_64-unknown-linux-gnu/rustc/bin/rust-lldb
-rwxr-xr-x 1000/1000  66622720 2023-08-08 11:11 rustc-1.73.0-dev-x86_64-unknown-linux-gnu/rustc/bin/rustdoc
-rwxrwxr-x 1000/1000     20944 2023-08-08 11:11 rustc-1.73.0-dev-x86_64-unknown-linux-gnu/rustc/bin/rustc
-rwxr-xr-x 1000/1000      2164 2023-08-08 11:11 rustc-1.73.0-dev-x86_64-unknown-linux-gnu/rustc/bin/rust-gdbgui

Now running with build.extended=true. Same result, but also found an orthogonal issue:

thread 'main' panicked at 'Error: File "/home/ubuntu/rustc/rust7/build/x86_64-unknown-linux-gnu/stage1/bin/rls" not found!', lib.rs:1683:13
stack backtrace:
   0: rust_begin_unwind
             at /rustc/7ba605cd90d6467bda469d1cd2148ac32ea562bf/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/7ba605cd90d6467bda469d1cd2148ac32ea562bf/library/core/src/panicking.rs:67:14
   2: bootstrap::Build::install
             at ./src/bootstrap/lib.rs:1683:13
   3: bootstrap::tarball::Tarball::add_file
             at ./src/bootstrap/tarball.rs:181:9
   4: <bootstrap::dist::Rls as bootstrap::builder::Step>::run
             at ./src/bootstrap/dist.rs:1128:9
   5: bootstrap::builder::Builder::ensure
             at ./src/bootstrap/builder.rs:2089:23
   6: <bootstrap::dist::Rls as bootstrap::builder::Step>::make_run
             at ./src/bootstrap/dist.rs:1107:9
   7: bootstrap::builder::StepDescription::maybe_run
             at ./src/bootstrap/builder.rs:315:13
   8: bootstrap::builder::StepDescription::run
             at ./src/bootstrap/builder.rs:356:21
   9: bootstrap::builder::Builder::run_step_descriptions
             at ./src/bootstrap/builder.rs:961:9
  10: bootstrap::builder::Builder::execute_cli
             at ./src/bootstrap/builder.rs:942:9
  11: bootstrap::Build::build
             at ./src/bootstrap/lib.rs:704:13
  12: bootstrap::main
             at ./src/bootstrap/bin/main.rs:74:5
  13: core::ops::function::FnOnce::call_once
             at /rustc/7ba605cd90d6467bda469d1cd2148ac32ea562bf/library/core/src/ops/function.rs:250:5

@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

No branches or pull requests

5 participants