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

ureq 2.x: Make MSRV 1.63 work (by bumping to MSRV 1.67) #878

Merged
merged 6 commits into from
Nov 27, 2024
Merged

Conversation

algesten
Copy link
Owner

@algesten algesten commented Nov 14, 2024

Close #877

@algesten algesten changed the base branch from main to 2.x November 14, 2024 11:47
@algesten algesten changed the title Make MSRV 1.63 work ureq 2.x: Make MSRV 1.63 work Nov 23, 2024
@algesten
Copy link
Owner Author

I found a combination of versions that work to compile 1.63 locally. However the CI now fails with:

error[E0282]: type annotations needed for `Box<_>`
Error:   --> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/time-0.3.20/src/format_description/parse/mod.rs:83:9
   |
83 |     let items = format_items
   |         ^^^^^
...
86 |     Ok(items.into())
   |              ---- type must be known at this point
   |
   = note: this is an inference error on crate `time` caused by an API change in Rust 1.80.0; update `time` to version `>=0.3.35` by calling `cargo update`

@algesten
Copy link
Owner Author

I think that means we might be stuck in a situation where ureq 2.x either must bump MSRV to whatever time requires at 0.3.35, or not be able to compile with rustc >= 1.80

@algesten algesten changed the title ureq 2.x: Make MSRV 1.63 work ureq 2.x: Make MSRV 1.63 work (by bumping to MSRV 1.67) Nov 27, 2024
@algesten algesten marked this pull request as ready for review November 27, 2024 08:44
@algesten
Copy link
Owner Author

Alright. This fixes MSRV... Not in a satisfactory way, but in a way that is releasable.

The RUSTSEC-2024-0399 error in the CI I'll deal with separately.

@algesten algesten merged commit 83afaff into 2.x Nov 27, 2024
76 of 100 checks passed
@algesten algesten deleted the fix/2.x-msrv branch November 27, 2024 08:45
@epage
Copy link

epage commented Dec 2, 2024

FYI the Cargo team discourages using non ^ version requirements except in some very specific cases. Enforcing of MSRV is not one of those and is in fact the motivating reason for us writing that documentation.

There are two problems with enforcing MSRV this way

  • This makes ureq incompatible with other packages that might require newer versions of these packages.
  • Not all users of ureq have the same MSRV and may be able to use newer versions of these packages and want to use them

From Cargo's documentation for rust-version

In support of the above, it is expected that each dependency’s version-requirement supports at least one version compatible with your rust-version. However, it is not expected that the dependency specification excludes versions incompatible with your rust-version. In fact, supporting both allows you to balance the needs of users that support older Rust versions with those that don’t.

The way I see people handle this include

  • Committing their lockfile and tracking MSRV-compatible versions with it (yes, its a pain)
  • Using -Zminimal-versions to approximate an MSRV-aware resolver

In 1.84, Cargo will include an MSRV-aware resolver, see https://doc.rust-lang.org/nightly/cargo/reference/config.html#resolverincompatible-rust-versions

Use of the MSRV-aware resolver does not require bumping your MSRV; you just need to change your Cargo.lock with 1.84+ and have it enabled. You can even have it enabled in a .cargo/config.toml and Cargo will ignore it in older versions.

@algesten
Copy link
Owner Author

algesten commented Dec 2, 2024

Hi @epage, thanks for the feedback!

Since I'm close to releasing ureq 3.x (RC3 is out), my hope is to be able to abandon the 2.x version soon anyway (apart from security and potentially more MSRV fixes). This pinned dep is a band aid until such time.

For 2.x I think I'm stuck between a rock and a hard place.

The MSRV table you provided here tells us that if I unlock the rustls dep, a user on anything below <1.71 will fail to compile with less than editing the lock file. That means ~25% of potential users of ureq could fail to use ureq out-of-the box. Did I get that right?

One of ureq's primary goals have always been to work out-of-the-box especially for beginners to Rust. A potential 25% failure rate is not good.

Or am I misunderstanding it?

@kamulos
Copy link

kamulos commented Dec 2, 2024

There is also another angle to this matter: I think in the case of rustls, pinning is especially bad. If a CVE occurs in rustls people will be unable to update without an update to ureq. In the worst case this is really time sensitive.

@epage
Copy link

epage commented Dec 2, 2024

The MSRV table you provided here tells us that if I unlock the rustls dep, a user on anything below <1.71 will fail to compile with less than editing the lock file. That means ~25% of potential users of ureq could fail to use ureq out-of-the box. Did I get that right?

That table was published on 11/10/2023. Whats more important is to look at the relative value. 1.71 was released on 2023-07-13 which is almost 1.5 years ago. For a release that was a year old at the time the table was made, 98.766% of requests to crates.io would be satisfied.

One of ureq's primary goals have always been to work out-of-the-box especially for beginners to Rust.

In considering the out-of-the-box experience for beginniners, also consider the esoteric errors you put your users at risk of seeing, e.g. https://www.reddit.com/r/rust/comments/p8clcx/how_to_fix_cargo_dependency_issue/

@algesten
Copy link
Owner Author

algesten commented Dec 3, 2024

@epage I have now released 2.12.0. This is MSRV 1.71 with dependencies unpinned.

I am going to maintain two lines:

  • 2.11.x -> MSRV 1.67
  • 2.12.x -> MSRV 1.71

I will pin dependencies in those lines as needed by releasing patch versions. If I must bump MSRV again, it will be in another minor version.

2.x will not get any more features. So I will only release patch to keep it alive (upon request).

@algesten algesten mentioned this pull request Dec 3, 2024
@epage
Copy link

epage commented Dec 3, 2024

@algesten is there a reason you are still pinning versions in your Cargo.toml file?

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

Successfully merging this pull request may close these issues.

3 participants