Skip to content

ci: create rustfmt configs when compiling each rustfmt binary #6775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ytmimi merged 1 commit into from
Jan 20, 2026
+77 −82
Merged

ci: create rustfmt configs when compiling each rustfmt binary #6775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 71 additions & 62 deletions check_diff/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::env;
use std::fmt::{Debug, Display};
use std::io::{self, Write};
Expand Down Expand Up @@ -181,19 +182,11 @@ pub struct CheckDiffRunners<F, S> {
}

pub trait CodeFormatter {
fn format_code<T: AsRef<str>>(
&self,
code: &str,
config: Option<&[T]>,
) -> Result<String, FormatCodeError>;

fn format_code_from_path<T: AsRef<str>, P: AsRef<Path>>(
&self,
path: P,
config: Option<&[T]>,
) -> Result<String, FormatCodeError> {
fn format_code(&self, code: &str) -> Result<String, FormatCodeError>;

fn format_code_from_path<P: AsRef<Path>>(&self, path: P) -> Result<String, FormatCodeError> {
let code = std::fs::read_to_string(path)?;
self.format_code(&code, config)
self.format_code(&code)
}
}

Expand All @@ -202,6 +195,7 @@ pub struct RustfmtRunner {
binary_path: PathBuf,
edition: Edition,
style_edition: StyleEdition,
config: Cow<'static, str>,
}

impl<F, S> CheckDiffRunners<F, S> {
Expand All @@ -219,17 +213,9 @@ where
S: CodeFormatter,
{
/// Creates a diff generated by running the source and feature binaries on the same file path
pub fn create_diff<T: AsRef<str>, P: AsRef<Path>>(
&self,
path: P,
additional_configs: Option<&[T]>,
) -> Result<Diff, CreateDiffError> {
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);
pub fn create_diff<P: AsRef<Path>>(&self, path: P) -> Result<Diff, CreateDiffError> {
let src_format = self.src_runner.format_code_from_path(&path);
let feature_format = self.feature_runner.format_code_from_path(&path);

match (src_format, feature_format) {
(Ok(s), Ok(f)) => Ok(Diff {
Expand Down Expand Up @@ -272,6 +258,10 @@ impl RustfmtRunner {

Ok(buffer_into_utf8_lossy(command.stdout))
}

fn command_line_configs(&self) -> &str {
&self.config
}
}

/// Convert a buffer of u8 into a String.
Expand Down Expand Up @@ -303,12 +293,7 @@ impl CodeFormatter for RustfmtRunner {
// 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<T: AsRef<str>, P: AsRef<Path>>(
&self,
path: P,
config: Option<&[T]>,
) -> Result<String, FormatCodeError> {
let config = create_config_arg(config);
fn format_code_from_path<P: AsRef<Path>>(&self, path: P) -> Result<String, FormatCodeError> {
let command = Command::new(&self.binary_path)
.env(
dynamic_library_path_env_var_name(),
Expand All @@ -322,7 +307,7 @@ impl CodeFormatter for RustfmtRunner {
"--unstable-features",
"--skip-children",
"--emit=stdout",
config.as_str(),
self.command_line_configs(),
])
.arg(path.as_ref())
.stdin(Stdio::piped())
Expand Down Expand Up @@ -351,12 +336,7 @@ impl CodeFormatter for RustfmtRunner {
// code: Code to run the binary on
// config: Any additional configuration options to pass to rustfmt
//
fn format_code<T: AsRef<str>>(
&self,
code: &str,
config: Option<&[T]>,
) -> Result<String, FormatCodeError> {
let config = create_config_arg(config);
fn format_code(&self, code: &str) -> Result<String, FormatCodeError> {
let mut command = Command::new(&self.binary_path)
.env(
dynamic_library_path_env_var_name(),
Expand All @@ -370,7 +350,7 @@ impl CodeFormatter for RustfmtRunner {
"--unstable-features",
"--skip-children",
"--emit=stdout",
config.as_str(),
self.command_line_configs(),
])
.stdin(Stdio::piped())
.stdout(Stdio::piped())
Expand All @@ -394,25 +374,42 @@ impl CodeFormatter for RustfmtRunner {
}
}

const DEFAULT_CONFIG: &str = "--config=error_on_line_overflow=false,error_on_unformatted=false";

/// Creates a configuration in the following form:
/// <config_name>=<config_val>, <config_name>=<config_val>, ...
fn create_config_arg<T: AsRef<str>>(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_ref());
}
result
}
None => String::new(),
fn create_config_arg<T: AsRef<str>>(configs: Option<&[T]>) -> Cow<'static, str> {
let Some(configs) = configs else {
return Cow::Borrowed(DEFAULT_CONFIG);
};
let config = format!(
"--config=error_on_line_overflow=false,error_on_unformatted=false{}",
config_arg.as_str()
);
config

let mut configs_len = 0;
let mut num_configs = 0;

// Determine how many non empty configs we've got
for c in configs.iter().map(AsRef::as_ref) {
if c.is_empty() {
continue;
}

configs_len += c.len();
num_configs += 1;
}

if num_configs == 0 {
// All configs were empty so return the default.
return Cow::Borrowed(DEFAULT_CONFIG);
}

// We need capacity for the default configs len + one ',' per config + total config element len
let mut result = String::with_capacity(DEFAULT_CONFIG.len() + num_configs + configs_len);

for c in configs.iter().map(AsRef::as_ref) {
result.push(',');
result.push_str(c);
}

Cow::Owned(result)
}
/// Clone a git repository
///
Expand Down Expand Up @@ -537,11 +534,12 @@ pub fn get_cargo_version() -> Result<String, CheckDiffError> {

/// Obtains the ld_lib path and then builds rustfmt from source
/// If that operation succeeds, the source is then copied to the output path specified
pub fn build_rustfmt_from_src(
pub fn build_rustfmt_from_src<T: AsRef<str>>(
binary_path: PathBuf,
dir: &Path,
edition: Edition,
style_edition: StyleEdition,
config: Option<&[T]>,
) -> Result<RustfmtRunner, CheckDiffError> {
// Because we're building standalone binaries we need to set the dynamic library path
// so each rustfmt binary can find it's runtime dependencies.
Expand All @@ -565,20 +563,22 @@ pub fn build_rustfmt_from_src(
binary_path,
edition,
style_edition,
config: create_config_arg(config),
})
}

// Compiles and produces two rustfmt binaries.
// One for the current main branch, and another for the feature branch
// Parameters:
// dest: Directory where rustfmt will be cloned
pub fn compile_rustfmt(
pub fn compile_rustfmt<T: AsRef<str>>(
dest: &Path,
remote_repo_url: String,
feature_branch: String,
edition: Edition,
style_edition: StyleEdition,
commit_hash: Option<String>,
config: Option<&[T]>,
) -> Result<CheckDiffRunners<RustfmtRunner, RustfmtRunner>, CheckDiffError> {
const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git";

Expand All @@ -589,16 +589,26 @@ 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, edition, style_edition)?;
let src_runner = build_rustfmt_from_src(
dest.join("src_rustfmt"),
dest,
edition,
style_edition,
config,
)?;
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, edition, style_edition)?;
let feature_runner = build_rustfmt_from_src(
dest.join("feature_rustfmt"),
dest,
edition,
style_edition,
config,
)?;
info!("RUSFMT_BIN {}", src_runner.get_binary_version()?);
let dynamic_library_path_env_var = dynamic_library_path_env_var_name();
info!(
Expand Down Expand Up @@ -633,8 +643,7 @@ pub fn search_for_rs_files(repo: &Path) -> impl Iterator<Item = PathBuf> {

/// 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<T: AsRef<str>, P: AsRef<Path>>(
config: Option<&[T]>,
pub fn check_diff<P: AsRef<Path>>(
runners: &CheckDiffRunners<impl CodeFormatter, impl CodeFormatter>,
repo: P,
repo_url: &str,
Expand All @@ -652,7 +661,7 @@ pub fn check_diff<T: AsRef<str>, P: AsRef<Path>>(
relative_path.display()
);

match runners.create_diff(file.as_path(), config) {
match runners.create_diff(file.as_path()) {
Ok(diff) => {
if !diff.is_empty() {
error!(
Expand Down
10 changes: 2 additions & 8 deletions check_diff/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ fn main() -> Result<ExitCode, Error> {
args.edition,
args.style_edition,
args.commit_hash,
args.rustfmt_config.as_deref(),
);

let check_diff_runners = match compilation_result {
Expand All @@ -88,13 +89,11 @@ fn main() -> Result<ExitCode, Error> {
};

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);
Expand All @@ -115,12 +114,7 @@ fn main() -> Result<ExitCode, Error> {
return;
};

let error_count = check_diff(
rustfmt_config.as_deref(),
&check_diff_runners,
tmp_dir.path(),
url,
);
let error_count = check_diff(&check_diff_runners, tmp_dir.path(), url);

errors.fetch_add(error_count as usize, Ordering::Relaxed);
});
Expand Down
16 changes: 4 additions & 12 deletions check_diff/tests/check_diff.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,7 @@ use tempfile::Builder;
struct DoNothingFormatter;

impl CodeFormatter for DoNothingFormatter {
fn format_code<T: AsRef<str>>(
&self,
_code: &str,
_config: Option<&[T]>,
) -> Result<String, FormatCodeError> {
fn format_code(&self, _code: &str) -> Result<String, FormatCodeError> {
Ok(String::new())
}
}
Expand All @@ -21,11 +17,7 @@ impl CodeFormatter for DoNothingFormatter {
struct AddWhiteSpaceFormatter;

impl CodeFormatter for AddWhiteSpaceFormatter {
fn format_code<T: AsRef<str>>(
&self,
code: &str,
_config: Option<&[T]>,
) -> Result<String, FormatCodeError> {
fn format_code(&self, code: &str) -> Result<String, FormatCodeError> {
let result = code.to_string() + " ";
Ok(result)
}
Expand Down Expand Up @@ -80,7 +72,7 @@ fn check_diff_test_no_formatting_difference() -> Result<(), CheckDiffError> {
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(), repo_url);
let errors = check_diff(&runners, dir.path(), repo_url);
assert_eq!(errors, 0);
Ok(())
}
Expand All @@ -93,7 +85,7 @@ fn check_diff_test_formatting_difference() -> Result<(), CheckDiffError> {
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(), repo_url);
let errors = check_diff(&runners, dir.path(), repo_url);
assert_ne!(errors, 0);
Ok(())
}
Loading