Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Silently ignore cargo::rustc-check-cfg to avoid MSRV annoyance when stabilizing -Zcheck-cfg #13438

Merged
merged 1 commit into from
Feb 28, 2024

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Feb 13, 2024

This PR, removes the warning when trying to use cargo::rustc-check-cfg on stable or nightly (without the nightly-only -Zcheck-cfg flag) to avoid MSRV annoyance when stabilizing -Zcheck-cfg.

See this Zulip thread for more information and context.

cc @ehuss

@rustbot
Copy link
Collaborator

rustbot commented Feb 13, 2024

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

@rustbot rustbot added A-build-scripts Area: build.rs scripts S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 13, 2024
Comment on lines 917 to 922
check_cfgs.push(value.to_string());
} else {
warnings.push(format!(
"{}{} requires -Zcheck-cfg flag",
syntax_prefix, key
));
// silently ignoring the instruction to try to
// minimise MSRV annoyance when stabilizing -Zcheck-cfg
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

imo we should make each of these conditioned this on nightly vs stable

Copy link
Member Author

@Urgau Urgau Feb 13, 2024

Choose a reason for hiding this comment

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

wouldn't this create the same issue we trying to avoid?

or do you mean that the silencing part should be conditioned on nightly/stable ?

Copy link
Member

Choose a reason for hiding this comment

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

Do you mean that it should not be silenced on nightly, so that nightly users get the warning (which is useful!)?

Copy link
Contributor

Choose a reason for hiding this comment

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

The main problem we are trying to avoid is when this is stabilized but not yet in people's MSRV: they will either have to live with the build script warning on their MSRV or deal with the check-cfg warning on the latest Rust toolchain.

(btw I keep going back and forth between "meh, this is fine" and "we probably should still warn")

@bors
Copy link
Contributor

bors commented Feb 20, 2024

☔ The latest upstream changes (presumably #13409) made this pull request unmergeable. Please resolve the merge conflicts.

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

@ehuss
Copy link
Contributor

ehuss commented Feb 28, 2024

@bors r+

@ehuss
Copy link
Contributor

ehuss commented Feb 28, 2024

(Oh, I think homu is down. Will probably need to kick this once it is fixed.)

@ehuss
Copy link
Contributor

ehuss commented Feb 28, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Feb 28, 2024

📌 Commit da59007 has been approved by ehuss

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 28, 2024
@bors
Copy link
Contributor

bors commented Feb 28, 2024

⌛ Testing commit da59007 with merge a3ba4ee...

@bors
Copy link
Contributor

bors commented Feb 28, 2024

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing a3ba4ee to master...

@bors bors merged commit a3ba4ee into rust-lang:master Feb 28, 2024
21 checks passed
@Urgau Urgau deleted the check-cfg-msrv-annoyance branch February 28, 2024 18:02
ehuss pushed a commit to ehuss/cargo that referenced this pull request Feb 28, 2024
Silently ignore `cargo::rustc-check-cfg` to avoid MSRV annoyance when stabilizing `-Zcheck-cfg`

This PR, removes the warning when trying to use `cargo::rustc-check-cfg` on stable or nightly (without the nightly-only `-Zcheck-cfg` flag) to avoid MSRV annoyance when stabilizing `-Zcheck-cfg`.

See this [Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/check-cfg.20backwards.20compatible.20warnings) for more information and context.

cc `@ehuss`
bors added a commit that referenced this pull request Feb 29, 2024
[beta-1.77] Backport ignore `cargo::rustc-check-cfg`

This is a beta backport of #13438 (comment) to disable the warning when `cargo::rustc-check-cfg` is used in a build script on stable in order to provide a broader window where we can stabilize check-cfg, and allow developers to avoid the warning when they use a range of different Rust versions (such as stable and nightly).

Also backports to fix CI:
* #13455 — Remove unnecessary use statement in metabuild
@rustbot rustbot added this to the 1.77.0 milestone Mar 1, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 2, 2024
Update cargo

12 commits in 8964c8ccff6e420e2a38b8696d178d69fab84d9d..f772ec0224d3755ce52ac5128a80319fb2eb45d0
2024-02-27 19:22:46 +0000 to 2024-03-01 22:57:35 +0000
- feat(toml): Warn on unset Edition (rust-lang/cargo#13505)
- fix(msrv): Report all incompatible packages, not just a random one (rust-lang/cargo#13514)
- refactor: abstract `std::fs` away from on-disk index cache (rust-lang/cargo#13515)
- chore(deps): update compatible (rust-lang/cargo#13507)
- chore(deps): update rust crate rusqlite to 0.31.0 (rust-lang/cargo#13510)
- [docs]: Clarify vendored sources as read-only and way to modify (rust-lang/cargo#13512)
- chore(deps): update rust crate supports-hyperlinks to v3 (rust-lang/cargo#13511)
- refactor: Clarify more Config -> Context (rust-lang/cargo#13506)
- test: Make `edition` explicit in packages (rust-lang/cargo#13504)
- Add all unit's children recursively for `doc.extern-map` option (rust-lang/cargo#13481)
- fix(rustc): Always pass --edition to rustc (rust-lang/cargo#13499)
- Silently ignore `cargo::rustc-check-cfg` to avoid MSRV annoyance when stabilizing `-Zcheck-cfg` (rust-lang/cargo#13438)

r? ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-scripts Area: build.rs scripts S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants