refactor src/tools/x

Signed-off-by: ozkanonur <[email protected]>

Author: onur-ozkan

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<ExitStatus> {
+        use std::os::unix::process::CommandExt;
+        Err(command.exec())
+    }
+
+    #[cfg(not(unix))]
+    fn exec_or_status(command: &mut Command) -> io::Result<ExitStatus> {
+        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::<Vec<_>>();
@@ -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<String> {
     config.last_modified_commit(
-        &["src/bootstrap", "src/tools/build_helper"],
+        &["src/bootstrap", "src/tools/build_helper", "src/tools/x"],
         "download-bootstrap",
         true,
     )

src/bootstrap/config.rs

@@ -84,7 +84,6 @@ pub struct Config {
     pub jobs: Option<u32>,
     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<String>,
@@ -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<usize>,
     build: Option<Build>,
     install: Option<Install>,
@@ -745,17 +744,19 @@ impl Config {
         config
     }
 
-    pub fn parse(args: &[String], custom_toml_config: Option<TomlConfig>) -> 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.

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]

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")
     }
 

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<PathBuf> {
@@ -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);
-    }
-}

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

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<PathBuf>,
-    // 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<String, String>,
     pub rustfmt: Option<RustfmtMetadata>,
 }
+
 #[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<T: Deserialize<'static> + Default>(_file: &Path) -> T {
+pub(crate) fn get_toml<T: serde::Deserialize<'static> + 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<T: Deserialize<'static> + Default>(file: &Path) -> T {
+pub(crate) fn get_toml<T: serde::Deserialize<'static> + 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<T: Deserialize<'static> + Default>(
+pub(crate) fn set_cfg_path_and_return_toml_cfg<T: serde::Deserialize<'static> + Default>(
     src: PathBuf,
     config_flag: Option<PathBuf>,
     cfg_path: &mut Option<PathBuf>,

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,24 +98,16 @@ fn handle_result(result: io::Result<ExitStatus>, 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}");
             process::exit(1);
         }
     };
 
-    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`."