Skip to content

fix: forward build.warnings config to external subcommands#17035

Open
raushan728 wants to merge 3 commits into
rust-lang:masterfrom
raushan728:fix/forward-config-to-external-subcommands
Open

fix: forward build.warnings config to external subcommands#17035
raushan728 wants to merge 3 commits into
rust-lang:masterfrom
raushan728:fix/forward-config-to-external-subcommands

Conversation

@raushan728
Copy link
Copy Markdown
Contributor

cargo --config='build.warnings="deny"' clippy silently exits 0 even when warnings are present. Outer cargo consumes --config but never forwards it to external subcommand binaries.

Forwards resolved build.warnings as CARGO_BUILD_WARNINGS env var when spawning external subcommands.

Fixes rust-lang/rust-clippy#16963

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 26, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 26, 2026

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @ehuss, @epage, @weihanglo
  • @ehuss, @epage, @weihanglo expanded to ehuss, epage, weihanglo
  • Random selection from ehuss, epage, weihanglo

Comment thread tests/testsuite/warning_override.rs
Comment thread src/bin/cargo/main.rs
None => ProcessBuilder::new(&cargo_exe),
};
cmd.env(cargo::CARGO_ENV, cargo_exe).args(args);
if let Ok(handling) = gctx.warning_handling() {
Copy link
Copy Markdown
Member

@weihanglo weihanglo May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable and useful enough to special-case it, though we might want to discuss in team meeting

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@epage
It seems worth to me as it is pretty common with clippy in CI we set env var for overrides. However, I don't know if we want to restrict this to clippy-like binary only to avoid over-expanding the stable interface to all external commands

Copy link
Copy Markdown
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry didn't mean to approve yet

View changes since this review

@rustbot rustbot added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 26, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 26, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@raushan728 raushan728 force-pushed the fix/forward-config-to-external-subcommands branch from d692f70 to 37b23cf Compare May 26, 2026 10:15
@raushan728
Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. labels May 26, 2026
@raushan728 raushan728 requested a review from weihanglo May 26, 2026 10:16
@epage
Copy link
Copy Markdown
Contributor

epage commented May 26, 2026

This is a more broad problem than just this case. Should we be handling this in a one-off manner?

@raushan728
Copy link
Copy Markdown
Contributor Author

This is a broader issue. Open to a generic approach if the team prefers it.

@epage
Copy link
Copy Markdown
Contributor

epage commented May 26, 2026

@epage
Copy link
Copy Markdown
Contributor

epage commented May 26, 2026

I personally would only handle this generically or not at all.

What we should do should be discussed in an Issue. Do ue have an existing one for this class of problem? If not, should we tranfer the clippy issue?

@raushan728
Copy link
Copy Markdown
Contributor Author

I came across rust-lang/rust-clippy#16963 and wasn't aware of an existing cargo issue for this. Transferring that issue here seems like the right move to continue the discussion.

@epage
Copy link
Copy Markdown
Contributor

epage commented May 26, 2026

#11390 is our issue for this.

It might be good to keep an issue on clippy's side in case they need to make changes to adapt to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area: Command-line interface, option parsing, etc. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support build.warnings

4 participants