-
Notifications
You must be signed in to change notification settings - Fork 24
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
Fixes for nightly features, bump MSRV #21
base: main
Are you sure you want to change the base?
Conversation
`maybe_uninit_ref` has stabilized since 1.55, and `read_initializer` is no longer a nightly feature. Remove both features now that stable versions on supported (run in CI) packages include 1.55+ compilers. Consider bumping MSRV to 1.55+
All latest stable on supported OSes is at least ten versions ahead of the MSRV. Fixes MSRV build on MacOS
Ping @bbqsrc |
Ping @bbqsrc, is this repository still maintained? |
Not sure why this bumps the MSRV? |
The MacOS CI builds fail without it. You can confirm the build failure by reverting the MSRV in the CI builder script, and running the CI build against the MacOS targets. You can also confirm by checking the error messages for the currently failing build on |
Sure, but what was the issue with macOS, why does it fail? Also you can just bump the macOS job - just because macOS is broken in some way or another doesn't imply that should require an MSRV bump for all machines. |
OK, if macOS is not a supported platform, then it should be removed from the CI. If it is a supported platform, the MSRV needs to be bumped so the crate can actually build on macOS. A third way could be to have a split MSRV where you say "Alright so we have MSRV 1.x.y on this that and the other platform, and MSRV 1.z.p on macOS". The last option looks decidedly wonky to me. |
Given the crate seems unmaintained I'm not sure it matters, but that's not that wonky - Afaiu older rustc doesn't support modern macOS, which is fine, MSRV can still be checked on older macOS - "if your system can run rustc X, we support it" seems like a great policy to me. |
Right, I had to fork it for another project I was working on, and the stuff in this crate really belong upstream. Last I checked there is still ongoing work for that. Though, the RFC looks a little dormant, too.
I think we are talking past each other. You basically just said the |
FWIW Rust 1.55 is also nearly two years old itself, much less the current MSRV. |
I think you missed my note that newer rustc doesn't work on older macOS - so you can't support new macOS on older rustc, hence if you limit to "if your system can run..." then the MSRV can absolutely be older ;)
I'm aware. Projects I work on sadly have to support 1.48 still both for validation and to build for old targets. |
Fair enough, but you see the contradiction here? The MSRV to get the build to work under the macOS in the CI is higher than what is available on the older macOS (making older macOS a non-supported platform). With CI breaking, it limits the usability for all the other targets not just the older macOS targets (which are technically unsupported until added to the CI). It truly doesn't matter to me, there is a fork of this code, the fork does what I need it to, and I'll replace it as soon as upstream implements these traits in I submitted this patch in an attempt to help the users/maintainers of this crate have something that builds in the CI. If there is a similar fix that doesn't require the MSRV bump, and achieves the same as the changes in 699986b, great. Maybe something like https://crates.io/crates/rustversion would work, but that comes with an additional dependency. Edit: other errors also come up using
From changes in #24 If the maintainer(s) have abandoned the crate, then this whole discussion is pointless. Your patch also fails CI checks, so no further additions will pass CI until the changes in this PR are addressed. |
That's not what I'm suggesting at all, you can also run older macOS in CI :) |
Some fixes for features that have become stable, and an MSRV bump to fix MacOS CI builds.