From 2ac0e9b6944bcff607e453ad8bd27800318de64f Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 28 Feb 2021 23:51:15 -0500 Subject: [PATCH] Update all submodules that rustbuild doesn't depend on lazily This only updates the submodules the first time they're needed, instead of unconditionally the first time you run x.py. Ideally, this would move *all* submodules and not exclude some tools and backtrace. Unfortunately, cargo requires all `Cargo.toml` files in the whole workspace to be present to build any crate. On my machine, this takes the time for an initial submodule clone (for `x.py --help`) from 55.70 to 15.87 seconds. This uses exactly the same logic as the LLVM update used, modulo some minor cleanups: - Use a local variable for `src.join(relative_path)` - Remove unnecessary arrays for `book!` macro and make the macro simpler to use - Add more comments --- src/bootstrap/bootstrap.py | 27 ++++++--- src/bootstrap/check.rs | 4 +- src/bootstrap/compile.rs | 2 + src/bootstrap/doc.rs | 39 +++++++++---- src/bootstrap/lib.rs | 113 +++++++++++++++++++++++++++++++++++-- src/bootstrap/native.rs | 86 +--------------------------- src/bootstrap/test.rs | 5 +- src/bootstrap/tool.rs | 10 +++- 8 files changed, 173 insertions(+), 113 deletions(-) diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py index f9904cb610d2d..3d46548807705 100644 --- a/src/bootstrap/bootstrap.py +++ b/src/bootstrap/bootstrap.py @@ -989,21 +989,30 @@ def update_submodules(self): slow_submodules = self.get_toml('fast-submodules') == "false" start_time = time() if slow_submodules: - print('Unconditionally updating all submodules') + print('Unconditionally updating submodules') else: print('Updating only changed submodules') default_encoding = sys.getdefaultencoding() - submodules = [s.split(' ', 1)[1] for s in subprocess.check_output( - ["git", "config", "--file", - os.path.join(self.rust_root, ".gitmodules"), - "--get-regexp", "path"] - ).decode(default_encoding).splitlines()] + # Only update submodules that are needed to build bootstrap. These are needed because Cargo + # currently requires everything in a workspace to be "locally present" when starting a + # build, and will give a hard error if any Cargo.toml files are missing. + # FIXME: Is there a way to avoid cloning these eagerly? Bootstrap itself doesn't need to + # share a workspace with any tools - maybe it could be excluded from the workspace? + # That will still require cloning the submodules the second you check the standard + # library, though... + # FIXME: Is there a way to avoid hard-coding the submodules required? + # WARNING: keep this in sync with the submodules hard-coded in bootstrap/lib.rs + submodules = [ + "src/tools/rust-installer", + "src/tools/cargo", + "src/tools/rls", + "src/tools/miri", + "library/backtrace", + "library/stdarch" + ] filtered_submodules = [] submodules_names = [] for module in submodules: - # This is handled by native::Llvm in rustbuild, not here - if module.endswith("llvm-project"): - continue check = self.check_submodule(module, slow_submodules) filtered_submodules.append((module, check)) submodules_names.append(module) diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs index 5003d6693f72c..bc106746e57e0 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -7,7 +7,7 @@ use crate::config::TargetSelection; use crate::tool::{prepare_tool_cargo, SourceType}; use crate::INTERNER; use crate::{Compiler, Mode, Subcommand}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub struct Std { @@ -72,6 +72,8 @@ impl Step for Std { } fn run(self, builder: &Builder<'_>) { + builder.update_submodule(&Path::new("library").join("stdarch")); + let target = self.target; let compiler = builder.compiler(builder.top_stage, builder.config.build); diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index 1fae4bee732c0..8c28d0b60fa32 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -79,6 +79,8 @@ impl Step for Std { return; } + builder.update_submodule(&Path::new("library").join("stdarch")); + let mut target_deps = builder.ensure(StartupObjects { compiler, target }); let compiler_to_use = builder.compiler_for(compiler.stage, compiler.host, target); diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index d3f2c87c0d22f..9ec5d4d8ccdb4 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -22,8 +22,17 @@ use crate::config::{Config, TargetSelection}; use crate::tool::{self, prepare_tool_cargo, SourceType, Tool}; use crate::util::symlink_dir; +macro_rules! submodule_helper { + ($path:expr, submodule) => { + $path + }; + ($path:expr, submodule = $submodule:literal) => { + $submodule + }; +} + macro_rules! book { - ($($name:ident, $path:expr, $book_name:expr;)+) => { + ($($name:ident, $path:expr, $book_name:expr $(, submodule $(= $submodule:literal)? )? ;)+) => { $( #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] pub struct $name { @@ -46,6 +55,10 @@ macro_rules! book { } fn run(self, builder: &Builder<'_>) { + $( + let path = Path::new(submodule_helper!( $path, submodule $( = $submodule )? )); + builder.update_submodule(&path); + )? builder.ensure(RustbookSrc { target: self.target, name: INTERNER.intern_str($book_name), @@ -59,13 +72,16 @@ macro_rules! book { // NOTE: When adding a book here, make sure to ALSO build the book by // adding a build step in `src/bootstrap/builder.rs`! +// NOTE: Make sure to add the corresponding submodule when adding a new book. +// FIXME: Make checking for a submodule automatic somehow (maybe by having a list of all submodules +// and checking against it?). book!( - CargoBook, "src/tools/cargo/src/doc", "cargo"; - EditionGuide, "src/doc/edition-guide", "edition-guide"; - EmbeddedBook, "src/doc/embedded-book", "embedded-book"; - Nomicon, "src/doc/nomicon", "nomicon"; - Reference, "src/doc/reference", "reference"; - RustByExample, "src/doc/rust-by-example", "rust-by-example"; + CargoBook, "src/tools/cargo/src/doc", "cargo", submodule = "src/tools/cargo"; + EditionGuide, "src/doc/edition-guide", "edition-guide", submodule; + EmbeddedBook, "src/doc/embedded-book", "embedded-book", submodule; + Nomicon, "src/doc/nomicon", "nomicon", submodule; + Reference, "src/doc/reference", "reference", submodule; + RustByExample, "src/doc/rust-by-example", "rust-by-example", submodule; RustdocBook, "src/doc/rustdoc", "rustdoc"; ); @@ -197,6 +213,9 @@ impl Step for TheBook { /// * Index page /// * Redirect pages fn run(self, builder: &Builder<'_>) { + let relative_path = Path::new("src").join("doc").join("book"); + builder.update_submodule(&relative_path); + let compiler = self.compiler; let target = self.target; @@ -204,7 +223,7 @@ impl Step for TheBook { builder.ensure(RustbookSrc { target, name: INTERNER.intern_str("book"), - src: INTERNER.intern_path(builder.src.join("src/doc/book")), + src: INTERNER.intern_path(builder.src.join(&relative_path)), }); // building older edition redirects @@ -212,7 +231,7 @@ impl Step for TheBook { builder.ensure(RustbookSrc { target, name: INTERNER.intern_string(format!("book/{}", edition)), - src: INTERNER.intern_path(builder.src.join("src/doc/book").join(edition)), + src: INTERNER.intern_path(builder.src.join(&relative_path).join(edition)), }); } @@ -221,7 +240,7 @@ impl Step for TheBook { // build the redirect pages builder.info(&format!("Documenting book redirect pages ({})", target)); - for file in t!(fs::read_dir(builder.src.join("src/doc/book/redirects"))) { + for file in t!(fs::read_dir(builder.src.join(&relative_path).join("redirects"))) { let file = t!(file); let path = file.path(); let path = path.to_str().unwrap(); diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 69c5de0b40831..6bcdbe3e4bbbb 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -477,17 +477,120 @@ impl Build { slice::from_ref(&self.build.triple) } + // modified from `check_submodule` and `update_submodule` in bootstrap.py + /// Given a path to the directory of a submodule, update it. + /// + /// `relative_path` should be relative to the root of the git repository, not an absolute path. + pub(crate) fn update_submodule(&self, relative_path: &Path) { + fn dir_is_empty(dir: &Path) -> bool { + t!(std::fs::read_dir(dir)).next().is_none() + } + + if !self.config.submodules { + return; + } + + let absolute_path = self.config.src.join(relative_path); + + // NOTE: The check for the empty directory is here because when running x.py the first time, + // the submodule won't be checked out. Check it out now so we can build it. + if !channel::GitInfo::new(false, relative_path).is_git() && !dir_is_empty(&absolute_path) { + return; + } + + // check_submodule + if self.config.fast_submodules { + let checked_out_hash = output( + Command::new("git").args(&["rev-parse", "HEAD"]).current_dir(&absolute_path), + ); + // update_submodules + let recorded = output( + Command::new("git") + .args(&["ls-tree", "HEAD"]) + .arg(relative_path) + .current_dir(&self.config.src), + ); + let actual_hash = recorded + .split_whitespace() + .nth(2) + .unwrap_or_else(|| panic!("unexpected output `{}`", recorded)); + + // update_submodule + if actual_hash == checked_out_hash.trim_end() { + // already checked out + return; + } + } + + println!("Updating submodule {}", relative_path.display()); + self.run( + Command::new("git") + .args(&["submodule", "-q", "sync"]) + .arg(relative_path) + .current_dir(&self.config.src), + ); + + // Try passing `--progress` to start, then run git again without if that fails. + let update = |progress: bool| { + let mut git = Command::new("git"); + git.args(&["submodule", "update", "--init", "--recursive"]); + if progress { + git.arg("--progress"); + } + git.arg(relative_path).current_dir(&self.config.src); + git + }; + // NOTE: doesn't use `try_run` because this shouldn't print an error if it fails. + if !update(true).status().map_or(false, |status| status.success()) { + self.run(&mut update(false)); + } + + self.run(Command::new("git").args(&["reset", "-q", "--hard"]).current_dir(&absolute_path)); + self.run(Command::new("git").args(&["clean", "-qdfx"]).current_dir(absolute_path)); + } + + /// 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) { + // WARNING: keep this in sync with the submodules hard-coded in bootstrap.py + const BOOTSTRAP_SUBMODULES: &[&str] = &[ + "src/tools/rust-installer", + "src/tools/cargo", + "src/tools/rls", + "src/tools/miri", + "library/backtrace", + "library/stdarch", + ]; + // Avoid running git when there isn't a git checkout. + if !self.config.submodules { + return; + } + let output = output( + Command::new("git") + .args(&["config", "--file"]) + .arg(&self.config.src.join(".gitmodules")) + .args(&["--get-regexp", "path"]), + ); + for line in output.lines() { + // Look for `submodule.$name.path = $path` + // Sample output: `submodule.src/rust-installer.path src/tools/rust-installer` + let submodule = Path::new(line.splitn(2, ' ').nth(1).unwrap()); + // avoid updating submodules twice + if !BOOTSTRAP_SUBMODULES.iter().any(|&p| Path::new(p) == submodule) + && channel::GitInfo::new(false, submodule).is_git() + { + self.update_submodule(submodule); + } + } + } + /// Executes the entire build, as configured by the flags and configuration. pub fn build(&mut self) { unsafe { job::setup(self); } - // If the LLVM submodule has been initialized already, sync it unconditionally. This avoids - // contributors checking in a submodule change by accident. - if self.in_tree_llvm_info.is_git() { - native::update_llvm_submodule(self); - } + self.maybe_update_submodules(); if let Subcommand::Format { check, paths } = &self.config.cmd { return format::format(self, *check, &paths); diff --git a/src/bootstrap/native.rs b/src/bootstrap/native.rs index 0be42d9b23486..1be414b29a1ae 100644 --- a/src/bootstrap/native.rs +++ b/src/bootstrap/native.rs @@ -21,7 +21,7 @@ use build_helper::{output, t}; use crate::builder::{Builder, RunConfig, ShouldRun, Step}; use crate::config::TargetSelection; use crate::util::{self, exe}; -use crate::{Build, GitRepo}; +use crate::GitRepo; use build_helper::up_to_date; pub struct Meta { @@ -91,86 +91,6 @@ pub fn prebuilt_llvm_config( Err(Meta { stamp, build_llvm_config, out_dir, root: root.into() }) } -// modified from `check_submodule` and `update_submodule` in bootstrap.py -pub(crate) fn update_llvm_submodule(build: &Build) { - let llvm_project = &Path::new("src").join("llvm-project"); - - fn dir_is_empty(dir: &Path) -> bool { - t!(std::fs::read_dir(dir)).next().is_none() - } - - if !build.config.submodules { - return; - } - - // NOTE: The check for the empty directory is here because when running x.py - // the first time, the llvm submodule won't be checked out. Check it out - // now so we can build it. - if !build.in_tree_llvm_info.is_git() && !dir_is_empty(&build.config.src.join(llvm_project)) { - return; - } - - // check_submodule - if build.config.fast_submodules { - let checked_out_hash = output( - Command::new("git") - .args(&["rev-parse", "HEAD"]) - .current_dir(build.config.src.join(llvm_project)), - ); - // update_submodules - let recorded = output( - Command::new("git") - .args(&["ls-tree", "HEAD"]) - .arg(llvm_project) - .current_dir(&build.config.src), - ); - let actual_hash = recorded - .split_whitespace() - .nth(2) - .unwrap_or_else(|| panic!("unexpected output `{}`", recorded)); - - // update_submodule - if actual_hash == checked_out_hash.trim_end() { - // already checked out - return; - } - } - - println!("Updating submodule {}", llvm_project.display()); - build.run( - Command::new("git") - .args(&["submodule", "-q", "sync"]) - .arg(llvm_project) - .current_dir(&build.config.src), - ); - - // Try passing `--progress` to start, then run git again without if that fails. - let update = |progress: bool| { - let mut git = Command::new("git"); - git.args(&["submodule", "update", "--init", "--recursive"]); - if progress { - git.arg("--progress"); - } - git.arg(llvm_project).current_dir(&build.config.src); - git - }; - // NOTE: doesn't use `try_run` because this shouldn't print an error if it fails. - if !update(true).status().map_or(false, |status| status.success()) { - build.run(&mut update(false)); - } - - build.run( - Command::new("git") - .args(&["reset", "-q", "--hard"]) - .current_dir(build.config.src.join(llvm_project)), - ); - build.run( - Command::new("git") - .args(&["clean", "-qdfx"]) - .current_dir(build.config.src.join(llvm_project)), - ); -} - #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] pub struct Llvm { pub target: TargetSelection, @@ -208,9 +128,7 @@ impl Step for Llvm { Err(m) => m, }; - if !builder.config.dry_run { - update_llvm_submodule(builder); - } + builder.update_submodule(&Path::new("src").join("llvm-project")); if builder.config.llvm_link_shared && (target.contains("windows") || target.contains("apple-darwin")) { diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index e4d6a3f587b05..ad7868f118030 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -1835,7 +1835,10 @@ impl Step for RustcGuide { } fn run(self, builder: &Builder<'_>) { - let src = builder.src.join("src/doc/rustc-dev-guide"); + let relative_path = Path::new("src").join("doc").join("rustc-dev-guide"); + builder.update_submodule(&relative_path); + + let src = builder.src.join(relative_path); let mut rustbook_cmd = builder.tool_cmd(Tool::Rustbook); let toolstate = if try_run(builder, rustbook_cmd.arg("linkcheck").arg(&src)) { ToolState::TestPass diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index aa7fe658df320..f5e3f61dcc88f 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -1,7 +1,7 @@ use std::collections::HashSet; use std::env; use std::fs; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::{exit, Command}; use build_helper::t; @@ -670,7 +670,8 @@ macro_rules! tool_extended { $path:expr, $tool_name:expr, stable = $stable:expr, - $(in_tree = $in_tree:expr,)* + $(in_tree = $in_tree:expr,)? + $(submodule = $submodule:literal,)? $extra_deps:block;)+) => { $( #[derive(Debug, Clone, Hash, PartialEq, Eq)] @@ -714,6 +715,7 @@ macro_rules! tool_extended { #[allow(unused_mut)] fn run(mut $sel, $builder: &Builder<'_>) -> Option { $extra_deps + $( $builder.update_submodule(&Path::new("src").join("tools").join($submodule)); )? $builder.ensure(ToolBuild { compiler: $sel.compiler, target: $sel.target, @@ -736,6 +738,8 @@ macro_rules! tool_extended { // Note: tools need to be also added to `Builder::get_step_descriptions` in `builder.rs` // to make `./x.py build ` work. +// Note: Most submodule updates for tools are handled by bootstrap.py, since they're needed just to +// invoke Cargo to build bootstrap. See the comment there for more details. tool_extended!((self, builder), Cargofmt, rustfmt, "src/tools/rustfmt", "cargo-fmt", stable=true, in_tree=true, {}; CargoClippy, clippy, "src/tools/clippy", "cargo-clippy", stable=true, in_tree=true, {}; @@ -752,7 +756,7 @@ tool_extended!((self, builder), }; RustDemangler, rust_demangler, "src/tools/rust-demangler", "rust-demangler", stable=false, in_tree=true, {}; Rustfmt, rustfmt, "src/tools/rustfmt", "rustfmt", stable=true, in_tree=true, {}; - RustAnalyzer, rust_analyzer, "src/tools/rust-analyzer/crates/rust-analyzer", "rust-analyzer", stable=false, {}; + RustAnalyzer, rust_analyzer, "src/tools/rust-analyzer/crates/rust-analyzer", "rust-analyzer", stable=false, submodule="rust-analyzer", {}; ); impl<'a> Builder<'a> {