Skip to content

Bump parking_lot to 0.10 #332

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.

Sign up for GitHub

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

Merged
NikVolf merged 7 commits into from
Feb 6, 2020
Merged

Bump parking_lot to 0.10 #332

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
287ea37
bump versions
NikVolf Feb 5, 2020
89448b6
update changelog
NikVolf Feb 5, 2020
564ead9
change bump to 0.5.0
NikVolf Feb 5, 2020
ed74d1f
update deps
NikVolf Feb 5, 2020
29edac1
major bump
NikVolf Feb 6, 2020
45a4a12
also kvdb-web
NikVolf Feb 6, 2020
3078dc3
also shared tests to 0.2
NikVolf Feb 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions kvdb-memorydb/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog].

## [Unreleased]

## [0.3.2] - 2019-02-05
- Bump parking_lot to 0.10. [#332](https://github.com/paritytech/parity-common/pull/332)

## [0.3.1] - 2019-01-06
- Updated features and feature dependencies. [#307](https://github.com/paritytech/parity-common/pull/307)

Expand Down
6 changes: 3 additions & 3 deletions kvdb-memorydb/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
[package]
name = "kvdb-memorydb"
version = "0.3.1"
version = "0.3.2"
authors = ["Parity Technologies <admin@parity.io>"]
repository = "https://github.com/paritytech/parity-common"
description = "A key-value in-memory database that implements the `KeyValueDB` trait"
license = "GPL-3.0"
edition = "2018"

[dependencies]
parity-util-mem = { path = "../parity-util-mem", version = "0.4", default-features = false, features = ["std"] }
parking_lot = "0.9.0"
parity-util-mem = { path = "../parity-util-mem", version = "0.5", default-features = false, features = ["std"] }
parking_lot = "0.10.0"
kvdb = { version = "0.3", path = "../kvdb" }

[dev-dependencies]
Expand Down
4 changes: 4 additions & 0 deletions kvdb-rocksdb/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog].

## [Unreleased]


## [0.4.3] - 2019-02-05
- Bump parking_lot to 0.10. [#332](https://github.com/paritytech/parity-common/pull/332

## [0.4.2] - 2019-02-04
### Fixes
- Fixed `iter_from_prefix` being slow. [#326](https://github.com/paritytech/parity-common/pull/326)
Expand Down
6 changes: 3 additions & 3 deletions kvdb-rocksdb/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "kvdb-rocksdb"
version = "0.4.2"
version = "0.4.3"
authors = ["Parity Technologies <admin@parity.io>"]
repository = "https://github.com/paritytech/parity-common"
description = "kvdb implementation backed by RocksDB"
Expand All @@ -18,11 +18,11 @@ interleaved-ordered = "0.1.1"
kvdb = { path = "../kvdb", version = "0.3" }
log = "0.4.8"
num_cpus = "1.10.1"
parking_lot = "0.9.0"
parking_lot = "0.10.0"
regex = "1.3.1"
rocksdb = { version = "0.13", features = ["snappy"], default-features = false }
owning_ref = "0.4.0"
parity-util-mem = { path = "../parity-util-mem", version = "0.4", default-features = false, features = ["std", "smallvec"] }
parity-util-mem = { path = "../parity-util-mem", version = "0.5", default-features = false, features = ["std", "smallvec"] }

[dev-dependencies]
alloc_counter = "0.0.4"
Expand Down
2 changes: 1 addition & 1 deletion kvdb-web/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ kvdb-memorydb = { version = "0.3", path = "../kvdb-memorydb" }
futures = "0.3"
log = "0.4.8"
send_wrapper = "0.3.0"
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 }

[dependencies.web-sys]
version = "0.3.31"
Expand Down
2 changes: 1 addition & 1 deletion kvdb/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ edition = "2018"
[dependencies]
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 }

@ordian ordian Feb 5, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for being semver-nazzi, but this is a breaking change for all kvdb-* libs now :)

@NikVolf NikVolf Feb 6, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@ordian ordian Feb 6, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@NikVolf NikVolf Feb 6, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@ordian ordian Feb 6, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@NikVolf NikVolf Feb 6, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

@ordian ordian Feb 6, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, KeyValueDBv0.3.1 inherits from parity_util_mem::MallocSizeOfv0.4.0

parity-common/kvdb/src/lib.rs

Line 112 in 996d370

pub trait KeyValueDB: Sync + Send + parity_util_mem::MallocSizeOf {

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 :)

@ordian ordian Feb 6, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@NikVolf NikVolf Feb 6, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

3 changes: 3 additions & 0 deletions parity-util-mem/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog].

## [Unreleased]

## [0.5.0] - 2019-02-05
- Bump parking_lot to 0.10. [#332](https://github.com/paritytech/parity-common/pull/332

## [0.4.2] - 2020-02-04
- Implementation of `MallocSizeOf` for `BTreeSet`. [#325](https://github.com/paritytech/parity-common/pull/325)
- Split off implementation of `MallocSizeOf` for `primitive-types`. [#323](https://github.com/paritytech/parity-common/pull/323)
Expand Down
4 changes: 2 additions & 2 deletions parity-util-mem/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "parity-util-mem"
version = "0.4.2"
version = "0.5.0"
authors = ["Parity Technologies <admin@parity.io>"]
repository = "https://github.com/paritytech/parity-common"
description = "Collection of memory related utilities"
Expand All @@ -25,7 +25,7 @@ impl-trait-for-tuples = "0.1.3"

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 }

@ordian ordian Feb 5, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that a breaking change for parity-util-mem?

@NikVolf NikVolf Feb 5, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like no.

@ordian ordian Feb 5, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

@NikVolf NikVolf Feb 5, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm technically yes, but it will require major bump just for everything
ok to me though

@ordian ordian Feb 5, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

primitive-types = { version = "0.6", path = "../primitive-types", default-features = false, optional = true }

[target.'cfg(target_os = "windows")'.dependencies]
Expand Down