Skip to content

Commit

Permalink
Use target.rustflags instead of build.rustflags
Browse files Browse the repository at this point in the history
  • Loading branch information
Kobzol committed Jul 22, 2024
1 parent 4f69cbb commit a3fd32b
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 3 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
7 changes: 5 additions & 2 deletions src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(',');
Expand Down
30 changes: 29 additions & 1 deletion tests/integration/pgo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(());
}
Expand All @@ -296,6 +296,34 @@ 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.{target}]

Check failure on line 321 in tests/integration/pgo.rs

View workflow job for this annotation

GitHub Actions / Test on Windows

there is no argument named `target`
rustflags = ["-Ctarget-cpu=native"]
"#
),
);

let output = project.cmd(&["build", "--", "-v"]).run()?;
println!("{}", output.stderr());
assert!(output.stderr().contains("-Ctarget-cpu=native"));
Expand Down

0 comments on commit a3fd32b

Please sign in to comment.