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

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

Open
notgull opened this issue Dec 8, 2022 · 11 comments
Open

Comments

@notgull
Copy link
Member

notgull commented Dec 8, 2022

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.

  • 1.64
    • We may want to implement IntoFuture on certain structures (like async_process::Child).
    • futures-lite may want to re-export the newly stable ready!() and poll_fn items. As before, this would be a breaking change.
  • 1.65
  • 1.66
    • The std::os::fd module could be used to replace parts of std::os::unix::io in a way that helps support WASI.
  • 1.67
    • must_use can now be used successfully on async fn.
  • 1.68
    • The pin! macro could be used to reimplement the pin macro in futures-lite without unsafe code. This means futures-lite could not contain any unsafe code.
    • We can use the constant VecDeque::new to avoid using OnceCell in blocking. (Remove the dependency on async-lock blocking#59)
  • Nightly
    • Async functions in traits will likely cause the entire async ecosystem to be revamped, we should look out for potentially breaking bumps in futures-lite or async-io.
    • AsyncIterator may replace Stream in futures.
    • Custom allocators would be nice to use for several instances of allocation. See Custom allocators async-task#24
    • Once ReadBuf is supported, we can use that in piper to avoid needing to initialize/zero out parts of the pipe.
      • I consider this to be a blocker for piper v1.0.

(Feel free to add any that I missed)

@taiki-e
Copy link
Collaborator

taiki-e commented Dec 28, 2022

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
Copy link
Member Author

notgull commented Apr 12, 2023

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
Copy link

KodrAus commented Apr 13, 2023

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
Copy link
Member Author

notgull commented Apr 13, 2023

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
Copy link

sdroege commented Apr 13, 2023

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
Copy link
Member Author

notgull commented Apr 13, 2023

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
Copy link

sdroege commented Apr 13, 2023

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
Copy link
Member Author

notgull commented Apr 13, 2023

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
Copy link

sdroege commented Apr 13, 2023

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
Copy link
Member Author

notgull commented Apr 28, 2023

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
Copy link
Member Author

notgull commented Jun 10, 2023

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.

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

No branches or pull requests

4 participants