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.60 #2453

Merged
merged 1 commit into from
Sep 2, 2022
Merged

Bump MSRV to 1.60 #2453

merged 1 commit into from
Sep 2, 2022

Conversation

madsmtm
Copy link
Member

@madsmtm madsmtm commented Sep 1, 2022

Since objc2 uses new cargo features, it is not possible to compile the dependency on 1.57, even if this is done on a platform which shouldn't need that dependency (this could be reverted, but I'd rather avoid doing so, as it is a bit of a usability regression when you want to use its cargo features).

I recognize that distributions will need to have caught up with 1.60 before we can really do this, but maybe we can do this now, and then time our next breaking release of winit with that?

  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior

CC @Lokathor @chrisduerr @kchibisov @rib

@madsmtm madsmtm added the S - maintenance Repaying technical debt label Sep 1, 2022
@madsmtm madsmtm mentioned this pull request Sep 1, 2022
8 tasks
@chrisduerr
Copy link
Contributor

IIRC @kchibisov mentioned that there are some IME issues that need fixing on X11, might be smart to hold off until the bugs of the last release are ironed out?

@kchibisov
Copy link
Member

We already have breaking changes on master branch. So while I can fix them, we can't make a patch release. Though, we can create a branch for old release and I can cherry pick on it? But in that case it doesn't matter.

@Lokathor
Copy link

Lokathor commented Sep 1, 2022

If the main branch already doesn't represent the latest release code (which is the case for this repo, as it is for nearly all repos) then we should just land this as soon as it's ready and just hold off on the release to crates.io itself.

As for the MSRV bump, unfortunate but it can't be helped, and we're still keeping nearly a 6 month window, so as madsmtm says we could also hold the crates.io release off by only a fairly short period of time and linux distros will likely be able to catch up.

@rib
Copy link
Contributor

rib commented Sep 1, 2022

This sounds good to me and would let me revert recent changes to make android-activity compile with 1.57 which I didn't really want to make.

Since it seems like the main motivation behind the MSRV is to try and help with packaging for Linux distros I also wonder whether we could limit the MSRV constraint to just the platforms that have a clear reason for it (maybe just Linux currently)?

On Android I don't think there is the (Rust) ecosystem maturity yet to justify being constrained to using older toolchains and using newer features in the Android backend wouldn't be a problem for Linux distros. For Android the priority is still with bootstrapping basic platform support, so it would be nice to not add additional constraints on that effort. One specific awkwardness I hit with the previous threshold of 1.57 was that it also, effectively, meant I would need to upstream changes to cargo ndk (because thats a tool that android-activity uses in CI) but it would seem a bit silly to justify limiting an Android build tool to older toolchains due to projects wanting to support Linux distros.

rib added a commit to rust-mobile/android-activity that referenced this pull request Sep 1, 2022
This reverts the changes from 66e3293
to support the 1.57.0 compiler (leaving the rustdoc fixes).

Making the changes to support 1.57 felt disappointing to require for the
sake of Winit being able to better support Linux distro packaging. Even
with the changes we still wouldn't really support
1.57 without also upstreaming changes to `cargo ndk`.

The new plan is to bank on Winit bumping its own MSRV to at least
1.58 which seems more than reasonable considering that even 1.59
is already >6months old.

Ref: rust-windowing/winit#2453

This updates ci.yml to now build with 1.60, assuming that the above
PR will be accepted.
@rib
Copy link
Contributor

rib commented Sep 1, 2022

For reference, I've now reverted the recent changes to android-activity to support 1.57.0 (considering that we weren't really able to build with 1.57.0 without also updating cargo ndk) and CI is now building against 1.60.0 instead, anticipating that this PR will land.

I'll wait until this lands before spinning a 0.2.1 release for android-activity so I can update my PR

@madsmtm madsmtm marked this pull request as ready for review September 1, 2022 13:04
@madsmtm madsmtm requested a review from kchibisov as a code owner September 1, 2022 13:04
@madsmtm
Copy link
Member Author

madsmtm commented Sep 1, 2022

Sounds like consensus is to do the bump and then wait with releasing new winit version until distributions have caught up / 6 months have passed (that would be the 7th of October).

I'll give this PR a day or so to simmer, in case someone wants to raise further concerns.

@rib
Copy link
Contributor

rib commented Sep 1, 2022

sounds fair to me. I probably wouldn't self impose a strict constraint on not releasing before 7th of october just on account of this though. It's commendable for Winit to try and support older compiler releases but if Winit were to end up being ready to release a bit before that 6month threshold no distro is going to be pointing fingers just because this is off by e.g. 2 weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S - maintenance Repaying technical debt
Development

Successfully merging this pull request may close these issues.

5 participants