From 6a3d223213bfaa8e67a9a52c603f06a60cd0cd54 Mon Sep 17 00:00:00 2001 From: ozkanonur Date: Tue, 28 Mar 2023 22:46:50 +0300 Subject: [PATCH] refactor `src/tools/x` Signed-off-by: ozkanonur --- src/bootstrap/bin/bootstrap-shim.rs | 56 ++++++++- src/bootstrap/config.rs | 21 ++-- src/bootstrap/config/tests.rs | 8 +- src/bootstrap/dist.rs | 1 - src/bootstrap/download.rs | 188 ++++++++++++++-------------- src/bootstrap/lib.rs | 4 +- src/bootstrap/min_config.rs | 18 +-- src/tools/x/src/main.rs | 30 +++-- 8 files changed, 190 insertions(+), 136 deletions(-) diff --git a/src/bootstrap/bin/bootstrap-shim.rs b/src/bootstrap/bin/bootstrap-shim.rs index d9a92ecda03fe..d44ca8f994716 100644 --- a/src/bootstrap/bin/bootstrap-shim.rs +++ b/src/bootstrap/bin/bootstrap-shim.rs @@ -1,9 +1,56 @@ -use std::{env, process::Command}; +use std::{ + env, io, + process::{self, Command, ExitStatus}, +}; use bootstrap::{Flags, MinimalConfig}; #[path = "../../../src/tools/x/src/main.rs"] -mod run_python; +mod x; + +/// We are planning to exclude python logic from x script by executing bootstrap-shim +/// immediately. Since `find_and_run_available_bootstrap_script` executes x script first, +/// any changes on bootstrap will not be seen. To prevent this problem, in bootstrap-shim +/// we want to call the python script directly. +fn find_and_run_py_bootstrap_script() { + #[cfg(unix)] + fn exec_or_status(command: &mut Command) -> io::Result { + use std::os::unix::process::CommandExt; + Err(command.exec()) + } + + #[cfg(not(unix))] + fn exec_or_status(command: &mut Command) -> io::Result { + command.status() + } + + let current_path = match env::current_dir() { + Ok(dir) => dir, + Err(err) => { + eprintln!("Failed to get current directory: {err}"); + process::exit(1); + } + }; + + for dir in current_path.ancestors() { + let candidate = dir.join("x.py"); + if candidate.exists() { + let mut cmd: Command; + cmd = Command::new(x::python()); + cmd.arg(&candidate).args(env::args().skip(1)).current_dir(dir); + let result = exec_or_status(&mut cmd); + + match result { + Err(error) => { + eprintln!("Failed to invoke `{:?}`: {}", cmd, error); + } + Ok(status) => { + process::exit(status.code().unwrap_or(1)); + } + } + } + } +} fn main() { let args = env::args().skip(1).collect::>(); @@ -15,16 +62,17 @@ fn main() { let bootstrap_bin = if let Some(commit) = last_modified_bootstrap_commit(&config) { config.download_bootstrap(&commit) } else { - return run_python::main(); + return find_and_run_py_bootstrap_script(); }; let args: Vec<_> = std::env::args().skip(1).collect(); + println!("Running pre-compiled bootstrap binary"); Command::new(bootstrap_bin).args(args).status().expect("failed to spawn bootstrap binairy"); } fn last_modified_bootstrap_commit(config: &MinimalConfig) -> Option { config.last_modified_commit( - &["src/bootstrap", "src/tools/build_helper"], + &["src/bootstrap", "src/tools/build_helper", "src/tools/x"], "download-bootstrap", true, ) diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index c19c5f078b17b..799c258675905 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -84,7 +84,6 @@ pub struct Config { pub jobs: Option, pub cmd: Subcommand, pub incremental: bool, - pub dry_run: DryRun, /// Arguments appearing after `--` to be forwarded to tools, /// e.g. `--fix-broken` or test arguments. pub free_args: Vec, @@ -391,7 +390,7 @@ impl Target { /// `Config` structure. #[derive(Deserialize, Default)] #[serde(deny_unknown_fields, rename_all = "kebab-case")] -pub struct TomlConfig { +struct TomlConfig { changelog_seen: Option, build: Option, install: Option, @@ -745,17 +744,19 @@ impl Config { config } - pub fn parse(args: &[String], custom_toml_config: Option) -> Config { + pub fn parse(args: &[String], custom_toml_config: Option<&str>) -> Config { let flags = Flags::parse(&args); let mut config = Config::default_opts(); - let mut toml: TomlConfig = custom_toml_config.unwrap_or_else(|| { + let mut toml: TomlConfig = if let Some(custom_toml_config) = custom_toml_config { + toml::from_str(custom_toml_config).unwrap() + } else { set_cfg_path_and_return_toml_cfg( config.src.clone(), flags.config.clone(), &mut config.config, ) - }); + }; config.minimal_config = MinimalConfig::parse(&flags, toml.build.clone()); @@ -787,11 +788,6 @@ impl Config { crate::detail_exit(1); } - let build = toml.build.clone().unwrap_or_default(); - if let Some(file_build) = build.build.as_ref() { - config.build = TargetSelection::from_user(file_build); - }; - if let Some(include) = &toml.profile { let mut include_path = config.src.clone(); include_path.push("src"); @@ -804,6 +800,11 @@ impl Config { config.changelog_seen = toml.changelog_seen; + let build = toml.build.unwrap_or_default(); + if let Some(file_build) = build.build { + config.build = TargetSelection::from_user(&file_build); + }; + set(&mut config.out, flags.build_dir.or_else(|| build.build_dir.map(PathBuf::from))); // NOTE: Bootstrap spawns various commands with different working directories. // To avoid writing to random places on the file system, `config.out` needs to be an absolute path. diff --git a/src/bootstrap/config/tests.rs b/src/bootstrap/config/tests.rs index 2e71ba045ffe0..7eb3e1000240a 100644 --- a/src/bootstrap/config/tests.rs +++ b/src/bootstrap/config/tests.rs @@ -1,12 +1,8 @@ -use super::{Config, TomlConfig}; +use super::Config; use std::{env, path::Path}; -fn toml(config: &str) -> TomlConfig { - toml::from_str(config).unwrap() -} - fn parse(config: &str) -> Config { - Config::parse(&["check".to_owned(), "--config=/does/not/exist".to_owned()], Some(toml(config))) + Config::parse(&["check".to_owned(), "--config=/does/not/exist".to_owned()], Some(config)) } #[test] diff --git a/src/bootstrap/dist.rs b/src/bootstrap/dist.rs index d727f23b22fc0..40b1b9f49075e 100644 --- a/src/bootstrap/dist.rs +++ b/src/bootstrap/dist.rs @@ -2312,7 +2312,6 @@ impl Step for BootstrapShim { const ONLY_HOSTS: bool = true; fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { - // Create this even with only `dist bootstrap` to avoid having to update all CI builders. run.alias("bootstrap-shim") } diff --git a/src/bootstrap/download.rs b/src/bootstrap/download.rs index 6b33374e55472..fe41d1297378f 100644 --- a/src/bootstrap/download.rs +++ b/src/bootstrap/download.rs @@ -11,8 +11,8 @@ use once_cell::sync::OnceCell; use xz2::bufread::XzDecoder; use crate::{ - min_config::RustfmtMetadata, llvm::detect_llvm_sha, + min_config::RustfmtMetadata, t, util::{check_run, exe, output, program_out_of_date, try_run}, Config, MinimalConfig, @@ -343,6 +343,99 @@ enum DownloadSource { Dist, } +impl MinimalConfig { + pub fn download_bootstrap(&self, commit: &str) -> PathBuf { + self.verbose(&format!("downloading bootstrap from CI (commit {commit})")); + let host = self.build.triple; + let bin_root = self.out.join(host).join("bootstrap"); + let stamp = bin_root.join(".bootstrap-stamp"); + let bootstrap_bin = bin_root.join("bin").join("bootstrap"); + + if !bootstrap_bin.exists() || program_out_of_date(&stamp, commit) { + let version = self.git_artifact_version_part(commit); + let filename = format!("bootstrap-{version}-{host}.tar.xz"); + self.download_component(DownloadSource::CI, filename, "bootstrap", commit, ""); + if self.should_fix_bins_and_dylibs() { + self.fix_bin_or_dylib(&bootstrap_bin); + } + t!(fs::write(stamp, commit)); + } + + bootstrap_bin + } + + fn download_component( + &self, + mode: DownloadSource, + filename: String, + prefix: &str, + key: &str, + destination: &str, + ) { + let cache_dst = self.out.join("cache"); + let cache_dir = cache_dst.join(key); + if !cache_dir.exists() { + t!(fs::create_dir_all(&cache_dir)); + } + + let bin_root = self.out.join(self.build.triple).join(destination); + let tarball = cache_dir.join(&filename); + let (base_url, url, should_verify) = match mode { + DownloadSource::CI => ( + self.stage0_metadata.config.artifacts_server.clone(), + format!("{key}/{filename}"), + false, + ), + DownloadSource::Dist => { + let dist_server = env::var("RUSTUP_DIST_SERVER") + .unwrap_or(self.stage0_metadata.config.dist_server.to_string()); + // NOTE: make `dist` part of the URL because that's how it's stored in src/stage0.json + (dist_server, format!("dist/{key}/{filename}"), true) + } + }; + + // 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 ... + let checksum = if should_verify { + let error = format!( + "src/stage0.json doesn't contain a checksum for {url}. \ + Pre-built artifacts might not be available for this \ + target at this time, see https://doc.rust-lang.org/nightly\ + /rustc/platform-support.html for more information." + ); + let sha256 = self.stage0_metadata.checksums_sha256.get(&url).expect(&error); + if tarball.exists() { + if self.verify(&tarball, sha256) { + self.unpack(&tarball, &bin_root, prefix); + return; + } else { + self.verbose(&format!( + "ignoring cached file {} due to failed verification", + tarball.display() + )); + self.remove(&tarball); + } + } + Some(sha256) + } else if tarball.exists() { + self.unpack(&tarball, &bin_root, prefix); + return; + } else { + None + }; + + self.download_file(&format!("{base_url}/{url}"), &tarball, ""); + if let Some(sha256) = checksum { + if !self.verify(&tarball, sha256) { + panic!("failed to verify {}", tarball.display()); + } + } + + self.unpack(&tarball, &bin_root, prefix); + } +} + /// Functions that are only ever called once, but named for clarify and to avoid thousand-line functions. impl Config { pub(crate) fn maybe_download_rustfmt(&self) -> Option { @@ -509,96 +602,3 @@ impl Config { self.unpack(&tarball, &llvm_root, "rust-dev"); } } - -impl MinimalConfig { - pub fn download_bootstrap(&self, commit: &str) -> PathBuf { - self.verbose(&format!("downloading bootstrap from CI (commit {commit})")); - let host = self.build.triple; - let bin_root = self.out.join(host).join("bootstrap"); - let stamp = bin_root.join(".bootstrap-stamp"); - let bootstrap_bin = bin_root.join("bin").join("bootstrap"); - - if !bootstrap_bin.exists() || program_out_of_date(&stamp, commit) { - let version = self.git_artifact_version_part(commit); - let filename = format!("bootstrap-{version}-{host}.tar.xz"); - self.download_component(DownloadSource::CI, filename, "bootstrap", commit, ""); - if self.should_fix_bins_and_dylibs() { - self.fix_bin_or_dylib(&bootstrap_bin); - } - t!(fs::write(stamp, commit)); - } - - bootstrap_bin - } - - fn download_component( - &self, - mode: DownloadSource, - filename: String, - prefix: &str, - key: &str, - destination: &str, - ) { - let cache_dst = self.out.join("cache"); - let cache_dir = cache_dst.join(key); - if !cache_dir.exists() { - t!(fs::create_dir_all(&cache_dir)); - } - - let bin_root = self.out.join(self.build.triple).join(destination); - let tarball = cache_dir.join(&filename); - let (base_url, url, should_verify) = match mode { - DownloadSource::CI => ( - self.stage0_metadata.config.artifacts_server.clone(), - format!("{key}/{filename}"), - false, - ), - DownloadSource::Dist => { - let dist_server = env::var("RUSTUP_DIST_SERVER") - .unwrap_or(self.stage0_metadata.config.dist_server.to_string()); - // NOTE: make `dist` part of the URL because that's how it's stored in src/stage0.json - (dist_server, format!("dist/{key}/{filename}"), true) - } - }; - - // 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 ... - let checksum = if should_verify { - let error = format!( - "src/stage0.json doesn't contain a checksum for {url}. \ - Pre-built artifacts might not be available for this \ - target at this time, see https://doc.rust-lang.org/nightly\ - /rustc/platform-support.html for more information." - ); - let sha256 = self.stage0_metadata.checksums_sha256.get(&url).expect(&error); - if tarball.exists() { - if self.verify(&tarball, sha256) { - self.unpack(&tarball, &bin_root, prefix); - return; - } else { - self.verbose(&format!( - "ignoring cached file {} due to failed verification", - tarball.display() - )); - self.remove(&tarball); - } - } - Some(sha256) - } else if tarball.exists() { - self.unpack(&tarball, &bin_root, prefix); - return; - } else { - None - }; - - self.download_file(&format!("{base_url}/{url}"), &tarball, ""); - if let Some(sha256) = checksum { - if !self.verify(&tarball, sha256) { - panic!("failed to verify {}", tarball.display()); - } - } - - self.unpack(&tarball, &bin_root, prefix); - } -} diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index ffaffb4afe3d1..3a45e83faa9a7 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -55,8 +55,8 @@ mod format; mod install; mod llvm; mod metadata; -mod render_tests; mod min_config; +mod render_tests; mod run; mod sanity; mod setup; @@ -91,8 +91,8 @@ use crate::cache::{Interned, INTERNER}; pub use crate::config::Config; pub use crate::flags::Flags; pub use crate::flags::Subcommand; -use termcolor::{ColorChoice, StandardStream, WriteColor}; pub use crate::min_config::MinimalConfig; +use termcolor::{ColorChoice, StandardStream, WriteColor}; const LLVM_TOOLS: &[&str] = &[ "llvm-cov", // used to generate coverage report diff --git a/src/bootstrap/min_config.rs b/src/bootstrap/min_config.rs index 8f8063f213b18..4c45955c6f124 100644 --- a/src/bootstrap/min_config.rs +++ b/src/bootstrap/min_config.rs @@ -7,7 +7,7 @@ use std::{ process::Command, }; -use serde::Deserialize; +use serde_derive::Deserialize; use crate::{ cache::{Interned, INTERNER}, @@ -18,6 +18,9 @@ use crate::{ }; /// The bare minimum config, suitable for `bootstrap-shim`, but sharing code with the main `bootstrap` binary. +/// Unlike bootstrap itself, bootstrap-shim needs to be compatible across multiple versions of the Rust repo. +/// +/// **DO NOT CHANGE THIS CONFIG WITHOUT CHANGING THE SHELL SCRIPTS** to avoid regressing the shim accidentally. #[derive(Default, Clone)] pub struct MinimalConfig { // Needed so we know where to store the unpacked bootstrap binary. @@ -29,12 +32,11 @@ pub struct MinimalConfig { pub patch_binaries_for_nix: bool, // Needed to know which commit to download. pub stage0_metadata: Stage0Metadata, - // This isn't currently used, but will eventually let people configure whether to download or build bootstrap. pub config: Option, - // Not currently used in the shim. + // General need for verbose mode logging in `bootstrap-shim`. pub verbose: usize, - // Not currently used in the shim. + // Needed for `bootstrap::download` module pub dry_run: DryRun, } @@ -45,11 +47,13 @@ pub struct Stage0Metadata { pub checksums_sha256: HashMap, pub rustfmt: Option, } + #[derive(Clone, Default, Deserialize)] pub struct CompilerMetadata { pub date: String, pub version: String, } + #[derive(Default, Deserialize, Clone)] pub struct Stage0Config { pub dist_server: String, @@ -227,12 +231,12 @@ impl MinimalConfig { #[cfg(test)] /// Shared helper function to be used in `MinimalConfig::parse` and `bootstrap::config::Config::parse` -pub(crate) fn get_toml + Default>(_file: &Path) -> T { +pub(crate) fn get_toml + Default>(_file: &Path) -> T { T::default() } #[cfg(not(test))] /// Shared helper function to be used in `MinimalConfig::parse` and `bootstrap::config::Config::parse` -pub(crate) fn get_toml + Default>(file: &Path) -> T { +pub(crate) fn get_toml + Default>(file: &Path) -> T { let contents = t!(fs::read_to_string(file), format!("config file {} not found", file.display())); // Deserialize to Value and then TomlConfig to prevent the Deserialize impl of @@ -261,7 +265,7 @@ fn set_config_output_dir(output_path: &mut PathBuf) { } /// Shared helper function to be used in `MinimalConfig::parse` and `bootstrap::config::Config::parse` -pub(crate) fn set_cfg_path_and_return_toml_cfg + Default>( +pub(crate) fn set_cfg_path_and_return_toml_cfg + Default>( src: PathBuf, config_flag: Option, cfg_path: &mut Option, diff --git a/src/tools/x/src/main.rs b/src/tools/x/src/main.rs index 344cb3e48b76f..bb506ca4ebf4f 100644 --- a/src/tools/x/src/main.rs +++ b/src/tools/x/src/main.rs @@ -19,7 +19,7 @@ const PYTHON: &str = "python"; const PYTHON2: &str = "python2"; const PYTHON3: &str = "python3"; -fn python() -> &'static str { +pub fn python() -> &'static str { let val = match env::var_os("PATH") { Some(val) => val, None => return PYTHON, @@ -98,16 +98,8 @@ fn handle_result(result: io::Result, cmd: Command) { } } -pub fn main() { - match env::args().skip(1).next().as_deref() { - Some("--wrapper-version") => { - let version = env!("CARGO_PKG_VERSION"); - println!("{}", version); - return; - } - _ => {} - } - let current = match env::current_dir() { +fn find_and_run_available_bootstrap_script() { + let current_path = match env::current_dir() { Ok(dir) => dir, Err(err) => { eprintln!("Failed to get current directory: {err}"); @@ -115,7 +107,7 @@ pub fn main() { } }; - for dir in current.ancestors() { + for dir in current_path.ancestors() { let candidate = dir.join("x.py"); if candidate.exists() { let shell_script_candidate = dir.join("x"); @@ -132,6 +124,20 @@ pub fn main() { handle_result(result, cmd); } } +} + +#[allow(dead_code)] +fn main() { + match env::args().skip(1).next().as_deref() { + Some("--wrapper-version") => { + let version = env!("CARGO_PKG_VERSION"); + println!("{}", version); + return; + } + _ => {} + } + + find_and_run_available_bootstrap_script(); eprintln!( "x.py not found. Please run inside of a checkout of `https://github.com/rust-lang/rust`."