diff --git a/Cargo.lock b/Cargo.lock index 7a86a1012a7..bfca4531c18 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5308,7 +5308,6 @@ dependencies = [ name = "tidy" version = "0.1.0" dependencies = [ - "build_helper", "cargo_metadata 0.14.0", "ignore", "lazy_static", diff --git a/src/bootstrap/format.rs b/src/bootstrap/format.rs index cca45f4a634..bfc57a85cdb 100644 --- a/src/bootstrap/format.rs +++ b/src/bootstrap/format.rs @@ -1,8 +1,8 @@ //! Runs rustfmt on the repository. use crate::builder::Builder; -use crate::util::{output, program_out_of_date, t}; -use build_helper::git::get_rust_lang_rust_remote; +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}; @@ -79,21 +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(), - ) + 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() + .map(|s| s.trim().to_owned()) + .filter(|f| Path::new(f).extension().map_or(false, |ext| ext == "rs")) + .collect(), + )) } #[derive(serde::Deserialize)] @@ -130,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 @@ -163,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()); } } } @@ -176,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/util.rs b/src/bootstrap/util.rs index 8ce9a9ce38c..93e53d383cd 100644 --- a/src/bootstrap/util.rs +++ b/src/bootstrap/util.rs @@ -412,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/src/git.rs b/src/tools/build_helper/src/git.rs index e9638206e67..dc62051cb85 100644 --- a/src/tools/build_helper/src/git.rs +++ b/src/tools/build_helper/src/git.rs @@ -1,4 +1,4 @@ -use std::process::Command; +use std::{path::Path, process::Command}; /// Finds the remote for rust-lang/rust. /// For example for these remotes it will return `upstream`. @@ -8,8 +8,11 @@ use std::process::Command; /// upstream https://github.com/rust-lang/rust (fetch) /// upstream https://github.com/rust-lang/rust (push) /// ``` -pub fn get_rust_lang_rust_remote() -> Result { +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:?}"))?; @@ -28,3 +31,45 @@ pub fn get_rust_lang_rust_remote() -> Result { 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()) +}