moka-rs / moka

A high performance concurrent caching library for Rust
Apache License 2.0
1.51k stars 68 forks source link

Locking `triomphe` is the wrong approach to preserving MSRV #440

Open aumetra opened 1 month ago

aumetra commented 1 month ago

The recent release locked the triomphe crate version to >=0.1.3, <0.1.12 which kinda works(?) but isn't a good solution. IMO it's fine if, by default, the MSRV breaks if there is a simple way of getting the MSRV to compile (in this case, by adding =0.1.11 to the manifest of projects that absolutely need the old Rust version).

There have been prior examples where a version lock like this has broken compilation for many downstream users, such as with the *-dalek crates, where zeroize used to be locked (1, 2, 3, 4).

MSRV should, in my opinion at least, not block new versions if there is a simple fix to getting that Rust version running.

tatsuya6502 commented 1 month ago

Thank you for your comment. We know it is not an ideal approach to lock dependency in older versions , but we had no other options.

Locking dependency in older versions may become a problem in the future, but no downstream user has reported yet. Meanwhile, some downstream users already reported problems with a newer version of the same dependency.

Anyway. To prevent the potential future problem, I will remove triomphe from moka's dependency. moka uses a small portion of triomphe code base, which seems very stable. And triomphe is a fork of std::sync::Arc. I will copy necessary part of triomphe or std::sync::Arc code into moka, and modify it to meet our needs. (Their OSS licenses allow it)

aumetra commented 1 week ago

Is there any reason why those downstream users can't pin triomphe themselves? I don't see why they would need moka to pin versions for them (and potentially breaking downstream uses of moka because of this requirement) if they can just fix it themselves.

It also seems like this approach of making users pin crate versions themselves isn't foreign to moka crates, looking at the .ci_extras directory over on the mini-moka repo (https://github.com/moka-rs/mini-moka/blob/main/.ci_extras/pin-crate-vers-msrv.sh)

tatsuya6502 commented 1 week ago

Is there any reason why those downstream users can't pin triomphe themselves?

No. Anybody can pin trionmphe version in their Cargo.lock by using cargo update -p PACKAGE --precise VERSION. However, Cargo ignores Cargo.lock provided by dependencies, so if they distribute library crates, pinning in Cargo.lock has no effect for their downstream users.

I don't see why they would need moka to pin versions for them

The main purpose to pin triomphe in moka is to keep our MSRV, not to keep their MSRVs. We needed it for ourselves in the first place.

Our policy ensures moka to compile with Rust versions at least 6 months old. triomphe@v0.1.12's MSRV is Rust 1.76.0 (2024-02-08), and at the time v0.1.12 was released, Rust 1.76.0 was only 3 moths old. If moka depended on triomphe@v0.1.12, it meant its MSRV was broken.

As for dashmap in mini-moka, I did not pin it in Cargo.toml but pinned it in Cargo.lock only for our CI. This is because the MSRV of the latest dashmap (5.x) did not conflict with mini-moka's 6-month MSRV policy. dashmap@v5.5.3's MSRV is Rust 1.65.0 2022-11-03. So I was thinking if mini-moka user reports mini-moka's MSRV Rust 1.61.0 2022-05-19 is broken, I would simply raise the MSRV to Rust 1.65.0.

tatsuya6502 commented 1 week ago

As I said before, I agree with you that locking triomphe in Cargo.toml is a wrong approach.

We know it is not an ideal approach to lock dependency in older versions , but we had no other options.

Now it is August 2024 and Rust 1.76.0 is 6 months old. We now have following options:

  1. Raise moka's MSRV from 1.65.0 to 1.76.0 and unpin triomphe version in Cargo.toml.
  2. Remove triomphe from moka's dependency. Take necessary codes from triomphe into moka.

I will take 2. because moka uses only a small portion of triomphe code base, which seems very stable.

After writing my first comment on July 13th, 2024, I did some experiments to take necessary triomphe code into moka. I was able to solve compile errors, but I was thinking to clean the code up further. Then I had no time to make any progress. I am hoping I will have some time in mid September to finish it.

moka-triomphe

aumetra commented 1 week ago

Ah, don't worry. I didn't mean to annoy you, but if you need help with integrating the vendored code, I'd be happy to help out.