Get the Check Diff rust binary working #6751

ytmimi · 2025-12-21T16:43:05Z

The Diff Check Job is integral to making sure we don't introduce breaking formatting changes. An effort was started by a contributor a while ago to rewrite the tool in rust, but that stalled out. I'd like to cut over to this rewritten rust version, which I think is an improvement over the current shell script and I think it should should help us get things unstuck and allow us to complete the next subtree sync with confidence that we're not introducing any breaking changes.

The PR is broken up into smaller commits, and I'd recommend reviewing things one commit at a time, but also happy to break this up into smaller commits if that's preferred.

Note: I plan to update the .github/workflows/check_diff.yml in a follow PR after this lands

r? @calebcartwright @jieyouxu @Manishearth

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.

I think this makes the overall code better. Also removes an unecessary lifetime annotation.

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.

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.

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.

This updates some logging output to be more descriptive and also adds some `debug!` and `error!` logging.

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.

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.

`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.

This let's us controll the rust language edition used to parse source code and the rustfmt style edition used for code formatting.

jieyouxu

Thanks! Looks good to me overall, a few nits

(Reviewed commit-by-commit, not sure if new github review experience is handling that quite right)

check_diff/src/lib.rs

check_diff/src/main.rs

jieyouxu · 2026-01-18T12:17:39Z

check_diff/src/main.rs

+                let Ok(tmp_dir) = tempdir() else {
+                    warn!(
+                        "Failed to create a tempdir for {}. Can't check formatting diff for {}",
+                        &url, repo_name
+                    );
+                    return;
+                };


Discussion: should this just error¹? Feels a bit strange if diffchecks between different runs of the job can be over a different set of repos (over possibly transient fs/env issues).

Ditto for the git repo clone below, but that one is particularly susceptible to network conditions and github's own availability...

EDIT: I realize (as a counter-argument to erroring) that it's possible to have timings where the main/master/default branch of these repos don't necessarily build against the nightly toolchain corresponding to the main/feature rustfmt (in theory), due to some recently bumped lints and such. But yeah.

Footnotes

As in, we can (in a follow-up) either add retry logic to tempdir creation, or retry the entire job, or a combination of both ↩

Adding some retry logic here would be really valuable. I'd be happy to work on that in a follow up PR. The one thing I was trying to avoid was having the whole job fail just because we failed to create the tempdir or clone one or two of the repos that we wanted to check.

I feel like the main purpose of this job is to give us confidence that we're not releasing any breaking changes, and as long as we cover a decent number of codebases I think we'll still be okay even if some fail.

Yeah, that makes sense 👍

check_diff/src/main.rs

check_diff/src/lib.rs

jieyouxu · 2026-01-18T12:26:09Z

check_diff/src/lib.rs

+    /// rust edition 2024
+    Edition2024,


Remark (non-actionable): internally in rustc, there's also edition=next

ytmimi · 2026-01-18T15:37:22Z

@jieyouxu Happy to address any nits before moving forward. Thanks for taking a look at the PR!

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.

jieyouxu

The current version looks good to me, thanks!

rustbot added the pr-not-reviewed label Dec 21, 2025

ytmimi marked this pull request as draft December 21, 2025 16:43

rustbot removed the pr-not-reviewed label Dec 21, 2025

ytmimi force-pushed the update_rust_check_diff branch from db0480a to 01e646c Compare December 21, 2025 16:49

ytmimi mentioned this pull request Dec 21, 2025

[DO NOT MERGE] subtree-push nightly-2025-12-06 #6743

Draft

ytmimi force-pushed the update_rust_check_diff branch 4 times, most recently from c3cd596 to 8bf953a Compare December 23, 2025 17:30

ytmimi force-pushed the update_rust_check_diff branch from 8bf953a to 78fdb19 Compare January 8, 2026 00:48

ytmimi added 12 commits January 10, 2026 13:16

Set dynamic library path env var name based on operating system

33bb89a

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.

Make format_code and associated functions more generic

3f40009

I think this makes the overall code better. Also removes an unecessary lifetime annotation.

remove utf-8 error variant

e9f06be

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.

return Err(FormatCodeError) from CodeFormatter::format_code

abac13b

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.

update check_diff logging

bc67c7d

This updates some logging output to be more descriptive and also adds some `debug!` and `error!` logging.

copy over the constant list of repositories from ci/check_diff.sh

7648720

Apply some clippy suggestions

837c4ee

Change the return type of check_diff to u8

1125576

`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.

Add --edition and --style-edition options to the check_diff CLI

45967f5

This let's us controll the rust language edition used to parse source code and the rustfmt style edition used for code formatting.

ytmimi force-pushed the update_rust_check_diff branch from 78fdb19 to 45967f5 Compare January 11, 2026 19:51

ytmimi marked this pull request as ready for review January 11, 2026 19:52

rustbot added the pr-not-reviewed label Jan 11, 2026

rustbot assigned calebcartwright Jan 11, 2026

ytmimi changed the title ~~WIP: Get the Check Diff rust binary working~~ Get the Check Diff rust binary working Jan 13, 2026

jieyouxu self-assigned this Jan 13, 2026

jieyouxu approved these changes Jan 18, 2026

View reviewed changes

ytmimi added 4 commits January 18, 2026 12:11

Apply PR suggestion for grouping repositories

2fc3ff8

show rustfmt compilation error

ab3f2ae

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.

just return ExitCode::FAILURE when we've found formatting diffs

f1c0a28

Add note about why format_code_from_path is better for rustfmt

55348f6

jieyouxu approved these changes Jan 19, 2026

View reviewed changes

ytmimi merged commit 75d056a into rust-lang:main Jan 19, 2026
26 checks passed

rustbot removed the pr-not-reviewed label Jan 19, 2026

ytmimi deleted the update_rust_check_diff branch January 19, 2026 03:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Get the Check Diff rust binary working #6751

Get the Check Diff rust binary working #6751

ytmimi commented Dec 21, 2025 •

edited

Loading

Uh oh!

jieyouxu left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jieyouxu Jan 18, 2026 •

edited

Loading

Uh oh!

ytmimi Jan 18, 2026

Uh oh!

jieyouxu Jan 18, 2026

Uh oh!

Uh oh!

Uh oh!

jieyouxu Jan 18, 2026

Uh oh!

ytmimi commented Jan 18, 2026

Uh oh!

jieyouxu left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Get the Check Diff rust binary working #6751

Get the Check Diff rust binary working #6751

Conversation

ytmimi commented Dec 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jieyouxu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jieyouxu Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Footnotes

Uh oh!

ytmimi Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

jieyouxu Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jieyouxu Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

ytmimi commented Jan 18, 2026

Uh oh!

jieyouxu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ytmimi commented Dec 21, 2025 •

edited

Loading

jieyouxu left a comment •

edited

Loading

jieyouxu Jan 18, 2026 •

edited

Loading