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

Disable std feature by default in rand_core and rand_jitter #703

Merged
merged 2 commits into from
Jan 25, 2019

Conversation

dhardy
Copy link
Member

@dhardy dhardy commented Jan 23, 2019

Implements #702
Resolves #645

Publishing under a new version is the correct way of doing things.

I took the liberty of applying the same logic to rand_jitter, which does have more requirement to use its std feature but is also a "low level" crate (and not yet published, hence not a breaking change).

I also forced rand_jitter to forward the std feature to rand_core, not because it needs it but because it re-exports rand_core and is thus more consistent for users. By the same logic, rand_os now requires std on rand_core.

@dhardy dhardy requested a review from vks January 23, 2019 12:49
@vks
Copy link
Collaborator

vks commented Jan 23, 2019

Does the test fail because of the dead links?

@dhardy
Copy link
Member Author

dhardy commented Jan 23, 2019

I don't know; I'm looking into the deadlinks now.

@dhardy
Copy link
Member Author

dhardy commented Jan 23, 2019

AppVeyor fails since cargo test --package rand_core identifies multiple versions of rand_core — we should use the manifest trick instead. Not sure why it is refering to rand_core 0.3 however. Edit: it's rdrand.

Of course, we also need a compatibility shim for rand_core 0.3 on 0.4, but that has to happen afterwards.

@dhardy
Copy link
Member Author

dhardy commented Jan 23, 2019

I can't find the deadlinks when following the Travis CI procedure. If I just do cargo doc --all there are a few, but looks like all are from external dependencies.

I think deploying this will break builds on SGX since that uses RDRAND which depends on rand_core 0.3, so we will need to deploy the compatibility shim immediately. (Probably the only issue though is that sgx.rs uses rand_core from its own dependency, instead of the the version used by rdrand, though since it's not re-exported as rdrand::rand_core getting it right isn't possible now.)

--package causes problems where multiple versions exist
(an issue since rdrand depends on rand_core:0.3).
Also, we definitely don't need both.
@dhardy
Copy link
Member Author

dhardy commented Jan 23, 2019

The Travis deadlinks check is still going on about

Linked file at path /home/travis/build/rust-random/rand/target/doc//index.html does not exist!

many times over. Perhaps there's a missing crate name between the double //, but I'm not sure why it's linking to the index page (I checked all mentions of 'index').

@dhardy
Copy link
Member Author

dhardy commented Jan 24, 2019

I was planning on publishing this today, but without further feedback or a solution to that CI error I'd rather hold off a little longer. Any ideas?

@dhardy
Copy link
Member Author

dhardy commented Jan 25, 2019

Well, both these CI errors are happening on other PRs, so I will go ahead and merge.

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.

3 participants