From 88d6348f6eb1d7a1021958bc3bb42b4c22eb9881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 16 Jun 2024 23:30:34 +0200 Subject: [PATCH 1/3] Add `TrackedCommand` to bootstrap --- src/bootstrap/src/utils/cmd.rs | 92 ++++++++++++++++++++++++++++++++++ src/bootstrap/src/utils/mod.rs | 2 + 2 files changed, 94 insertions(+) create mode 100644 src/bootstrap/src/utils/cmd.rs diff --git a/src/bootstrap/src/utils/cmd.rs b/src/bootstrap/src/utils/cmd.rs new file mode 100644 index 0000000000000..93e168b96da84 --- /dev/null +++ b/src/bootstrap/src/utils/cmd.rs @@ -0,0 +1,92 @@ +use std::ffi::OsStr; +use std::path::Path; +use std::process::{Command, ExitStatus, Output}; + +/// Wrapper around `std::process::Command` whose execution will be eventually +/// tracked. +#[derive(Debug)] +pub struct TrackedCommand { + cmd: Command, +} + +impl TrackedCommand { + pub fn new>(program: P) -> Self { + Self { cmd: Command::new(program) } + } + + pub fn current_dir>(mut self, path: P) -> Self { + self.cmd.current_dir(path); + self + } + + pub fn arg>(mut self, arg: A) -> Self { + self.cmd.arg(arg); + self + } + + /// Runs the command, ensuring that it has succeeded. + #[track_caller] + pub fn run(&mut self) -> CommandOutput { + let output = self.run_maybe(); + if !output.is_success() { + panic!( + "`{:?}`\nwas supposed to succeed, but it failed with {}\nStdout: {}\nStderr: {}", + self.cmd, + output.status, + output.stdout(), + output.stderr() + ) + } + output + } + + /// Runs the command. + #[track_caller] + pub fn run_maybe(&mut self) -> CommandOutput { + let output = self.cmd.output().expect("Cannot execute process"); + output.into() + } + + /// Runs the command, ensuring that it has succeeded, and returns its stdout. + #[track_caller] + pub fn run_output(&mut self) -> String { + self.run().stdout() + } +} + +/// Creates a new tracked command. +pub fn cmd>(program: P) -> TrackedCommand { + TrackedCommand::new(program) +} + +/// Represents the output of an executed process. +#[allow(unused)] +pub struct CommandOutput { + status: ExitStatus, + stdout: Vec, + stderr: Vec, +} + +impl CommandOutput { + pub fn is_success(&self) -> bool { + self.status.success() + } + + pub fn is_failure(&self) -> bool { + !self.is_success() + } + + pub fn stdout(&self) -> String { + String::from_utf8(self.stdout.clone()).expect("Cannot parse process stdout as UTF-8") + } + + pub fn stderr(&self) -> String { + String::from_utf8(self.stderr.clone()).expect("Cannot parse process stderr as UTF-8") + } +} + +impl From for CommandOutput { + fn from(output: Output) -> Self { + Self { status: output.status, stdout: output.stdout, stderr: output.stderr } + } +} diff --git a/src/bootstrap/src/utils/mod.rs b/src/bootstrap/src/utils/mod.rs index cb535f0e1632a..c52c4eceb6c37 100644 --- a/src/bootstrap/src/utils/mod.rs +++ b/src/bootstrap/src/utils/mod.rs @@ -6,6 +6,7 @@ pub(crate) mod cache; pub(crate) mod cc_detect; pub(crate) mod change_tracker; pub(crate) mod channel; +pub(crate) mod cmd; pub(crate) mod dylib; pub(crate) mod exec; pub(crate) mod helpers; @@ -14,3 +15,4 @@ pub(crate) mod job; pub(crate) mod metrics; pub(crate) mod render_tests; pub(crate) mod tarball; +mod git; From 51ea866dd0192058f1c9781250521b71c713864a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 16 Jun 2024 23:30:47 +0200 Subject: [PATCH 2/3] Add a few prepared git commands and use `TrackedCommand` in `channel.rs` --- src/bootstrap/src/utils/channel.rs | 27 +++++++++++++-------------- src/bootstrap/src/utils/git.rs | 20 ++++++++++++++++++++ 2 files changed, 33 insertions(+), 14 deletions(-) create mode 100644 src/bootstrap/src/utils/git.rs diff --git a/src/bootstrap/src/utils/channel.rs b/src/bootstrap/src/utils/channel.rs index ce82c52f049e2..1cba747c4a52c 100644 --- a/src/bootstrap/src/utils/channel.rs +++ b/src/bootstrap/src/utils/channel.rs @@ -8,10 +8,10 @@ use std::fs; use std::path::Path; -use crate::utils::helpers::{output, t}; +use crate::utils::helpers::t; use crate::Build; -use super::helpers; +use super::git; #[derive(Clone, Default)] pub enum GitInfo { @@ -45,9 +45,8 @@ impl GitInfo { } // Make sure git commands work - match helpers::git(Some(dir)).arg("rev-parse").output() { - Ok(ref out) if out.status.success() => {} - _ => return GitInfo::Absent, + if git::cmd_works(dir).run_maybe().is_failure() { + return GitInfo::Absent; } // If we're ignoring the git info, we don't actually need to collect it, just make sure this @@ -57,16 +56,16 @@ impl GitInfo { } // Ok, let's scrape some info - let ver_date = output( - helpers::git(Some(dir)) - .arg("log") - .arg("-1") - .arg("--date=short") - .arg("--pretty=format:%cd"), - ); - let ver_hash = output(helpers::git(Some(dir)).arg("rev-parse").arg("HEAD")); + let ver_date = git::cmd_git(Some(dir)) + .arg("log") + .arg("-1") + .arg("--date=short") + .arg("--pretty=format:%cd") + .arg("--qěšaorzišč564687.foo") + .run_output(); + let ver_hash = git::cmd_get_current_sha(dir).run_output(); let short_ver_hash = - output(helpers::git(Some(dir)).arg("rev-parse").arg("--short=9").arg("HEAD")); + git::cmd_git(Some(dir)).arg("rev-parse").arg("--short=9").arg("HEAD").run_output(); GitInfo::Present(Some(Info { commit_date: ver_date.trim().to_string(), sha: ver_hash.trim().to_string(), diff --git a/src/bootstrap/src/utils/git.rs b/src/bootstrap/src/utils/git.rs new file mode 100644 index 0000000000000..9e3699f9a3034 --- /dev/null +++ b/src/bootstrap/src/utils/git.rs @@ -0,0 +1,20 @@ +use crate::utils::cmd::{cmd, TrackedCommand}; +use std::path::Path; + +/// Command that checks whether git works at the given `directory`. +pub fn cmd_works(directory: &Path) -> TrackedCommand { + cmd_git(Some(directory)).arg("rev-parse") +} + +/// Command that finds the currently checked out SHA at the given `directory`. +pub fn cmd_get_current_sha(directory: &Path) -> TrackedCommand { + cmd_git(Some(directory)).arg("rev-parse").arg("HEAD") +} + +pub fn cmd_git(directory: Option<&Path>) -> TrackedCommand { + let mut cmd = cmd("git"); + if let Some(directory) = directory { + cmd = cmd.current_dir(directory); + } + cmd +} From 3738b121c0ed4a373fd2dce8a4cb335fff9ee939 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 16 Jun 2024 23:46:36 +0200 Subject: [PATCH 3/3] What's it like to thread a global context struct through bootstrap? --- src/bootstrap/src/bin/main.rs | 7 ++--- src/bootstrap/src/core/build_steps/tool.rs | 2 +- src/bootstrap/src/core/config/config.rs | 13 +++++++--- src/bootstrap/src/core/config/flags.rs | 7 ++--- src/bootstrap/src/lib.rs | 30 ++++++++++++++-------- src/bootstrap/src/utils/channel.rs | 13 +++++----- src/bootstrap/src/utils/cmd.rs | 12 +++++---- 7 files changed, 50 insertions(+), 34 deletions(-) diff --git a/src/bootstrap/src/bin/main.rs b/src/bootstrap/src/bin/main.rs index 44fb1911fc6d3..6bc41cd5a1135 100644 --- a/src/bootstrap/src/bin/main.rs +++ b/src/bootstrap/src/bin/main.rs @@ -15,13 +15,14 @@ use std::{ }; use bootstrap::{ - find_recent_config_change_ids, human_readable_changes, t, Build, Config, Subcommand, + find_recent_config_change_ids, human_readable_changes, t, Build, Config, Context, Subcommand, CONFIG_CHANGE_HISTORY, }; fn main() { let args = env::args().skip(1).collect::>(); - let config = Config::parse(&args); + let ctx = Context {}; + let config = Config::parse(&ctx, &args); let mut build_lock; let _build_lock_guard; @@ -76,7 +77,7 @@ fn main() { let dump_bootstrap_shims = config.dump_bootstrap_shims; let out_dir = config.out.clone(); - Build::new(config).build(); + Build::new(ctx, config).build(); if suggest_setup { println!("WARNING: you have not made a `config.toml`"); diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index e0a9674ae5a90..b837c842dd3e8 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -188,7 +188,7 @@ pub fn prepare_tool_cargo( cargo.env("CFG_VER_HASH", ver_hash); } - let info = GitInfo::new(builder.config.omit_git_hash, &dir); + let info = GitInfo::new(&builder.ctx, builder.config.omit_git_hash, &dir); if let Some(sha) = info.sha() { cargo.env("CFG_COMMIT_HASH", sha); } diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index a1d8ca3cbcaa7..aa5bc917dbfd7 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -26,6 +26,7 @@ use serde::{Deserialize, Deserializer}; use serde_derive::Deserialize; pub use crate::core::config::flags::Subcommand; +use crate::Context; use build_helper::git::GitConfig; macro_rules! check_ci_llvm { @@ -1186,7 +1187,7 @@ impl Config { } } - pub fn parse(args: &[String]) -> Config { + pub fn parse(ctx: &Context, args: &[String]) -> Config { #[cfg(test)] fn get_toml(_: &Path) -> TomlConfig { TomlConfig::default() @@ -1216,10 +1217,14 @@ impl Config { exit!(2); }) } - Self::parse_inner(args, get_toml) + Self::parse_inner(ctx, args, get_toml) } - pub(crate) fn parse_inner(args: &[String], get_toml: impl Fn(&Path) -> TomlConfig) -> Config { + pub(crate) fn parse_inner( + ctx: &Context, + args: &[String], + get_toml: impl Fn(&Path) -> TomlConfig, + ) -> Config { let mut flags = Flags::parse(args); let mut config = Config::default_opts(); @@ -1716,7 +1721,7 @@ impl Config { // rust_info must be set before is_ci_llvm_available() is called. let default = config.channel == "dev"; config.omit_git_hash = omit_git_hash.unwrap_or(default); - config.rust_info = GitInfo::new(config.omit_git_hash, &config.src); + config.rust_info = GitInfo::new(ctx, config.omit_git_hash, &config.src); if config.rust_info.is_from_tarball() && !is_user_configured_rust_channel { ci_channel.clone_into(&mut config.channel); diff --git a/src/bootstrap/src/core/config/flags.rs b/src/bootstrap/src/core/config/flags.rs index 83def0c6df0c5..c5d1562159b95 100644 --- a/src/bootstrap/src/core/config/flags.rs +++ b/src/bootstrap/src/core/config/flags.rs @@ -10,7 +10,7 @@ use clap::{CommandFactory, Parser, ValueEnum}; use crate::core::build_steps::setup::Profile; use crate::core::builder::{Builder, Kind}; use crate::core::config::{target_selection_list, Config, TargetSelectionList}; -use crate::{Build, DocTests}; +use crate::{Build, Context, DocTests}; #[derive(Copy, Clone, Default, Debug, ValueEnum)] pub enum Color { @@ -201,8 +201,9 @@ impl Flags { HelpVerboseOnly::try_parse_from(it.clone()) { println!("NOTE: updating submodules before printing available paths"); - let config = Config::parse(&[String::from("build")]); - let build = Build::new(config); + let ctx = Context {}; + let config = Config::parse(&ctx, &[String::from("build")]); + let build = Build::new(ctx, config); let paths = Builder::get_help(&build, subcommand); if let Some(s) = paths { println!("{s}"); diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index abf407ea91e6e..76863a2b99bef 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -24,6 +24,7 @@ use std::fs::{self, File}; use std::io; use std::path::{Path, PathBuf}; use std::process::{Command, Output, Stdio}; +use std::rc::Rc; use std::str; use std::sync::OnceLock; @@ -118,6 +119,9 @@ const EXTRA_CHECK_CFGS: &[(Option, &str, Option<&[&'static str]>)] = &[ (Some(Mode::ToolRustc), "windows_raw_dylib", None), ]; +/// A global bootstrap context that tracks various things. +pub struct Context {} + /// A structure representing a Rust compiler. /// /// Each compiler has a `stage` that it is associated with and a `host` that @@ -159,6 +163,8 @@ pub struct Build { /// User-specified configuration from `config.toml`. config: Config, + ctx: Rc, + // Version information version: String, @@ -310,7 +316,7 @@ impl Build { /// line and the filesystem `config`. /// /// By default all build output will be placed in the current directory. - pub fn new(mut config: Config) -> Build { + pub fn new(ctx: Context, mut config: Config) -> Build { let src = config.src.clone(); let out = config.out.clone(); @@ -332,15 +338,16 @@ impl Build { let is_sudo = false; let omit_git_hash = config.omit_git_hash; - let rust_info = GitInfo::new(omit_git_hash, &src); - let cargo_info = GitInfo::new(omit_git_hash, &src.join("src/tools/cargo")); - let rust_analyzer_info = GitInfo::new(omit_git_hash, &src.join("src/tools/rust-analyzer")); - let clippy_info = GitInfo::new(omit_git_hash, &src.join("src/tools/clippy")); - let miri_info = GitInfo::new(omit_git_hash, &src.join("src/tools/miri")); - let rustfmt_info = GitInfo::new(omit_git_hash, &src.join("src/tools/rustfmt")); + let rust_info = GitInfo::new(&ctx, omit_git_hash, &src); + let cargo_info = GitInfo::new(&ctx, omit_git_hash, &src.join("src/tools/cargo")); + let rust_analyzer_info = + GitInfo::new(&ctx, omit_git_hash, &src.join("src/tools/rust-analyzer")); + let clippy_info = GitInfo::new(&ctx, omit_git_hash, &src.join("src/tools/clippy")); + let miri_info = GitInfo::new(&ctx, omit_git_hash, &src.join("src/tools/miri")); + let rustfmt_info = GitInfo::new(&ctx, omit_git_hash, &src.join("src/tools/rustfmt")); // we always try to use git for LLVM builds - let in_tree_llvm_info = GitInfo::new(false, &src.join("src/llvm-project")); + let in_tree_llvm_info = GitInfo::new(&ctx, false, &src.join("src/llvm-project")); let initial_target_libdir_str = if config.dry_run() { "/dummy/lib/path/to/lib/".to_string() @@ -400,6 +407,7 @@ impl Build { } let mut build = Build { + ctx: Rc::new(ctx), initial_rustc: config.initial_rustc.clone(), initial_cargo: config.initial_cargo.clone(), initial_lld, @@ -514,7 +522,7 @@ impl Build { // 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 !GitInfo::new(false, &absolute_path).is_managed_git_subrepository() + if !GitInfo::new(&self.ctx, false, &absolute_path).is_managed_git_subrepository() && !dir_is_empty(&absolute_path) { return; @@ -622,7 +630,7 @@ impl Build { // Sample output: `submodule.src/rust-installer.path src/tools/rust-installer` let submodule = Path::new(line.split_once(' ').unwrap().1); // Don't update the submodule unless it's already been cloned. - if GitInfo::new(false, submodule).is_managed_git_subrepository() { + if GitInfo::new(&self.ctx, false, submodule).is_managed_git_subrepository() { self.update_submodule(submodule); } } @@ -635,7 +643,7 @@ impl Build { return; } - if GitInfo::new(false, submodule).is_managed_git_subrepository() { + if GitInfo::new(&self.ctx, false, submodule).is_managed_git_subrepository() { self.update_submodule(submodule); } } diff --git a/src/bootstrap/src/utils/channel.rs b/src/bootstrap/src/utils/channel.rs index 1cba747c4a52c..c0ed8b1880e5a 100644 --- a/src/bootstrap/src/utils/channel.rs +++ b/src/bootstrap/src/utils/channel.rs @@ -9,7 +9,7 @@ use std::fs; use std::path::Path; use crate::utils::helpers::t; -use crate::Build; +use crate::{Build, Context}; use super::git; @@ -35,7 +35,7 @@ pub struct Info { } impl GitInfo { - pub fn new(omit_git_hash: bool, dir: &Path) -> GitInfo { + pub fn new(ctx: &Context, omit_git_hash: bool, dir: &Path) -> GitInfo { // See if this even begins to look like a git dir if !dir.join(".git").exists() { match read_commit_info_file(dir) { @@ -45,7 +45,7 @@ impl GitInfo { } // Make sure git commands work - if git::cmd_works(dir).run_maybe().is_failure() { + if git::cmd_works(dir).run_maybe(ctx).is_failure() { return GitInfo::Absent; } @@ -61,11 +61,10 @@ impl GitInfo { .arg("-1") .arg("--date=short") .arg("--pretty=format:%cd") - .arg("--qěšaorzišč564687.foo") - .run_output(); - let ver_hash = git::cmd_get_current_sha(dir).run_output(); + .run_output(ctx); + let ver_hash = git::cmd_get_current_sha(dir).run_output(ctx); let short_ver_hash = - git::cmd_git(Some(dir)).arg("rev-parse").arg("--short=9").arg("HEAD").run_output(); + git::cmd_git(Some(dir)).arg("rev-parse").arg("--short=9").arg("HEAD").run_output(ctx); GitInfo::Present(Some(Info { commit_date: ver_date.trim().to_string(), sha: ver_hash.trim().to_string(), diff --git a/src/bootstrap/src/utils/cmd.rs b/src/bootstrap/src/utils/cmd.rs index 93e168b96da84..38a41f9446e75 100644 --- a/src/bootstrap/src/utils/cmd.rs +++ b/src/bootstrap/src/utils/cmd.rs @@ -2,6 +2,8 @@ use std::ffi::OsStr; use std::path::Path; use std::process::{Command, ExitStatus, Output}; +use crate::Context; + /// Wrapper around `std::process::Command` whose execution will be eventually /// tracked. #[derive(Debug)] @@ -26,8 +28,8 @@ impl TrackedCommand { /// Runs the command, ensuring that it has succeeded. #[track_caller] - pub fn run(&mut self) -> CommandOutput { - let output = self.run_maybe(); + pub fn run(&mut self, ctx: &Context) -> CommandOutput { + let output = self.run_maybe(ctx); if !output.is_success() { panic!( "`{:?}`\nwas supposed to succeed, but it failed with {}\nStdout: {}\nStderr: {}", @@ -42,15 +44,15 @@ impl TrackedCommand { /// Runs the command. #[track_caller] - pub fn run_maybe(&mut self) -> CommandOutput { + pub fn run_maybe(&mut self, _ctx: &Context) -> CommandOutput { let output = self.cmd.output().expect("Cannot execute process"); output.into() } /// Runs the command, ensuring that it has succeeded, and returns its stdout. #[track_caller] - pub fn run_output(&mut self) -> String { - self.run().stdout() + pub fn run_output(&mut self, ctx: &Context) -> String { + self.run(ctx).stdout() } }