hyperium / hyper

An HTTP library for Rust
https://hyper.rs
MIT License
14.07k stars 1.55k forks source link

chore: fix unexpected cfg warning #3660

Closed tottoto closed 1 month ago

tottoto commented 1 month ago

Fixes unexpected cfg warnings, which cause the ci fail.

https://doc.rust-lang.org/nightly/cargo/reference/build-scripts.html#rustc-check-cfg

seanmonstar commented 1 month ago

Started with https://github.com/rust-lang/cargo/pull/13571

seanmonstar commented 1 month ago

A build file slows down compilation measurably. Can it be solved without?

tottoto commented 1 month ago

I think another workaround is to allow this lint.

tottoto commented 1 month ago

Changed to allow it.

Darksonn commented 1 month ago

Note that it's possible to add a ci check that still runs the lint, with your custom --cfgs allowed. See #6538.

tottoto commented 1 month ago

I have known it, but I do not think that it is a good idea to use --cfg for configs which are always intended to be given.

Darksonn commented 1 month ago

What do you mean by that?

sfackler commented 1 month ago

@dtolnay has a neat approach to avoid the build.rs build time penalty: https://github.com/tokio-rs/tokio/pull/6542

seanmonstar commented 1 month ago

That's clever, but I think currently my preferred solution is to just allow the lint normally, and optionally add a CI job checking it.

tottoto commented 1 month ago

Updated to allow unexpected_cfgs as default and check it with a new ci job.

Urgau commented 1 month ago

Heads up, with the release of rust-lang/cargo#13913 (in nightly-2024-05-19), there is no longer any need for the kind of workarounds employed in this PR. Cargo has now gain the ability to declare --check-cfg args directly inside the [lints] table with [lints.rust.unexpected_cfgs.check-cfg][^1]:

Cargo.toml:

[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ['cfg(foo)'] }

Note that the diagnostic output of the lint has been updated to suggest the [lints] approach first. You can use it to guide you through the --check-cfg arguments that may need to be added.

[^1]: take effect on Rust 1.80 (current nightly), is ignored on Rust 1.79 (current beta), and produce an unused warning below