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

Minimum Supported Rust Version #1075

Closed
Lokathor opened this issue Jul 29, 2019 · 25 comments · Fixed by #2400
Closed

Minimum Supported Rust Version #1075

Lokathor opened this issue Jul 29, 2019 · 25 comments · Fixed by #2400
Labels
C - needs discussion Direction must be ironed out S - meta Project governance

Comments

@Lokathor
Copy link

Back in #531 it seems that it was agreed to test against 1.24 in CI, and now travis and appveyor just test against stable.

It would be better for the long term health of the community if foundational crates such as winit had a particular version number for minimum rust requirement. Could we move winit back in that direction? Since everyone is working on winit-0.20 being a "big change" with event loop 2.0 and all that, it'd be nice to also use that as a time to transition back to a policy of having a specific minimum rust version and trying to be aware of when that minimum version goes up.

@goddessfreya
Copy link
Contributor

Personally, it is of my opinion that we should support the last three stable rust versions, so 1.36, 1.35 and 1.34 currently. This was discussed between me and @chrisduerr in the past, but I never got around to updating the CI or the readme. I'm not really in favor of a MSRV.

@goddessfreya goddessfreya added C - needs discussion Direction must be ironed out S - meta Project governance labels Jul 29, 2019
@Lokathor
Copy link
Author

I've since had some additional information brought to my attention

Now, I'm not asking that you always keep the same minimum rust version if your dependencies aren't willing to play along. However, it should be entirely possible for you at least go back to tracking a minimum version and then having any minimum version bumps force a breaking change version bump in your own release.

@chrisduerr
Copy link
Contributor

Personally, it is of my opinion that we should support the last three stable rust versions, so 1.36, 1.35 and 1.34 currently. This was discussed between me and @chrisduerr in the past, but I never got around to updating the CI or the readme. I'm not really in favor of a MSRV.

I think this would be a great idea and offer quite a bit of room for users and distributions to update their systems. Note that I've proposed this policy based on a discussion with alexcrichton here. So winit wouldn't be first to adopt this policy.

I'd also like to note that this doesn't mean that winit can't support older versions, just that changing the minimum version to something older than the last three versions is not a breaking change.

This is an important detail to make maintenance and CI a bit easier. There's no need to bump winit's minimum supported CI version whenever a new Rust version is released, the CI can just stay on 1.20.0 for example but as soon as a change is required which updates to something that was not introduced in the last two rust versions, CI can just be bumped and everything's good again.

This policy would also mean that if there's really a necessity to update to something even more recent, that would be possible too. However, in that case it needs to clearly be marked as a breaking change so no downstream dependencies run into any issues. However I think generally this policy should be lax enough that this shouldn't usually be necessary, but it's an important tool to have to clearly communicate when this is the case.

@Lokathor
Copy link
Author

Well, code actually breaks. It worked one moment and doesn't after a cargo update.

That doesn't mean that you can't ever move your minimum rust version, just also reflect it as a breaking change in your version numbering.

@Osspial
Copy link
Contributor

Osspial commented Jul 29, 2019

We've gone through this before. I'm extremely wary about doing this without official support from Cargo (such as with rust-lang/rfcs#2495), since it's pretty much impossible to enforce a MSRV if you have dependencies that don't enforce it.

However, it should be entirely possible for you at least go back to tracking a minimum version and then having any minimum version bumps force a breaking change version bump in your own release.

Since we have dependencies that don't enforce a MSRV, this isn't feasible. If you have dependencies that are willing to bump the MSRV in patch releases (which we do), this sort of policy can and will lead to scenarios where breaking changes happen upon running cargo update, even if no changes are made in our repository (see #743, which @Lokathor already linked). It seems awfully arbitrary to force a version bump on our end, especially given that in this scenario any pre-version-bump versions on crates.io are still going to be broken on old versions of the Rust compiler.

I'm not in theory opposed to doing this, but I don't expect it to be feasible without broader coordination in the Rust community. Until then, I have no interest in putting up with the headache that this induces.

@chrisduerr
Copy link
Contributor

it's pretty much impossible to enforce a MSRV if you have dependencies that don't enforce it.

I guess binaries have it a little easier than libraries here, since lockfiles prevent dependencies from getting updated automatically downstream.

However if the library is built sufficient times and people are willing to report issues related to this, cargo is capable of fixing any issues related to this and locking down breaking dependencies temporarily is possible. To make sure this doesn't break releases it would be necessary to always specify fixed dependency versions though, which would likely cause lots of trouble downstream too with having to deal with outdated dependencies.

The reason why I personally requested a minimum supported Rust version to be specified was because winit actually broke things though. And I think that it would definitely be possible to at least track winit's minimum Rust version requirements. At the end of the day, breaking stuff because others do it isn't really a great argument, so that might at least be a good middle ground until proper support is added in cargo (which I'd expect to take at least a couple of years).

@Osspial What do you think about at least having an internal guarantee about winit not breaking stuff like accepting PRs requiring features from the latest stable right after it was released? That should be a good way to at least show that the desire for a little bit of consistency is there and might potentially inspire others to follow suite (and eventually bring around proper cargo support)?

@Lokathor
Copy link
Author

If the winit team is interested in moving in this direction but doesn't feel that they have the person-hours needed, I will volunteer to do it.

@chrisduerr
Copy link
Contributor

If the winit team is interested in moving in this direction but doesn't feel that they have the person-hours needed, I will volunteer to do it.

I think the problem isn't necessarily person-hours, but more that it's impossible for libraries to make any guarantees for past releases since the lock file is not provided with libraries.

So any depedency of winit could get updated by a downstream user who still uses winit 0.0.1 which might have coincidentally broken the minimum supported Rust version and the only thing winit could do is yank 0.0.1 and release a new version (probably 0.0.2) which locks down the dependency to a fixed version. This would have to be done for all past releases, which is pretty much impossible to do in practice.

@Lokathor
Copy link
Author

Oh, no no, I don't think all winit versions back through time need this. Just moving to this policy going forward, starting with winit-0.20.0 would be satisfactory.

And yes, dependencies doing a min version bump without also doing a breaking release are a problem. My suggestion there would be similar to what you said: temporarily issue a winit patch that holds that dependency back to the last version that worked, and then development going forward can bump the min version, bump the dependency to newest, and then bump the winit version.

@chrisduerr
Copy link
Contributor

Oh, no no, I don't think all winit versions back through time need this. Just moving to this policy going forward, starting with winit-0.20.0 would be satisfactory.

The thing is, all future winit versions would have to be maintained at the same time then. It's just not feasible to yank and push new versions constantly, that'll just create a huge mess.

@Lokathor
Copy link
Author

Well, the more you do this, and the more you spread awareness of this to other crates to start playing by the same rules, the easier this process becomes.

I do not think that you can give an absolute guarantee immediately starting in 0.20. That's definitely an unrealistic demand. However, I think that the intent can be broadcast starting with 0.20, and then the situation can become more solid over time.

I don't think that versions would need to be yanked over this until quite far in the future when much more of the ecosystem is on-board with the idea. However, this is a very visible crate to help get the ball rolling with.

@francesca64
Copy link
Member

Rather than pushing the burden of this onto every library maintainer in the ecosystem, wouldn't it be better to work on pushing forward the internals discussion on Cargo features, policies, etc. that would enable making these guarantees? You have to persuade a bunch of people either way, and some people will never be convinced unless it's made official.

I've tried playing the "please yank and bump" game before, and it isn't fun. It leads to emotional friction, which leads to me feeling upset and having a bad day. I understand that your goal is to reduce that friction by making Rust version policies a de facto convention, but every maintainer ultimately has their own beliefs and values, and that will lead to many more threads at least as long as this one.

@Osspial
Copy link
Contributor

Osspial commented Jul 30, 2019

What do you think about at least having an internal guarantee about winit not breaking stuff like accepting PRs requiring features from the latest stable right after it was released? That should be a good way to at least show that the desire for a little bit of consistency is there and might potentially inspire others to follow suite (and eventually bring around proper cargo support)?

I'm entirely okay with giving the ecosystem a few weeks to catch up to the newest stable version before accepting PRs with new features.

Beyond that, @francesca64's response reflects my views well, and I don't have much more to add at the moment. It'll be much less of headache for everyone involved to defer tackling this until the ecosystem supports it.

@Liamolucko
Copy link
Contributor

I'm extremely wary about doing this without official support from Cargo (such as with rust-lang/rfcs#2495), since it's pretty much impossible to enforce a MSRV if you have dependencies that don't enforce it.

rust-version in Cargo.toml was stabilised in Rust 1.56, so that's now an option.

kchibisov added a commit to kchibisov/winit that referenced this issue Jul 27, 2022
This should help with distributing apps using winit.

Fixes rust-windowing#1075.
kchibisov added a commit to kchibisov/winit that referenced this issue Jul 27, 2022
This should help with distributing apps using winit.

Fixes rust-windowing#1075.
kchibisov added a commit to kchibisov/winit that referenced this issue Jul 27, 2022
This should help with distributing apps using winit.

Fixes rust-windowing#1075.
kchibisov added a commit to kchibisov/winit that referenced this issue Jul 29, 2022
This should help with distributing apps using winit.

Fixes rust-windowing#1075.
kchibisov added a commit that referenced this issue Jul 29, 2022
This should help with distributing apps using winit.

Fixes #1075.
a-llie pushed a commit to a-llie/winit that referenced this issue Aug 24, 2022
Unify `with_app_id` and `with_class` methods

Both APIs are used to set application name. This commit unifies the API
between Wayland and X11, so downstream applications can remove platform
specific code when using `WindowBuilderExtUnix`.

Fixes rust-windowing#1739.

Unify behavior of `resizable` across platforms

This makes X11 and Wayland follow Windows and macOS, so the size of the
window could be set even though it has resizable attribute set to false.

Fixes rust-windowing#2242.

Fix assigning the wrong monitor when receiving Windows move events (rust-windowing#2266)

Fix embedded NULs in C wide strings returned from Windows API (rust-windowing#2264)

On Wayland, fix hiding cursors on GNOME

`wl_pointer::set_cursor` expects a serial number of the last
`wl_pointer::enter` event. However other calls expect latest
observed pointer serial, so this commit tracks both and
use them as required by specification.

Fixes rust-windowing#2273.

Bump windows-sys version to 0.36 (rust-windowing#2277)

Add new `Ime` event for desktop platforms

This commit brings new Ime event to account for preedit state of input
method, also adding `Window::set_ime_allowed` to toggle IME input on
the particular window.

This commit implements API as designed in rust-windowing#1497 for desktop platforms.

On Wayland, provide option for better CSD

While most compositors provide server side decorations, the GNOME
does not, and won't provide them. Also Wayland clients must render
client side decorations.

Winit was already drawing some decorations, however they were bad
looking and provided no text rendering, so the title was missing.
However this commit makes use of the SCTK external frame similar to
GTK's Adwaita theme supporting text rendering and looking similar to
other GTK applications.

Fixes rust-windowing#1967.

Fix warnings on nightly rust (rust-windowing#2295)

This was causing CI to fail: https://github.com/rust-windowing/winit/runs/6506026326

On macOS, emit resize event on `frame_did_change`

When the window switches mode from normal to tabbed one, it doesn't
get resized, however the frame gets resized. This commit makes
winit to track resizes when frame changes instead of window.

Fixes rust-windowing#2191.

Reorganize `EventLoopBuilder::build()` platform documentation

Since there's a "Platform-specific" header, it makes sense to put the
Linux-specific part under it. On the other hand, "Can only be called on
the main thread." is true for all platforms, not just iOS, so there is
no reason to call it out for iOS specifically.

[Windows] Avoid GetModuleHandle(NULL) (rust-windowing#2301)

Use get_instance_handle() over GetModuleHandle(NULL)

On Windows, fix reported cursor position. (rust-windowing#2311)

When clicking and moving the cursor out of the window negative coordinates were not handled correctly.

Revert "On Wayland, fix resize not propagating properly"

This reverts commit 78e5a39.

It was discovered that in some cases mesa will lock the back
buffer, e.g. when making context current, leading to resize
missing. Given that applications can restructure their rendering
to account for that, and that winit isn't limited to playing
nice with mesa reverting the original commit.

Set `WindowBuilder` to must_use

Add X11 opt-in function for device events

Previously on X11, by default all global events were broadcasted to
every winit application. This unnecessarily drains battery due to
excessive CPU usage when moving the mouse.

To resolve this, device events are now ignored by default and users must
manually opt into it using
`EventLoopWindowTarget::set_filter_device_events`.

Fixes (rust-windowing#1634) on Linux.

Prevent null dereference on X11 with bad locale

Remove old dialog fix that is superseded by rust-windowing#2027 (rust-windowing#2292)

This fixes the run_return loop never returning on macos when using multiple windows

Migrate from lazy_static to once_cell

macOS: Emit LoopDestroyed on CMD+Q (rust-windowing#2073)

override applicationWillTerminate:

On Android, use `HasRawWindowHandle` directly from the `ndk` crate (rust-windowing#2318)

The `ndk` crate now implements [`HasRawWindowHandle` directly on
`NativeWindow`], relieving the burden to reimplement it on `winit`.

[`HasRawWindowHandle` directly on `NativeWindow`]: rust-mobile/ndk#274

Run clippy on CI

Fixes rust-windowing#1402.

Make `set_device_event_filter` non-mut

Commit f10a984 added `EventLoopWindowTarget::set_device_event_filter`
with for a mutable reference, however most winit APIs work with
immutable references, so altering API to play nicely with existing APIs.

This also disables device event filtering on debug example.

Make `WindowAttributes` private (rust-windowing#2134)

* Make `WindowAttributes` private, and move its documentation

* Reorder WindowAttributes title and fullscreen to match method order

Build docs on `docs.rs` for iOS and Android as well (rust-windowing#2324)

Remove core-video-sys dependency (rust-windowing#2326)

Hasn't been updated in over 2 years - many open PRs, seems abandoned. Is the cause of several duplicate dependencies in our dependency tree!

Fix macOS 32bit (rust-windowing#2327)

Documentation cleanup (rust-windowing#2328)

* Remove redundant documentation links

* Add note to README about windows not showing up on Wayland

* Fix documentation links

* Small documentation fixes

* Add note about doing stuff after StartCause::Init on macOS

Add `WindowBuilder::transparent`

This is required to help hardware accelerated libraries like glutin
that accept WindowBuilder instead of RawWindowHandle, since the api
to access builder properties directly was removed.

Follow up to 44288f6.

Refine `Window::set_cursor_grab` API

This commit renames `Window::set_cursor_grab` to
`Window::set_cursor_grab_mode`. The new API now accepts enumeration
to control the way cursor grab is performed. The value could be: `lock`,
`confine`, or `none`.

This commit also implements `Window::set_cursor_position` for Wayland,
since it's tied to locked cursor.

Implements API from rust-windowing#1677.

examples/window_run_return: Enable on Android (rust-windowing#2321)

Android also supports `EventLoopExtRunReturn`.  The user will still have
to follow the README to turn this example into a `cdylib` and add the
`ndk_glue::main()` initialization attribute, though.

Fix doubled device events on X11

Fixes rust-windowing#2332

macOS: disallow_highdpi will set explicity the value to avoid the SO value by default (rust-windowing#2339)

ci: Disallow warnings in rustdoc and test private items (rust-windowing#2341)

Make sure `cargo doc` runs cleanly without any warnings in the CI - some
recently introduced but still allowing a PR to get merged.

In case someone wishes to add docs on private items, make sure those
adhere to the same standards.

Bump smithay-client-toolkit to v0.16.0

Disallow multiple EventLoop creation

Fix conflict in `WindowFlags` on Windows

Map XK_Caps_Lock to VirtualKeyCode::Capital (rust-windowing#1864)

This allows applications to handle events for the caps lock key under X11

Less redundancy and improve fullscreen in examples

Remove examples/minimize which is redundant

Implement From<u64> for WindowId and vise-versa

This should help downstream applications to expose WindowId to the end
users via e.g. IPC to control particular windows in multi window
systems.

examples/multiwindow.rs: ignore synthetic key press events

Fix infinite recursion in `WindowId` conversion methods

Add 'WindowEvent::Occluded(bool)'

This commits and an event to track window occlusion state,
which could help optimize rendering downstream.

Add `refresh_rate_millihertz` for `MonitorHandle`

This also alters `VideoMode::refresh_rate` to
`VideoMode::refresh_rate_millihertz` which now returns monitor refresh rate in
mHz.

On Wayland send Focused(false) for new window

On Wayland winit will always get an explicit focused event from the
system and will transfer it downstream. So send focused false to enforce
it.

On Wayland, drop wl_surface on window close

web: Manually emit focused event on mouse click (rust-windowing#2202)

* Manually emit focused event on mouse click

* Update CHANGELOG.md

Co-authored-by: Markus Røyset <[email protected]>

web: Add `EventLoop::spawn` (rust-windowing#2208)

* web: Add `EventLoop::spawn`

This is the same as `EventLoop::run`, but doesn't throw an exception in order to return `!`.

I decided to name it `spawn` rather than `run_web` because I think that's more descriptive, but I'm happy to change it to `run_web`.

Resolves rust-windowing#1714

* Update src/platform/web.rs

* Fix outdated names

Co-authored-by: Markus Røyset <[email protected]>

Fix changelog entry for `EventLoopExtWebSys` (rust-windowing#2372)

android: Hold `NativeWindow` lock until after notifying the user with `Event::Suspended` (rust-windowing#2307)

This applies rust-mobile/ndk#117
on the `winit` side: Android destroys its window/surface as soon as the
user returns from [`onNativeWindowDestroyed`], and we "fixed" this on
the `ndk-glue` side by sending the `WindowDestroyed` event before
locking the window and removing it: this lock has to wait for any user
of `ndk-glue` - ie. `winit` - to give up its readlock on the window,
which is what we utilize here to give users of `winit` "time" to destroy
any resource created on top of a `RawWindowHandle`.

since we can't pass the user a `RawWindowHandle` through the
`HasRawWindowHandle` trait we have to document this case explicitly and
keep the lock alive on the `winit` side instead.

[`onNativeWindowDestroyed`]: https://developer.android.com/ndk/reference/struct/a-native-activity-callbacks#onnativewindowdestroyed

web: add `with_prevent_default`, `with_focusable` (rust-windowing#2365)

* web: add `with_prevent_default`, `with_focusable`

`with_prevent_default` controls whether `event.preventDefault` is called

`with_focusable` controls whether `tabindex` is added

Fixes rust-windowing#1768

* Remove extra space from CHANGELOG

windows: Use correct value for mouse wheel delta (rust-windowing#2374)

Make winit focus take activity into account on Windows (rust-windowing#2159)

winit's notion of "focus" is very simple; you're either focused or not.
However, Windows has both notions of focused window and active window
and paying attention only to WM_SETFOCUS/WM_KILLFOCUS can cause a window
to believe the user is interacting with it when they're not. (this
manifests when a user switches to another application between when a
winit application starts and it creates its first window)

Fix typos (rust-windowing#2375)

Bump sctk-adwaita to 0.4.1

This should force the use of system libraries for Fontconfig
and freetype instead of building them with cmake if missing.

This also fixes compilation failures on nightly.

Fixes rust-windowing#2373.

Tidy up "platform-specifc" doc sections (rust-windowing#2356)

* Tidy up "platform-specific" doc sections

* Unrelated grammatical fix

* Subjective improvements

Android: avoid deadlocks while handling UserEvent (rust-windowing#2343)

Replace `Arc<Mutex<VecDeque<T>>` by `mpsc`

Update raw-window-handle to v0.5.0

This updates raw-window-handle to v0.5.0.

On macOS, fix confirmed character inserted

When confirming input in e.g. Korean IME or using characters like
`+` winit was sending those twice, once via `Ime::Commit` and the
other one via `ReceivedCharacter`, since those events weren't generating
any `Ime::Preedit` and were forwarded due to `do_command_by_selector`.

Add method to hook xlib error handler

This should help glutin to handle errors coming from GLX
and offer multithreading support in a safe way.

Fixes rust-windowing#2378.

Windows: apply skip taskbar state when taskbar is restarted (rust-windowing#2380)

Fix hiding a maximized window On Windows (rust-windowing#2336)

Bump `ndk` and `ndk-glue` dependencies to stable `0.7.0` release (rust-windowing#2392)

Fix type hint reference for xlib hook

Consistently deliver a Resumed event on all platforms

To be more consistent with mobile platforms this updates the Windows,
macOS, Wayland, X11 and Web backends to all emit a Resumed event
immediately after the initial `NewEvents(StartCause::Init)` event.

The documentation for Suspended and Resumed has also been updated
to provide general recommendations for how to handle Suspended and
Resumed events in portable applications as well as providing
Android and iOS specific details.

This consistency makes it possible to write applications that lazily
initialize their graphics state when the application resumes without
any platform-specific knowledge. Previously, applications that wanted
to run on Android and other systems would have to maintain two,
mutually-exclusive, initialization paths.

Note: This patch does nothing to guarantee that Suspended events will
be delivered. It's still reasonable to say that most OSs without a
formal lifecycle for applications will simply never "suspend" your
application. There are currently no known portability issues caused
by not delivering `Suspended` events consistently and technically
it's not possible to guarantee the delivery of `Suspended` events if
the OS doesn't define an application lifecycle. (app can always be
terminated without any kind of clean up notification on most
non-mobile OSs)

Fixes rust-windowing#2185.

ci: manually point ANDROID_NDK_ROOT to latest supplied version

It seems the symlink to `ndk-bundle` and this environment variable
pointing to it have been removed to prevent the sdkmanager from failing,
when finding the SDK setup to be in an "indeterminate" state.  It is now
up to the users themselves to install an NDK through that tool or point
the right variables to a preinstalled "latest" NDK.

actions/runner-images#2689
actions/runner-images#5926

Fix changelog entry wrt scrolling

The breaking change was put into the wrong release section.

Release 0.27.0 version

Explicitly specify minimum supported rust version

This should help with distributing apps using winit.

Fixes rust-windowing#1075.

On X11, fix crash when can't disable IME

Fixes rust-windowing#2402.

Release 0.27.1 version

Windows: respect min/max sizes when creating the window (rust-windowing#2393)

On X11, fix window hints not persisting

This commit fixes the issue with min, max, and resize increments
not persisting across the dpi changes.

Fix tracking of phase changes for mousewheel on trackpad (rust-windowing#2158)

On Windows, add opt-in function for device events (rust-windowing#2409)

Add CODEOWNERS file (rust-windowing#2420)

* Add CODEOWNERS file

This makes it very clear when you're stepping down from the post as a maintainer, and makes it clear for users who is expected to review their PR

* Fix grammar

* Make @kchibisov receive pings for the X11 platform

* Fix typo

Implement version 0.4 of the HasRawWindowHandle trait

This makes Winit 0.27 compatible with crates like Wgpu 0.13 that are
using the raw_window_handle v0.4 crate and aren't able to upgrade to 0.5
until they do a new release (since it requires a semver change).

The change is intended to be self-contained (instead of pushing
the details into all the platform_impl backends) since this is only
intended to be a temporary trait implementation for backwards
compatibility that will likely be removed before the next Winit release.

Fixes rust-windowing#2415.

Fix missleading breaking change on Windows

The applications should not rely on not-implemented behavior and
should use the right functions for that.

Remove redundant steps from CI

Tests are already building the entire crate, so no need for a
separate builds slowing down the CI.

On Wayland, fix `Window::request_redraw` being delayed

On Waylnad when asking for redraw before `MainEventsCleared`
would result for redraw being send on the next event loop tick,
which is not expectable given that it must be delivered on the same
event loop tick.

Release 0.27.2 version

On Windows, improve support for undecorated windows (rust-windowing#2419)

Add touchpad magnify and rotate gestures support for macOS (rust-windowing#2157)

* Add touchpad magnify support for macOS

* Add touchpad rotate support for macOS

* Add macOS rotate and magnify gesture cancelled phases

* Correct docs for TouchpadRotate event

* Fix tracing macros

Document `WindowEvent::Moved` as unsupported on Wayland

Update `sctk-adwaita` to use `ab_glyph`

The crossfont will still be available under the option.

Mark new events as breaking change

Adding a new enum variant is a breaking change in winit.

Co-Authored-By: kas <[email protected]>
Co-Authored-By: Artur Kovacs <[email protected]>
Co-Authored-By: Markus Siglreithmaier <[email protected]>
Co-Authored-By: Murarth <[email protected]>
Co-Authored-By: Yusuke Kominami <[email protected]>
Co-Authored-By: moko256 <[email protected]>
Co-Authored-By: Mads Marquart <[email protected]>
Co-Authored-By: Markus Røyset <[email protected]>
Co-Authored-By: Marijn Suijten <[email protected]>
Co-Authored-By: Kirill Chibisov <[email protected]>
@ajtribick
Copy link
Contributor

Looks like the MSRV imposed by dependencies is now 1.59, due to the MSRV of time (imported via simple_logger) changing in version 0.3.14 (i.e. they consider it a non-breaking change).

@Lokathor
Copy link
Author

a dev-dependency "mostly doesn't count". You can still use the lib.

@rib
Copy link
Contributor

rib commented Aug 31, 2022

Out of curiosity was there a specific reason for picking 1.57.0 as the MSRV? At the outset of this issue @goddessfreya had suggested supporting the last three stable releases which would be 1.63, 1.62 and 1.61 - is there a particular need to go back further than that?

Since I was looking at updating the Android backend to use the android-activity crate I've just had to remove any use of formatted strings like info!("This is the value {foo}") where foo is a variable in the scope of that line - since that wasn't introduced until 1.58. Personally I've come to use that formatting sugar a lot and was a bit disappointed to have to revert to passing explicit arguments.

Another related issue I hit with trying to update the CI for android-activity to also ensure it builds with 1.57.0 was realizing that cargo ndk doesn't build with 1.57. So this means I now also need to upstream patches for cargo ndk to build 1.57 and wait for a corresponding release.

Of course there will always be something you miss out on with any MSRV policy but I figured I'd check in case it was fairly arbitrary and maybe the MSRV can be moved forwards to 1.58.0?

@Lokathor
Copy link
Author

This came up in the libc MSRV discussion, and the short story is that six months to a year of lag time is needed for most of the moderate paced Linux distros (eg: Fedora, not Debian). We don't need to get there instantly, but we need to try and get there over time if we want people's winit based programs to be accepted into linux repositories without the Linux distros shipping some ancient version of the program and all it's deps.

It's not always easy, but if it's a small thing like formatting args and not an absolute requirement, then we should make an effort to slow down our msrv bumps when we can.

@chrisduerr
Copy link
Contributor

Yeah I think a good rule of thumb is usually to look at what Ubuntu stable is shipping. If your application can't be built on Ubuntu because its Rust version is too old, you'll probably miss out on a big portion of Linux users right there. So I'd always recommend trying to at least cover Ubuntu stable, if not Ubuntu LTS.

@rib
Copy link
Contributor

rib commented Aug 31, 2022

Thanks for the extra context. right, I had feeling it might be related to Linux distro release cycles.

I wonder if the MSRV could somehow just be enforced for Linux since I'm not sure the same pressing need to support older compilers exists for other platforms?

The case I've hit, with needing to now update cargo ndk feels somewhat awkward since I'm now looking to upstream a patch that I'm essentially going to be justifying in terms of wanting to support Winit's requirements - that mainly come from Linux distros, which should essentially be irrelevant for cargo ndk.

The patch for cargo ndk is trivial, and hopefully it will be possible to upstream and get a new release but the previous code was notably neater.

@Lokathor
Copy link
Author

It's probably reasonable if more niche targets like Android an iOS, which have their own special workflow, don't hold to the same standard.

You could open a new issue for that idea, because having a discussion in a closed issue is a good way to not have many eyes see it.

@madsmtm
Copy link
Member

madsmtm commented Aug 31, 2022

Relatedly, can someone help me to somewhere that I can check which version of rustc "Ubuntu Stable" ships with?

EDIT: I found https://packages.ubuntu.com/kinetic/rust, but unsure if that's the correct to be looking?

@Lokathor
Copy link
Author

Lokathor commented Sep 1, 2022

https://packages.ubuntu.com/jammy/rustc is one link, and if you click along the top like "bionic" or "kenetic" you can see what each LTS version uses. Looks like 1.59.

Fedora goes back as far as 1.55: https://packages.fedoraproject.org/pkgs/rust/rust/, but then also jumps to 1.59

So probably, "soon enough" we could bring winit up to a 1.59 baseline

Which... double checks is about the 6 month window that was proposed in the libc discussion.

@chrisduerr
Copy link
Contributor

Yeah generally sticking to the Ubuntu rule of thumb works out pretty well and covers Fedora too. And honestly I really don't think it's too much to ask to support compilers that aren't even half a year old yet. In most scenarios the changes are just small stylistic stuff, which aren't worth breaking compatibility for.

I think the bigger issue is usually just with dependencies, considering a lot of people simple don't care. But I'd argue that's just more reason to care and encourage others to do the same.

@rib
Copy link
Contributor

rib commented Sep 1, 2022

6 months as a threshold sounds a bit more reasonable, though (just in my personal opinion) I suppose even that seems overly conservative for Winit (especially for non-Linux platforms). 6 months would suggest setting the initial minimum version to 1.60 I think (guessing the next Winit release will likely be > 1 month from now. This initial threshold of 1.57 will be closer to one year (10 months now, likely 11+ months by the time of the next release).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - needs discussion Direction must be ironed out S - meta Project governance
Development

Successfully merging a pull request may close this issue.

9 participants