deny warnings only when compiling for tests

[bluss]

deny warnings only when compiling for tests

This makes cargo forward-compatible with lint changes upstream in rust.

This relates to rust-lang/rust/pull/33091. This PR makes cargo never stop compiling from denied lints, but tests may still fail. This allows using cap-lints warn sensibly in rust's cargotest, since we know it will not break cargo itself.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @huonw (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[bluss]

cc @alexcrichton

[bluss]

So this is the simplest approach and needs upstream rust to use cap-lints.

Another approach could be to never use deny by default (neither build nor test), but opt into it when running CI.

[alexcrichton]

Would this actually help the compiler? We always run cargo test anyway which would deny warnings eventually?

[bluss]

This allows using cap-lints warn sensibly in rust's cargotest, since we know it will not break cargo itself.

[alexcrichton]

I'm not sure I understand, if we use --cap-lints warn then it essentially ignores these #[deny] directives, so why would we also need to change them?

[bluss]

Please see the issue rust-lang/rust/pull/33091 for context and also the wish expressed here rust-lang/rust#33091 (comment)

With no deny in the regular build, lint changes will not break cargo. Thus we can accept using cap-lints warn in cargotest like @brson wishes. I think it makes sense.

Alternatively, we can change cargo around to not use deny at all in the default build / test configuration and only enable it in cargo's CI. I thought it would be unbeautiful to add some configuration variable in cargo code just for that.

[alexcrichton]

Ok, sorted this out on IRC with @bluss.

Specifically:

• The combination of #2617 and rust-lang/rust#33215 means that no lint changes will bounce Rust code.
• This PR should guarantee that cargo build then always works, but not necessarily cargo test.

I think that basically only guaranteeing cargo build probably isn't too useful for now, as we still wouldn't be able to land changes until cargo test was fixed. As a result I'm gonna close this for now, but we can certainly revisit if it becomes a problem

[bluss]

I did mention the alternative above

Another approach could be to never use deny by default (neither build nor test), but opt into it when running CI.

from our discussion I think it's the best approach.

[bluss]

In that sentence, CI is this repository's CI.

[alexcrichton]

Unfortunately that still wouldn't help landing changes in Cargo (as they'd still be blocked on broken lints) :(

[bluss]

improved lints 😉

[alexcrichton]

right!