From e1f0c8e5190f3f7bdd6d02613e67be8d51aab5e5 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 4 Mar 2026 19:00:41 +0100 Subject: [PATCH 1/5] Rustify `ci/integration.sh` script --- .github/workflows/integration.yml | 2 +- Cargo.toml | 5 + ci/integration.rs | 237 ++++++++++++++++++++++++++++++ ci/integration.sh | 121 --------------- 4 files changed, 243 insertions(+), 122 deletions(-) create mode 100644 ci/integration.rs delete mode 100755 ci/integration.sh diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index c1b04721cb1..a0760d27e2e 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -76,5 +76,5 @@ jobs: env: INTEGRATION: ${{ matrix.integration }} TARGET: x86_64-unknown-linux-gnu - run: ./ci/integration.sh + run: cargo run --bin ci-integration continue-on-error: ${{ matrix.allow-failure == true }} diff --git a/Cargo.toml b/Cargo.toml index 6ac140f2e09..562a27af581 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,10 @@ path = "src/format-diff/main.rs" name = "git-rustfmt" path = "src/git-rustfmt/main.rs" +[[bin]] +name = "ci-integration" +path = "ci/integration.rs" + [features] default = ["cargo-fmt", "rustfmt-format-diff"] cargo-fmt = [] @@ -70,3 +74,4 @@ tempfile = "3.23.0" [package.metadata.rust-analyzer] # This package uses #[feature(rustc_private)] rustc_private = true + diff --git a/ci/integration.rs b/ci/integration.rs new file mode 100644 index 00000000000..be900dd55af --- /dev/null +++ b/ci/integration.rs @@ -0,0 +1,237 @@ +use std::collections::HashMap; +use std::env::var; +use std::ffi::OsStr; +use std::path::Path; +use std::process::Command; + +macro_rules! map { + ($($key:expr => $value:expr),* $(,)?) => {{ + let mut __map: HashMap = HashMap::new(); + + $(__map.insert($key.into(), $value.into());)* + __map + }} +} + +fn write_file(file_path: impl AsRef, content: &str) -> Result<(), String> { + std::fs::write(&file_path, content).map_err(|error| { + format!( + "Failed to create empty `{}` file: {error:?}", + file_path.as_ref().display(), + ) + }) +} + +fn run_command_with_env( + bin: &str, + args: I, + current_dir: &str, + env: &HashMap, +) -> Result<(), String> +where + I: IntoIterator, + S: AsRef, +{ + let exit_status = Command::new(bin) + .args(args) + .envs(env) + .current_dir(current_dir) + .spawn() + .map_err(|error| format!("Failed to spawn command `{bin}`: {error:?}"))? + .wait() + .map_err(|error| format!("Failed to wait command `{bin}`: {error:?}"))?; + if exit_status.success() { + Ok(()) + } else { + Err(format!("Command `{bin}` failed")) + } +} + +fn run_command(bin: &str, args: I, current_dir: &str) -> Result<(), String> +where + I: IntoIterator, + S: AsRef, +{ + run_command_with_env(bin, args, current_dir, &HashMap::new()) +} + +fn run_command_with_output_and_env( + bin: &str, + args: I, + current_dir: &str, + env: &HashMap, +) -> Result +where + I: IntoIterator, + S: AsRef, +{ + let cmd_output = Command::new(bin) + .args(args) + .envs(env) + .current_dir(current_dir) + .output() + .map_err(|error| format!("Failed to spawn command `{bin}`: {error:?}"))?; + let mut output = String::from_utf8_lossy(&cmd_output.stdout).into_owned(); + output.push_str(&String::from_utf8_lossy(&cmd_output.stderr)); + Ok(output) +} + +fn run_command_with_output(bin: &str, args: I, current_dir: &str) -> Result +where + I: IntoIterator, + S: AsRef, +{ + run_command_with_output_and_env(bin, args, current_dir, &HashMap::new()) +} + +// Checks that: +// +// * `cargo fmt --all` succeeds without any warnings or errors +// * `cargo fmt --all -- --check` after formatting returns success +// * `cargo test --all` still passes (formatting did not break the build) +fn check_fmt_with_all_tests(env: HashMap, current_dir: &str) -> Result<(), String> { + check_fmt_base("--all", env, current_dir) +} + +// Checks that: +// +// * `cargo fmt --all` succeeds without any warnings or errors +// * `cargo fmt --all -- --check` after formatting returns success +// * `cargo test --lib` still passes (formatting did not break the build) +fn check_fmt_with_lib_tests(env: HashMap, current_dir: &str) -> Result<(), String> { + check_fmt_base("--lib", env, current_dir) +} + +fn check_fmt_base( + test_args: &str, + env: HashMap, + current_dir: &str, +) -> Result<(), String> { + fn check_output_does_not_contain(output: &str, needle: &str) -> Result<(), String> { + if output.contains(needle) { + Err(format!("`cargo fmt --all -v` contains `{needle}`")) + } else { + Ok(()) + } + } + + let output = run_command_with_output_and_env("cargo", &["test", test_args], current_dir, &env)?; + if ["build failed", "test result: FAILED."] + .iter() + .any(|needle| output.contains(needle)) + { + return Ok(()); + } + + write_file(Path::new(current_dir).join("rustfmt.toml"), "")?; + + let output = + run_command_with_output_and_env("cargo", &["fmt", "--all", "-v"], current_dir, &env)?; + + println!("{output}"); + + check_output_does_not_contain(&output, "internal error")?; + check_output_does_not_contain(&output, "warning")?; + check_output_does_not_contain(&output, "Warning")?; + + let output = run_command_with_output_and_env( + "cargo", + &["fmt", "--all", "--", "--check"], + current_dir, + &env, + )?; + write_file(Path::new(current_dir).join("rustfmt_check_output"), &output)?; + + run_command_with_env("cargo", &["test", test_args], current_dir, &env) +} + +fn show_head(integration: &str) -> Result<(), String> { + let head = run_command_with_output("git", &["rev-parse", "HEAD"], integration)?; + println!("Head commit of {integration}: {head}"); + Ok(()) +} + +fn run_test, &str) -> Result<(), String>>( + integration: &str, + git_repository: String, + env: HashMap, + test_fn: F, +) -> Result<(), String> { + run_command_with_output("git", &["clone", "--depth=1", git_repository.as_str()], ".")?; + show_head(integration)?; + test_fn(env, integration) +} + +fn runner() -> Result<(), String> { + let integration = match var("INTEGRATION") { + Ok(value) if !value.is_empty() => value, + _ => { + return Err("The INTEGRATION environment variable must be set.".into()); + } + }; + + // FIXME: this means we can get a stale cargo-fmt from a previous run. + // + // `which rustfmt` fails if rustfmt is not found. Since we don't install + // `rustfmt` via `rustup`, this is the case unless we manually install it. Once + // that happens, `cargo install --force` will be called, which installs + // `rustfmt`, `cargo-fmt`, etc to `~/.cargo/bin`. This directory is cached by + // travis (see `.travis.yml`'s "cache" key), such that build-bots that arrive + // here after the first installation will find `rustfmt` and won't need to build + // it again. + // + //which cargo-fmt || cargo install --force + run_command_with_env( + "cargo", + &["install", "--path", ".", "--force", "--locked"], + ".", + &map!( + "CFG_RELEASE" => "nightly", + "CFG_RELEASE_CHANNEL" => "nightly", + ), + )?; + + println!("Integration tests for {integration}"); + + run_command("cargo", &["fmt", "--", "--version"], ".")?; + + match integration.as_str() { + "cargo" => run_test( + &integration, + format!("https://github.com/rust-lang/{integration}.git"), + map!("CFG_DISABLE_CROSS_TESTS" => "1"), + check_fmt_with_all_tests, + ), + "crater" => run_test( + &integration, + format!("https://github.com/rust-lang/{integration}.git"), + map!(), + check_fmt_with_lib_tests, + ), + "bitflags" => run_test( + &integration, + format!("https://github.com/bitflags/{integration}.git"), + map!(), + check_fmt_with_all_tests, + ), + "tempdir" => run_test( + &integration, + format!("https://github.com/rust-lang-deprecated/{integration}.git"), + map!(), + check_fmt_with_all_tests, + ), + _ => run_test( + &integration, + format!("https://github.com/rust-lang/{integration}.git"), + map!(), + check_fmt_with_all_tests, + ), + } +} + +fn main() { + if let Err(error) = runner() { + eprintln!("{error}"); + std::process::exit(1); + } +} diff --git a/ci/integration.sh b/ci/integration.sh deleted file mode 100755 index ea96e4be130..00000000000 --- a/ci/integration.sh +++ /dev/null @@ -1,121 +0,0 @@ -#!/usr/bin/env bash - -set -ex - -: ${INTEGRATION?"The INTEGRATION environment variable must be set."} - -# FIXME: this means we can get a stale cargo-fmt from a previous run. -# -# `which rustfmt` fails if rustfmt is not found. Since we don't install -# `rustfmt` via `rustup`, this is the case unless we manually install it. Once -# that happens, `cargo install --force` will be called, which installs -# `rustfmt`, `cargo-fmt`, etc to `~/.cargo/bin`. This directory is cached by -# travis (see `.travis.yml`'s "cache" key), such that build-bots that arrive -# here after the first installation will find `rustfmt` and won't need to build -# it again. -# -#which cargo-fmt || cargo install --force -CFG_RELEASE=nightly CFG_RELEASE_CHANNEL=nightly cargo install --path . --force --locked - -echo "Integration tests for: ${INTEGRATION}" -cargo fmt -- --version - -# Checks that: -# -# * `cargo fmt --all` succeeds without any warnings or errors -# * `cargo fmt --all -- --check` after formatting returns success -# * `cargo test --all` still passes (formatting did not break the build) -function check_fmt_with_all_tests { - check_fmt_base "--all" - return $? -} - -# Checks that: -# -# * `cargo fmt --all` succeeds without any warnings or errors -# * `cargo fmt --all -- --check` after formatting returns success -# * `cargo test --lib` still passes (formatting did not break the build) -function check_fmt_with_lib_tests { - check_fmt_base "--lib" - return $? -} - -function check_fmt_base { - local test_args="$1" - local build=$(cargo test $test_args 2>&1) - if [[ "$build" =~ "build failed" ]] || [[ "$build" =~ "test result: FAILED." ]]; then - return 0 - fi - touch rustfmt.toml - cargo fmt --all -v |& tee rustfmt_output - if [[ ${PIPESTATUS[0]} != 0 ]]; then - cat rustfmt_output - return 1 - fi - cat rustfmt_output - ! cat rustfmt_output | grep -q "internal error" - if [[ $? != 0 ]]; then - return 1 - fi - ! cat rustfmt_output | grep -q "warning" - if [[ $? != 0 ]]; then - return 1 - fi - ! cat rustfmt_output | grep -q "Warning" - if [[ $? != 0 ]]; then - return 1 - fi - cargo fmt --all -- --check |& tee rustfmt_check_output - if [[ ${PIPESTATUS[0]} != 0 ]]; then - cat rustfmt_check_output - return 1 - fi - cargo test $test_args - if [[ $? != 0 ]]; then - return $? - fi -} - -function show_head { - local head=$(git rev-parse HEAD) - echo "Head commit of ${INTEGRATION}: $head" -} - -case ${INTEGRATION} in - cargo) - git clone --depth=1 https://github.com/rust-lang/${INTEGRATION}.git - cd ${INTEGRATION} - show_head - export CFG_DISABLE_CROSS_TESTS=1 - check_fmt_with_all_tests - cd - - ;; - crater) - git clone --depth=1 https://github.com/rust-lang/${INTEGRATION}.git - cd ${INTEGRATION} - show_head - check_fmt_with_lib_tests - cd - - ;; - bitflags) - git clone --depth=1 https://github.com/bitflags/${INTEGRATION}.git - cd ${INTEGRATION} - show_head - check_fmt_with_all_tests - cd - - ;; - tempdir) - git clone --depth=1 https://github.com/rust-lang-deprecated/${INTEGRATION}.git - cd ${INTEGRATION} - show_head - check_fmt_with_all_tests - cd - - ;; - *) - git clone --depth=1 https://github.com/rust-lang/${INTEGRATION}.git - cd ${INTEGRATION} - show_head - check_fmt_with_all_tests - cd - - ;; -esac From 04840f6eabb5f44f4e1b1820718070f3a59af36c Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 6 Mar 2026 11:27:33 +0100 Subject: [PATCH 2/5] Replace `map!` macro with plain `HashMap` calls --- ci/integration.rs | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/ci/integration.rs b/ci/integration.rs index be900dd55af..ad83d091cac 100644 --- a/ci/integration.rs +++ b/ci/integration.rs @@ -4,15 +4,6 @@ use std::ffi::OsStr; use std::path::Path; use std::process::Command; -macro_rules! map { - ($($key:expr => $value:expr),* $(,)?) => {{ - let mut __map: HashMap = HashMap::new(); - - $(__map.insert($key.into(), $value.into());)* - __map - }} -} - fn write_file(file_path: impl AsRef, content: &str) -> Result<(), String> { std::fs::write(&file_path, content).map_err(|error| { format!( @@ -26,7 +17,7 @@ fn run_command_with_env( bin: &str, args: I, current_dir: &str, - env: &HashMap, + env: &HashMap<&str, &str>, ) -> Result<(), String> where I: IntoIterator, @@ -59,7 +50,7 @@ fn run_command_with_output_and_env( bin: &str, args: I, current_dir: &str, - env: &HashMap, + env: &HashMap<&str, &str>, ) -> Result where I: IntoIterator, @@ -89,7 +80,7 @@ where // * `cargo fmt --all` succeeds without any warnings or errors // * `cargo fmt --all -- --check` after formatting returns success // * `cargo test --all` still passes (formatting did not break the build) -fn check_fmt_with_all_tests(env: HashMap, current_dir: &str) -> Result<(), String> { +fn check_fmt_with_all_tests(env: HashMap<&str, &str>, current_dir: &str) -> Result<(), String> { check_fmt_base("--all", env, current_dir) } @@ -98,13 +89,13 @@ fn check_fmt_with_all_tests(env: HashMap, current_dir: &str) -> // * `cargo fmt --all` succeeds without any warnings or errors // * `cargo fmt --all -- --check` after formatting returns success // * `cargo test --lib` still passes (formatting did not break the build) -fn check_fmt_with_lib_tests(env: HashMap, current_dir: &str) -> Result<(), String> { +fn check_fmt_with_lib_tests(env: HashMap<&str, &str>, current_dir: &str) -> Result<(), String> { check_fmt_base("--lib", env, current_dir) } fn check_fmt_base( test_args: &str, - env: HashMap, + env: HashMap<&str, &str>, current_dir: &str, ) -> Result<(), String> { fn check_output_does_not_contain(output: &str, needle: &str) -> Result<(), String> { @@ -151,10 +142,10 @@ fn show_head(integration: &str) -> Result<(), String> { Ok(()) } -fn run_test, &str) -> Result<(), String>>( +fn run_test, &str) -> Result<(), String>>( integration: &str, git_repository: String, - env: HashMap, + env: HashMap<&str, &str>, test_fn: F, ) -> Result<(), String> { run_command_with_output("git", &["clone", "--depth=1", git_repository.as_str()], ".")?; @@ -185,10 +176,10 @@ fn runner() -> Result<(), String> { "cargo", &["install", "--path", ".", "--force", "--locked"], ".", - &map!( - "CFG_RELEASE" => "nightly", - "CFG_RELEASE_CHANNEL" => "nightly", - ), + &HashMap::from([ + ("CFG_RELEASE", "nightly"), + ("CFG_RELEASE_CHANNEL", "nightly"), + ]), )?; println!("Integration tests for {integration}"); @@ -199,31 +190,31 @@ fn runner() -> Result<(), String> { "cargo" => run_test( &integration, format!("https://github.com/rust-lang/{integration}.git"), - map!("CFG_DISABLE_CROSS_TESTS" => "1"), + HashMap::from([("CFG_DISABLE_CROSS_TESTS", "1")]), check_fmt_with_all_tests, ), "crater" => run_test( &integration, format!("https://github.com/rust-lang/{integration}.git"), - map!(), + HashMap::new(), check_fmt_with_lib_tests, ), "bitflags" => run_test( &integration, format!("https://github.com/bitflags/{integration}.git"), - map!(), + HashMap::new(), check_fmt_with_all_tests, ), "tempdir" => run_test( &integration, format!("https://github.com/rust-lang-deprecated/{integration}.git"), - map!(), + HashMap::new(), check_fmt_with_all_tests, ), _ => run_test( &integration, format!("https://github.com/rust-lang/{integration}.git"), - map!(), + HashMap::new(), check_fmt_with_all_tests, ), } From 5706ca5ef7c0ad0158d77da609880d4d89a7f8c1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 6 Mar 2026 11:30:38 +0100 Subject: [PATCH 3/5] Only create `rustfmt.toml` if it does not already exist --- ci/integration.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ci/integration.rs b/ci/integration.rs index ad83d091cac..9f14743e243 100644 --- a/ci/integration.rs +++ b/ci/integration.rs @@ -114,7 +114,10 @@ fn check_fmt_base( return Ok(()); } - write_file(Path::new(current_dir).join("rustfmt.toml"), "")?; + let rustfmt_toml = Path::new(current_dir).join("rustfmt.toml"); + if !rustfmt_toml.is_file() { + write_file(rustfmt_toml, "")?; + } let output = run_command_with_output_and_env("cargo", &["fmt", "--all", "-v"], current_dir, &env)?; From f07689e18e0ab6c0392c201c7325b550f66174a9 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 9 Mar 2026 14:57:15 +0100 Subject: [PATCH 4/5] Stick closer to original shell script behaviour --- ci/integration.rs | 44 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/ci/integration.rs b/ci/integration.rs index 9f14743e243..0ec3c4dfe8b 100644 --- a/ci/integration.rs +++ b/ci/integration.rs @@ -46,12 +46,17 @@ where run_command_with_env(bin, args, current_dir, &HashMap::new()) } +struct CommandOutput { + output: String, + exited_successfully: bool, +} + fn run_command_with_output_and_env( bin: &str, args: I, current_dir: &str, env: &HashMap<&str, &str>, -) -> Result +) -> Result where I: IntoIterator, S: AsRef, @@ -64,10 +69,17 @@ where .map_err(|error| format!("Failed to spawn command `{bin}`: {error:?}"))?; let mut output = String::from_utf8_lossy(&cmd_output.stdout).into_owned(); output.push_str(&String::from_utf8_lossy(&cmd_output.stderr)); - Ok(output) + Ok(CommandOutput { + output, + exited_successfully: cmd_output.status.success(), + }) } -fn run_command_with_output(bin: &str, args: I, current_dir: &str) -> Result +fn run_command_with_output( + bin: &str, + args: I, + current_dir: &str, +) -> Result where I: IntoIterator, S: AsRef, @@ -109,7 +121,7 @@ fn check_fmt_base( let output = run_command_with_output_and_env("cargo", &["test", test_args], current_dir, &env)?; if ["build failed", "test result: FAILED."] .iter() - .any(|needle| output.contains(needle)) + .any(|needle| output.output.contains(needle)) { return Ok(()); } @@ -121,12 +133,16 @@ fn check_fmt_base( let output = run_command_with_output_and_env("cargo", &["fmt", "--all", "-v"], current_dir, &env)?; + println!("{}", output.output); - println!("{output}"); + if !output.exited_successfully { + return Err("`cargo fmt --all -v` failed".to_string()); + } - check_output_does_not_contain(&output, "internal error")?; - check_output_does_not_contain(&output, "warning")?; - check_output_does_not_contain(&output, "Warning")?; + let output = &output.output; + check_output_does_not_contain(output, "internal error")?; + check_output_does_not_contain(output, "warning")?; + check_output_does_not_contain(output, "Warning")?; let output = run_command_with_output_and_env( "cargo", @@ -134,13 +150,21 @@ fn check_fmt_base( current_dir, &env, )?; - write_file(Path::new(current_dir).join("rustfmt_check_output"), &output)?; + + if !output.exited_successfully { + return Err("`cargo fmt --all -- -v --check` failed".to_string()); + } + let output = &output.output; + if let Err(error) = write_file(Path::new(current_dir).join("rustfmt_check_output"), output) { + println!("{output}"); + return Err(error); + } run_command_with_env("cargo", &["test", test_args], current_dir, &env) } fn show_head(integration: &str) -> Result<(), String> { - let head = run_command_with_output("git", &["rev-parse", "HEAD"], integration)?; + let head = run_command_with_output("git", &["rev-parse", "HEAD"], integration)?.output; println!("Head commit of {integration}: {head}"); Ok(()) } From 178961706e433f64d0253251d8feaf9b24461540 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 10 Mar 2026 14:07:39 +0100 Subject: [PATCH 5/5] Improve `ci/integration.rs` code --- ci/integration.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/ci/integration.rs b/ci/integration.rs index 0ec3c4dfe8b..dbfecf181ab 100644 --- a/ci/integration.rs +++ b/ci/integration.rs @@ -118,11 +118,13 @@ fn check_fmt_base( } } - let output = run_command_with_output_and_env("cargo", &["test", test_args], current_dir, &env)?; + let output = + run_command_with_output_and_env("cargo", &["test", test_args], current_dir, &env)?.output; if ["build failed", "test result: FAILED."] .iter() - .any(|needle| output.output.contains(needle)) + .any(|needle| output.contains(needle)) { + println!("`cargo test {test_args}` failed: {output}"); return Ok(()); } @@ -141,6 +143,7 @@ fn check_fmt_base( let output = &output.output; check_output_does_not_contain(output, "internal error")?; + check_output_does_not_contain(output, "internal compiler error")?; check_output_does_not_contain(output, "warning")?; check_output_does_not_contain(output, "Warning")?; @@ -160,6 +163,7 @@ fn check_fmt_base( return Err(error); } + // This command allows to ensure that no source file was modified while running the tests. run_command_with_env("cargo", &["test", test_args], current_dir, &env) } @@ -188,17 +192,6 @@ fn runner() -> Result<(), String> { } }; - // FIXME: this means we can get a stale cargo-fmt from a previous run. - // - // `which rustfmt` fails if rustfmt is not found. Since we don't install - // `rustfmt` via `rustup`, this is the case unless we manually install it. Once - // that happens, `cargo install --force` will be called, which installs - // `rustfmt`, `cargo-fmt`, etc to `~/.cargo/bin`. This directory is cached by - // travis (see `.travis.yml`'s "cache" key), such that build-bots that arrive - // here after the first installation will find `rustfmt` and won't need to build - // it again. - // - //which cargo-fmt || cargo install --force run_command_with_env( "cargo", &["install", "--path", ".", "--force", "--locked"],