Skip to content

ci: switch to Cargo build.warnings#6212

Open
jinmay wants to merge 1 commit into
PyO3:mainfrom
jinmay:ci-build-warnings
Open

ci: switch to Cargo build.warnings#6212
jinmay wants to merge 1 commit into
PyO3:mainfrom
jinmay:ci-build-warnings

Conversation

@jinmay

@jinmay jinmay commented Jul 20, 2026

Copy link
Copy Markdown

Closes #6205.

Replaces RUSTFLAGS=-D warnings with Cargo's build.warnings (CARGO_BUILD_WARNINGS=deny) for the rustc-warning paths, so CI no longer cache-busts local builds — RUSTFLAGS is part of cargo's fingerprint, build.warnings is not.

Two things I kept separate from a straight swap:

  • build.warnings is only respected on Cargo 1.97+. PyO3's MSRV is 1.83 and the MSRV jobs run through build.yml / check-feature-powerset, so there the setting is silently ignored and warnings stay non-fatal until the MSRV reaches 1.97; stable jobs still enforce them. I kept this simple rather than gating per-toolchain, since it self-resolves.
  • build.warnings covers rustc build warnings but not rustdoc, so I left RUSTDOCFLAGS=-D warnings (and the docs nox session) in place.

Verified locally on 1.97.1 that CARGO_BUILD_WARNINGS=deny turns warnings into errors for cargo build, test, and hack check, and is ignored on 1.95.

(AI-assisted; the MSRV/rustdoc analysis and final decisions are my own.)

@jinmay
jinmay marked this pull request as ready for review July 20, 2026 04:43
@jinmay

jinmay commented Jul 20, 2026

Copy link
Copy Markdown
Author

Heads up — CI surfaced a subtlety that makes this more than a drop-in swap.

The Windows build jobs fail in ffi-check:

warning: linker stdout: LINK : warning LNK4044: unrecognized option '/Wl,-rpath,...'; ignored
  = note: `#[warn(linker_messages)]` on by default
  = note: the `linker_messages` lint ignores `-D warnings`
error: warnings are denied by `build.warnings` configuration

This isn't a real code warning — it's the linker_messages lint. On main the same warning is emitted but stays a warning under RUSTFLAGS=-D warnings, because that lint deliberately ignores -D warnings. CARGO_BUILD_WARNINGS=deny doesn't honor that exception, so it's promoted to an error and breaks the build.

This seemed to contradict the stabilization promise that build.warnings "matches RUSTFLAGS=-Dwarnings", so I looked upstream — and it's already a known bug: rust-lang/cargo#17142 (linker messages), with rust-lang/cargo#17193 covering the same divergence for removed lints. Until that's resolved, switching the rustc-warning path to build.warnings is a Windows regression.

(The python3.15-dev failure is unrelated — a pre-existing pyo3-ffi-check MACRO_EXCLUSIONS mismatch on 3.15, and that job is continue-on-error anyway.)

Options as I see them:

  1. Hold until cargo#17142 is fixed.
  2. Keep rustc warnings on -D warnings, adopt build.warnings only where it's safe.
  3. Suppress the linker warning separately (treats the symptom, not the cause).

Leaning towards 1/2 — happy to adjust to whichever you prefer.

@davidhewitt

Copy link
Copy Markdown
Member

I disagree with any of the (seemingly LLM-assisted) root-cause analysis/options above. The linker warning is almost certainly from add_libpython_rpath_link_args - we should probably not emit anything from that function on Windows. Worth fixing in a precursor PR.

@jinmay

jinmay commented Jul 20, 2026

Copy link
Copy Markdown
Author

You're right, and apologies. I think my mistake came before the analysis itself: CI broke on my change, and instead of questioning my own approach, I assumed the code where the failure showed up was the root cause and started digging into it (with AI). That assumption was wrong from the start.

Your point is the actual issue: print_libpython_rpath_link_args still emits -Wl,-rpath on Windows (and Cygwin), where it's a no-op that only produces a linker warning.

I'd like to fix that first and rebase this on top. Shall I open a separate issue + PR for it?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

switch to build.warnings in CI

2 participants