Auto merge of rust-lang#92260 - jyn514:less-python-logic, r=Mark-Simu…

…lacrum

Move some more bootstrap logic from python to rust

Same rationale as rust-lang#76544; it would be nice to make python entirely optional at some point.

This also removes $ROOT as an option for the build directory; I haven't been using it, and like Alex
said in rust-lang#76544 (comment) it seems like a misfeature.

This allows running `cargo run` from src/bootstrap, although that still gives
lots of compile errors if you don't use the beta toolchain. It's not exactly the same as using `x.py`, since it won't have `BOOTSTRAP_DOWNLOAD_RUSTC` set, but it's pretty close. Doing this from the top-level directory requires rust-lang/cargo#7290 to be fixed, or using `cargo run -p bootstrap`.

The next steps for making python optional are to move download-ci-llvm and download-rustc support into rustbuild, likely be shelling out as the python scripts do today.

It would also be nice (although not required) to move submodule support there, but that would require taking bootstrap out of the workspace to avoid errors from crates that haven't been cloned yet.

r? `@Mark-Simulacrum`

src/bootstrap/Cargo.toml

@@ -3,6 +3,7 @@ name = "bootstrap"
 version = "0.0.0"
 edition = "2021"
 build = "build.rs"
+default-run = "bootstrap"
 
 [lib]
 path = "lib.rs"

src/bootstrap/bootstrap.py

@@ -1267,7 +1267,7 @@ def bootstrap(help_triggered):
     build.check_vendored_status()
 
     build_dir = build.get_toml('build-dir', 'build') or 'build'
-    build.build_dir = os.path.abspath(build_dir.replace("$ROOT", build.rust_root))
+    build.build_dir = os.path.abspath(build_dir)
 
     with open(os.path.join(build.rust_root, "src", "stage0.json")) as f:
         data = json.load(f)
@@ -1302,10 +1302,7 @@ def bootstrap(help_triggered):
     env = os.environ.copy()
     env["BOOTSTRAP_PARENT_ID"] = str(os.getpid())
     env["BOOTSTRAP_PYTHON"] = sys.executable
-    env["BUILD_DIR"] = build.build_dir
     env["RUSTC_BOOTSTRAP"] = '1'
-    if toml_path:
-        env["BOOTSTRAP_CONFIG"] = toml_path
     if build.rustc_commit is not None:
         env["BOOTSTRAP_DOWNLOAD_RUSTC"] = '1'
     run(args, env=env, verbose=build.verbose, is_bootstrap=True)

src/bootstrap/builder.rs

@@ -883,7 +883,7 @@ impl<'a> Builder<'a> {
     }
 
     pub fn rustdoc_cmd(&self, compiler: Compiler) -> Command {
-        let mut cmd = Command::new(&self.out.join("bootstrap/debug/rustdoc"));
+        let mut cmd = Command::new(&self.bootstrap_out.join("rustdoc"));
         cmd.env("RUSTC_STAGE", compiler.stage.to_string())
             .env("RUSTC_SYSROOT", self.sysroot(compiler))
             // Note that this is *not* the sysroot_libdir because rustdoc must be linked
@@ -1249,7 +1249,7 @@ impl<'a> Builder<'a> {
             .env("RUSTC_STAGE", stage.to_string())
             .env("RUSTC_SYSROOT", &sysroot)
             .env("RUSTC_LIBDIR", &libdir)
-            .env("RUSTDOC", self.out.join("bootstrap/debug/rustdoc"))
+            .env("RUSTDOC", self.bootstrap_out.join("rustdoc"))
             .env(
                 "RUSTDOC_REAL",
                 if cmd == "doc" || cmd == "rustdoc" || (cmd == "test" && want_rustdoc) {
@@ -1263,7 +1263,7 @@ impl<'a> Builder<'a> {
         // Clippy support is a hack and uses the default `cargo-clippy` in path.
         // Don't override RUSTC so that the `cargo-clippy` in path will be run.
         if cmd != "clippy" {
-            cargo.env("RUSTC", self.out.join("bootstrap/debug/rustc"));
+            cargo.env("RUSTC", self.bootstrap_out.join("rustc"));
         }
 
         // Dealing with rpath here is a little special, so let's go into some

src/bootstrap/builder/tests.rs

@@ -8,10 +8,10 @@ fn configure(cmd: &str, host: &[&str], target: &[&str]) -> Config {
     config.save_toolstates = None;
     config.dry_run = true;
     config.ninja_in_file = false;
-    // try to avoid spurious failures in dist where we create/delete each others file
     config.out = PathBuf::from(env::var_os("BOOTSTRAP_OUTPUT_DIRECTORY").unwrap());
     config.initial_rustc = PathBuf::from(env::var_os("RUSTC").unwrap());
     config.initial_cargo = PathBuf::from(env::var_os("BOOTSTRAP_INITIAL_CARGO").unwrap());
+    // try to avoid spurious failures in dist where we create/delete each others file
     let dir = config
         .out
         .join("tmp-rustbuild-tests")

src/bootstrap/config.rs

@@ -6,7 +6,6 @@
 use std::cmp;
 use std::collections::{HashMap, HashSet};
 use std::env;
-use std::ffi::OsString;
 use std::fmt;
 use std::fs;
 use std::path::{Path, PathBuf};
@@ -392,7 +391,6 @@ derive_merge! {
         build: Option<String>,
         host: Option<Vec<String>>,
         target: Option<Vec<String>>,
-        // This is ignored, the rust code always gets the build directory from the `BUILD_DIR` env variable
         build_dir: Option<String>,
         cargo: Option<String>,
         rustc: Option<String>,
@@ -588,18 +586,6 @@ derive_merge! {
 }
 
 impl Config {
-    fn path_from_python(var_key: &str) -> PathBuf {
-        match env::var_os(var_key) {
-            Some(var_val) => Self::normalize_python_path(var_val),
-            _ => panic!("expected '{}' to be set", var_key),
-        }
-    }
-
-    /// Normalizes paths from Python slightly. We don't trust paths from Python (#49785).
-    fn normalize_python_path(path: OsString) -> PathBuf {
-        Path::new(&path).components().collect()
-    }
-
     pub fn default_opts() -> Config {
         let mut config = Config::default();
         config.llvm_optimize = true;
@@ -625,7 +611,7 @@ impl Config {
         let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
         // Undo `src/bootstrap`
         config.src = manifest_dir.parent().unwrap().parent().unwrap().to_owned();
-        config.out = Config::path_from_python("BUILD_DIR");
+        config.out = PathBuf::from("build");
 
         config.initial_cargo = PathBuf::from(env!("CARGO"));
         config.initial_rustc = PathBuf::from(env!("RUSTC"));
@@ -655,12 +641,6 @@ impl Config {
         config.llvm_profile_use = flags.llvm_profile_use;
         config.llvm_profile_generate = flags.llvm_profile_generate;
 
-        if config.dry_run {
-            let dir = config.out.join("tmp-dry-run");
-            t!(fs::create_dir_all(&dir));
-            config.out = dir;
-        }
-
         #[cfg(test)]
         let get_toml = |_| TomlConfig::default();
         #[cfg(not(test))]
@@ -677,7 +657,15 @@ impl Config {
             }
         };
 
-        let mut toml = flags.config.as_deref().map(get_toml).unwrap_or_else(TomlConfig::default);
+        // check --config first, then `$RUST_BOOTSTRAP_CONFIG` first, then `config.toml`
+        let toml_path = flags
+            .config
+            .clone()
+            .or_else(|| env::var_os("RUST_BOOTSTRAP_CONFIG").map(PathBuf::from))
+            .unwrap_or_else(|| PathBuf::from("config.toml"));
+        let mut toml =
+            if toml_path.exists() { get_toml(&toml_path) } else { TomlConfig::default() };
+
         if let Some(include) = &toml.profile {
             let mut include_path = config.src.clone();
             include_path.push("src");
@@ -689,12 +677,25 @@ impl Config {
         }
 
         config.changelog_seen = toml.changelog_seen;
-        if let Some(cfg) = flags.config {
-            config.config = cfg;
-        }
+        config.config = toml_path;
 
         let build = toml.build.unwrap_or_default();
 
+        set(&mut config.initial_rustc, build.rustc.map(PathBuf::from));
+        set(&mut config.out, 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.
+        if !config.out.is_absolute() {
+            // `canonicalize` requires the path to already exist. Use our vendored copy of `absolute` instead.
+            config.out = crate::util::absolute(&config.out);
+        }
+
+        if config.dry_run {
+            let dir = config.out.join("tmp-dry-run");
+            t!(fs::create_dir_all(&dir));
+            config.out = dir;
+        }
+
         config.hosts = if let Some(arg_host) = flags.host {
             arg_host
         } else if let Some(file_host) = build.host {

src/bootstrap/flags.rs

@@ -3,7 +3,6 @@
 //! This module implements the command-line parsing of the build system which
 //! has various flags to configure how it's run.
 
-use std::env;
 use std::path::PathBuf;
 use std::process;
 
@@ -541,7 +540,6 @@ Arguments:
         // Get any optional paths which occur after the subcommand
         let mut paths = matches.free[1..].iter().map(|p| p.into()).collect::<Vec<PathBuf>>();
 
-        let cfg_file = env::var_os("BOOTSTRAP_CONFIG").map(PathBuf::from);
         let verbose = matches.opt_present("verbose");
 
         // User passed in -h/--help?
@@ -671,7 +669,7 @@ Arguments:
             } else {
                 None
             },
-            config: cfg_file,
+            config: matches.opt_str("config").map(PathBuf::from),
             jobs: matches.opt_str("jobs").map(|j| j.parse().expect("`jobs` should be a number")),
             cmd,
             incremental: matches.opt_present("incremental"),

src/bootstrap/lib.rs

@@ -261,6 +261,7 @@ pub struct Build {
     // Properties derived from the above configuration
     src: PathBuf,
     out: PathBuf,
+    bootstrap_out: PathBuf,
     rust_info: channel::GitInfo,
     cargo_info: channel::GitInfo,
     rls_info: channel::GitInfo,
@@ -435,6 +436,20 @@ impl Build {
             .expect("failed to read src/version");
         let version = version.trim();
 
+        let bootstrap_out = if std::env::var("BOOTSTRAP_PYTHON").is_ok() {
+            out.join("bootstrap").join("debug")
+        } else {
+            let workspace_target_dir = std::env::var("CARGO_TARGET_DIR")
+                .map(PathBuf::from)
+                .unwrap_or_else(|_| src.join("target"));
+            let bootstrap_out = workspace_target_dir.join("debug");
+            if !bootstrap_out.join("rustc").exists() {
+                // this restriction can be lifted whenever https://github.com/rust-lang/rfcs/pull/3028 is implemented
+                panic!("run `cargo build --bins` before `cargo run`")
+            }
+            bootstrap_out
+        };
+
         let mut build = Build {
             initial_rustc: config.initial_rustc.clone(),
             initial_cargo: config.initial_cargo.clone(),
@@ -453,6 +468,7 @@ impl Build {
             version: version.to_string(),
             src,
             out,
+            bootstrap_out,
 
             rust_info,
             cargo_info,
@@ -629,7 +645,7 @@ impl Build {
         }
 
         if let Subcommand::Setup { profile } = &self.config.cmd {
-            return setup::setup(&self.config.src, *profile);
+            return setup::setup(&self.config, *profile);
         }
 
         {

src/bootstrap/setup.rs

@@ -1,5 +1,5 @@
-use crate::TargetSelection;
 use crate::{t, VERSION};
+use crate::{Config, TargetSelection};
 use std::env::consts::EXE_SUFFIX;
 use std::fmt::Write as _;
 use std::fs::File;
@@ -81,24 +81,22 @@ impl fmt::Display for Profile {
     }
 }
 
-pub fn setup(src_path: &Path, profile: Profile) {
-    let cfg_file = env::var_os("BOOTSTRAP_CONFIG").map(PathBuf::from);
+pub fn setup(config: &Config, profile: Profile) {
+    let path = &config.config;
 
-    if cfg_file.as_ref().map_or(false, |f| f.exists()) {
-        let file = cfg_file.unwrap();
+    if path.exists() {
         println!(
             "error: you asked `x.py` to setup a new config file, but one already exists at `{}`",
-            file.display()
+            path.display()
         );
-        println!("help: try adding `profile = \"{}\"` at the top of {}", profile, file.display());
+        println!("help: try adding `profile = \"{}\"` at the top of {}", profile, path.display());
         println!(
             "note: this will use the configuration in {}",
-            profile.include_path(src_path).display()
+            profile.include_path(&config.src).display()
         );
         std::process::exit(1);
     }
 
-    let path = cfg_file.unwrap_or_else(|| "config.toml".into());
     let settings = format!(
         "# Includes one of the default files in src/bootstrap/defaults\n\
     profile = \"{}\"\n\
@@ -107,7 +105,7 @@ pub fn setup(src_path: &Path, profile: Profile) {
     );
     t!(fs::write(path, settings));
 
-    let include_path = profile.include_path(src_path);
+    let include_path = profile.include_path(&config.src);
     println!("`x.py` will now use the configuration at {}", include_path.display());
 
     let build = TargetSelection::from_user(&env!("BUILD_TRIPLE"));
@@ -138,7 +136,7 @@ pub fn setup(src_path: &Path, profile: Profile) {
 
     println!();
 
-    t!(install_git_hook_maybe(src_path));
+    t!(install_git_hook_maybe(&config.src));
 
     println!();
 

src/bootstrap/test.rs

@@ -730,7 +730,7 @@ impl Step for RustdocTheme {
     }
 
     fn run(self, builder: &Builder<'_>) {
-        let rustdoc = builder.out.join("bootstrap/debug/rustdoc");
+        let rustdoc = builder.bootstrap_out.join("rustdoc");
         let mut cmd = builder.tool_cmd(Tool::RustdocTheme);
         cmd.arg(rustdoc.to_str().unwrap())
             .arg(builder.src.join("src/librustdoc/html/static/css/themes").to_str().unwrap())
@@ -2346,6 +2346,8 @@ impl Step for Bootstrap {
             .current_dir(builder.src.join("src/bootstrap"))
             .env("RUSTFLAGS", "-Cdebuginfo=2")
             .env("CARGO_TARGET_DIR", builder.out.join("bootstrap"))
+            // HACK: bootstrap's tests want to know the output directory, but there's no way to set
+            // it except through config.toml. Set it through an env variable instead.
             .env("BOOTSTRAP_OUTPUT_DIRECTORY", &builder.config.out)
             .env("BOOTSTRAP_INITIAL_CARGO", &builder.config.initial_cargo)
             .env("RUSTC_BOOTSTRAP", "1")