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

Please update/remove rand_pcg #780

Closed
dhardy opened this issue Feb 22, 2022 · 9 comments
Closed

Please update/remove rand_pcg #780

dhardy opened this issue Feb 22, 2022 · 9 comments
Labels

Comments

@dhardy
Copy link

dhardy commented Feb 22, 2022

rand_pcg v0.3 was released around a year ago, yet the version on the playground is v0.2. (This is not surprising, since v0.2 was a dependency of rand v0.7, but rand v0.8 does not depend on rand_pcg.)

Similar to #531, this fails:

// workaround:
// extern crate rand_0_7_3 as rand;

use rand::{Rng, SeedableRng};

fn main() {
    let mut rng = rand_pcg::Pcg32::seed_from_u64(123);
    println!("{}", rng.gen::<i32>());
}

The error message here is confusing.

Suggestion: either include the latest version of rand_pcg (even though it's not in the top 100 list) or exclude all versions of rand_pcg, because having the playground default to an outdated version is confusing.

@dhardy dhardy added the wontfix label Feb 22, 2022
@dhardy dhardy changed the title Please add/update rand_pcg Please update/remove rand_pcg Feb 22, 2022
@dhardy
Copy link
Author

dhardy commented Feb 22, 2022

No idea why this got a wontfix label.

@shepmaster
Copy link
Member

No idea why this got a wontfix label.

Because you used the issue template that contains the text:

Any issue opened for a new or updated crate version will be closed with a reference to the same policy.

@shepmaster
Copy link
Member

As stated in #531:

file an issue with rand. They can specify metadata for extra features that they think should be enabled on the playground

I think you might have a connection to a rand crate maintainer 😉

@shepmaster
Copy link
Member

shepmaster commented Feb 22, 2022

either include the latest version of rand_pcg

As stated multiple locations, including the issue template linked above, I don't want to get into the business of deciding which crates are privileged or not on a crate-by-crate basis.

or exclude all versions of rand_pcg

This can't be done as rand_pcg is a dependency via:

rand v0.7.3
└── phf_generator v0.8.0
    ├── phf_codegen v0.8.0
    │   [build-dependencies]
    │   └── markup5ever v0.10.1
    │       ├── html5ever v0.25.1
    │       │   ├── markup5ever_rcdom v0.1.0
    │       │   │   └── select v0.5.0
    │       │   └── select v0.5.0 (*)
    │       ├── markup5ever_rcdom v0.1.0 (*)
    │       └── xml5ever v0.16.2
    │           └── markup5ever_rcdom v0.1.0 (*)
    └── string_cache_codegen v0.5.1
        [build-dependencies]
        └── markup5ever v0.10.1 (*)

Counter suggestion: update those crate(s) to use rand 0.8 and rand_pcg will naturally fall off of the playground's list.

@dhardy
Copy link
Author

dhardy commented Feb 22, 2022

They can specify #192 (comment)

Not relevant. rand_pcg is not a "feature" of rand; it's simply not a dependency any more (except under dev-dependencies).

I don't want to get into the business of deciding which crates are privileged or not on a crate-by-crate basis.

So add a rule: if crate foo v0.x is included, then include all later versions too. Simple? No? Fine, I'll leave this to you.

Counter suggestion: update those crate(s) to use rand 0.8 and rand_pcg will naturally fall off of the playground's list.

Really? So the latest release of markup5ever depends on an old version of phf_codegen (in fact, it was updated in master, but still not to the latest version). Why is fixing this my problem? In fact, why is use of outdated crates a problem in the first place (other than with regards to security, for which we already have https://rustsec.org/).

@shepmaster
Copy link
Member

shepmaster commented Feb 22, 2022

why is use of outdated crates a problem in the first place

didn’t you open this issue because of the use of an outdated crate? ("because having the playground default to an outdated version is confusing")

Why is fixing this my problem?

It doesn't have to be, but considering you opened this issue, it seemed like you were interested in seeing it resolved. That is one avenue to resolving it.

Not relevant. rand_pcg is not a "feature" of rand

I see; I assume that changed from when the original comment was posted.

So add a rule: if crate foo v0.x is included, then include all later versions too. Simple? No? Fine, I'll leave this to you.

I honestly don't know. Certainly including all later versions would not be a good idea, so I assume you meant "the latest" version. That would involve someone making the appropriate changes to the script and seeing what the net change in new compilations would be.

@shepmaster
Copy link
Member

The error message here is confusing.

You may also want to consider opening/adding to a Rust issue about this case as it applies to any user, not just the playground. There have been previous issues around poor error messages due to duplicate / mismatched crate versions, so that is encouraging that someone could improve the situation overall.

@dhardy
Copy link
Author

dhardy commented Feb 22, 2022

didn’t you open this issue because of the use of an outdated crate?

Yes, because using rand_pcg in the playground uses an old version by default, and because of this causing an incompatibility with a confusing message. By default I think we should use the most recent version of a crate (if we use that crate at all), but I see no problem with keeping old versions around too.

I see; I assume that changed from when the original comment was posted.

Yes, in rand v0.7 rand_pcg was optional, so the proposal would have made sense.

There have been previous issues around poor error messages due to duplicate / mismatched crate versions, so that is encouraging that someone could improve the situation overall.

Good point. That's already been improved in some cases, but clearly not this one. Already reported: rust-lang/rust#81659

@shepmaster
Copy link
Member

I think we should use the most recent version of a crate (if we use that crate at all), but I see no problem with keeping old versions around too.

The current behavior is that the current version of the top 100 crates (plus Rust Cookbook and compiler-suggested crates) are added. Since all of their dependencies must be built anyway, we go ahead and expose those dependencies at their specific versions to the playground user.

We don't then also go off and get a newer version of those dependencies, which would then potentially bring in more dependencies. Some of those new dependencies wouldn't be the newest version and then we'd have to repeat this process until we reach a fixpoint.

An alternative solution would be to track top level vs transitive dependencies. We could suffix all transitive dependencies with the version number, or perhaps just those transitive dependencies that aren't the newest version.

Applied to this issue, there wouldn't be a rand_pcg crate in the playground, only rand_pcg_0_2_1.

@shepmaster shepmaster closed this as not planned Won't fix, can't repro, duplicate, stale Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants