Make rand dependency optional

[ghost]

In the light of the rand crate pushing semver-incompatible changes (again) and breaking every (even indirectly) dependent crate, this pull request makes the rand dependency in nalgebra optional. See issues rust-random/rand#818, rust-random/rand#645, rust-random/rand#766, rust-random/rand#754.

Frankly, I'm tired of dealing with this and I would suggest removing the rand crate from the default features outright, but this would be a breaking change, and I'm not sure how to structure Cargo.toml so that the tests can still enable this feature.

[Ralith]

I think the offending rand_hc release has already been yanked, but I like the idea of this anyway because I often write medium-sized utility libraries that use nalgebra and have no business depending on rand, and a quicker build/smaller dep graph is always welcome.

[Ralith]

Do we have tests confirming that this section works as expected, in both positive and negative cases?

[ghost]

I've tested it by hand, but I'll look into if we can do this in the CI.

[ghost]

So looking into it a bit more, looks like neither of the conditional sections here were actually getting activated. I don't think it's possible to pass feature flags to optional dependencies without activating the dependency. The only solution seems to be to provide new features "std-no-rand" and "stdweb-no-rand". As a sidenote, while investigating this, I noticed that the "stdweb" feature is currently broken in master.

[0xpr03]

Note that it's semver compatible to make breaking changes pre 1.0 in minor versions.

[0xpr03]

See Point 4

Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable.

[dhardy]

Note that rust-random/rand#818 was a release issue resolved within a two-hour window (the breaking part within half an hour IIRC), so it hardly seems fair to use as motivation here. The other reports appear to relate to version 0.6.5, which, yes, causes issues for those doing selective updates from previous versions.

That said, I don't object to the idea of making the dependency optional, though I don't believe there remains a big motivation (rand = 0.7 reduces dependencies significantly and without semver breakage I believe).

What I do find strange is this std-without-rand feature. Is it intended to be temporary? If not, is it better to require uses to opt-in to "rand" when using it?

Maybe it would be best to update to rand = 0.7 first then revisit whether this is useful.

[ghost]

I only know that I've had my builds break twice now within a few months of each other just because rand pushed out a new version. I think it's entirely fair to not want to use a crate that behaves in this manner. That aside, I think it's a good change to make this crate optional, as I would imagine quite a large portion of the users of nalgebra aren't interested in the random number generation aspects, and removing any additional dependencies will improve compile times for users.

I'm not particularly happy with how the features turned out here. I don't remember what exactly was the problem, but it's a weird quirk of the Cargo.toml syntax that makes us have to do this. It was some weird issue in how the web and core targets are defined with features, and those features need to be passed into the dependencies as well, but this is impossible or very hard to do with optional dependencies. I wonder if we really need to do this, or if these crates could check the compile target instead of using features for it?

[repi]

I would prefer rand to be optional in nalgebra as well, we have a lot of code that we do not allow any system calls and that have to be deterministic, and actually doesn't have a random number generator available in it (it is for wasm-unknown-unknown and not in a browser) so any rand call will panic.

So we would want the random functionality simply to not be available when compiling instead

[dhardy]

@repi the rand lib does also support deterministic generation (using fixed seeds). It can even come without support for any system calls if default features are disabled. However this is all irrelevant if you have no need for pseudo-random values.

[repi]

Yes I should have been more precise, it is the optional but default getrandom feature in rand that brings in the OS random number generators that is the main challenge for us.
And looking at nalgebra it actually does disable the default features of rand so looks like it doesn't bring in the OS random number generators? which would be great.

rand           = { version = "0.6", default-features = false }

Though still think it would be nice to have a stronger way to disable the support explicitly like with this suggested PR feature toggle, but is at least less of an issue for us if it already doesn't bring in the OS functions

[dhardy]

Using rand without the std feature and using a fixed seed for tests would be enough.

[newpavlov]

@aleksijuvani
Please take a look at #622, it also makes rand optional. Maybe it's worth to close this issue in favor of that one?

[ghost]

Sure, that seems fine. I would prefer rand to be opt-in in the first place, and if it allows us to avoid the awkward "std-no-rand" feature flag, that's good. Not sure if it makes sense to bundle into the same pull request as #622 however, as it's essentially a different issue.

[ghost]

@sebcrozet What are your thoughts on this whole rand situation?

[sebcrozet]

I'm in favor of making the dependency to rand optional and opt-in. I will review #622 which appears to address both making rand optional and updating it to 0.7.

I'm closing this PR because it does not seem to bring anything more than what #622 already covers, but feel free to reopen and correct me if I am wrong.

Thank you @aleksijuvani for preparing this PR anyway.