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

memchr v2.6.0 broke 1.20/1.25 tokio LTS MSRVs #5957

Closed
TheBlueMatt opened this issue Aug 28, 2023 · 5 comments
Closed

memchr v2.6.0 broke 1.20/1.25 tokio LTS MSRVs #5957

TheBlueMatt opened this issue Aug 28, 2023 · 5 comments
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug.

Comments

@TheBlueMatt
Copy link

The memchr project's MSRV guidelines mention that MSRVs are only supported across patch versions, not minor versions. Tokio 1.27 switched to libc memchr (in #5558), dropping the dependency, but the 1.20/1.25 LTS releases depend on memchr via simply memchr = "2.2". Thus, the MSRV is liable to be broken on any minor release of memchr, which finally happened today with the memchr 2.6.0 release. I understand this may not be supported and pinning is pretty easy on the building end, but this isn't architecture-specific, and impacts all users of the io-util feature and would be pretty easy to pin the dependency directly in the LTS releases.

@TheBlueMatt TheBlueMatt added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels Aug 28, 2023
@Darksonn
Copy link
Contributor

Pinning has negative impacts on dependency resolution. Switching to libc memchr seems better to me. Thoughts?

@hawkw
Copy link
Member

hawkw commented Aug 28, 2023

Users who need to compile on Tokio's MSRV may also wish to compile with -Z minimal-versions to avoid potential MSRV breakage from transitive dependencies in the future.

I agree with @Darksonn that pinning in Tokio itself is not great, and that backporting the change to depend on libc's memchr to the LTS releases is probably a better approach.

@carllerche
Copy link
Member

I'm a +1 w/ @darkson as well

@sunshowers
Copy link
Contributor

sunshowers commented Aug 29, 2023

Agreed, pinning breaks cargo in some unfortunate ways. Downstream projects with an old MSRV should check in a Cargo.lock that works with that MSRV (e.g. for one of my projects: https://github.com/sunshowers-code/debug-ignore/blob/47a911cefdba591b75fba4a3c8e208558d11ebfd/Cargo.lock.rust134)

@TheBlueMatt
Copy link
Author

Makes sense to me - the change to drop the dependency and use libc was pretty trivial, backporting it should be pretty low-risk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

5 participants