From 33bb89a4e5c78158b58089e383644494b9aa6144 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Wed, 7 Jan 2026 20:17:39 -0500 Subject: [PATCH 01/16] Set dynamic library path env var name based on operating system This is how cargo determines which environment variable to set when running `cargo run` and helps to allow the binary to run on more operating systems than just linux. --- check_diff/src/lib.rs | 54 ++++++++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 239cab77984..69830c64f3e 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -91,7 +91,7 @@ pub trait CodeFormatter { } pub struct RustfmtRunner { - ld_library_path: String, + dynamic_library_path: String, binary_path: PathBuf, } @@ -128,7 +128,10 @@ where impl RustfmtRunner { fn get_binary_version(&self) -> Result { let Ok(command) = Command::new(&self.binary_path) - .env("LD_LIBRARY_PATH", &self.ld_library_path) + .env( + dynamic_library_path_env_var_name(), + &self.dynamic_library_path, + ) .args(["--version"]) .output() else { @@ -139,6 +142,18 @@ impl RustfmtRunner { let binary_version = std::str::from_utf8(&command.stdout)?.trim(); return Ok(binary_version.to_string()); + +/// Returns the name of the environment variable used to search for dynamic libraries. +/// This is the same logic that cargo uses when setting these environment variables +fn dynamic_library_path_env_var_name() -> &'static str { + if cfg!(windows) { + "PATH" + } else if cfg!(target_os = "macos") { + "DYLD_FALLBACK_LIBRARY_PATH" + } else if cfg!(target_os = "aix") { + "LIBPATH" + } else { + "LD_LIBRARY_PATH" } } @@ -156,7 +171,10 @@ impl CodeFormatter for RustfmtRunner { ) -> Result { let config = create_config_arg(config); let mut command = Command::new(&self.binary_path) - .env("LD_LIBRARY_PATH", &self.ld_library_path) + .env( + dynamic_library_path_env_var_name(), + &self.dynamic_library_path, + ) .args([ "--unstable-features", "--skip-children", @@ -292,7 +310,7 @@ pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { return Ok(()); } -pub fn get_ld_library_path(dir: &Path) -> Result { +pub fn get_dynamic_library_path(dir: &Path) -> Result { let Ok(command) = Command::new("rustc") .current_dir(dir) .args(["--print", "sysroot"]) @@ -301,8 +319,7 @@ pub fn get_ld_library_path(dir: &Path) -> Result { return Err(CheckDiffError::FailedCommand("Error getting sysroot")); }; let sysroot = std::str::from_utf8(&command.stdout)?.trim_end(); - let ld_lib_path = format!("{}/lib", sysroot); - return Ok(ld_lib_path); + Ok(format!("{}/lib", sysroot)) } pub fn get_cargo_version() -> Result { @@ -322,11 +339,9 @@ pub fn build_rustfmt_from_src( binary_path: PathBuf, dir: &Path, ) -> Result { - //Because we're building standalone binaries we need to set `LD_LIBRARY_PATH` so each - // binary can find it's runtime dependencies. - // See https://github.com/rust-lang/rustfmt/issues/5675 - // This will prepend the `LD_LIBRARY_PATH` for the main rustfmt binary - let ld_lib_path = get_ld_library_path(&dir)?; + // Because we're building standalone binaries we need to set the dynamic library path + // so each rustfmt binary can find it's runtime dependencies. + let dynamic_library_path = get_dynamic_library_path(dir)?; info!("Building rustfmt from source"); let Ok(_) = Command::new("cargo") @@ -341,10 +356,10 @@ pub fn build_rustfmt_from_src( std::fs::copy(dir.join("target/release/rustfmt"), &binary_path)?; - return Ok(RustfmtRunner { - ld_library_path: ld_lib_path, + Ok(RustfmtRunner { + dynamic_library_path, binary_path, - }); + }) } // Compiles and produces two rustfmt binaries. @@ -369,20 +384,21 @@ pub fn compile_rustfmt( let src_runner = build_rustfmt_from_src(dest.join("src_rustfmt"), dest)?; let should_detach = commit_hash.is_some(); git_switch( - commit_hash.unwrap_or(feature_branch).as_str(), + commit_hash.as_ref().unwrap_or(&feature_branch), should_detach, )?; let feature_runner = build_rustfmt_from_src(dest.join("feature_rustfmt"), dest)?; info!("RUSFMT_BIN {}", src_runner.get_binary_version()?); + let dynamic_library_path_env_var = dynamic_library_path_env_var_name(); info!( - "Runtime dependencies for (src) rustfmt -- LD_LIBRARY_PATH: {}", - src_runner.ld_library_path + "Runtime dependencies for (main) rustfmt -- {}: {}", + dynamic_library_path_env_var, src_runner.dynamic_library_path ); info!("FEATURE_BIN {}", feature_runner.get_binary_version()?); info!( - "Runtime dependencies for (feature) rustfmt -- LD_LIBRARY_PATH: {}", - feature_runner.ld_library_path + "Runtime dependencies for ({}) rustfmt -- {}: {}", + feature_branch, dynamic_library_path_env_var, feature_runner.dynamic_library_path ); return Ok(CheckDiffRunners { From 3f400097c88c5282d571b243a0dbe1461f27491f Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Wed, 7 Jan 2026 21:35:21 -0500 Subject: [PATCH 02/16] Make `format_code` and associated functions more generic I think this makes the overall code better. Also removes an unecessary lifetime annotation. --- check_diff/src/lib.rs | 30 ++++++++++++++++-------------- check_diff/tests/check_diff.rs | 16 ++++++++-------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 69830c64f3e..3e5921e3989 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -83,10 +83,10 @@ pub struct CheckDiffRunners { } pub trait CodeFormatter { - fn format_code<'a>( + fn format_code>( &self, - code: &'a str, - config: &Option>, + code: &str, + config: Option<&[T]>, ) -> Result; } @@ -110,10 +110,10 @@ where S: CodeFormatter, { /// Creates a diff generated by running the source and feature binaries on the same file path - pub fn create_diff( + pub fn create_diff>( &self, path: &Path, - additional_configs: &Option>, + additional_configs: Option<&[T]>, ) -> Result { let code = std::fs::read_to_string(path)?; let src_format = self.src_runner.format_code(&code, additional_configs)?; @@ -142,6 +142,8 @@ impl RustfmtRunner { let binary_version = std::str::from_utf8(&command.stdout)?.trim(); return Ok(binary_version.to_string()); + } +} /// Returns the name of the environment variable used to search for dynamic libraries. /// This is the same logic that cargo uses when setting these environment variables @@ -164,10 +166,10 @@ impl CodeFormatter for RustfmtRunner { // code: Code to run the binary on // config: Any additional configuration options to pass to rustfmt // - fn format_code<'a>( + fn format_code>( &self, - code: &'a str, - config: &Option>, + code: &str, + config: Option<&[T]>, ) -> Result { let config = create_config_arg(config); let mut command = Command::new(&self.binary_path) @@ -194,13 +196,13 @@ impl CodeFormatter for RustfmtRunner { /// Creates a configuration in the following form: /// =, =, ... -fn create_config_arg(config: &Option>) -> String { +fn create_config_arg>(config: Option<&[T]>) -> String { let config_arg: String = match config { Some(configs) => { let mut result = String::new(); for arg in configs.iter() { result.push(','); - result.push_str(arg.as_str()); + result.push_str(arg.as_ref()); } result } @@ -423,15 +425,15 @@ pub fn search_for_rs_files(repo: &Path) -> impl Iterator { /// Calculates the number of errors when running the compiled binary and the feature binary on the /// repo specified with the specific configs. -pub fn check_diff( - config: Option>, - runners: CheckDiffRunners, +pub fn check_diff>( + config: Option<&[T]>, + runners: &CheckDiffRunners, repo: &Path, ) -> i32 { let mut errors = 0; let iter = search_for_rs_files(repo); for file in iter { - match runners.create_diff(file.as_path(), &config) { + match runners.create_diff(file.as_path(), config) { Ok(diff) => { if !diff.is_empty() { eprint!("{diff}"); diff --git a/check_diff/tests/check_diff.rs b/check_diff/tests/check_diff.rs index 17297c13043..57dadaaa6a6 100644 --- a/check_diff/tests/check_diff.rs +++ b/check_diff/tests/check_diff.rs @@ -7,10 +7,10 @@ use tempfile::Builder; struct DoNothingFormatter; impl CodeFormatter for DoNothingFormatter { - fn format_code<'a>( + fn format_code>( &self, - _code: &'a str, - _config: &Option>, + _code: &str, + _config: Option<&[T]>, ) -> Result { Ok(String::new()) } @@ -20,10 +20,10 @@ impl CodeFormatter for DoNothingFormatter { struct AddWhiteSpaceFormatter; impl CodeFormatter for AddWhiteSpaceFormatter { - fn format_code<'a>( + fn format_code>( &self, - code: &'a str, - _config: &Option>, + code: &str, + _config: Option<&[T]>, ) -> Result { let result = code.to_string() + " "; Ok(result) @@ -78,7 +78,7 @@ fn check_diff_test_no_formatting_difference() -> Result<(), CheckDiffError> { let file_path = dir.path().join("test.rs"); let _tmp_file = File::create(file_path)?; - let errors = check_diff(None, runners, dir.path()); + let errors = check_diff(None::<&[&str]>, &runners, dir.path()); assert_eq!(errors, 0); Ok(()) } @@ -90,7 +90,7 @@ fn check_diff_test_formatting_difference() -> Result<(), CheckDiffError> { let file_path = dir.path().join("test.rs"); let _tmp_file = File::create(file_path)?; - let errors = check_diff(None, runners, dir.path()); + let errors = check_diff(None::<&[&str]>, &runners, dir.path()); assert_ne!(errors, 0); Ok(()) } From e9f06be608dc671d55188bffddc353d33ec12842 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Fri, 9 Jan 2026 12:20:22 -0500 Subject: [PATCH 03/16] remove utf-8 error variant in most cases I'd expect to get valid UTF-8, and in the rare event that we don't get valid UTF-8 including some replacement characters should be fine. --- check_diff/src/lib.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 3e5921e3989..49d141b95c8 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -4,7 +4,6 @@ use std::fmt::{Debug, Display}; use std::io::{self, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; -use std::str::Utf8Error; use tracing::info; use walkdir::WalkDir; @@ -14,8 +13,6 @@ pub enum CheckDiffError { FailedGit(GitError), /// Error for generic commands FailedCommand(&'static str), - /// UTF8 related errors - FailedUtf8(Utf8Error), /// Error for building rustfmt from source FailedSourceBuild(&'static str), /// Error when obtaining binary version @@ -37,12 +34,6 @@ impl From for CheckDiffError { } } -impl From for CheckDiffError { - fn from(error: Utf8Error) -> Self { - CheckDiffError::FailedUtf8(error) - } -} - #[derive(Debug)] pub enum GitError { FailedClone { stdout: Vec, stderr: Vec }, @@ -140,11 +131,20 @@ impl RustfmtRunner { )); }; - let binary_version = std::str::from_utf8(&command.stdout)?.trim(); - return Ok(binary_version.to_string()); + Ok(buffer_into_utf8_lossy(command.stdout)) } } +/// Convert a buffer of u8 into a String. +fn buffer_into_utf8_lossy(buffer: Vec) -> String { + let mut s = match String::from_utf8(buffer) { + Ok(s) => s, + Err(e) => String::from_utf8_lossy(e.as_bytes()).to_string(), + }; + s.truncate(s.trim_end().len()); + s +} + /// Returns the name of the environment variable used to search for dynamic libraries. /// This is the same logic that cargo uses when setting these environment variables fn dynamic_library_path_env_var_name() -> &'static str { @@ -190,7 +190,7 @@ impl CodeFormatter for RustfmtRunner { command.stdin.as_mut().unwrap().write_all(code.as_bytes())?; let output = command.wait_with_output()?; - Ok(std::str::from_utf8(&output.stdout)?.to_string()) + Ok(buffer_into_utf8_lossy(output.stdout)) } } @@ -320,8 +320,9 @@ pub fn get_dynamic_library_path(dir: &Path) -> Result { else { return Err(CheckDiffError::FailedCommand("Error getting sysroot")); }; - let sysroot = std::str::from_utf8(&command.stdout)?.trim_end(); - Ok(format!("{}/lib", sysroot)) + let mut sysroot = buffer_into_utf8_lossy(command.stdout); + sysroot.push_str("/lib"); + Ok(sysroot) } pub fn get_cargo_version() -> Result { @@ -331,8 +332,7 @@ pub fn get_cargo_version() -> Result { )); }; - let cargo_version = std::str::from_utf8(&command.stdout)?.trim_end(); - return Ok(cargo_version.to_string()); + Ok(buffer_into_utf8_lossy(command.stdout)) } /// Obtains the ld_lib path and then builds rustfmt from source From abac13b324445b43e4aff632fd74f127863d2aaf Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Fri, 9 Jan 2026 18:06:27 -0500 Subject: [PATCH 04/16] return `Err(FormatCodeError)` from `CodeFormatter::format_code` The `CheckDiffError` that was returned in the error case was too broad and I felt that the code could be simplified if we returned a type that specifically captured what went wrong. --- check_diff/src/lib.rs | 42 +++++++++++++++++++++++++++++++--- check_diff/tests/check_diff.rs | 7 +++--- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 49d141b95c8..5832250f8fc 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -7,6 +7,31 @@ use std::process::{Command, Stdio}; use tracing::info; use walkdir::WalkDir; +pub enum FormatCodeError { + // IO Error when running code formatter + Io(std::io::Error), + /// An error occured that prevents code formatting. For example, a parse error. + CodeNotFormatted(Vec), +} + +impl From for FormatCodeError { + fn from(error: std::io::Error) -> Self { + Self::Io(error) + } +} + +impl std::fmt::Debug for FormatCodeError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Io(e) => std::fmt::Debug::fmt(e, f), + Self::CodeNotFormatted(e) => { + let data = String::from_utf8_lossy(e); + f.write_str(&data) + } + } + } +} + #[derive(Debug)] pub enum CheckDiffError { /// Git related errors @@ -78,7 +103,7 @@ pub trait CodeFormatter { &self, code: &str, config: Option<&[T]>, - ) -> Result; + ) -> Result; } pub struct RustfmtRunner { @@ -170,7 +195,7 @@ impl CodeFormatter for RustfmtRunner { &self, code: &str, config: Option<&[T]>, - ) -> Result { + ) -> Result { let config = create_config_arg(config); let mut command = Command::new(&self.binary_path) .env( @@ -190,7 +215,18 @@ impl CodeFormatter for RustfmtRunner { command.stdin.as_mut().unwrap().write_all(code.as_bytes())?; let output = command.wait_with_output()?; - Ok(buffer_into_utf8_lossy(output.stdout)) + let formatted_code = buffer_into_utf8_lossy(output.stdout); + + match output.status.code() { + Some(0) => Ok(formatted_code), + Some(_) | None => { + if !formatted_code.is_empty() { + Ok(formatted_code) + } else { + Err(FormatCodeError::CodeNotFormatted(output.stderr)) + } + } + } } } diff --git a/check_diff/tests/check_diff.rs b/check_diff/tests/check_diff.rs index 57dadaaa6a6..5926fc30366 100644 --- a/check_diff/tests/check_diff.rs +++ b/check_diff/tests/check_diff.rs @@ -1,5 +1,6 @@ use check_diff::{ - CheckDiffError, CheckDiffRunners, CodeFormatter, check_diff, search_for_rs_files, + CheckDiffError, CheckDiffRunners, CodeFormatter, FormatCodeError, check_diff, + search_for_rs_files, }; use std::fs::File; use tempfile::Builder; @@ -11,7 +12,7 @@ impl CodeFormatter for DoNothingFormatter { &self, _code: &str, _config: Option<&[T]>, - ) -> Result { + ) -> Result { Ok(String::new()) } } @@ -24,7 +25,7 @@ impl CodeFormatter for AddWhiteSpaceFormatter { &self, code: &str, _config: Option<&[T]>, - ) -> Result { + ) -> Result { let result = code.to_string() + " "; Ok(result) } From bbd754220e3ff156334d913811d5c57a39b9d72d Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Fri, 9 Jan 2026 19:15:31 -0500 Subject: [PATCH 05/16] return `Err(CreateDiffError)` from `CheckDiffRunners::create_diff` The `CheckDiffError` that was previously returned in the error case was too broad and I felt that the code could be simplified if we returned a type that specifically captured what went wrong when trying to create a diff between two formatter. --- check_diff/src/lib.rs | 63 ++++++++++++++++++++++++++++++++----------- 1 file changed, 48 insertions(+), 15 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 5832250f8fc..b24db65db4b 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -32,6 +32,20 @@ impl std::fmt::Debug for FormatCodeError { } } +pub enum CreateDiffError { + /// Couldn't create a diff because the rustfmt binary compiled from the `main` branch + /// failed to format the input. + MainRustfmtFailed(FormatCodeError), + /// Couldn't create a diff because the rustfmt binary compiled from the `feature` branch + /// failed to format the input. + FeatureRustfmtFailed(FormatCodeError), + /// Couldn't create a diff because both rustfmt binaries failed to format the input + BothRustfmtFailed { + src: FormatCodeError, + feature: FormatCodeError, + }, +} + #[derive(Debug)] pub enum CheckDiffError { /// Git related errors @@ -130,14 +144,32 @@ where &self, path: &Path, additional_configs: Option<&[T]>, - ) -> Result { - let code = std::fs::read_to_string(path)?; - let src_format = self.src_runner.format_code(&code, additional_configs)?; - let feature_format = self.feature_runner.format_code(&code, additional_configs)?; - Ok(Diff { - src_format, - feature_format, - }) + ) -> Result { + let code = std::fs::read_to_string(path).expect("we can read the file"); + let src_format = self.src_runner.format_code(&code, additional_configs); + let feature_format = self.feature_runner.format_code(&code, additional_configs); + + match (src_format, feature_format) { + (Ok(s), Ok(f)) => Ok(Diff { + src_format: s, + feature_format: f, + }), + (Err(error), Ok(_)) => { + // main formatting failed. + Err(CreateDiffError::MainRustfmtFailed(error)) + } + (Ok(_), Err(error)) => { + // feature formatting failed + Err(CreateDiffError::FeatureRustfmtFailed(error)) + } + (Err(src_error), Err(feature_error)) => { + // Both main formatting and feature formatting failed + Err(CreateDiffError::BothRustfmtFailed { + src: src_error, + feature: feature_error, + }) + } + } } } @@ -476,13 +508,14 @@ pub fn check_diff>( errors += 1; } } - Err(e) => { - eprintln!( - "Error creating diff for {:?}: {:?}", - file.as_path().display(), - e - ); - errors += 1; + Err(CreateDiffError::MainRustfmtFailed(_)) => { + continue; + } + Err(CreateDiffError::FeatureRustfmtFailed(_)) => { + continue; + } + Err(CreateDiffError::BothRustfmtFailed { .. }) => { + continue; } } } From bc67c7d2f254e4f6385bf11337a9592c7f57b11c Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Fri, 9 Jan 2026 23:08:16 -0500 Subject: [PATCH 06/16] update check_diff logging This updates some logging output to be more descriptive and also adds some `debug!` and `error!` logging. --- check_diff/src/lib.rs | 67 +++++++++++++++++++++++++++++----- check_diff/tests/check_diff.rs | 6 ++- 2 files changed, 62 insertions(+), 11 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index b24db65db4b..565eaa29b8f 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -4,7 +4,7 @@ use std::fmt::{Debug, Display}; use std::io::{self, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; -use tracing::info; +use tracing::{debug, error, info, trace}; use walkdir::WalkDir; pub enum FormatCodeError { @@ -310,8 +310,8 @@ pub fn clone_git_repo(url: &str, dest: &Path) -> Result<(), GitError> { return Err(error); } - info!("Successfully clone repository."); - return Ok(()); + info!("Successfully cloned repository {url} to {}", dest.display()); + Ok(()) } pub fn git_remote_add(url: &str) -> Result<(), GitError> { @@ -374,7 +374,7 @@ pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { let dest_path = Path::new(&dest); env::set_current_dir(&dest_path)?; info!( - "Current directory: {}", + "Setting current directory to: {}", env::current_dir().unwrap().display() ); return Ok(()); @@ -497,28 +497,77 @@ pub fn check_diff>( config: Option<&[T]>, runners: &CheckDiffRunners, repo: &Path, + repo_url: &str, ) -> i32 { let mut errors = 0; let iter = search_for_rs_files(repo); for file in iter { + let relative_path = file.strip_prefix(repo).unwrap_or(&file); + let repo_name = get_repo_name(repo_url); + + trace!( + "Formatting '{0}' file {0}/{1}", + repo_name, + relative_path.display() + ); + match runners.create_diff(file.as_path(), config) { Ok(diff) => { if !diff.is_empty() { - eprint!("{diff}"); + error!( + "Diff found in '{0}' when formatting {0}/{1}\n{2}", + repo_name, + relative_path.display(), + diff, + ); errors += 1; + } else { + trace!( + "No diff found in '{0}' when formatting {0}/{1}", + repo_name, + relative_path.display(), + ) } } - Err(CreateDiffError::MainRustfmtFailed(_)) => { + Err(CreateDiffError::MainRustfmtFailed(e)) => { + debug!( + "`main` rustfmt failed to format {}/{}\n{:?}", + repo_name, + relative_path.display(), + e, + ); continue; } - Err(CreateDiffError::FeatureRustfmtFailed(_)) => { + Err(CreateDiffError::FeatureRustfmtFailed(e)) => { + debug!( + "`feature` rustfmt failed to format {}/{}\n{:?}", + repo_name, + relative_path.display(), + e, + ); continue; } - Err(CreateDiffError::BothRustfmtFailed { .. }) => { + Err(CreateDiffError::BothRustfmtFailed { src, feature }) => { + debug!( + "Both rustfmt binaries failed to format {}/{}\n{:?}\n{:?}", + repo_name, + relative_path.display(), + src, + feature, + ); continue; } } } - return errors; + errors +} + +/// parse out the repository name from a GitHub Repository name. +pub fn get_repo_name(git_url: &str) -> &str { + let strip_git_prefix = git_url.strip_suffix(".git").unwrap_or(git_url); + let (_, repo_name) = strip_git_prefix + .rsplit_once('/') + .unwrap_or(("", strip_git_prefix)); + repo_name } diff --git a/check_diff/tests/check_diff.rs b/check_diff/tests/check_diff.rs index 5926fc30366..ddd39c0dfc8 100644 --- a/check_diff/tests/check_diff.rs +++ b/check_diff/tests/check_diff.rs @@ -78,8 +78,9 @@ fn check_diff_test_no_formatting_difference() -> Result<(), CheckDiffError> { let dir = Builder::new().tempdir_in("").unwrap(); let file_path = dir.path().join("test.rs"); let _tmp_file = File::create(file_path)?; + let repo_url = "https://github.com/rust-lang/rustfmt.git"; - let errors = check_diff(None::<&[&str]>, &runners, dir.path()); + let errors = check_diff(None::<&[&str]>, &runners, dir.path(), repo_url); assert_eq!(errors, 0); Ok(()) } @@ -90,8 +91,9 @@ fn check_diff_test_formatting_difference() -> Result<(), CheckDiffError> { let dir = Builder::new().tempdir_in("").unwrap(); let file_path = dir.path().join("test.rs"); let _tmp_file = File::create(file_path)?; + let repo_url = "https://github.com/rust-lang/rustfmt.git"; - let errors = check_diff(None::<&[&str]>, &runners, dir.path()); + let errors = check_diff(None::<&[&str]>, &runners, dir.path(), repo_url); assert_ne!(errors, 0); Ok(()) } From 76487205154bb7df0d71be1a39b4b0c37d53013a Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sat, 10 Jan 2026 10:59:54 -0500 Subject: [PATCH 07/16] copy over the constant list of repositories from `ci/check_diff.sh` --- check_diff/src/main.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/check_diff/src/main.rs b/check_diff/src/main.rs index 9c14c5f08cd..3469b7ea216 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -3,6 +3,32 @@ use clap::Parser; use tempfile::Builder; use tracing::info; +const REPOS: &[&str] = &[ + "https://github.com/rust-lang/rust.git", + "https://github.com/rust-lang/cargo.git", + "https://github.com/rust-lang/miri.git", + "https://github.com/rust-lang/rust-analyzer.git", + "https://github.com/bitflags/bitflags.git", + "https://github.com/rust-lang/log.git", + "https://github.com/rust-lang/mdBook.git", + "https://github.com/rust-lang/packed_simd.git", + "https://github.com/rust-lang/rust-semverver.git", + "https://github.com/Stebalien/tempfile.git", + "https://github.com/rust-lang/futures-rs.git", + "https://github.com/dtolnay/anyhow.git", + "https://github.com/dtolnay/thiserror.git", + "https://github.com/dtolnay/syn.git", + "https://github.com/serde-rs/serde.git", + "https://github.com/rust-lang/rustlings.git", + "https://github.com/rust-lang/rustup.git", + "https://github.com/SergioBenitez/Rocket.git", + "https://github.com/rustls/rustls.git", + "https://github.com/rust-lang/rust-bindgen.git", + "https://github.com/hyperium/hyper.git", + "https://github.com/actix/actix.git", + "https://github.com/denoland/deno.git", +]; + /// Inputs for the check_diff script #[derive(Parser)] struct CliInputs { From dcf369e20782a3486fddfb64f2359dffcb753010 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sat, 10 Jan 2026 11:38:48 -0500 Subject: [PATCH 08/16] rewrite the `main` function to actually perform the diff check Now after we compile both rustfmt binaries we iterate through each repository and process each check in its own thread. This implementation more or less follows the behavior from `ci/check_diff.sh`, but it's faster since we're able to process each repository in parallel. --- check_diff/src/main.rs | 77 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 67 insertions(+), 10 deletions(-) diff --git a/check_diff/src/main.rs b/check_diff/src/main.rs index 3469b7ea216..a6bb50549a4 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -1,7 +1,15 @@ -use check_diff::{CheckDiffError, check_diff, compile_rustfmt}; +use std::io::Error; +use std::process::ExitCode; +use std::sync::Arc; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::thread; + +use check_diff::{ + check_diff, clone_git_repo, compile_rustfmt, get_repo_name, +}; use clap::Parser; -use tempfile::Builder; -use tracing::info; +use tempfile::tempdir; +use tracing::{error, info, warn}; const REPOS: &[&str] = &[ "https://github.com/rust-lang/rust.git", @@ -45,22 +53,71 @@ struct CliInputs { rustfmt_config: Option>, } -fn main() -> Result<(), CheckDiffError> { +fn main() -> Result { tracing_subscriber::fmt() .with_env_filter(tracing_subscriber::EnvFilter::from_env("CHECK_DIFF_LOG")) .init(); let args = CliInputs::parse(); - let tmp_dir = Builder::new().tempdir_in("").unwrap(); + let tmp_dir = tempdir()?; info!("Created tmp_dir {:?}", tmp_dir); - let check_diff_runners = compile_rustfmt( + let Ok(check_diff_runners) = compile_rustfmt( tmp_dir.path(), args.remote_repo_url, args.feature_branch, args.commit_hash, - )?; + ) else { + error!("Failed to compile rustfmt"); + return Ok(ExitCode::FAILURE); + }; + + let errors = Arc::new(AtomicUsize::new(0)); + let rustfmt_config = Arc::new(args.rustfmt_config); + let check_diff_runners = Arc::new(check_diff_runners); + + thread::scope(|s| { + for url in REPOS { + let errors = Arc::clone(&errors); + let rustfmt_config = Arc::clone(&rustfmt_config); + let check_diff_runners = Arc::clone(&check_diff_runners); + s.spawn(move || { + let repo_name = get_repo_name(url); + info!("Processing repo: {repo_name}"); + let Ok(tmp_dir) = tempdir() else { + warn!( + "Failed to create a tempdir for {}. Can't check formatting diff for {}", + &url, repo_name + ); + return; + }; + + let Ok(_) = clone_git_repo(url, tmp_dir.path()) else { + warn!( + "Failed to clone repo {}. Can't check formatting diff for {}", + &url, repo_name + ); + return; + }; + + let error_count = check_diff( + rustfmt_config.as_deref(), + &check_diff_runners, + tmp_dir.path(), + url, + ); - // TODO: currently using same tmp dir path for sake of compilation - let _ = check_diff(args.rustfmt_config, check_diff_runners, tmp_dir.path()); + errors.fetch_add(error_count as usize, Ordering::Relaxed); + }); + } + }); - Ok(()) + let error_count = Arc::into_inner(errors) + .expect("All other threads are done") + .load(Ordering::Relaxed); + if error_count > 0 { + error!("Formatting diff found 💔"); + Ok(ExitCode::from(u8::try_from(error_count).unwrap_or(u8::MAX))) + } else { + info!("No diff found 😊"); + Ok(ExitCode::SUCCESS) + } } From f1a0dcdd11700ba9abdac4c47d879ae38460b434 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sat, 10 Jan 2026 12:22:25 -0500 Subject: [PATCH 09/16] Use `format_code_from_path` instead of `format_code` When rustfmt knows the file path it's able to skip formatting for files listed in the repo's rustfmt.toml `ignore` list. This helps when us process files in r-l/rust that have been explicitly skipped. Otherwise we'll try to format files that cause rustfmt to hang. --- check_diff/src/lib.rs | 69 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 62 insertions(+), 7 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 565eaa29b8f..b941faa60de 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -118,6 +118,15 @@ pub trait CodeFormatter { code: &str, config: Option<&[T]>, ) -> Result; + + fn format_code_from_path, P: AsRef>( + &self, + path: P, + config: Option<&[T]>, + ) -> Result { + let code = std::fs::read_to_string(path)?; + self.format_code(&code, config) + } } pub struct RustfmtRunner { @@ -140,14 +149,17 @@ where S: CodeFormatter, { /// Creates a diff generated by running the source and feature binaries on the same file path - pub fn create_diff>( + pub fn create_diff, P: AsRef>( &self, - path: &Path, + path: P, additional_configs: Option<&[T]>, ) -> Result { - let code = std::fs::read_to_string(path).expect("we can read the file"); - let src_format = self.src_runner.format_code(&code, additional_configs); - let feature_format = self.feature_runner.format_code(&code, additional_configs); + let src_format = self + .src_runner + .format_code_from_path(&path, additional_configs); + let feature_format = self + .feature_runner + .format_code_from_path(&path, additional_configs); match (src_format, feature_format) { (Ok(s), Ok(f)) => Ok(Diff { @@ -217,6 +229,48 @@ fn dynamic_library_path_env_var_name() -> &'static str { } impl CodeFormatter for RustfmtRunner { + fn format_code_from_path, P: AsRef>( + &self, + path: P, + config: Option<&[T]>, + ) -> Result { + let config = create_config_arg(config); + let command = Command::new(&self.binary_path) + .env( + dynamic_library_path_env_var_name(), + &self.dynamic_library_path, + ) + .args([ + "--edition", + self.edition.as_str(), + "--style-edition", + self.style_edition.as_str(), + "--unstable-features", + "--skip-children", + "--emit=stdout", + config.as_str(), + ]) + .arg(path.as_ref()) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + + let output = command.wait_with_output()?; + let formatted_code = buffer_into_utf8_lossy(output.stdout); + + match output.status.code() { + Some(0) => Ok(formatted_code), + Some(_) | None => { + if !formatted_code.is_empty() { + Ok(formatted_code) + } else { + Err(FormatCodeError::CodeNotFormatted(output.stderr)) + } + } + } + } + // Run rusfmt to see if a diff is produced. Runs on the code specified // // Parameters: @@ -493,13 +547,14 @@ pub fn search_for_rs_files(repo: &Path) -> impl Iterator { /// Calculates the number of errors when running the compiled binary and the feature binary on the /// repo specified with the specific configs. -pub fn check_diff>( +pub fn check_diff, P: AsRef>( config: Option<&[T]>, runners: &CheckDiffRunners, - repo: &Path, + repo: P, repo_url: &str, ) -> i32 { let mut errors = 0; + let repo = repo.as_ref(); let iter = search_for_rs_files(repo); for file in iter { let relative_path = file.strip_prefix(repo).unwrap_or(&file); From 837c4ee1da435c973869e29364a924cc30cd93ea Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sat, 10 Jan 2026 12:37:48 -0500 Subject: [PATCH 10/16] Apply some clippy suggestions --- check_diff/src/lib.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index b941faa60de..c431906fcc0 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -1,4 +1,3 @@ -use diffy; use std::env; use std::fmt::{Debug, Display}; use std::io::{self, Write}; @@ -384,7 +383,7 @@ pub fn git_remote_add(url: &str) -> Result<(), GitError> { } info!("Successfully added remote: {url}"); - return Ok(()); + Ok(()) } pub fn git_fetch(branch_name: &str) -> Result<(), GitError> { @@ -403,7 +402,7 @@ pub fn git_fetch(branch_name: &str) -> Result<(), GitError> { } info!("Successfully fetched: {branch_name}"); - return Ok(()); + Ok(()) } pub fn git_switch(git_ref: &str, should_detach: bool) -> Result<(), GitError> { @@ -421,17 +420,17 @@ pub fn git_switch(git_ref: &str, should_detach: bool) -> Result<(), GitError> { return Err(error); } info!("Successfully switched to {git_ref}"); - return Ok(()); + Ok(()) } pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { let dest_path = Path::new(&dest); - env::set_current_dir(&dest_path)?; + env::set_current_dir(dest_path)?; info!( "Setting current directory to: {}", env::current_dir().unwrap().display() ); - return Ok(()); + Ok(()) } pub fn get_dynamic_library_path(dir: &Path) -> Result { @@ -525,24 +524,24 @@ pub fn compile_rustfmt( feature_branch, dynamic_library_path_env_var, feature_runner.dynamic_library_path ); - return Ok(CheckDiffRunners { + Ok(CheckDiffRunners { src_runner, feature_runner, - }); + }) } /// Searches for rust files in the particular path and returns an iterator to them. pub fn search_for_rs_files(repo: &Path) -> impl Iterator { - return WalkDir::new(repo).into_iter().filter_map(|e| match e.ok() { + WalkDir::new(repo).into_iter().filter_map(|e| match e.ok() { Some(entry) => { let path = entry.path(); - if path.is_file() && path.extension().map_or(false, |ext| ext == "rs") { + if path.is_file() && path.extension().is_some_and(|ext| ext == "rs") { return Some(entry.into_path()); } - return None; + None } None => None, - }); + }) } /// Calculates the number of errors when running the compiled binary and the feature binary on the From 11255762aaed191f8bdb4df4568c16d18b3c8e8d Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sat, 10 Jan 2026 12:48:07 -0500 Subject: [PATCH 11/16] Change the return type of `check_diff` to `u8` `i32` didn't seem like the right return type to me. Since we're just recording that any error happened `u8` seemed good enough for that. --- check_diff/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index c431906fcc0..b78f4a35256 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -551,8 +551,8 @@ pub fn check_diff, P: AsRef>( runners: &CheckDiffRunners, repo: P, repo_url: &str, -) -> i32 { - let mut errors = 0; +) -> u8 { + let mut errors: u8 = 0; let repo = repo.as_ref(); let iter = search_for_rs_files(repo); for file in iter { @@ -574,7 +574,7 @@ pub fn check_diff, P: AsRef>( relative_path.display(), diff, ); - errors += 1; + errors = errors.saturating_add(1); } else { trace!( "No diff found in '{0}' when formatting {0}/{1}", From 45967f53d392d18ac9e45676d3e4423d64176a80 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sat, 10 Jan 2026 13:07:47 -0500 Subject: [PATCH 12/16] Add `--edition` and `--style-edition` options to the check_diff CLI This let's us controll the rust language edition used to parse source code and the rustfmt style edition used for code formatting. --- check_diff/src/lib.rs | 87 +++++++++++++++++++++++++++++++++++++++++- check_diff/src/main.rs | 10 ++++- 2 files changed, 94 insertions(+), 3 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index b78f4a35256..be5a2e09b15 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -3,9 +3,78 @@ use std::fmt::{Debug, Display}; use std::io::{self, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; +use std::str::FromStr; use tracing::{debug, error, info, trace}; use walkdir::WalkDir; +#[derive(Debug, Clone, Copy)] +pub enum Edition { + /// rust edition 2015 + Edition2015, + /// rust edition 2018 + Edition2018, + /// rust edition 2021 + Edition2021, + /// rust edition 2024 + Edition2024, +} + +impl Edition { + fn as_str(&self) -> &str { + match self { + Edition::Edition2015 => "2015", + Edition::Edition2018 => "2018", + Edition::Edition2021 => "2021", + Edition::Edition2024 => "2024", + } + } +} + +impl FromStr for Edition { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "2015" => Ok(Edition::Edition2015), + "2018" => Ok(Edition::Edition2018), + "2021" => Ok(Edition::Edition2021), + "2024" => Ok(Edition::Edition2024), + _ => Err(format!("Invalid rust language edition {s}")), + } + } +} + +#[derive(Debug, Clone, Copy)] +pub enum StyleEdition { + // rustfmt style_edition 2021. Also equivaluent to 2015 and 2018. + Edition2021, + // rustfmt style_edition 2024 + Edition2024, +} + +impl StyleEdition { + fn as_str(&self) -> &str { + match self { + StyleEdition::Edition2021 => "2021", + StyleEdition::Edition2024 => "2024", + } + } +} + +impl FromStr for StyleEdition { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "2015" => Ok(StyleEdition::Edition2021), + "2018" => Ok(StyleEdition::Edition2021), + "2021" => Ok(StyleEdition::Edition2021), + "2024" => Ok(StyleEdition::Edition2024), + _ => Err(format!("Invalid rustfmt style edition {s}")), + } + } +} + pub enum FormatCodeError { // IO Error when running code formatter Io(std::io::Error), @@ -131,6 +200,8 @@ pub trait CodeFormatter { pub struct RustfmtRunner { dynamic_library_path: String, binary_path: PathBuf, + edition: Edition, + style_edition: StyleEdition, } impl CheckDiffRunners { @@ -288,6 +359,10 @@ impl CodeFormatter for RustfmtRunner { &self.dynamic_library_path, ) .args([ + "--edition", + self.edition.as_str(), + "--style-edition", + self.style_edition.as_str(), "--unstable-features", "--skip-children", "--emit=stdout", @@ -461,6 +536,8 @@ pub fn get_cargo_version() -> Result { pub fn build_rustfmt_from_src( binary_path: PathBuf, dir: &Path, + edition: Edition, + style_edition: StyleEdition, ) -> Result { // Because we're building standalone binaries we need to set the dynamic library path // so each rustfmt binary can find it's runtime dependencies. @@ -482,6 +559,8 @@ pub fn build_rustfmt_from_src( Ok(RustfmtRunner { dynamic_library_path, binary_path, + edition, + style_edition, }) } @@ -493,6 +572,8 @@ pub fn compile_rustfmt( dest: &Path, remote_repo_url: String, feature_branch: String, + edition: Edition, + style_edition: StyleEdition, commit_hash: Option, ) -> Result, CheckDiffError> { const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; @@ -504,14 +585,16 @@ pub fn compile_rustfmt( let cargo_version = get_cargo_version()?; info!("Compiling with {}", cargo_version); - let src_runner = build_rustfmt_from_src(dest.join("src_rustfmt"), dest)?; + let src_runner = + build_rustfmt_from_src(dest.join("src_rustfmt"), dest, edition, style_edition)?; let should_detach = commit_hash.is_some(); git_switch( commit_hash.as_ref().unwrap_or(&feature_branch), should_detach, )?; - let feature_runner = build_rustfmt_from_src(dest.join("feature_rustfmt"), dest)?; + let feature_runner = + build_rustfmt_from_src(dest.join("feature_rustfmt"), dest, edition, style_edition)?; info!("RUSFMT_BIN {}", src_runner.get_binary_version()?); let dynamic_library_path_env_var = dynamic_library_path_env_var_name(); info!( diff --git a/check_diff/src/main.rs b/check_diff/src/main.rs index a6bb50549a4..b3cc3c883ce 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -5,7 +5,7 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use std::thread; use check_diff::{ - check_diff, clone_git_repo, compile_rustfmt, get_repo_name, + Edition, StyleEdition, check_diff, clone_git_repo, compile_rustfmt, get_repo_name, }; use clap::Parser; use tempfile::tempdir; @@ -44,6 +44,12 @@ struct CliInputs { remote_repo_url: String, /// Name of the feature branch on the forked repo feature_branch: String, + /// Rust language `edition` used to parse code. Possible values {2015, 2018, 2021, 2024} + #[arg(short, long, default_value = "2015")] + edition: Edition, + /// rustfmt `style_edition` used when formatting code. Possible vales {2015, 2018, 2021, 2024}. + #[arg(short, long, default_value = "2021")] + style_edition: StyleEdition, /// Optional commit hash from the feature branch #[arg(short, long)] commit_hash: Option, @@ -64,6 +70,8 @@ fn main() -> Result { tmp_dir.path(), args.remote_repo_url, args.feature_branch, + args.edition, + args.style_edition, args.commit_hash, ) else { error!("Failed to compile rustfmt"); From 2fc3ff8b623c5f4ebdede3cad6d1186a3ff0a27e Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sun, 18 Jan 2026 12:11:03 -0500 Subject: [PATCH 13/16] Apply PR suggestion for grouping repositories --- check_diff/src/main.rs | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/check_diff/src/main.rs b/check_diff/src/main.rs index b3cc3c883ce..f6e218c294f 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -11,30 +11,33 @@ use clap::Parser; use tempfile::tempdir; use tracing::{error, info, warn}; +/// A curated set of `rust-lang/*` and popular ecosystem repositories to compare `rustfmt`s against. const REPOS: &[&str] = &[ - "https://github.com/rust-lang/rust.git", + // `rust-lang/*` repositories. "https://github.com/rust-lang/cargo.git", - "https://github.com/rust-lang/miri.git", - "https://github.com/rust-lang/rust-analyzer.git", - "https://github.com/bitflags/bitflags.git", + "https://github.com/rust-lang/futures-rs.git", "https://github.com/rust-lang/log.git", "https://github.com/rust-lang/mdBook.git", + "https://github.com/rust-lang/miri.git", "https://github.com/rust-lang/packed_simd.git", + "https://github.com/rust-lang/rust-analyzer.git", + "https://github.com/rust-lang/rust-bindgen.git", "https://github.com/rust-lang/rust-semverver.git", - "https://github.com/Stebalien/tempfile.git", - "https://github.com/rust-lang/futures-rs.git", - "https://github.com/dtolnay/anyhow.git", - "https://github.com/dtolnay/thiserror.git", - "https://github.com/dtolnay/syn.git", - "https://github.com/serde-rs/serde.git", + "https://github.com/rust-lang/rust.git", "https://github.com/rust-lang/rustlings.git", "https://github.com/rust-lang/rustup.git", - "https://github.com/SergioBenitez/Rocket.git", - "https://github.com/rustls/rustls.git", - "https://github.com/rust-lang/rust-bindgen.git", - "https://github.com/hyperium/hyper.git", + // Ecosystem repositories "https://github.com/actix/actix.git", + "https://github.com/bitflags/bitflags.git", "https://github.com/denoland/deno.git", + "https://github.com/dtolnay/anyhow.git", + "https://github.com/dtolnay/syn.git", + "https://github.com/dtolnay/thiserror.git", + "https://github.com/hyperium/hyper.git", + "https://github.com/rustls/rustls.git", + "https://github.com/serde-rs/serde.git", + "https://github.com/SergioBenitez/Rocket.git", + "https://github.com/Stebalien/tempfile.git", ]; /// Inputs for the check_diff script From ab3f2aeb6e3da5281f6622ec8bb4ca136d23fe71 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sun, 18 Jan 2026 12:16:48 -0500 Subject: [PATCH 14/16] show rustfmt compilation error Now we'll output the error that lead to us failing to compile one or both of the rustfmt binaries before exiting with a failure. --- check_diff/src/main.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/check_diff/src/main.rs b/check_diff/src/main.rs index f6e218c294f..2c5078b2469 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -69,16 +69,22 @@ fn main() -> Result { let args = CliInputs::parse(); let tmp_dir = tempdir()?; info!("Created tmp_dir {:?}", tmp_dir); - let Ok(check_diff_runners) = compile_rustfmt( + + let compilation_result = compile_rustfmt( tmp_dir.path(), args.remote_repo_url, args.feature_branch, args.edition, args.style_edition, args.commit_hash, - ) else { - error!("Failed to compile rustfmt"); - return Ok(ExitCode::FAILURE); + ); + + let check_diff_runners = match compilation_result { + Ok(runner) => runner, + Err(e) => { + error!("Failed to compile rustfmt:\n{e:?}"); + return Ok(ExitCode::FAILURE); + } }; let errors = Arc::new(AtomicUsize::new(0)); From f1c0a286d2944fcfa5fa6e52bc36579ebb1ad02b Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sun, 18 Jan 2026 12:34:12 -0500 Subject: [PATCH 15/16] just return `ExitCode::FAILURE` when we've found formatting diffs --- check_diff/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/check_diff/src/main.rs b/check_diff/src/main.rs index 2c5078b2469..50bda185398 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -131,8 +131,8 @@ fn main() -> Result { .expect("All other threads are done") .load(Ordering::Relaxed); if error_count > 0 { - error!("Formatting diff found 💔"); - Ok(ExitCode::from(u8::try_from(error_count).unwrap_or(u8::MAX))) + error!("{error_count} formatting diffs found 💔"); + Ok(ExitCode::FAILURE) } else { info!("No diff found 😊"); Ok(ExitCode::SUCCESS) From 55348f61064ff01378422e3deca9a47ed5f269e8 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Sun, 18 Jan 2026 12:42:16 -0500 Subject: [PATCH 16/16] Add note about why `format_code_from_path` is better for rustfmt --- check_diff/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index be5a2e09b15..4b975fa1d59 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -299,6 +299,10 @@ fn dynamic_library_path_env_var_name() -> &'static str { } impl CodeFormatter for RustfmtRunner { + // When rustfmt knows the file path it's able to skip formatting for files listed in the repo's + // rustfmt.toml `ignore` list. For example, this helps us skip files in r-l/rust that have + // been explicitly skipped because trying to format them causes rustfmt to hang or rustfmt. + // doesn't do a good job at formatting those files. fn format_code_from_path, P: AsRef>( &self, path: P,