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

Bump MSRV to 1.56 (Edition 2021) #1269

Merged
merged 6 commits into from
Dec 6, 2022
Merged

Bump MSRV to 1.56 (Edition 2021) #1269

merged 6 commits into from
Dec 6, 2022

Conversation

dhardy
Copy link
Member

@dhardy dhardy commented Nov 22, 2022

We recently bumped the MSRV to 1.51 (#1246), but we could bump it further. This version is just over a year old, and the next release will be breaking anyway.

  • for: Lets us use edition = "2021" and rust-version = "1.56"
  • for: We currently have a CI failure since one of our dev-dependencies, Rayon, requires 1.56
  • against: ...?

This PR also applies a few Clippy recommendations. Tempting to invoke rustfmt too, but there are too many open PRs which would conflict.

The README now notes:

Older releases may work (depending on feature configuration) but are untested.

If you object, please reply.

@dhardy dhardy requested a review from newpavlov November 22, 2022 12:51
Copy link
Member

@newpavlov newpavlov left a comment

Choose a reason for hiding this comment

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

Maybe also bump crate versions to make it clear that next releases will be breaking ones?

@dhardy
Copy link
Member Author

dhardy commented Nov 22, 2022

Maybe also bump crate versions to make it clear that next releases will be breaking ones?

Unless I forgot something we didn't yet make any breaking changes to rand_core, so non-breaking releases of this plus the RNG crates would still be possible. So I guess we should ask: do we want to make a breaking release to those too?

  • For: keeping a single MSRV for everything here is slightly easier for us (and one less CI test)
  • For: we have some pending breaking changes: fill_via_chunks: mutate src on BE (small optimisation) #1182, Add read_adapter to avoid dynamic dispatch #1267
  • Against: avoiding a breaking release to rand_core makes migration to Rand v0.9 easier
  • Some ideas for breaking changes to RngCore have been floated but no PR yet and quite possibly we want new language features first, hence we expect a breaking release in the future anyway (possibly years away so could be 2.0)
  • We can't release 1.0 without getrandom 1.0 anyway

So maybe I should revert the MSRV bump to core + RNG crates and add another CI MSRV test?

I'll cross-post to the tracker for anyone watching that.

@dhardy dhardy mentioned this pull request Nov 22, 2022
23 tasks
@dhardy
Copy link
Member Author

dhardy commented Nov 22, 2022

test-avr fails due to using too old pinned version of nightly:

toolchain: nightly-2021-01-07 # Pinned compiler version due to rust-lang/compiler-builtins#400

@dhardy
Copy link
Member Author

dhardy commented Nov 22, 2022

AVR failure is rust-lang/rust#102278 ?

@dhardy
Copy link
Member Author

dhardy commented Nov 22, 2022

Latest AVR failure:

error: ran out of registers during register allocation

At this point I'm tempted to remove AVR support. CC @Palladinium who added this CI test in #1144.

@vks
Copy link
Collaborator

vks commented Nov 22, 2022

Tests requiring unstable features are expected to fail from time to time, so maybe we should just ignore this failure? I expect it will be fixed eventually.

@dhardy
Copy link
Member Author

dhardy commented Nov 23, 2022

Is there any point having a test if we ignore failures?

Anyway, I was planning on leaving this open for a few more days.

I added another PR to the list of pending breaking changes for rand_core, so maybe we should just go ahead with a breaking release? I'm still on the fence about this question.

@vks
Copy link
Collaborator

vks commented Nov 23, 2022

Ideally, we would notice when it works again. However, in this case we pin the nightly, so I guess it won't fix itself.

Does it make sense to keep the test as a comment?

@dhardy
Copy link
Member Author

dhardy commented Dec 6, 2022

Okay, I commented the AVR test out.

Lets go with a breaking release for rand_core since we have a few proposed changes and no objections.

@dhardy
Copy link
Member Author

dhardy commented Dec 12, 2022

Apparently AVR works on nightly-2022-12-09 works.

I won't bother re-enabling AVR (no motivation) but if someone else wants to, go ahead and make a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants