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

rand_distr: fix no_std build #1208

Merged
merged 3 commits into from Jan 13, 2022
Merged

Conversation

newpavlov
Copy link
Member

@newpavlov newpavlov commented Dec 28, 2021

Closes #1205.

Note that the test-no-std job checks only rand and rand_core and does not affect rand_distr and other crates . IIUC we need a virtual manifest for it to work, not the current repository structure.

Changing working directory to rand_distr and running cargo build in it will not work due to the dev feature unification, i.e. we would need resolver = "2" for no_std tests to work in that case.

@@ -13,6 +13,8 @@ use crate::{Distribution, Uniform};
use rand::Rng;
use core::fmt;
use core::cmp::Ordering;
#[allow(unused_imports)]
use num_traits::Float;
Copy link
Member Author

@newpavlov newpavlov Dec 28, 2021

Choose a reason for hiding this comment

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

The allow attributes is a bit unfortunate, but we currently don't have a way of checking whether std is enabled for current build, note that it's not equivalent to checking feature = "std".

@vks
Copy link
Collaborator

vks commented Dec 28, 2021

I believe we can add resolver = "2" without bumping the MSRV, because it will only result in a warning for old Cargo versions.

@newpavlov
Copy link
Member Author

IIRC it's not true for Rust 1.36.

@dhardy
Copy link
Member

dhardy commented Dec 29, 2021

In that case it sounds like the best thing to do is release a patch now to fix this, then bump the MSRV for rand_distr only. Thanks @newpavlov .

@vks
Copy link
Collaborator

vks commented Dec 29, 2021

@newpavlov I'm using it here, and it seems to work for Rust 1.36.

@dhardy
Copy link
Member

dhardy commented Jan 10, 2022

resolver = 2 is supported since Rust 1.51.0 (March 2021), which is old enough that I don't think we need to worry about an MSRV bump for rand v0.9 or rand_distr v0.5.

Copy link
Member

@dhardy dhardy left a comment

Choose a reason for hiding this comment

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

In any case, I see nothing against releasing this patch version. There's nothing else worth mentioning in the changelog.

@vks vks merged commit 8f37250 into rust-random:master Jan 13, 2022
@vks
Copy link
Collaborator

vks commented Jan 13, 2022

@dhardy I'll take care of the release tonight, unless you get to it before then.

@newpavlov newpavlov deleted the rand_distr/fix_no_std branch January 13, 2022 20:29
@dhardy
Copy link
Member

dhardy commented Jan 18, 2022

Published

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.

None yet

3 participants