Closed ecton closed 2 years ago
README needs to be updated to reflect 1.59, it still says 1.58 is the minimum rust.
README needs to be updated to reflect 1.59, it still says 1.58 is the minimum rust.
This isn't implemented yet, and the minimum version hasn't officially been updated yet. The currently released version builds on 1.58 (just tested), and the current branch builds with 1.58 unless you enable the test-util
feature flag.
Ah, ok. Running the test suite failed on 1.58 because of available_parallelism
. I guess I misunderstood the README. Is there a documented minimum version for developing on bonsaidb (including running tests)?
Honestly, because of the lack of CI testing the current MSRV, I hadn't noticed it yet. Thank you for bringing it to my attention. Testing the MSRV is something I have on my list to check before doing a release, but it's also nice to know about the issue ahead of time.
In the process of working on adding the blocking unit test suite, I reached for that out of "not wanting to add another dependency right now", and it just slipped my mind ever since. I most likely will be changing it to num_cpus since its already a dependency in upstream crates, which will revert the requirement change.
I'll try to get MSRV CI hooked up soon. Thank you for checking out BonsaiDb, and apologies for the friction when checking it out!
I've pushed the change to revert the MSRV back to 1.58.
After further investigation, I've decided to not use available_parallelism. num_cpus
supports some features that available_parallelism doesn't support, such as cgroups.
Via @daxpedda in Discord, a new api has been stabilized in Rust 1.59 that allows querying the "available parallelism":
https://doc.rust-lang.org/stable/std/thread/fn.available_parallelism.html
We could replace the sysinfo dependency with this new API for querying the CPU count. We have to update the MSRV to use this.