Release new parking_lot_core?

[dekellum]

I was personally pleased to see this particular change released, as mentioned in the CHANGELOG:

0.8.1 (2019-07-03)
==================

- Removed dependency on rand crate. (#144)

Its also mentioned in final comments of #138. I saw this release was yanked per #156, but I also noticed that localized PRNG was actually added to parking_lot_core (not parking_lot) and there has been no new release attempt for parking_lot_core. This is the git hash for the last parking_lot_core-0.5.0 release:

cat ~/.cargo/registry/src/*/parking_lot_core-0.5.0/.cargo_vcs_info.json

{
  "git": {
    "sha1": "3259fd72e0d76e9c9a6f31ff41a996648fe9ccfa"
  }
}

Friendly suggestion: its a pretty standard approach for a git repo workspace of multiple crates to have crate specific tags, e.g.:

git tag core-0.5.0 3259fd72

…and then either give it its own core/CHANGELOG.md, or seperately give it its own version sections in the top-level CHANGELOG.

Here is what has changed between that release and master in the core crate:

3259fd7...master

Or git log 3259fd72.. -- core produces:

7b403d3 Use hint::unreachable_unchecked instead of Void enum
013e482 Add new wasm thread parker for non-atomic libstds
a31781b Rename existing wasm ThreadParker
3a395e1 Merge pull request #140 from faern/fix-feedback-from-libstd-1
92498d5 Remove invalid use of word "random"
757a640 Use incrementing index instead of ptr address as seed
4d78285 Implement own xorshift PRNG to avoid rand dependency
6607fd3 Fix CloudABI ThreadParker
c763967 Remove not needed unsafe blocks
5e2d677 Use FUTEX_ constants from libc
3132b95 Make Android use futex based parker
2fb27d3 Create a ThreadParker trait, to unify API
c162cfc Remove #[inline(never)]
f9f951b Format comment
c8c5041 Use impl trait instead of generic in park
c775469 Simplify unlock_bucket_pair
4594729 Run latest rustfmt (1.2.2-nightly)

Without knowing more, I would cautiously suggest this warrants a parking_lot_core-0.6.0 release, with the presumed (suggested in #156) to be released parking_lot_0.9.0 depending on that. If you release this as parking_lot_core-0.5.1, it would mean its compatible for use with the existing parking_lot-0.8.0 and that likely would warrant testing.

If it would be helpful, I'm happy to provide a PR with those suggested changes to the two Cargo.toml. I'm not sure I'm well enough versed to completely fix up the CHANGELOG(s) though. Thanks for all your work on this project!

[dekellum]

If I check out tag/0.8.0 locally and patch it with the current master branch (ace8280) version of parking_lot_core, it compiles and tests pass for me. So maybe it is compatible with 0.8.0 and therefore could be released as 0.5.1, and independent of parking_lot-0.9.0? Not sure what is best here. Are all the changes SemVer PATCH release appropriate?

[Amanieu]

I'm quite busy this week and would definitely appreciate a PR to update the versions and changelog.

Regarding your question: parking_lot_core is an implementation detail of parking_lot, so breaking changes to parking_lot_core do not case breaking changes in parking_lot. However lock_api is part of the public API of parking_lot, so breaking changes to lock_api will break parking_lot semver compatibility.

[dekellum]

OK I'll start a PR branch. So parking_lot_core is a private dependency, do I understand that correctly? Is your preference then to release this as parking_lot_core-0.5.1?

An example of a change that could still be breaking for users (and warrant releasing as 0.6.0 IMO) would be if MSRV increased, but at least according to .travis.yml, that hasn't changed from 0.5.0 to master for core (still 1.31.0). Is there anything else you know of that is like that kind of breaking change in the above diffs?

[Amanieu]

None of the parking_lot_core changes are breaking, so 0.5.1 is fine.

[faern]

@dekellum Each crate is to be considered on its own. If the changes to parking_lot_core since the 0.5.0 release in any way change the public API of parking_lot_core it must be bumped to 0.6.0, otherwise 0.5.1 is fine. The fact that (or how) parking_lot uses parking_lot_core has no relevance in the context of version bumping parking_lot_core itself.

Whether or not parking_lot should have a breaking version bump or not depends on if the public API of parking_lot changed since the last release. This includes if a dependency of it is re-exported and that part of the API changes, because that means the result is that the public API of parking_lot also changes.

That's a very non-direct answer to the problem at hand, more how to think about semver overall :)

[dekellum]

As possible exhibit to @faern's point of view, there are, intended or otherwise, parking_lot_core dependees beyond just parking_lot:

https://crates.io/crates/parking_lot_core/reverse_dependencies

If those uses are unsupported, perhaps stronger language should be added to the parking_lot_core crate's top-level rustdoc or elsewhere? The start of the current lib module doc might be construed as public API:

//! This library exposes a low-level API for creating your own efficient
//! synchronization primitives.

The c8c5041 change of replacing generics with impl Trait in arg position of pub fn might be breaking if the generic parameters are explicitly used?

-pub unsafe fn park<V, B, T>(
+pub unsafe fn park(
     key: usize,
-    validate: V,
-    before_sleep: B,
-    timed_out: T,
+    validate: impl FnOnce() -> bool,
+    before_sleep: impl FnOnce(),
+    timed_out: impl FnOnce(usize, bool),
     park_token: ParkToken,
     timeout: Option<Instant>,
-) -> ParkResult
-where
-    V: FnOnce() -> bool,
-    B: FnOnce(),
-    T: FnOnce(usize, bool),
-{
+) -> ParkResult {

That said I think I might have gotten away with calling a similar change compatible™ in a much less popular crate of my own! Could I get more eyeballs on the actual diff (linked above) please? It's clearly safer to release this as parking_lot_core-0.6.0, but that requires a parking_lot-0.9.0 release to use it.

[faern]

The generic to impl trait change is definitely a breaking change. Because it was previously possible to call it with park<_, _, _>(...) and now it's not. See this playground:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=bc0050e149acb351907dfa519de49649