Bump parking_lot to 0.10#332
Conversation
|
Took me a while to realise that it is different versions of |
| smallvec = { version = "1.0.0", optional = true } | ||
| ethereum-types = { version = "0.8.0", optional = true, path = "../ethereum-types" } | ||
| parking_lot = { version = "0.9.0", optional = true } | ||
| parking_lot = { version = "0.10.0", optional = true } |
There was a problem hiding this comment.
Isn't that a breaking change for parity-util-mem?
There was a problem hiding this comment.
But that's public API. parity-util-mem::MallocSizeOf v0.4.2 is implemented for parking_lot v0.9.0, but parity-util-mem::MallocSizeOf v0.4.3 is not (as 0.10.0 is a breaking change to 0.9.0)
There was a problem hiding this comment.
hmm technically yes, but it will require major bump just for everything
ok to me though
There was a problem hiding this comment.
yeah, that's version bump creep is unfortunate, but we already have to do this e.g. for rand and other libs, so it's better to safe than sorry
| smallvec = "1.0.0" | ||
| bytes = { package = "parity-bytes", version = "0.1", path = "../parity-bytes" } | ||
| parity-util-mem = { path = "../parity-util-mem", version = "0.4", default-features = false } | ||
| parity-util-mem = { path = "../parity-util-mem", version = "0.5", default-features = false } |
There was a problem hiding this comment.
sorry for being semver-nazzi, but this is a breaking change for all kvdb-* libs now :)
There was a problem hiding this comment.
Now I think not.
How do you think it can break?
in previous example it could break because of implementation for parking-lot:0.9.0 gone missing, seems this is not the case here.
There was a problem hiding this comment.
The way to think about breaking changes is imagine you bump a version of only this crate in a lib that uses it as a dependency, will the compilation of the lib break? In this case the answer is yes, because the lib uses parity-util-mem::MallocSizeOf v0.4.0 and e.g. kvdb-rocksdb v0.4.2, but kvdb-rocksdb v0.4.3 (if you bump it that way) doesn't implement it (as it implements parity-util-mem::MallocSizeOf v0.5.0) and hence it doesn't implement kvdb 0.4.0 etc, so you will get compilation errors.
There was a problem hiding this comment.
This breakage can be avoided by requiring kvdb ^0.4.3 (which is default when requiring just 0.4.3) in kvdb-rocksdb
So if you upgrade one to latest, you are forced to upgrade another
There was a problem hiding this comment.
sometimes you can upgrade to the latest breaking change easily, but it doesn't make the change non-breaking
sometimes you can't control all of your dependencies, imagine that parity-util-mem is a transitive dependency of some lib, so the lib can't just upgrade it by bumping in its Cargo.toml;
that way the lib will end up with 2 versions of parity-util-mem 0.4.0 and 0.5.0, which are incompatible
There was a problem hiding this comment.
Why incompatible? If you use it directly, you use only one of the versions. If you don’t use it directly, you don’t care if they are duplicated :)
There was a problem hiding this comment.
Well, KeyValueDBv0.3.1 inherits from parity_util_mem::MallocSizeOfv0.4.0
Line 112 in 996d370
You don't bump the kvdb version here, but bump only kvdb-rocksdb to 0.4.3, so it no longer implements KeyValueDBv0.3.1, right? Now, if you bump kvdb to 0.3.2, then kvdb-rocksdb_v0.4.2 no longer implements that.
Also parity-util-mem has a feature of global allocator which we use in parity-ethereum, so having two versions there will probably not work.
But I feel like we've spend too much time debating on this issue already and it's easier to just bump the major versions :)
There was a problem hiding this comment.
hy incompatible? If you use it directly, you use only one of the versions. If you don’t use it directly, you don’t care if they are duplicated :)
Imagine this code
#[derive(MallocSizeOf)]
pub struct Foo {
foo: third_party_lib::implements_parity_util_mem::MallocSizeOf_v_0_4_0,
bar: parking_lot::RwLock<Baz>,
kvdb: Arc<dyn KeyValueDB>,
}this will break
There was a problem hiding this comment.
easier to just bump the major versions
Yeah, of course it is easier, I just thought if we could avoid this cascading bump spam
But seems like no way :)
|
(also kvdb-shared-tests) |
9bdc869 to
45a4a12
Compare
|
I imagine doing this for substrate crates if they go to crates.io @bkchr The whole thing just to update parking_lot 0.9 -> 0.10 |
df47594 to
3078dc3
Compare
* master: Add different mode for malloc_size_of_is_0 macro dealing with generics (#334) [parity-crypto] Use upstream secp256k1 (#258) Bump parking_lot to 0.10 and minor versions (#332) Remove libc completely (#333) update changelogs (#329) bump parity-util-mem to 0.4.2 (#328) remove libc feature from fixed-hash (#317) kvdb-rocksdb: release 0.4.2 (#327) kvdb-rocksdb: fix iter_from_prefix being slow (#326) MallocSizeOf for BTreeSet (#325) split off primitives (#323) travis: disable kvdb-web tests for chrome (#324) Expand const fn coverage (#319) uint: make zero const fn (#318) README: fix appveyor badge (#316)
No description provided.