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

rustc is using three versions of rand #57724

Closed
scottmcm opened this issue Jan 18, 2019 · 27 comments · Fixed by #62026
Closed

rustc is using three versions of rand #57724

scottmcm opened this issue Jan 18, 2019 · 27 comments · Fixed by #62026
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@scottmcm
Copy link
Member

scottmcm commented Jan 18, 2019

I noticed this scrolling by while "Building stage0 compiler artifacts" today:

   Compiling rand_pcg v0.1.1
   Compiling rand v0.6.1
   Compiling flate2 v1.0.6
   Compiling rand v0.4.3
   Compiling rand v0.5.5
   Compiling winapi-util v0.1.1

Optimistically flagging E-easy

@scottmcm scottmcm added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Jan 18, 2019
@scottmcm scottmcm changed the title rustc is using two versions of rand rustc is using three versions of rand Jan 18, 2019
@JohnTitor
Copy link
Member

@scottmcm I'd like to work on this, how can I fix? make rustc use one version(maybe v0.6.1)?

@ishitatsuyuki
Copy link
Contributor

@JohnTitor You will need to look into each dependency that depends on rand (by searching through Cargo.lock), check if they have a newer version that depends on new rand, and finally use cargo update -p <crate> to upgrade those crates.

@JohnTitor
Copy link
Member

@ishitatsuyuki Thanks! When I checked, it seems each rand version is used.

@ishitatsuyuki
Copy link
Contributor

Yeah, it's natural that they are all used, and you could try to eliminate those crates using old rand by upgrading or refactoring. That said, there isn't much drawback with an older rand, so in that case this issue can be just closed.

@JohnTitor
Copy link
Member

Oh, I'm sorry, I misunderstood. Hmmm, I think we don't have to fix this quickly and this may be a catalyst for a newbie like me to contribute, so I leave this, okay?

@hellow554
Copy link
Contributor

@ishitatsuyuki but one could argue that it will lower the time to compile so it's worth to fix IMHO ;)

@JohnTitor
Copy link
Member

Uh, would it be better to fix?

@hellow554
Copy link
Contributor

Disclaimer: I'm not a core member, but IMHO this is worth fixing. Also scottmcm who is a core member opened this issue, so I guess you're good to go and fix that issue @JohnTitor :)

@JohnTitor
Copy link
Member

Okay, I want to try to fix. For instance, crossbeam-channel depends on rand v0.5.5, how can I update or refactor this?

@hellow554
Copy link
Contributor

That's one thing you can't edit, because rand is a dependency of crossbeam-channel here, so the crate crossbeam-channel needs to update its dependecy (maybe it already did and you just need to update crossbeam-channel itself)?

@Aaron1011
Copy link
Member

For future reference, running cargo tree -d from src/rustc is an easy way to identify the source of all duplicates.

@JohnTitor
Copy link
Member

JohnTitor commented Jan 19, 2019

Thanks @Aaron1011 ! I got this tree(gist).
I updated parking_lot, but rustc-rayon v0.1.1 is latest and depends on rand v0.4.3(https://crates.io/crates/rustc-rayon).
What should I do?

@scottmcm
Copy link
Member Author

@hellow554 I'm not on compiler (or core), so don't take my word on this one 🙂

@JohnTitor It looks like upstream rayon needs 0.5, so you could consider making a PR to upgrade it to 0.6. (Maybe ask someone on rayon first.) Also, it seems that rustc-rayon hasn't been updated in 6+ months, so they'd probably appreciate a PR to upgrade it to newer upstream.

@cuviper
Copy link
Member

cuviper commented Jan 20, 2019

Current rayon only uses rand as a dev-dependency, since rayon-rs/rayon#571. That wouldn't show up in Cargo.lock here at all, if rustc-rayon follows suit.

@lnicola
Copy link
Member

lnicola commented Feb 3, 2019

So the latest versions are rustc-rayon 1.0.3 and rustc-rayon-core 1.4.1, but 0.1.1 is used in this repository. They come from librustc_data_structures. Maybe those dependencies should be updated or made optional?

Centril added a commit to Centril/rust that referenced this issue Mar 13, 2019
@polybuildr
Copy link
Contributor

It looks like this was fixed in PR #58908 and merged by bors. Should this issue be closed now?

@sfackler
Copy link
Member

There are still 3 versions of rand in use: https://github.com/rust-lang/rust/blob/master/Cargo.lock#L4173-L4175

@mati865
Copy link
Contributor

mati865 commented Mar 25, 2019

#58805 will remove rand 0.5, I can get rid of 0.4 anytime but there are many PRs modifying Cargo.lock right now.

@lnicola
Copy link
Member

lnicola commented Jun 6, 2019

Update: rand 0.4.3 is used by phf_generator, rayon_core and redox_users (via dirs).

@cuviper
Copy link
Member

cuviper commented Jun 6, 2019

That's with rayon-core 1.4.0 -- 1.4.1 only uses rand 0.5 as a dev-dependency.

@lnicola
Copy link
Member

lnicola commented Jun 6, 2019

I didn't check now, but at some point rustc was using an old version of a custom fork of rayon.

@cuviper
Copy link
Member

cuviper commented Jun 6, 2019

rustc-rayon has rebased since then, so it already reflects that dependency change.

Centril added a commit to Centril/rust that referenced this issue Jun 7, 2019
Centril added a commit to Centril/rust that referenced this issue Jun 7, 2019
Centril added a commit to Centril/rust that referenced this issue Jun 7, 2019
@lnicola
Copy link
Member

lnicola commented Jun 8, 2019

I tried to fix a part of this in #61597, but it didn't come out as planned. There is dependency and license whitelist that needs to be updated.

I think just running cargo update leaves only one usage of rand 0.4 via ammonia and mdbook 1 (which can be updated). But then there is the licensing story, and it's not very clear to me which dependencies are used by the tools, and which by the compiler.

@mati865
Copy link
Contributor

mati865 commented Jun 8, 2019

Last time I checked mdbook and rustc-rayon were the only thing preventing of rand 0.4, otherwise I'd fix it already.
We are working on removing it but it's blocked on releasing new version of mdbook.

@lnicola
Copy link
Member

lnicola commented Jun 8, 2019

I think rustc-rayon goes away after a cargo update, so that one might be easy to fix.

I don't know how dependencies are updated across the rust repository. Is it just that most of them are for the tools, not for std and rustc? Does somebody run cargo update from time to time? And why does the tidy license check fail on bors on my PR, but not on Travis or on my computer?

@mati865
Copy link
Contributor

mati865 commented Jun 8, 2019

So rustc-rayon released new version since.

Is it just that most of them are for the tools, not for std and rustc?

Correct, dependencies of everything that is created by dist are tracked by single Cargo.lock.

Does somebody run cargo update from time to time?

Dependencies are often updated separately as needed because upgrading everything at once is likely to break something and it's easier to find what is wrong when you change like 5 dependencies instead of 50.

And why does the tidy license check fail on bors on my PR, but not on Travis or on my computer?

I think it should error when you run ./x.py build.

@mati865
Copy link
Contributor

mati865 commented Jun 8, 2019

Also you can run the same image that found the error (x86_64-gnu-llvm-6.0 in this case) by using these steps https://github.com/rust-lang/rust/blob/master/src/ci/docker/README.md

bors added a commit that referenced this issue Jun 13, 2019
Bump dirs, rand and redox_users

Part of #57724.
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 2, 2019
bors added a commit that referenced this issue Jul 7, 2019
Final nail in `rand 0.4` coffin

Closes #57724
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants