Skip to content
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

Update optional parking_lot dependency to 0.11.0 #2676

Merged
merged 2 commits into from
Jul 28, 2020
Merged

Update optional parking_lot dependency to 0.11.0 #2676

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
33429eb
Update optional parking_lot dependency to 0.11.0
jbg Jul 22, 2020
f647acf
Merge 'upstream/master' into 'jbg/update-parking-lot'
Darksonn Jul 27, 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
2 changes: 1 addition & 1 deletion tokio/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ memchr = { version = "2.2", optional = true }
mio = { version = "0.6.20", optional = true }
iovec = { version = "0.1.4", optional = true }
num_cpus = { version = "1.8.0", optional = true }
parking_lot = { version = "0.10.0", optional = true } # Not in full
parking_lot = { version = "0.11.0", optional = true } # Not in full
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about a more permissive requirement that allows both, like

Suggested change
parking_lot = { version = "0.11.0", optional = true } # Not in full
parking_lot = { version = ">= 0.10.0, <= 0.11.0", optional = true } # Not in full

If the goal is just to reduce duplicate dependency versions, this would be the ideal choice --- if other dependencies in tree are already pulling 0.10, Cargo would select that version. Tokio doesn't really use any of the APIs that have changed between those releases...

Copy link
Contributor

Choose a reason for hiding this comment

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

Would that prevent using 0.11.1 once that is released?

Copy link
Member

Choose a reason for hiding this comment

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

What would be the advantage? Seems best to track deps?

Copy link
Member

Choose a reason for hiding this comment

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

Would that prevent using 0.11.1 once that is released?

Err, I meant < 0.12.0.

What would be the advantage? Seems best to track deps?

The idea is that Tokio should be able to use whatever parking_lot version is pulled in by another crate, if there is one in-tree, to minimize compiling duplicate versions. If another crate has an older dependency, we could just use it, and prefer the latest if no other crate depends on some other version.

I'm not sure if this is the right choice or not, but the only parking_lot APIs we depend on are the ones that mimic std, and therefore will probably not change very much. We can ignore breaking changes to unrelated APIs that we don't care about in order to reduce the total number of parking_lots in a crate where multiple dependencies use parking_lot.

This is a loosely-held opinion...I'm willing to be convinced this is a bad idea.

Copy link
Member

Choose a reason for hiding this comment

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

I don't have any data on whether or not it works well. If you think it is worth it, we can try it. I also do not know if the cargo dependency resolver is smart enough to dedup the way you are describing (it is a very hard problem).

Copy link
Contributor

Choose a reason for hiding this comment

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

I recall having heard issues with the resolver in cases like this before on the user's forum. I would want to test that it really works properly before doing this.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, not a big deal if this doesn't work, then!

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless something changed within the last ~year, cargo will prefer using the latest versions possible for everything even if deduplication would be possible, unless the crate being deduplicated uses links (out of necessity) - see also rust-lang/cargo#4902 (comment).

slab = { version = "0.4.1", optional = true } # Backs `DelayQueue`
tracing = { version = "0.1.16", default-features = false, features = ["std"], optional = true } # Not in full

Expand Down