Skip to content

bootstrap minor improvements and clean-ups #128588

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

Merged
merged 3 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 9 additions & 25 deletions src/bootstrap/src/core/build_steps/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use object::BinaryFormat;

use crate::core::build_steps::doc::DocumentationFormat;
use crate::core::build_steps::tool::{self, Tool};
use crate::core::build_steps::vendor::default_paths_to_vendor;
use crate::core::build_steps::{compile, llvm};
use crate::core::builder::{Builder, Kind, RunConfig, ShouldRun, Step};
use crate::core::config::TargetSelection;
Expand Down Expand Up @@ -1016,35 +1017,18 @@ impl Step for PlainSourceTarball {
if builder.rust_info().is_managed_git_subrepository()
|| builder.rust_info().is_from_tarball()
{
// FIXME: This code looks _very_ similar to what we have in `src/core/build_steps/vendor.rs`
// perhaps it should be removed in favor of making `dist` perform the `vendor` step?

builder.require_and_update_all_submodules();

// Vendor all Cargo dependencies
let mut cmd = command(&builder.initial_cargo);
cmd.arg("vendor")
.arg("--versioned-dirs")
.arg("--sync")
.arg(builder.src.join("./src/tools/cargo/Cargo.toml"))
.arg("--sync")
.arg(builder.src.join("./src/tools/rust-analyzer/Cargo.toml"))
.arg("--sync")
.arg(builder.src.join("./compiler/rustc_codegen_cranelift/Cargo.toml"))
.arg("--sync")
.arg(builder.src.join("./compiler/rustc_codegen_gcc/Cargo.toml"))
.arg("--sync")
.arg(builder.src.join("./library/Cargo.toml"))
.arg("--sync")
.arg(builder.src.join("./src/bootstrap/Cargo.toml"))
.arg("--sync")
.arg(builder.src.join("./src/tools/opt-dist/Cargo.toml"))
.arg("--sync")
.arg(builder.src.join("./src/tools/rustc-perf/Cargo.toml"))
.arg("--sync")
.arg(builder.src.join("./src/tools/rustbook/Cargo.toml"))
// Will read the libstd Cargo.toml
// which uses the unstable `public-dependency` feature.
cmd.arg("vendor").arg("--versioned-dirs");

for p in default_paths_to_vendor(builder) {
cmd.arg("--sync").arg(p);
}

cmd
// Will read the libstd Cargo.toml which uses the unstable `public-dependency` feature.
.env("RUSTC_BOOTSTRAP", "1")
.current_dir(plain_dst_src);

Expand Down
19 changes: 15 additions & 4 deletions src/bootstrap/src/core/build_steps/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,11 @@ pub struct Link;
impl Step for Link {
type Output = ();
const DEFAULT: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.alias("link")
}

fn make_run(run: RunConfig<'_>) {
if run.builder.config.dry_run() {
return;
Expand All @@ -262,21 +264,30 @@ impl Step for Link {
}
fn run(self, builder: &Builder<'_>) -> Self::Output {
let config = &builder.config;

if config.dry_run() {
return;
}

if !rustup_installed(builder) {
println!("WARNING: `rustup` is not installed; Skipping `stage1` toolchain linking.");
return;
}

let stage_path =
["build", config.build.rustc_target_arg(), "stage1"].join(MAIN_SEPARATOR_STR);
if !rustup_installed(builder) {
eprintln!("`rustup` is not installed; cannot link `stage1` toolchain");
} else if stage_dir_exists(&stage_path[..]) && !config.dry_run() {

if stage_dir_exists(&stage_path[..]) && !config.dry_run() {
attempt_toolchain_link(builder, &stage_path[..]);
}
}
}

fn rustup_installed(builder: &Builder<'_>) -> bool {
command("rustup").arg("--version").run_capture_stdout(builder).is_success()
let mut rustup = command("rustup");
rustup.arg("--version");

rustup.allow_failure().run_always().run_capture_stdout(builder).is_success()
}

fn stage_dir_exists(stage_path: &str) -> bool {
Expand Down
7 changes: 1 addition & 6 deletions src/bootstrap/src/core/build_steps/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1091,18 +1091,13 @@ macro_rules! tool_extended {
}
}

// NOTE: tools need to be also added to `Builder::get_step_descriptions` in `builder.rs`
// to make `./x.py build <tool>` work.
tool_extended!((self, builder),
Cargofmt, "src/tools/rustfmt", "cargo-fmt", stable=true;
CargoClippy, "src/tools/clippy", "cargo-clippy", stable=true;
Clippy, "src/tools/clippy", "clippy-driver", stable=true, add_bins_to_sysroot = ["clippy-driver", "cargo-clippy"];
Miri, "src/tools/miri", "miri", stable=false, add_bins_to_sysroot = ["miri"];
CargoMiri, "src/tools/miri/cargo-miri", "cargo-miri", stable=true, add_bins_to_sysroot = ["cargo-miri"];
// FIXME: tool_std is not quite right, we shouldn't allow nightly features.
// But `builder.cargo` doesn't know how to handle ToolBootstrap in stages other than 0,
// and this is close enough for now.
Rls, "src/tools/rls", "rls", stable=true, tool_std=true;
Comment on lines -1102 to -1105
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an outdated FIXME we had for rust-demangler.

Rls, "src/tools/rls", "rls", stable=true;
Rustfmt, "src/tools/rustfmt", "rustfmt", stable=true, add_bins_to_sysroot = ["rustfmt", "cargo-fmt"];
);

Expand Down
32 changes: 22 additions & 10 deletions src/bootstrap/src/core/build_steps/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,26 @@ use crate::core::build_steps::tool::SUBMODULES_FOR_RUSTBOOK;
use crate::core::builder::{Builder, RunConfig, ShouldRun, Step};
use crate::utils::exec::command;

/// List of default paths used for vendoring for `x vendor` and dist tarballs.
pub fn default_paths_to_vendor(builder: &Builder<'_>) -> Vec<PathBuf> {
let mut paths = vec![];
for p in [
"src/tools/cargo/Cargo.toml",
"src/tools/rust-analyzer/Cargo.toml",
"compiler/rustc_codegen_cranelift/Cargo.toml",
"compiler/rustc_codegen_gcc/Cargo.toml",
"library/Cargo.toml",
"src/bootstrap/Cargo.toml",
"src/tools/rustbook/Cargo.toml",
"src/tools/rustc-perf/Cargo.toml",
"src/tools/opt-dist/Cargo.toml",
] {
paths.push(builder.src.join(p));
}

paths
}

#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub(crate) struct Vendor {
sync_args: Vec<PathBuf>,
Expand Down Expand Up @@ -42,16 +62,8 @@ impl Step for Vendor {
}

// Sync these paths by default.
for p in [
"src/tools/cargo/Cargo.toml",
"src/tools/rust-analyzer/Cargo.toml",
"compiler/rustc_codegen_cranelift/Cargo.toml",
"compiler/rustc_codegen_gcc/Cargo.toml",
"library/Cargo.toml",
"src/bootstrap/Cargo.toml",
"src/tools/rustbook/Cargo.toml",
] {
cmd.arg("--sync").arg(builder.src.join(p));
for p in default_paths_to_vendor(builder) {
cmd.arg("--sync").arg(p);
}

// Also sync explicitly requested paths.
Expand Down
14 changes: 5 additions & 9 deletions src/bootstrap/src/core/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1449,15 +1449,11 @@ impl<'a> Builder<'a> {
assert_eq!(target, compiler.host);
}

if self.config.rust_optimize.is_release() {
// FIXME: cargo bench/install do not accept `--release`
// and miri doesn't want it
Comment on lines -1453 to -1454
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem like a FIXME note; it's more like a regular note.

match cmd_kind {
Kind::Bench | Kind::Install | Kind::Miri | Kind::MiriSetup | Kind::MiriTest => {}
_ => {
cargo.arg("--release");
}
}
if self.config.rust_optimize.is_release() &&
// cargo bench/install do not accept `--release` and miri doesn't want it
!matches!(cmd_kind, Kind::Bench | Kind::Install | Kind::Miri | Kind::MiriSetup | Kind::MiriTest)
{
cargo.arg("--release");
}

// Remove make-related flags to ensure Cargo can correctly set things up
Expand Down
2 changes: 0 additions & 2 deletions src/bootstrap/src/core/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,8 +616,6 @@ impl Config {
};

// For the beta compiler, put special effort into ensuring the checksums are valid.
// FIXME: maybe we should do this for download-rustc as well? but it would be a pain to update
// this on each and every nightly ...
Comment on lines -619 to -620
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds painful and I think we will never be doing this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a future possibility, maybe distribute the hashes with the tarballs?
Or resurrect the signing of the artifacts...

let checksum = if should_verify {
let error = format!(
"src/stage0 doesn't contain a checksum for {url}. \
Expand Down
Loading