From 422c5644f6534d4663df04f1c587e1808e2a320f Mon Sep 17 00:00:00 2001 From: Arthur Pastel Date: Thu, 10 Jul 2025 17:51:27 +0200 Subject: [PATCH 1/3] tests(cargo-codspeed): create a testcase for handling rustflags from the cargo config --- crates/cargo-codspeed/tests/.gitignore | 2 +- .../tests/cargo_config.in/.cargo/config.toml | 2 ++ .../tests/cargo_config.in/Cargo.toml | 14 ++++++++ .../benches/cargo_config_bench.rs | 17 +++++++++ .../tests/cargo_config.in/build.rs | 3 ++ crates/cargo-codspeed/tests/cargo_config.rs | 35 +++++++++++++++++++ 6 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 crates/cargo-codspeed/tests/cargo_config.in/.cargo/config.toml create mode 100644 crates/cargo-codspeed/tests/cargo_config.in/Cargo.toml create mode 100644 crates/cargo-codspeed/tests/cargo_config.in/benches/cargo_config_bench.rs create mode 100644 crates/cargo-codspeed/tests/cargo_config.in/build.rs create mode 100644 crates/cargo-codspeed/tests/cargo_config.rs diff --git a/crates/cargo-codspeed/tests/.gitignore b/crates/cargo-codspeed/tests/.gitignore index 35709a7c..661ad40f 100644 --- a/crates/cargo-codspeed/tests/.gitignore +++ b/crates/cargo-codspeed/tests/.gitignore @@ -1,2 +1,2 @@ +*/target */Cargo.lock -*/target/ diff --git a/crates/cargo-codspeed/tests/cargo_config.in/.cargo/config.toml b/crates/cargo-codspeed/tests/cargo_config.in/.cargo/config.toml new file mode 100644 index 00000000..7b4d0356 --- /dev/null +++ b/crates/cargo-codspeed/tests/cargo_config.in/.cargo/config.toml @@ -0,0 +1,2 @@ +[build] +rustflags = ["--cfg", "custom_feature_flag"] diff --git a/crates/cargo-codspeed/tests/cargo_config.in/Cargo.toml b/crates/cargo-codspeed/tests/cargo_config.in/Cargo.toml new file mode 100644 index 00000000..d8098d66 --- /dev/null +++ b/crates/cargo-codspeed/tests/cargo_config.in/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "cargo_config_test" +version = "0.1.0" +edition = "2021" +publish = false + +[dependencies] +codspeed-criterion-compat = { path = "../../../criterion_compat" } + +[workspace] + +[[bench]] +name = "cargo_config_bench" +harness = false diff --git a/crates/cargo-codspeed/tests/cargo_config.in/benches/cargo_config_bench.rs b/crates/cargo-codspeed/tests/cargo_config.in/benches/cargo_config_bench.rs new file mode 100644 index 00000000..d763db64 --- /dev/null +++ b/crates/cargo-codspeed/tests/cargo_config.in/benches/cargo_config_bench.rs @@ -0,0 +1,17 @@ +use codspeed_criterion_compat::{criterion_group, criterion_main, Criterion}; + +fn bench_failing_without_custom_flag(c: &mut Criterion) { + c.bench_function("custom_flag_disabled", |b| { + b.iter(|| { + // This will cause a compilation error if custom_feature_flag is not set + #[cfg(not(custom_feature_flag))] + compile_error!( + "custom_feature_flag is not enabled - .cargo/config.toml rustflags not applied" + ); + }) + }); +} + +criterion_group!(benches, bench_failing_without_custom_flag); + +criterion_main!(benches); diff --git a/crates/cargo-codspeed/tests/cargo_config.in/build.rs b/crates/cargo-codspeed/tests/cargo_config.in/build.rs new file mode 100644 index 00000000..0fc4f2c0 --- /dev/null +++ b/crates/cargo-codspeed/tests/cargo_config.in/build.rs @@ -0,0 +1,3 @@ +fn main() { + println!("cargo::rustc-check-cfg=cfg(custom_feature_flag)"); +} diff --git a/crates/cargo-codspeed/tests/cargo_config.rs b/crates/cargo-codspeed/tests/cargo_config.rs new file mode 100644 index 00000000..3882f087 --- /dev/null +++ b/crates/cargo-codspeed/tests/cargo_config.rs @@ -0,0 +1,35 @@ +use std::process::Command; + +mod helpers; +use helpers::*; + +#[test] +fn test_cargo_config_rustflags() { + let tmp_dir = setup("tests/cargo_config.in", Project::Simple); + + // Test that cargo bench works with the custom flag + let output = Command::new("cargo") + .arg("bench") + .arg("--no-run") + .current_dir(&tmp_dir) + .output() + .expect("Failed to execute cargo bench"); + + assert!( + output.status.success(), + "cargo codspeed bench should succeed with .cargo/config.toml rustflags", + ); + + // Test that cargo codspeed build also works with the custom flag + let output = cargo_codspeed(&tmp_dir) + .arg("build") + .output() + .expect("Failed to execute cargo codspeed build"); + + assert!( + output.status.success(), + "cargo codspeed build should succeed with .cargo/config.toml rustflags", + ); + + teardown(tmp_dir); +} From d5206e75de0a53aa314083492ffa256432290fdb Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Fri, 11 Jul 2025 12:29:02 +0200 Subject: [PATCH 2/3] feat(cargo-codspeed): handle rustflags from .cargo/config.toml --- crates/cargo-codspeed/Cargo.toml | 1 + crates/cargo-codspeed/src/build.rs | 68 ++++++++++++++++++++++-------- 2 files changed, 51 insertions(+), 18 deletions(-) diff --git a/crates/cargo-codspeed/Cargo.toml b/crates/cargo-codspeed/Cargo.toml index ee246080..3d24b52c 100644 --- a/crates/cargo-codspeed/Cargo.toml +++ b/crates/cargo-codspeed/Cargo.toml @@ -1,6 +1,7 @@ [package] name = "cargo-codspeed" version = "3.0.2" +rust-version = "1.74" # MSRV edition = "2021" description = "Cargo extension to build & run your codspeed benchmarks" authors = ["Arthur Pastel "] diff --git a/crates/cargo-codspeed/src/build.rs b/crates/cargo-codspeed/src/build.rs index 65646f99..ab099b6d 100644 --- a/crates/cargo-codspeed/src/build.rs +++ b/crates/cargo-codspeed/src/build.rs @@ -105,30 +105,62 @@ impl BuildOptions<'_> { Ok(built_benches) } + /// Adds debug flags and codspeed compilation + /// + /// If the user has set `RUSTFLAGS`, it will append the flags to it. + /// Else, and if the cargo version allows it, it will set the cargo config through + /// `--config 'build.rustflags=[ ... ]'` + /// + /// # Why we do this + /// As tracked in [https://github.com/rust-lang/cargo/issues/5376], setting `RUSTFLAGS` + /// completely overrides rustflags from cargo config + /// We use the cargo built-in config mechanism to set the flags if the user has not set + /// `RUSTFLAGS`. + fn add_rust_flags(&self, cargo: &mut Command, measurement_mode: MeasurementMode) { + let mut flags = vec![ + // Add debug info (equivalent to -g) + "-Cdebuginfo=2".to_owned(), + // Prevent debug info stripping + // https://doc.rust-lang.org/cargo/reference/profiles.html#release + // According to cargo docs, for release profile which we default to: + // `strip = "none"` and `debug = false`. + // In practice, if we set debug info through RUSTFLAGS, cargo still strips them, most + // likely because debug = false in the release profile. + // We also need to disable stripping through rust flags. + "-Cstrip=none".to_owned(), + ]; + + // Add the codspeed cfg flag if instrumentation mode is enabled + if measurement_mode == MeasurementMode::Instrumentation { + flags.push("--cfg=codspeed".to_owned()); + } + + match std::env::var("RUSTFLAGS") { + Result::Ok(existing_rustflags) => { + // Expand already existing RUSTFLAGS env var + let flags_str = flags.join(" "); + cargo.env("RUSTFLAGS", format!("{existing_rustflags} {flags_str}")); + } + Err(_) => { + // Use --config to set rustflags + // Our rust integration has an msrv of 1.74, --config is available since 1.63 + // https://doc.rust-lang.org/nightly/cargo/CHANGELOG.html#cargo-163-2022-08-11 + let config_value = format!( + "build.rustflags=[{}]", + flags.into_iter().map(|f| format!("\"{f}\"")).join(",") + ); + cargo.arg("--config").arg(config_value); + } + } + } + /// Generates a subcommand to build the benchmarks by invoking cargo and forwarding the filters /// This command explicitly ignores the `self.benches`: all benches are built fn build_command(&self, measurement_mode: MeasurementMode) -> Command { let mut cargo = Command::new("cargo"); cargo.args(["build", "--benches"]); - let mut rust_flags = std::env::var("RUSTFLAGS").unwrap_or_else(|_| "".into()); - // Add debug info (equivalent to -g) - rust_flags.push_str(" -C debuginfo=2"); - - // Prevent debug info stripping - // https://doc.rust-lang.org/cargo/reference/profiles.html#release - // According to cargo docs, for release profile which we default to: - // `strip = "none"` and `debug = false`. - // In practice, if we set debug info through RUSTFLAGS, cargo still strips them, most - // likely because debug = false in the release profile. - // We also need to disable stripping through rust flags. - rust_flags.push_str(" -C strip=none"); - - // Add the codspeed cfg flag if instrumentation mode is enabled - if measurement_mode == MeasurementMode::Instrumentation { - rust_flags.push_str(" --cfg codspeed"); - } - cargo.env("RUSTFLAGS", rust_flags); + self.add_rust_flags(&mut cargo, measurement_mode); if let Some(features) = self.features { cargo.arg("--features").arg(features.join(",")); From 0926c07b4935332ee2778a5244149ddfd133ee9d Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Fri, 11 Jul 2025 16:31:19 +0200 Subject: [PATCH 3/3] ci: fix walltime build within the repo With the fix from previous commit, the `--cfg codspeed` would be caught by the cargo config of the repo. --- .github/workflows/ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7921fcae..565193ab 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -107,7 +107,10 @@ jobs: - run: cargo install --path crates/cargo-codspeed --locked - - run: cargo codspeed build -p ${{ matrix.package }} + - run: | + # Remove the cargo config else it forces instrumentation mode + rm -f .cargo/config.toml + cargo codspeed build -p ${{ matrix.package }} - name: Run the benchmarks uses: CodSpeedHQ/action@main