-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
allow windows reserved names in CI #15135
allow windows reserved names in CI #15135
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
That check was added at the request of @ehuss at #10322 (comment) I feel like the comment in the code is ambiguous but in looking at that context, it seems like upgrading CI to a new Windows will cause us to lose coverage and this test was to make sure we got as much coverage as possible until that day. I don't see much we can do about ensuring old coverage except running two versions of Windows in Ci which doesn't seem ideal, so I'm assuming this should be fine but will wait a bit in case anyone else with more context can pipe in. |
This change makes sense to me. Eventually, once more users are on a Windows release that doesn't have this restriction, we can remove the test altogether. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems fine to me. My original intent was that once we drop Windows 10 as a tier-1 platform, we would migrate our CI to Windows 11 (and remove this test). However, it looks like CI migration is happening faster than our tier support migration. Probably not too much risk, though we could maybe consider running both windows-2022 and windows-2025 (at least on the cargo side), but probably not worth the bother.
I would just note that even the Windows 10 runner is a fair way above Rust's minimum tier 1 support (the runner is Windows Server 2019 whereas our minimum tier 1 supported version is Windows Server 2016 or Windows Desktop 1507). And that's not just a technicality as major "feature" releases were made on a yearly (or twice yearly) release schedule during Windows 10's lifetime. Ideally we would also test RTM Windows 10 but alas we don't have the infra. |
Update cargo 14 commits in 0e3d73849ab8cbbab3ec5c65cbd555586cb21339..2928e32734b04925ee51e1ae88bea9a83d2fd451 2025-02-01 20:14:40 +0000 to 2025-02-07 16:50:22 +0000 - Simplify backtrack (rust-lang/cargo#15150) - Don't use on Solaris libc::LOCK_* which were removed from libc in ver… (rust-lang/cargo#15143) - feat: emit error if package not found within workspace (rust-lang/cargo#15071) - Make cache tracking resilient to unexpected files (rust-lang/cargo#15147) - Small resolver cleanups (rust-lang/cargo#15040) - feat: add `cargo pkgid` support for cargo-script (rust-lang/cargo#14961) - Suggest similar feature names on CLI (rust-lang/cargo#15133) - fix: Don't use "did you mean" in errors (rust-lang/cargo#15138) - Fix changelog link (rust-lang/cargo#15142) - chore(deps): update rust crate rand to 0.9.0 (rust-lang/cargo#15129) - Remove the original changelog (rust-lang/cargo#15123) - chore(deps): update rust crate gix to 0.70.0 (rust-lang/cargo#15128) - allow windows reserved names in CI (rust-lang/cargo#15135) - removed a word that was repeated (rust-lang/cargo#15136)
We are progressively moving the windows CI from windows 2022 to windows 2025 because we found windows 2025 more stable.
In rust-lang/rust#136478 a cargo test failed and this might solve the issue. See this comment. What do you think?
If you have a better way of solving this, let me know 👍