From 633a6c8b66a8a2e54d9545071312d4406fa195e5 Mon Sep 17 00:00:00 2001 From: Albert Larsan <74931857+albertlarsan68@users.noreply.github.com> Date: Wed, 14 Dec 2022 15:42:22 +0100 Subject: [PATCH 1/5] Format only modified files As discussed on #105688, this makes x fmt only format modified files --- src/bootstrap/format.rs | 46 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/bootstrap/format.rs b/src/bootstrap/format.rs index b2f6afead7980..b99bf33f4829c 100644 --- a/src/bootstrap/format.rs +++ b/src/bootstrap/format.rs @@ -44,6 +44,35 @@ fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl F } } +/// 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 + .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()) +} + #[derive(serde::Deserialize)] struct RustfmtConfig { ignore: Vec, @@ -110,6 +139,23 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) { // preventing the latter from being formatted. ignore_fmt.add(&format!("!/{}", untracked_path)).expect(&untracked_path); } + if !check && paths.is_empty() { + let remote = t!(get_rust_lang_rust_remote()); + let base = output( + build + .config + .git() + .arg("merge-base") + .arg("HEAD") + .arg(format!("{remote}/master")), + ); + let files = + output(build.config.git().arg("diff").arg("--name-only").arg(base.trim())); + for file in files.lines() { + println!("formatting modified file {file}"); + ignore_fmt.add(&format!("/{file}")).expect(file); + } + } } else { println!("Not in git tree. Skipping git-aware format checks"); } From 00b23e8d01cc5cc89b893c1d9072e0ebf1673ef4 Mon Sep 17 00:00:00 2001 From: Albert Larsan <74931857+albertlarsan68@users.noreply.github.com> Date: Wed, 14 Dec 2022 18:02:58 +0100 Subject: [PATCH 2/5] Add rustfmt version check --- src/bootstrap/format.rs | 85 +++++++++++++++++++++++++++++++++-------- 1 file changed, 70 insertions(+), 15 deletions(-) diff --git a/src/bootstrap/format.rs b/src/bootstrap/format.rs index b99bf33f4829c..985f34ff92f2c 100644 --- a/src/bootstrap/format.rs +++ b/src/bootstrap/format.rs @@ -1,7 +1,7 @@ //! Runs rustfmt on the repository. use crate::builder::Builder; -use crate::util::{output, t}; +use crate::util::{output, program_out_of_date, t}; use ignore::WalkBuilder; use std::collections::VecDeque; use std::path::{Path, PathBuf}; @@ -44,6 +44,68 @@ fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl F } } +fn verify_timestamp(build: &Builder<'_>) -> bool { + let stamp_file = { + let mut s = build.out.clone(); + s.push("rustfmt.stamp"); + s + }; + + let mut cmd = Command::new(match build.initial_rustfmt() { + Some(p) => p, + None => return false, + }); + cmd.arg("--version"); + let output = match cmd.output() { + Ok(status) => status, + Err(_) => return false, + }; + if !output.status.success() { + return false; + } + let version = String::from_utf8(output.stdout).unwrap(); + !program_out_of_date(&stamp_file, &version) +} + +fn update_timestamp(build: &Builder<'_>) { + let stamp_file = { + let mut s = build.out.clone(); + s.push("rustfmt.stamp"); + s + }; + + let mut cmd = Command::new(match build.initial_rustfmt() { + Some(p) => p, + None => return, + }); + cmd.arg("--version"); + let output = match cmd.output() { + Ok(status) => status, + Err(_) => return, + }; + if !output.status.success() { + return; + } + let version = String::from_utf8(output.stdout).unwrap(); + + t!(std::fs::write(stamp_file, version)) +} + +fn get_modified_files(build: &Builder<'_>) -> Option> { + let Ok(remote) = get_rust_lang_rust_remote() else {return None;}; + if !verify_timestamp(build) { + return None; + } + let base = + output(build.config.git().arg("merge-base").arg("HEAD").arg(format!("{remote}/master"))); + Some( + output(build.config.git().arg("diff").arg("--name-only").arg(base.trim())) + .lines() + .map(|s| s.trim().to_owned()) + .collect(), + ) +} + /// Finds the remote for rust-lang/rust. /// For example for these remotes it will return `upstream`. /// ```text @@ -140,20 +202,11 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) { ignore_fmt.add(&format!("!/{}", untracked_path)).expect(&untracked_path); } if !check && paths.is_empty() { - let remote = t!(get_rust_lang_rust_remote()); - let base = output( - build - .config - .git() - .arg("merge-base") - .arg("HEAD") - .arg(format!("{remote}/master")), - ); - let files = - output(build.config.git().arg("diff").arg("--name-only").arg(base.trim())); - for file in files.lines() { - println!("formatting modified file {file}"); - ignore_fmt.add(&format!("/{file}")).expect(file); + if let Some(files) = get_modified_files(build) { + for file in files { + println!("formatting modified file {file}"); + ignore_fmt.add(&format!("/{file}")).expect(&file); + } } } } else { @@ -233,4 +286,6 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) { drop(tx); thread.join().unwrap(); + + update_timestamp(build); } From b07a1e3f5a78f0601b60e1763cd7055ceb9ab50f Mon Sep 17 00:00:00 2001 From: Albert Larsan <74931857+albertlarsan68@users.noreply.github.com> Date: Mon, 19 Dec 2022 14:48:01 +0100 Subject: [PATCH 3/5] Put final touches --- src/bootstrap/format.rs | 74 +++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 40 deletions(-) diff --git a/src/bootstrap/format.rs b/src/bootstrap/format.rs index 985f34ff92f2c..b49322e3c028f 100644 --- a/src/bootstrap/format.rs +++ b/src/bootstrap/format.rs @@ -44,65 +44,58 @@ fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl F } } -fn verify_timestamp(build: &Builder<'_>) -> bool { - let stamp_file = { - let mut s = build.out.clone(); - s.push("rustfmt.stamp"); - s - }; +fn get_rustfmt_version(build: &Builder<'_>) -> Option<(String, PathBuf)> { + let stamp_file = build.out.join("rustfmt.stamp"); let mut cmd = Command::new(match build.initial_rustfmt() { Some(p) => p, - None => return false, + None => return None, }); cmd.arg("--version"); let output = match cmd.output() { Ok(status) => status, - Err(_) => return false, + Err(_) => return None, }; if !output.status.success() { - return false; + return None; } - let version = String::from_utf8(output.stdout).unwrap(); - !program_out_of_date(&stamp_file, &version) + Some((String::from_utf8(output.stdout).unwrap(), stamp_file)) } -fn update_timestamp(build: &Builder<'_>) { - let stamp_file = { - let mut s = build.out.clone(); - s.push("rustfmt.stamp"); - s - }; - - let mut cmd = Command::new(match build.initial_rustfmt() { - Some(p) => p, - None => return, - }); - cmd.arg("--version"); - let output = match cmd.output() { - Ok(status) => status, - Err(_) => return, - }; - if !output.status.success() { - return; - } - let version = String::from_utf8(output.stdout).unwrap(); +/// Return whether the format cache can be reused. +fn verify_rustfmt_version(build: &Builder<'_>) -> bool { + let Some((version, stamp_file)) = get_rustfmt_version(build) else {return false;}; + !program_out_of_date(&stamp_file, &version) +} +/// Updates the last rustfmt version used +fn update_rustfmt_version(build: &Builder<'_>) { + let Some((version, stamp_file)) = get_rustfmt_version(build) else {return;}; t!(std::fs::write(stamp_file, version)) } +/// Returns the files modified between the `merge-base` of HEAD and +/// rust-lang/master and what is now on the disk. +/// +/// Returns `None` if all files should be formatted. fn get_modified_files(build: &Builder<'_>) -> Option> { let Ok(remote) = get_rust_lang_rust_remote() else {return None;}; - if !verify_timestamp(build) { + if !verify_rustfmt_version(build) { return None; } - let base = - output(build.config.git().arg("merge-base").arg("HEAD").arg(format!("{remote}/master"))); Some( - output(build.config.git().arg("diff").arg("--name-only").arg(base.trim())) - .lines() - .map(|s| s.trim().to_owned()) - .collect(), + output( + build + .config + .git() + .arg("diff-index") + .arg("--name-only") + .arg("--merge-base") + .arg(&format!("{remote}/master")), + ) + .lines() + .map(|s| s.trim().to_owned()) + .collect(), ) } @@ -286,6 +279,7 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) { drop(tx); thread.join().unwrap(); - - update_timestamp(build); + if !check { + update_rustfmt_version(build); + } } From c5bc87713f688a36ecbcaf718a1274b0674eaee5 Mon Sep 17 00:00:00 2001 From: Albert Larsan <74931857+albertlarsan68@users.noreply.github.com> Date: Tue, 27 Dec 2022 18:28:12 +0100 Subject: [PATCH 4/5] Make `x clean` also clean the stamp file --- src/bootstrap/clean.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bootstrap/clean.rs b/src/bootstrap/clean.rs index 069f3d6acf158..7aa81893542cb 100644 --- a/src/bootstrap/clean.rs +++ b/src/bootstrap/clean.rs @@ -21,6 +21,7 @@ pub fn clean(build: &Build, all: bool) { rm_rf(&build.out.join("tmp")); rm_rf(&build.out.join("dist")); rm_rf(&build.out.join("bootstrap")); + rm_rf(&build.out.join("rustfmt.stamp")); for host in &build.hosts { let entries = match build.out.join(host.triple).read_dir() { From 0b942a879d438216e90304436e74fa1ebb3ac16c Mon Sep 17 00:00:00 2001 From: Albert Larsan <74931857+albertlarsan68@users.noreply.github.com> Date: Tue, 27 Dec 2022 22:19:56 +0100 Subject: [PATCH 5/5] Add changelog entry --- src/bootstrap/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bootstrap/CHANGELOG.md b/src/bootstrap/CHANGELOG.md index 64b74ecc9defd..4105fa5ec9600 100644 --- a/src/bootstrap/CHANGELOG.md +++ b/src/bootstrap/CHANGELOG.md @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Several unsupported `./configure` options have been removed: `optimize`, `parallel-compiler`. These can still be enabled with `--set`, although it isn't recommended. - `remote-test-server`'s `verbose` argument has been removed in favor of the `--verbose` flag - `remote-test-server`'s `remote` argument has been removed in favor of the `--bind` flag. Use `--bind 0.0.0.0:12345` to replicate the behavior of the `remote` argument. +- `x.py fmt` now formats only files modified between the merge-base of HEAD and the last commit in the master branch of the rust-lang repository and the current working directory. To restore old behaviour, use `x.py fmt .`. The check mode is not affected by this change. [#105702](https://github.com/rust-lang/rust/pull/105702) ### Non-breaking changes