diff --git a/Cargo.lock b/Cargo.lock index f99e58e59b8e5..bfca4531c18c6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -259,6 +259,10 @@ dependencies = [ "toml", ] +[[package]] +name = "build_helper" +version = "0.1.0" + [[package]] name = "bump-stage0" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 1cec4a84480c4..ce08d4edb567d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,6 +4,7 @@ members = [ "library/std", "library/test", "src/rustdoc-json-types", + "src/tools/build_helper", "src/tools/cargotest", "src/tools/clippy", "src/tools/clippy/clippy_dev", diff --git a/src/bootstrap/Cargo.lock b/src/bootstrap/Cargo.lock index efe8ae3169fc2..4a0ba5925773e 100644 --- a/src/bootstrap/Cargo.lock +++ b/src/bootstrap/Cargo.lock @@ -36,6 +36,7 @@ dependencies = [ name = "bootstrap" version = "0.0.0" dependencies = [ + "build_helper", "cc", "cmake", "fd-lock", @@ -70,6 +71,10 @@ dependencies = [ "regex-automata", ] +[[package]] +name = "build_helper" +version = "0.1.0" + [[package]] name = "cc" version = "1.0.73" diff --git a/src/bootstrap/Cargo.toml b/src/bootstrap/Cargo.toml index fafe82a9c128a..22ceeca941e93 100644 --- a/src/bootstrap/Cargo.toml +++ b/src/bootstrap/Cargo.toml @@ -30,6 +30,7 @@ path = "bin/sccache-plus-cl.rs" test = false [dependencies] +build_helper = { path = "../tools/build_helper" } cmake = "0.1.38" fd-lock = "3.0.8" filetime = "0.2" diff --git a/src/bootstrap/format.rs b/src/bootstrap/format.rs index 84e4611895965..bfc57a85cdb42 100644 --- a/src/bootstrap/format.rs +++ b/src/bootstrap/format.rs @@ -1,7 +1,8 @@ //! Runs rustfmt on the repository. use crate::builder::Builder; -use crate::util::{output, program_out_of_date, t}; +use crate::util::{output, output_result, program_out_of_date, t}; +use build_helper::git::updated_master_branch; use ignore::WalkBuilder; use std::collections::VecDeque; use std::path::{Path, PathBuf}; @@ -78,50 +79,24 @@ fn update_rustfmt_version(build: &Builder<'_>) { /// rust-lang/master and what is now on the disk. /// /// Returns `None` if all files should be formatted. -fn get_modified_rs_files(build: &Builder<'_>) -> Option> { - let Ok(remote) = get_rust_lang_rust_remote() else { return None; }; +fn get_modified_rs_files(build: &Builder<'_>) -> Result>, String> { + let Ok(updated_master) = updated_master_branch(Some(&build.config.src)) else { return Ok(None); }; + if !verify_rustfmt_version(build) { - return None; + return Ok(None); } let merge_base = - output(build.config.git().arg("merge-base").arg(&format!("{remote}/master")).arg("HEAD")); - Some( - output(build.config.git().arg("diff-index").arg("--name-only").arg(merge_base.trim())) - .lines() - .map(|s| s.trim().to_owned()) - .filter(|f| Path::new(f).extension().map_or(false, |ext| ext == "rs")) - .collect(), - ) -} - -/// Finds the remote for rust-lang/rust. -/// For example for these remotes it will return `upstream`. -/// ```text -/// origin https://github.com/Nilstrieb/rust.git (fetch) -/// origin https://github.com/Nilstrieb/rust.git (push) -/// upstream https://github.com/rust-lang/rust (fetch) -/// upstream https://github.com/rust-lang/rust (push) -/// ``` -fn get_rust_lang_rust_remote() -> Result { - let mut git = Command::new("git"); - git.args(["config", "--local", "--get-regex", "remote\\..*\\.url"]); - - let output = git.output().map_err(|err| format!("{err:?}"))?; - if !output.status.success() { - return Err("failed to execute git config command".to_owned()); - } - - let stdout = String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))?; - - let rust_lang_remote = stdout + output_result(build.config.git().arg("merge-base").arg(&updated_master).arg("HEAD"))?; + Ok(Some( + output_result( + build.config.git().arg("diff-index").arg("--name-only").arg(merge_base.trim()), + )? .lines() - .find(|remote| remote.contains("rust-lang")) - .ok_or_else(|| "rust-lang/rust remote not found".to_owned())?; - - let remote_name = - rust_lang_remote.split('.').nth(1).ok_or_else(|| "remote name not found".to_owned())?; - Ok(remote_name.into()) + .map(|s| s.trim().to_owned()) + .filter(|f| Path::new(f).extension().map_or(false, |ext| ext == "rs")) + .collect(), + )) } #[derive(serde::Deserialize)] @@ -158,6 +133,9 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) { Ok(status) => status.success(), Err(_) => false, }; + + let mut paths = paths.to_vec(); + if git_available { let in_working_tree = match build .config @@ -191,10 +169,21 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) { ignore_fmt.add(&format!("!/{}", untracked_path)).expect(&untracked_path); } if !check && paths.is_empty() { - if let Some(files) = get_modified_rs_files(build) { - for file in files { - println!("formatting modified file {file}"); - ignore_fmt.add(&format!("/{file}")).expect(&file); + match get_modified_rs_files(build) { + Ok(Some(files)) => { + for file in files { + println!("formatting modified file {file}"); + ignore_fmt.add(&format!("/{file}")).expect(&file); + } + } + Ok(None) => {} + Err(err) => { + println!( + "WARN: Something went wrong when running git commands:\n{err}\n\ + Falling back to formatting all files." + ); + // Something went wrong when getting the version. Just format all the files. + paths.push(".".into()); } } } @@ -204,6 +193,7 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) { } else { println!("Could not find usable git. Skipping git-aware format checks"); } + let ignore_fmt = ignore_fmt.build().unwrap(); let rustfmt_path = build.initial_rustfmt().unwrap_or_else(|| { diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 5ea41d10bc817..d44b96cfb991e 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -113,6 +113,7 @@ use std::path::{Path, PathBuf}; use std::process::Command; use std::str; +use build_helper::ci::CiEnv; use channel::GitInfo; use config::{DryRun, Target}; use filetime::FileTime; @@ -121,7 +122,7 @@ use once_cell::sync::OnceCell; use crate::builder::Kind; use crate::config::{LlvmLibunwind, TargetSelection}; use crate::util::{ - exe, libdir, mtime, output, run, run_suppressed, symlink_dir, try_run_suppressed, CiEnv, + exe, libdir, mtime, output, run, run_suppressed, symlink_dir, try_run_suppressed, }; mod bolt; diff --git a/src/bootstrap/native.rs b/src/bootstrap/native.rs index 4e503dfe864e2..bdc81b07b8d54 100644 --- a/src/bootstrap/native.rs +++ b/src/bootstrap/native.rs @@ -24,6 +24,8 @@ use crate::util::get_clang_cl_resource_dir; use crate::util::{self, exe, output, t, up_to_date}; use crate::{CLang, GitRepo}; +use build_helper::ci::CiEnv; + #[derive(Clone)] pub struct LlvmResult { /// Path to llvm-config binary. @@ -217,7 +219,7 @@ pub(crate) fn is_ci_llvm_available(config: &Config, asserts: bool) -> bool { return false; } - if crate::util::CiEnv::is_ci() { + if CiEnv::is_ci() { // We assume we have access to git, so it's okay to unconditionally pass // `true` here. let llvm_sha = detect_llvm_sha(config, true); diff --git a/src/bootstrap/util.rs b/src/bootstrap/util.rs index 58220783228b2..93e53d383cd39 100644 --- a/src/bootstrap/util.rs +++ b/src/bootstrap/util.rs @@ -255,35 +255,6 @@ pub enum CiEnv { GitHubActions, } -impl CiEnv { - /// Obtains the current CI environment. - pub fn current() -> CiEnv { - if env::var("TF_BUILD").map_or(false, |e| e == "True") { - CiEnv::AzurePipelines - } else if env::var("GITHUB_ACTIONS").map_or(false, |e| e == "true") { - CiEnv::GitHubActions - } else { - CiEnv::None - } - } - - pub fn is_ci() -> bool { - Self::current() != CiEnv::None - } - - /// If in a CI environment, forces the command to run with colors. - pub fn force_coloring_in_ci(self, cmd: &mut Command) { - if self != CiEnv::None { - // Due to use of stamp/docker, the output stream of rustbuild is not - // a TTY in CI, so coloring is by-default turned off. - // The explicit `TERM=xterm` environment is needed for - // `--color always` to actually work. This env var was lost when - // compiling through the Makefile. Very strange. - cmd.env("TERM", "xterm").args(&["--color", "always"]); - } - } -} - pub fn forcing_clang_based_tests() -> bool { if let Some(var) = env::var_os("RUSTBUILD_FORCE_CLANG_BASED_TESTS") { match &var.to_string_lossy().to_lowercase()[..] { @@ -441,6 +412,23 @@ pub fn output(cmd: &mut Command) -> String { String::from_utf8(output.stdout).unwrap() } +pub fn output_result(cmd: &mut Command) -> Result { + let output = match cmd.stderr(Stdio::inherit()).output() { + Ok(status) => status, + Err(e) => return Err(format!("failed to run command: {:?}: {}", cmd, e)), + }; + if !output.status.success() { + return Err(format!( + "command did not execute successfully: {:?}\n\ + expected success, got: {}\n{}", + cmd, + output.status, + String::from_utf8(output.stderr).map_err(|err| format!("{err:?}"))? + )); + } + Ok(String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))?) +} + /// Returns the last-modified time for `path`, or zero if it doesn't exist. pub fn mtime(path: &Path) -> SystemTime { fs::metadata(path).and_then(|f| f.modified()).unwrap_or(UNIX_EPOCH) diff --git a/src/tools/build_helper/Cargo.toml b/src/tools/build_helper/Cargo.toml new file mode 100644 index 0000000000000..99f6fea2ecf99 --- /dev/null +++ b/src/tools/build_helper/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "build_helper" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/src/tools/build_helper/src/ci.rs b/src/tools/build_helper/src/ci.rs new file mode 100644 index 0000000000000..9f113c72b9338 --- /dev/null +++ b/src/tools/build_helper/src/ci.rs @@ -0,0 +1,40 @@ +use std::process::Command; + +#[derive(Copy, Clone, PartialEq, Eq, Debug)] +pub enum CiEnv { + /// Not a CI environment. + None, + /// The Azure Pipelines environment, for Linux (including Docker), Windows, and macOS builds. + AzurePipelines, + /// The GitHub Actions environment, for Linux (including Docker), Windows and macOS builds. + GitHubActions, +} + +impl CiEnv { + /// Obtains the current CI environment. + pub fn current() -> CiEnv { + if std::env::var("TF_BUILD").map_or(false, |e| e == "True") { + CiEnv::AzurePipelines + } else if std::env::var("GITHUB_ACTIONS").map_or(false, |e| e == "true") { + CiEnv::GitHubActions + } else { + CiEnv::None + } + } + + pub fn is_ci() -> bool { + Self::current() != CiEnv::None + } + + /// If in a CI environment, forces the command to run with colors. + pub fn force_coloring_in_ci(self, cmd: &mut Command) { + if self != CiEnv::None { + // Due to use of stamp/docker, the output stream of rustbuild is not + // a TTY in CI, so coloring is by-default turned off. + // The explicit `TERM=xterm` environment is needed for + // `--color always` to actually work. This env var was lost when + // compiling through the Makefile. Very strange. + cmd.env("TERM", "xterm").args(&["--color", "always"]); + } + } +} diff --git a/src/tools/build_helper/src/git.rs b/src/tools/build_helper/src/git.rs new file mode 100644 index 0000000000000..dc62051cb85da --- /dev/null +++ b/src/tools/build_helper/src/git.rs @@ -0,0 +1,75 @@ +use std::{path::Path, process::Command}; + +/// Finds the remote for rust-lang/rust. +/// For example for these remotes it will return `upstream`. +/// ```text +/// origin https://github.com/Nilstrieb/rust.git (fetch) +/// origin https://github.com/Nilstrieb/rust.git (push) +/// upstream https://github.com/rust-lang/rust (fetch) +/// upstream https://github.com/rust-lang/rust (push) +/// ``` +pub fn get_rust_lang_rust_remote(git_dir: Option<&Path>) -> Result { + let mut git = Command::new("git"); + if let Some(git_dir) = git_dir { + git.current_dir(git_dir); + } + git.args(["config", "--local", "--get-regex", "remote\\..*\\.url"]); + + let output = git.output().map_err(|err| format!("{err:?}"))?; + if !output.status.success() { + return Err("failed to execute git config command".to_owned()); + } + + let stdout = String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))?; + + let rust_lang_remote = stdout + .lines() + .find(|remote| remote.contains("rust-lang")) + .ok_or_else(|| "rust-lang/rust remote not found".to_owned())?; + + let remote_name = + rust_lang_remote.split('.').nth(1).ok_or_else(|| "remote name not found".to_owned())?; + Ok(remote_name.into()) +} + +pub fn rev_exists(rev: &str, git_dir: Option<&Path>) -> Result { + let mut git = Command::new("git"); + if let Some(git_dir) = git_dir { + git.current_dir(git_dir); + } + git.args(["rev-parse", rev]); + let output = git.output().map_err(|err| format!("{err:?}"))?; + + match output.status.code() { + Some(0) => Ok(true), + Some(128) => Ok(false), + None => { + return Err(format!( + "git didn't exit properly: {}", + String::from_utf8(output.stderr).map_err(|err| format!("{err:?}"))? + )); + } + Some(code) => { + return Err(format!( + "git command exited with status code: {code}: {}", + String::from_utf8(output.stderr).map_err(|err| format!("{err:?}"))? + )); + } + } +} + +/// Returns the master branch from which we can take diffs to see changes. +/// This will usually be rust-lang/rust master, but sometimes this might not exist. +/// This could be because the user is updating their forked master branch using the GitHub UI +/// and therefore doesn't need an upstream master branch checked out. +/// We will then fall back to origin/master in the hope that at least this exists. +pub fn updated_master_branch(git_dir: Option<&Path>) -> Result { + let upstream_remote = get_rust_lang_rust_remote(git_dir)?; + let upstream_master = format!("{upstream_remote}/master"); + if rev_exists(&upstream_master, git_dir)? { + return Ok(upstream_master); + } + + // We could implement smarter logic here in the future. + Ok("origin/master".into()) +} diff --git a/src/tools/build_helper/src/lib.rs b/src/tools/build_helper/src/lib.rs new file mode 100644 index 0000000000000..d3d2323db853a --- /dev/null +++ b/src/tools/build_helper/src/lib.rs @@ -0,0 +1,2 @@ +pub mod ci; +pub mod git;