smol-rs / smol

A small and fast async runtime for Rust
Apache License 2.0
3.68k stars 155 forks source link

Tracking issue for potentially useful features in future Rust versions #244

Open notgull opened 1 year ago

notgull commented 1 year ago

As of the time of writing (updated 2024-01-14), the current MSRV for every crate in the smol-rs organization is as follows:

Repository Name MSRV
async-broadcast N/A
async-channel 1.61
async-compat N/A
async-dup 1.59
async-executor 1.60
async-fs 1.63
async-io 1.63
async-lock 1.60
async-net 1.63
async-process 1.63
async-rustls 1.61
async-signal 1.63
async-task 1.57
atomic-waker 1.36
blocking 1.61
cache-padded 1.31
concurrent-queue 1.61
easy-parallel 1.63
event-listener 1.61
event-listener-strategy 1.60
fastrand 1.36
fastrand-contrib 1.43
futures-lite 1.61
nb-connect N/A
parking 1.51
piper 1.36
polling 1.63
smol 1.63
smol-macros 1.63
vec-arena N/A
waker-fn 1.51

The lowest MSRV here is 1.36 (aside from cache-padded, which we are planning on deprecating, see smol-rs/cache-padded#7). As a whole, the policy is usually to not bump the MSRV past the current Rust version used by Debian Stable (currenly 1.63), but once Debian Stable upgrades to a new Rust version, or a bump in an important crate (such as libc) happens, we will upgrade to a newer Rust version. This issue keeps track of features that may be useful to smol, but cannot be used yet because we don't have the required Rust version.

(Feel free to add any that I missed)

taiki-e commented 1 year ago

Thanks for writing this!

AFAIK, async-compat (tokio requires 1.49), async-rustls (rustls requires 1.56), and async-broadcast (parking_lot requires 1.49) cannot support Debian stable due to their dependencies. However, they are not dependencies of smol-rs/smol, so it is okay to have their own MSRV policy.

As for nb-connect and vec-arena, they are deprecated and archived, so you may want to remove them from the list.

notgull commented 1 year ago

rust-lang/log#552 may force us to break our MSRV policy sooner than expected. Short of removing logging entirely from smol, I don't think there's a way around this one.

Edit: it looks like Tokio is moving to excise logging entirely, see tokio-rs/mio#1666. I'm not sure if we should follow this precedent.

KodrAus commented 1 year ago

Hmm does your MSRV policy apply to optional features too? Would it be reasonable to, say, put logging behind a feature gate and shim the macros when it’s not available so that your default build targets whatever ships with Debian but logging requires something newer?

notgull commented 1 year ago

Hmm does your MSRV policy apply to optional features too? Would it be reasonable to, say, put logging behind a feature gate and shim the macros when it’s not available so that your default build targets whatever ships with Debian but logging requires something newer?

I avoid adding features, especially if we'd have to either remove them or make them no-ops when our MSRV goes up in the near future.

I could be talked into a --cfg guard, but personally I'm leaning towards "just take out logging entirely" if it comes down to it.

sdroege commented 1 year ago

Why does this matter for smol at all though? MSRV only really matters for two things: 1) final executables / cdylibs depending on smol (nobody is compiling smol individually, the final build artifact is the interesting aspect), and 2) smol tests that might want to ensure that everything still compiles with the declared MSRV.

In both cases this can be achieved by a Cargo.lock that depends on an old enough version of log. It's currently not very easy to create such a Cargo.lock after the fact, but that's a cargo / tooling problem.

The only aspect that should matter for smol is that it must not use too new features from e.g. log or other crates, which a CI job can enforce.

What am I missing?

notgull commented 1 year ago

In both cases this can be achieved by a Cargo.lock that depends on an old enough version of log. It's currently not very easy to create such a Cargo.lock after the fact, but that's a cargo / tooling problem.

Having a Cargo.lock.msrv file committed would work fine from a maintenance point of view, but would be frustrating for users that depend on the MSRV since there is additional effort needed to get that build. This was considered when once-cell bumped its MSRV, but ultimately we decided against it. See here for further discussion.

On top of that, pinning log to a specific version might lead to two versions of log being in the dependency graph. This is especially bad, since log uses static variables to hold the logging implementation, and can lead to hard-to-track-down bugs. For instance, if a user registers a logger in the log=0.4.17 crate, then logs from the log=0.4.18 crate will be dropped.

(Also, neat, I didn't know we had collaborators on this repo)

sdroege commented 1 year ago

but would be frustrating for users that depend on the MSRV since there is additional effort needed to get that build

You mean the cargo / tooling aspect I mentioned, i.e. that cargo can't figure that out automatically during dependency resolution based on the rust-version field?

Personally for my crates, I'm keeping a Cargo.lock for this purpose and if that's hard to maintain for downstream users then that's something that should be fixed in cargo.

On top of that, pinning log to a specific version might lead to two versions of log being in the dependency graph. This is especially bad, since log uses static variables to hold the logging implementation, and can lead to hard-to-track-down bugs. For instance, if a user registers a logger in the log=0.4.17 crate, then logs from the log=0.4.18 crate will be dropped.

Yeah that would be the worst possible solution. As cargo considers 0.4.17 and 0.4.18 compatible, this wouldn't even compile AFAIU and would be considered conflicting dependencies.

It also removes the choice of MSRV from your downstream users. They couldn't even depend on the very latest everything if they wanted to because your crate is holding them back.

Cargo.toml is for selecting minimum/compatible versions, while Cargo.lock is for selecting a very specific set of dependency versions. Only the latter is really relevant for MSRV concerns IMHO.

notgull commented 1 year ago

I agree that Cargo could be tooled better to handle rust-version and MSRV. However, I'd still be against committing a Cargo.lock. It's ignored by dependencies (see here), so even if we have it just for CI testing we would still need to effectively raise the MSRV to 1.60.

Having different setups for a 1.48 MSRV and a 1.60 MSRV is not only a maintenance burden but complicated for users to handle, especially when they don't depend on smol directly but through other dependencies (e.g. async-io is used indirectly by eframe though about 6 layers of dependencies).

Given the options of "bump practical MSRV to 1.60 but have a theoretical MSRV that takes a non-significant amount of effort to get to" or "remove logging, which we don't use very much anyways", I'd lean towards the second option.

sdroege commented 1 year ago

It's ignored by dependencies (see here), so even if we have it just for CI testing we would still need to effectively raise the MSRV to 1.60.

That it's ignored by downstream users is a feature though. Your MSRV is not your downstream user's MSRV, but they might want to depend on latest Rust instead.

If they don't then they right now have to hand-craft a suitable Cargo.lock for themselves, or put all their energy (and anger, see other issues) into improving cargo instead of making it the problem of individual crate maintainers. The current approach of blaming upstream crate maintainers whenever they increase their MSRV because some downstream user has special constraints and needs an older Rust version is not really sustainable for the ecosystem.

For the smol case it seems reasonable to just drop log though, I agree.

notgull commented 1 year ago

https://www.phoronix.com/news/Debian-12.0-Release-Date Debian Bookworm has just been announced to be released on June 10th! It looks like it will contain Rustc 1.63, which would check off a lot of items on my wishlist here.

notgull commented 1 year ago

Rise and grind, gamers!

Debian Bookworm has now officially released as stable, and with it comes rustc v1.63.0.

Meaning, we can now bump the MSRV for all of these crates much higher now. See all of the items above for a reason why this is very exciting!

For anyone looking for a good first patch to contribute to smol-rs, most of these items are now low hanging fruit!

Over the next week or so I'll start going down the list and taking care of this.