From 8dbe52d46d1f5f19d296ee77485cd0649f40b65e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Mon, 22 Jul 2024 16:26:41 +0200 Subject: [PATCH 1/2] Use `target.rustflags` instead of `build.rustflags` --- CHANGELOG.md | 8 ++++++++ src/build.rs | 7 +++++-- tests/integration/pgo.rs | 31 ++++++++++++++++++++++++++++++- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19cc955..bd37bfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +# Dev +## Changes + +- Use `target.rustflags` for passing PGO flags instead + of `build.rustflags` (https://github.com/Kobzol/cargo-pgo/issues/56). + This should make it less likely that PGO flags will be overridden by users' `.cargo/config.toml` files. + On the other hand, `cargo pgo` will now override `build.rustflags` set in `config.toml`. + # 0.2.8 (28. 3. 2024) ## Fixes diff --git a/src/build.rs b/src/build.rs index 7daf301..e677ea4 100644 --- a/src/build.rs +++ b/src/build.rs @@ -93,10 +93,13 @@ pub fn cargo_command_with_rustflags( (true, _) => { // If there's no RUSTFLAGS, and Cargo supports `--config`, use it. // The user might have some flags in their config.toml file(s), and - // `--config build.rustflags` will append to it instead of overwriting it. + // `--config target.*.rustflags` will append to it instead of overwriting it. + // We use target RUSTFLAGS instead of e.g. build rustflags, because it has a higher + // precedence. See https://github.com/Kobzol/cargo-pgo/issues/56 for more details. final_cargo_args.push("--config".to_string()); - let mut flags = String::from("build.rustflags=["); + // `cfg(all())` should match any target. + let mut flags = String::from("target.'cfg(all())'.rustflags=["); for (index, flag) in rustflags.into_iter().enumerate() { if index > 0 { flags.push(','); diff --git a/tests/integration/pgo.rs b/tests/integration/pgo.rs index 02daa5a..a60e198 100644 --- a/tests/integration/pgo.rs +++ b/tests/integration/pgo.rs @@ -282,7 +282,7 @@ fn test_respect_existing_rustflags() -> anyhow::Result<()> { /// This only works for Rust 1.63+. #[test] -fn test_respect_existing_rustflags_from_config() -> anyhow::Result<()> { +fn test_override_build_rustflags_from_config() -> anyhow::Result<()> { if !version_check::is_min_version("1.63.0").unwrap_or(false) { return Ok(()); } @@ -296,6 +296,35 @@ rustflags = ["-Ctarget-cpu=native"] "#, ); + let output = project.cmd(&["build", "--", "-v"]).run()?; + println!("{}", output.stderr()); + assert!(!output.stderr().contains("-Ctarget-cpu=native")); + assert!(output.stderr().contains("-Cprofile-generate")); + output.assert_ok(); + + Ok(()) +} + +#[test] +fn test_respect_existing_target_rustflags_from_config() -> anyhow::Result<()> { + if !version_check::is_min_version("1.63.0").unwrap_or(false) { + return Ok(()); + } + + let target = get_default_target()?; + + let mut project = init_cargo_project()?; + project.file( + ".cargo/config.toml", + &format!( + r#" +[target.{}] +rustflags = ["-Ctarget-cpu=native"] +"#, + target + ), + ); + let output = project.cmd(&["build", "--", "-v"]).run()?; println!("{}", output.stderr()); assert!(output.stderr().contains("-Ctarget-cpu=native")); From d6f4193c3ffcc8f2955a8bde91c146f0594b2304 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Mon, 22 Jul 2024 17:00:59 +0200 Subject: [PATCH 2/2] Fix clippy issues --- src/bolt/instrument.rs | 2 +- src/bolt/optimize.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bolt/instrument.rs b/src/bolt/instrument.rs index 5f14497..b031e8c 100644 --- a/src/bolt/instrument.rs +++ b/src/bolt/instrument.rs @@ -78,7 +78,7 @@ pub fn bolt_instrument(ctx: CargoContext, args: BoltInstrumentArgs) -> anyhow::R "{} {} instrumented successfully. Now run {} on your workload.", capitalize(get_artifact_kind(&artifact)).yellow(), artifact.target.name.blue(), - cli_format_path(&instrumented_path.display()) + cli_format_path(instrumented_path.display()) ); } } diff --git a/src/bolt/optimize.rs b/src/bolt/optimize.rs index ce4d8d4..61ef322 100644 --- a/src/bolt/optimize.rs +++ b/src/bolt/optimize.rs @@ -77,7 +77,7 @@ The optimization will probably not be very effective.", "{} {} successfully optimized with BOLT. You can find it at {}.", capitalize(get_artifact_kind(&artifact)).yellow(), artifact.target.name.blue(), - cli_format_path(&optimized_path.display()) + cli_format_path(optimized_path.display()) ); } }