search

mozilla / rkv

A simple, humane, typed key-value storage solution.
https://crates.io/crates/rkv
Apache License 2.0
304 stars 52 forks source link

Replace failure with thiserror #210

Closed upsuper closed 3 years ago

upsuper commented 3 years ago

failure has been deprecated in favor of thiserror. This PR replaces it.

victorporof commented 3 years ago

I wonder if this conflicts with usage in m-c at all.

upsuper commented 3 years ago

I'm going to update m-c as well.

victorporof commented 3 years ago

@upsuper Do you have a link to a bugzilla bug?

upsuper commented 3 years ago

Nope. I'll create one when I have the patch :)

I don't think this would affect downstream crates, as Fail is implemented for all Error types. The only meaningful difference is that backtrace support is dropped, which m-c doesn't use anyway.

upsuper commented 3 years ago

Meta bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1681898

upsuper commented 3 years ago

Oh actually there is a security advisory about the deprecation of failure: https://rustsec.org/advisories/RUSTSEC-2020-0036.html

upsuper commented 3 years ago

Based on experience on agashlin/comedy-rs#13, this would require a minor version bump, because failure without std feature doesn't really have impl Fail for error.

upsuper commented 3 years ago

Hey @ncloudioj , what do you think about this?

After removing failure from bits_client, dependents of rkv and crates of lucet sandbox compilers are the last crates which are still using failure. I've contacted maintainer of lucet sandbox compilers, and from January next year, they are going to work on catching up with their upstream which has dropped failure dependencies for long. By then dependents of rkv would be the only reason we can't remove dependency of failure in m-c.

If you can merge this and release a new version, I can raise patch to update m-c to vendor the new version and fix up things.

upsuper commented 3 years ago

Ping for this.

ncloudioj commented 3 years ago

@upsuper My apologies for the late response.

Looking into your patch now.

upsuper commented 3 years ago

Thanks! I'll look into this over this weekend.

victorporof commented 3 years ago

Done.