linker_messages force-warn lint level is not respected

[Mark-Simulacrum]

Not entirely sure if this is a bug in Cargo or in rustc or I'm misunderstanding the stabilization report, but my expectation based on https://doc.rust-lang.org/nightly/cargo/reference/config.html#buildwarnings ("Only warnings that are lints (i.e. level is adjustable) are affected") and the stabilization report ("this matches RUSTFLAGS=-Dwarnings") is that CARGO_BUILD_WARNINGS and RUSTFLAGS=-Dwarnings should have the same behavior. However, for linker messages that doesn't appear to be the case; -Dwarnings is ignored while CARGO_BUILD_WARNINGS denies the lint and breaks compilation.

It was intentional that linker messages don't get affected by deny warnings so this behavior is inconsistent with intent on the rustc side.

Both of these are stabilized in 1.97, which will become stable in ~1.5 weeks and are intended to both feature in the blog post, so it probably makes sense to figure out whether we're happy with this (to me unexpected) interaction.

$ cargo build --config "target.aarch64-apple-darwin.linker=\"$(pwd)/linker.rs\""
   Compiling test-linker-message v0.1.0 (/Volumes/Edit/Edit/test-linker-message)
warning: linker stderr: Linker emits a message
  |
  = note: `#[warn(linker_messages)]` on by default

warning: `test-linker-message` (bin "test-linker-message") generated 1 warning

$ RUSTFLAGS=-Dwarnings cargo build --config "target.aarch64-apple-darwin.linker=\"$(pwd)/linker.rs\""
   Compiling test-linker-message v0.1.0 (/Volumes/Edit/Edit/test-linker-message)
warning: linker stderr: Linker emits a message
  |
  = note: `#[warn(linker_messages)]` on by default
  = note: the `linker_messages` lint ignores `-D warnings`

$ CARGO_BUILD_WARNINGS=deny cargo build --config "target.aarch64-apple-darwin.linker=\"$(pwd)/linker.rs\""
warning: linker stderr: Linker emits a message
  |
  = note: `#[warn(linker_messages)]` on by default

error: `test-linker-message` (bin "test-linker-message") generated 1 warning
error: warnings are denied by `build.warnings` configuration

Linker warning-emitter:

Details

$ cat linker.rs
#!/usr/bin/env cargo -Zscript
---
[package]
edition = "2024"
---

fn main() -> Result<(), std::process::ExitCode> {
    eprintln!("Linker emits a message");
    let status = std::process::Command::new("cc")
        .args(std::env::args_os().skip(1))
        .status()
        .unwrap();
    if status.success() {
        Ok(())
    } else {
        Err(std::process::ExitCode::FAILURE)
    }
}

[Mark-Simulacrum]

cc @jyn514 @rust-lang/cargo since it's getting pretty close to stable for us to change something (if we decide to do so).

[weihanglo]

From rust-lang/rust@ff8faeb looks like linker_messages lint is the first lint that ignore -Dwarnings?

This is a really special lint that doesn't fully respect any previous lint logic (it is adjustable and not force-warn but ignore -Dwarnings). Some questions:

• How to generalize the ignore_deny_warnings idea in rustc/clippy/cargo lint system?
  • Should it be encoded in the diagnostic JSON message output, so Cargo can adjust its behavior accordingly
  • Are there other lints wanting this behavior?
• If user really wants to deny linker-messages in CI, what should they do?
  • They can definite set RUSTFLAGS=-Dlinker-messages along with CARGO_BUILD_WARNINGS=deny though that defeat the benefit of CARGO_BUILD_WARNINGS. That said, this can probably to mitigate by something like add a RUSTLINKFLAGS environment variable #4349.

BTW in my opinion Cargo can probably hardcode the behavior for linker_messages lint in short term, though it is a bit confusing and I think we should consider this mechanism as a whole.
(I would also love to hear opinions from t-clippy)

[epage]

Note that this came up on the tracking issue, starting at #14802 (comment)

[epage]

My quick thoughts:

• If this is a lint, then this is behaving as documented and intended on Cargo's side
• I think there is some wiggle room for changing this after stabilization
• Any change needs to be considered through the lens of "how do we explain this through principles cargo users know?"

[weihanglo]

@epage It is not uncommon that linker generates some amount of warnings (there are some examples in rust-lang/rust#136096 and at work I've seen more with older linkers). And that seems to be one of the main motivations of linker_messages ignoring -Dwarnings.

If Cargo's build.warnings denies linker messages, it may make this feature less cross-platform compatible and people may not switch to it due linker warning failing CI. Also, linker warnings are usually not trivial to suppress and often need some less-known ld flags harder to maintain.

edit: I meant to reply to "I think there is some wiggle room for changing this after stabilization".

[epage]

My comment was not to dismiss the use case but to answer to the original Issue about whether this was a bug or misunderstanding of the stabilization report. This is working as intended from Cargo's side. The comparisons to the warnings group was not prescriptive but descriptive and we admittedly have a poor understanding of the warnings group, as we discovered through this features development (and cements further concerns I would have over adding hacks to it).

This feature is effectively designed to say that warnings are soft errors. We have a carve out for hard warnings because the user doesn't have the contingency of changing the level. If something needs to be displayed but is not a soft error, that should likely be communicated through another means (e.g. a nit lint level like is talked about in rust-lang/rfcs#3730 or rust-lang/rfcs#3926, independent of the rest of the policy of those RFCs).

[Mark-Simulacrum]

I'm confused by the statement that this is working as intended: the stabilization report directly states that this is "equivalent to -Dwarnings". The way I've understood this feature, the intent is that users should migrate to it from setting RUSTFLAGS (seems like a good goal, there are real benefits). I repeated the same general statement in the proposed blog post announcing this feature (rust-lang/blog.rust-lang.org#1873).

Do we have suggestions for how to rephrase the announcement? My sense is that otherwise it's pretty likely we'll have to tell users to move back to -Dwarnings or add a -Alinker_messages if they're seeing a linker warning. The latter is not horrible, but it does seem unfortunate that these are landing in the same release :(

Maybe we can delay one or the other.

[epage]

I'm confused by the statement that this is working as intended: the stabilization report directly states that this is "equivalent to -Dwarnings"

Note that it never says "equivalent" in behavior but "similar" in purpose. It is intended to cover the same workflow for end users of allowing users to treat warnings as soft errors. build.warnings is not a 1:1 in behavior with the warnings group. The stabilization report discusses that the behavior "matches" -Dwarnings but that is being descriptive of the bullet points above that statement that do not cover the nuance of this case. I had either forgotten about that previous conversation and/or did not know of the timing of the linker feature landing when I wrote the stabilization report.

Do we have suggestions for how to rephrase the announcement?

Personally, I'm not seeing an issue with the wording on this subject at the time I write this:

It's common practice to deny warnings in CI. Historically, doing so is typically done through RUSTFLAGS=-Dwarnings.

This feature is about denying warnings in CI. -Dwarnings is typically how that has been done.

My sense is that otherwise it's pretty likely we'll have to tell users to move back to -Dwarnings or add a -Alinker_messages if they're seeing a linker warning. The latter is not horrible, but it does seem unfortunate that these are landing in the same release :(

Yes, it is unfortunate that the conversation fizzled out and nothing more was done and these ended up in the same release (even next to each other in the blog post).

For my framing of the situation (the role of warnings), -Alinker_messages seems the most appropriate recommendation if that is getting in people's way. Similarly, I run with -Adeprecated set in my CI.

I wonder how many people will be affected. For myself, I'm guessing this won't affect me as I've not seen linker warnings on my nightly runs. My general recommendation is to not combine -Dwarnings with other jobs in CI, like tests, so you don't have slower to report, likely more platform specific, and more important (e.g. logic errors) operations being blocked by fast, lower importance (e.g. clippy style) operations. I also have not found as much importance from checking for warnings under every configuration, so I only do so for more relatively mundane platforms. Unsure how representative this thinking is.

[jyn514]

My sense is that otherwise it's pretty likely we'll have to tell users to move back to -Dwarnings or add a -Alinker_messages if they're seeing a linker warning.

i think this is actually perfect timing because it forces an explicit decision: do people want to treat linker warnings as regular warnings? if so, they can change to CARGO_DENY_WARNINGS. do they want back-compat, or have they not read the announcement? then they can do nothing and continue to use -D warnings.

[jyn514]

@epage how does build.warnings interact with lint controls? is there a way to use build.warnings=deny while setting "-W linker_messages but in cargo", or something like that? we could recommend that in the release post if people really want 1-1 parity with -Dwarnings.

[weihanglo]

how does build.warnings interact with lint controls? is there a way to use build.warnings=deny while setting "-W linker_messages but in cargo", or something like that? we could recommend that in the release post if people really want 1-1 parity with -Dwarnings.

I was thinking whether --force-warn linker_messages would work, but turns out not. build.warnings doesn't respect --force-warn for other normal lints at all. Not even unused_veriables. I've read #14802 and we didn't go deeper for this interaction.

Not saying the force-warn interaction is a blocker, but it may be worth calling out explicitly.

With -Dwarnings -> error

$ RUSTFLAGS='-Dwarnings' cargo +nightly build
    Compiling xxx v0.1.0 (/home/projects/foo)
error: unused variable: `x`
 --> src/main.rs:2:9
  |
2 |     let x = 1;
  |         ^ help: if this is intentional, prefix it with an underscore: `_x`
  |
  = note: `-D unused-variables` implied by `-D warnings`
  = help: to override `-D warnings` add `#[allow(unused_variables)]`

error: could not compile `xxx` (bin "xxx") due to 1 previous error

With -Dwarnings + --force-warn -> warn

$ RUSTFLAGS='--force-warn unused-variables -Dwarnings' cargo +nightly build
   Compiling xxx v0.1.0 (/home/projects/foo)
warning: unused variable: `x`
 --> src/main.rs:2:9
  |
2 |     let x = 1;
  |         ^ help: if this is intentional, prefix it with an underscore: `_x`
  |
  = note: requested on the command line with `--force-warn unused-variables`
  = note: the `unused_variables` lint ignores `-D warnings`

warning: `xxx` (bin "xxx") generated 1 warning (run `cargo fix --bin "xxx" -p xxx` to apply 1 suggestion)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.27s

With CARGO_BUILD_WARNINGS=deny -> error

$ CARGO_BUILD_WARNINGS=deny cargo +nightly build
    Compiling xxx v0.1.0 (/home/projects/foo)
warning: unused variable: `x`
 --> src/main.rs:2:9
  |
2 |     let x = 1;
  |         ^ help: if this is intentional, prefix it with an underscore: `_x`
  |
  = note: `#[warn(unused_variables)]` (part of `#[warn(unused)]`) on by default

error: `xxx` (bin "xxx") generated 1 warning (run `cargo fix --bin "xxx" -p xxx` to apply 1 suggestion)
error: warnings are denied by `build.warnings` configuration

With CARGO_BUILD_WARNINGS=deny + --force-warn -> error

$ RUSTFLAGS='--force-warn unused_variables' CARGO_BUILD_WARNINGS=deny cargo +nightly build
   Compiling xxx v0.1.0 (/home/projects/foo)
warning: unused variable: `x`
 --> src/main.rs:2:9
  |
2 |     let x = 1;
  |         ^ help: if this is intentional, prefix it with an underscore: `_x`
  |
  = note: requested on the command line with `--force-warn unused-variables`

error: `xxx` (bin "xxx") generated 1 warning (run `cargo fix --bin "xxx" -p xxx` to apply 1 suggestion)
error: warnings are denied by `build.warnings` configuration

[Mark-Simulacrum]

"-W linker_messages but in cargo"

You can definitely allow the lint:

[lints.rust]
linker_messages = "allow"

But an explicit warn level there doesn't do any good -- which seems expected to me, since that's the behavior you generally want for warnings.

My suspicion is that long-term we may want to treat linker messages as special; I suspect in 90% of cases the configuration of whether to expect them / show them / etc belongs alongside the linker configuration (i.e., in per-target [target] configuration or otherwise "global"). It's unfortunate that there's no environment variable users can set for lints.rust.linker_messages = "allow". It may be that Cargo should 'take ownership' of the linker messages in general and render them similarly to build script warnings (https://doc.rust-lang.org/cargo/reference/build-scripts.html#cargo-warning).

Those aren't denied by CARGO_BUILD_WARNINGS=deny, presumably because there's no way to control them in a targeted fashion today? (per #16796 (comment) this is expected I think).

$ CARGO_BUILD_WARNINGS=deny cargo build --config "target.aarch64-apple-darwin.linker=\"$(pwd)/linker.rs\""
   Compiling test-linker-message v0.1.0 (/Volumes/Edit/Edit/test-linker-message)
warning: test-linker-message@0.1.0: this is a build script warning
    Finished [`dev` profile [unoptimized + debuginfo]](https://doc.rust-lang.org/cargo/reference/profiles.html#default-profiles) target(s) in 0.86s

[weihanglo]

You can definitely allow the lint:

[lints.rust]
linker_messages = "allow"

Just want to clarify this works but not the 1:1 parity with -Dwarnings jyn asked for