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.

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

Upgrade to crossbeam 0.4 #5790

Merged
merged 4 commits into from
Jul 26, 2018

Conversation

dwijnand
Copy link
Member

Supersedes #5789

@rust-highfive
Copy link

r? @matklad

(rust_highfive has picked a reviewer for you, use r? to override)

@matklad
Copy link
Member

matklad commented Jul 25, 2018

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Jul 25, 2018

📌 Commit 16681b3 has been approved by matklad

@bors
Copy link
Contributor

bors commented Jul 25, 2018

⌛ Testing commit 16681b3 with merge 853089a18f4f20f7ea5893ad580df70352f6d71f...

@bors
Copy link
Contributor

bors commented Jul 25, 2018

💔 Test failed - status-travis

@dwijnand
Copy link
Member Author

This is failing in the Rust 1.27.2 (which is the latest stable) job, with:

error: expected identifier, found `"std"`
  --> /home/travis/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.1.1/rust/src/liblibc/lib.rs:79:46
   |
79 | #[cfg(feature = "cargo-build")] extern crate "std" as core;
   |                                              ^^^^^ expected identifier

while the stable job is passing fine, which is confusing.

@matklad can you make sense of it?

@matklad
Copy link
Member

matklad commented Jul 25, 2018

This is certainly #5757

r? @Eh2406
:)

@dwijnand
Copy link
Member Author

Thanks @matklad.

lol, from just 2 days ago:

If it turns out that --minimum-versions is a constant painful effort to keep up to date then we could remove it again from CI.

Note: I'm not suggesting it's a constant painful effort or that we should remove it from CI, I just thought it was funny that within less than 24 hours it's affected a PR, albeit one about tweaking dependencies.

@Eh2406
Copy link
Contributor

Eh2406 commented Jul 25, 2018

I was not able to reproduce locally. I do not have libc 0.1.1 in my lock file with minimum-versions. I am updating the latest nightly in the vain hope that that is why.

Can I get an automatic ping on @dependabot PRs?

@dwijnand
Copy link
Member Author

Let's see if it fails after a rerunning (triggered by closing and reopening the PR).

For the record these are the failed CI runs:

@dwijnand dwijnand closed this Jul 25, 2018
@dwijnand dwijnand reopened this Jul 25, 2018
@Eh2406
Copy link
Contributor

Eh2406 commented Jul 25, 2018

douw My bad. :-( I did not have the new branch checked out. Sorry.

@Eh2406
Copy link
Contributor

Eh2406 commented Jul 25, 2018

it is a log v0.3.0 problem. From rand v0.3.0 From parking_lot_core v0.2.0

One fix is to add a synthetic dependency on parking_lot_core = "0.2.11".

edit: in #5757 @alexcrichton recommended adding a comment like # required only for minimal-versions. brought in by crossbeam.

@dwijnand
Copy link
Member Author

When you get a chance, if you could document how you figure that out, I think it would be useful.

If you have write permissions to the repo feel free to push to my branch, or continue in your own PR. I was just trying to unblock the original PR 🙂. If not I'll try and apply your comments.

@Eh2406
Copy link
Contributor

Eh2406 commented Jul 25, 2018

I do not have write permissions, It is all yours!

@Eh2406
Copy link
Contributor

Eh2406 commented Jul 25, 2018

https://travis-ci.org/rust-lang/cargo/jobs/408134134 is green! So that is a good sign!

@matklad
Copy link
Member

matklad commented Jul 25, 2018

One fix is to add a synthetic dependency on parking_lot_core = "0.2.11".

Adding synthetic deps seems like a wrong solution to the problem... I would expect that the right solution is to bump parking_lot dependency version in crossbeam.

OTOH, I came to really appreciate @Eh2406 idea about non-transitive minimal versions....

@Eh2406
Copy link
Contributor

Eh2406 commented Jul 25, 2018

I like upstreaming the fix, for now this works.

FYI I think it was @joshtriplett idea originally.

@alexcrichton
Copy link
Member

Thanks for this! I think that the crossbeam crate has actually been split up a bit, could crossbeam_utils be brought in instead? I think we may just need the scoped thread support there and it should sidestep the minimal versions issue!

@dwijnand
Copy link
Member Author

Oh that sounds plausible. I'll give it a try, Alex.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Jul 25, 2018

📌 Commit 8ea90e9 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jul 25, 2018

⌛ Testing commit 8ea90e9 with merge 80f6922...

bors added a commit that referenced this pull request Jul 25, 2018
…crichton

Upgrade to crossbeam 0.4

Supersedes #5789
@bors
Copy link
Contributor

bors commented Jul 26, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 80f6922 to master...

@bors bors merged commit 8ea90e9 into rust-lang:master Jul 26, 2018
@dwijnand dwijnand deleted the dependabot/cargo/crossbeam-0.4 branch July 26, 2018 05:51
@ehuss ehuss added this to the 1.29.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants