Ensure required submodules at the same time as updating existing submodules

src/bootstrap/flags.rs

@@ -143,7 +143,7 @@ pub enum Subcommand {
         args: Vec<String>,
     },
     Setup {
-        profile: Profile,
+        profile: Option<Profile>,
     },
 }
 
@@ -628,14 +628,15 @@ Arguments:
                         |path| format!("{} is not a valid UTF8 string", path.to_string_lossy())
                     ));
 
-                    profile_string.parse().unwrap_or_else(|err| {
+                    let profile = profile_string.parse().unwrap_or_else(|err| {
                         eprintln!("error: {}", err);
                         eprintln!("help: the available profiles are:");
                         eprint!("{}", Profile::all_for_help("- "));
                         crate::detail_exit(1);
-                    })
+                    });
+                    Some(profile)
                 } else {
-                    t!(crate::setup::interactive_path())
+                    None
                 };
                 Subcommand::Setup { profile }
             }

src/bootstrap/lib.rs

@@ -542,16 +542,6 @@ impl Build {
             metrics: metrics::BuildMetrics::init(),
         };
 
-        build.verbose("finding compilers");
-        cc_detect::find(&mut build);
-        // When running `setup`, the profile is about to change, so any requirements we have now may
-        // be different on the next invocation. Don't check for them until the next time x.py is
-        // run. This is ok because `setup` never runs any build commands, so it won't fail if commands are missing.
-        if !matches!(build.config.cmd, Subcommand::Setup { .. }) {
-            build.verbose("running sanity check");
-            sanity::check(&mut build);
-        }
-
         // If local-rust is the same major.minor as the current version, then force a
         // local-rebuild
         let local_version_verbose =
@@ -567,16 +557,34 @@ impl Build {
             build.local_rebuild = true;
         }
 
-        // Make sure we update these before gathering metadata so we don't get an error about missing
-        // Cargo.toml files.
-        let rust_submodules =
-            ["src/tools/rust-installer", "src/tools/cargo", "library/backtrace", "library/stdarch"];
-        for s in rust_submodules {
-            build.update_submodule(Path::new(s));
-        }
+        build.verbose("finding compilers");
+        cc_detect::find(&mut build);
+        // When running `setup`, the profile is about to change, so any requirements we have now may
+        // be different on the next invocation. Don't check for them until the next time x.py is
+        // run. This is ok because `setup` never runs any build commands, so it won't fail if commands are missing.
+        //
+        // Similarly, for `setup` we don't actually need submodules or cargo metadata.
+        if !matches!(build.config.cmd, Subcommand::Setup { .. }) {
+            build.verbose("running sanity check");
+            sanity::check(&mut build);
+
+            // Make sure we update these before gathering metadata so we don't get an error about missing
+            // Cargo.toml files.
+            let rust_submodules = [
+                "src/tools/rust-installer",
+                "src/tools/cargo",
+                "library/backtrace",
+                "library/stdarch",
+            ];
+            for s in rust_submodules {
+                build.update_submodule(Path::new(s));
+            }
+            // Now, update all existing submodules.
+            build.update_existing_submodules();
 
-        build.verbose("learning about cargo");
-        metadata::build(&mut build);
+            build.verbose("learning about cargo");
+            metadata::build(&mut build);
+        }
 
         build
     }
@@ -654,7 +662,7 @@ impl Build {
 
     /// If any submodule has been initialized already, sync it unconditionally.
     /// This avoids contributors checking in a submodule change by accident.
-    pub fn maybe_update_submodules(&self) {
+    pub fn update_existing_submodules(&self) {
         // Avoid running git when there isn't a git checkout.
         if !self.config.submodules(&self.rust_info()) {
             return;
@@ -683,8 +691,6 @@ impl Build {
             job::setup(self);
         }
 
-        self.maybe_update_submodules();
-
         if let Subcommand::Format { check, paths } = &self.config.cmd {
             return format::format(&builder::Builder::new(&self), *check, &paths);
         }

src/bootstrap/setup.rs

@@ -1,15 +1,13 @@
+use crate::Config;
 use crate::{t, VERSION};
-use crate::{Config, TargetSelection};
 use std::env::consts::EXE_SUFFIX;
 use std::fmt::Write as _;
 use std::fs::File;
+use std::io::Write;
 use std::path::{Path, PathBuf, MAIN_SEPARATOR};
 use std::process::Command;
 use std::str::FromStr;
-use std::{
-    env, fmt, fs,
-    io::{self, Write},
-};
+use std::{fmt, fs, io};
 
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub enum Profile {
@@ -81,38 +79,10 @@ impl fmt::Display for Profile {
     }
 }
 
-pub fn setup(config: &Config, profile: Profile) {
-    let path = &config.config.clone().unwrap_or(PathBuf::from("config.toml"));
-
-    if path.exists() {
-        eprintln!(
-            "error: you asked `x.py` to setup a new config file, but one already exists at `{}`",
-            path.display()
-        );
-        eprintln!("help: try adding `profile = \"{}\"` at the top of {}", profile, path.display());
-        eprintln!(
-            "note: this will use the configuration in {}",
-            profile.include_path(&config.src).display()
-        );
-        crate::detail_exit(1);
-    }
-
-    let settings = format!(
-        "# Includes one of the default files in src/bootstrap/defaults\n\
-    profile = \"{}\"\n\
-    changelog-seen = {}\n",
-        profile, VERSION
-    );
-    t!(fs::write(path, settings));
-
-    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"));
+pub fn setup(config: &Config, profile: Option<Profile>) {
+    let profile = profile.unwrap_or_else(|| t!(interactive_path()));
     let stage_path =
-        ["build", build.rustc_target_arg(), "stage1"].join(&MAIN_SEPARATOR.to_string());
-
-    println!();
+        ["build", config.build.rustc_target_arg(), "stage1"].join(&MAIN_SEPARATOR.to_string());
 
     if !rustup_installed() && profile != Profile::User {
         eprintln!("`rustup` is not installed; cannot link `stage1` toolchain");
@@ -134,8 +104,6 @@ pub fn setup(config: &Config, profile: Profile) {
         Profile::User => &["dist", "build"],
     };
 
-    println!();
-
     t!(install_git_hook_maybe(&config));
 
     println!();
@@ -150,6 +118,36 @@ pub fn setup(config: &Config, profile: Profile) {
             "For more suggestions, see https://rustc-dev-guide.rust-lang.org/building/suggested.html"
         );
     }
+
+    let path = &config.config.clone().unwrap_or(PathBuf::from("config.toml"));
+    setup_config_toml(path, profile, config);
+}
+
+fn setup_config_toml(path: &PathBuf, profile: Profile, config: &Config) {
+    if path.exists() {
+        eprintln!();
+        eprintln!(
+            "error: you asked `x.py` to setup a new config file, but one already exists at `{}`",
+            path.display()
+        );
+        eprintln!("help: try adding `profile = \"{}\"` at the top of {}", profile, path.display());
+        eprintln!(
+            "note: this will use the configuration in {}",
+            profile.include_path(&config.src).display()
+        );
+        crate::detail_exit(1);
+    }
+
+    let settings = format!(
+        "# Includes one of the default files in src/bootstrap/defaults\n\
+    profile = \"{}\"\n\
+    changelog-seen = {}\n",
+        profile, VERSION
+    );
+    t!(fs::write(path, settings));
+
+    let include_path = profile.include_path(&config.src);
+    println!("`x.py` will now use the configuration at {}", include_path.display());
 }
 
 fn rustup_installed() -> bool {
@@ -303,7 +301,18 @@ pub fn interactive_path() -> io::Result<Profile> {
 
 // install a git hook to automatically run tidy --bless, if they want
 fn install_git_hook_maybe(config: &Config) -> io::Result<()> {
+    let git = t!(config.git().args(&["rev-parse", "--git-common-dir"]).output().map(|output| {
+        assert!(output.status.success(), "failed to run `git`");
+        PathBuf::from(t!(String::from_utf8(output.stdout)).trim())
+    }));
+    let dst = git.join("hooks").join("pre-push");
+    if dst.exists() {
+        // The git hook has already been set up, or the user already has a custom hook.
+        return Ok(());
+    }
+
     let mut input = String::new();
+    println!();
     println!(
         "Rust's CI will automatically fail if it doesn't pass `tidy`, the internal tool for ensuring code quality.
 If you'd like, x.py can install a git hook for you that will automatically run `tidy --bless` before
@@ -329,12 +338,6 @@ undesirable, simply delete the `pre-push` file from .git/hooks."
 
     if should_install {
         let src = config.src.join("src").join("etc").join("pre-push.sh");
-        let git =
-            t!(config.git().args(&["rev-parse", "--git-common-dir"]).output().map(|output| {
-                assert!(output.status.success(), "failed to run `git`");
-                PathBuf::from(t!(String::from_utf8(output.stdout)).trim())
-            }));
-        let dst = git.join("hooks").join("pre-push");
         match fs::hard_link(src, &dst) {
             Err(e) => eprintln!(
                 "error: could not create hook {}: do you already have the git hook installed?\n{}",