Skip to content

Re-export rand_core#683

Merged
newpavlov merged 1 commit intomasterfrom
rand_reexport
Jul 11, 2021
Merged

Re-export rand_core#683
newpavlov merged 1 commit intomasterfrom
rand_reexport

Conversation

@newpavlov
Copy link
Copy Markdown
Member

Closes #681

Also issue compilation errors if unstable functionality in the signature crate is enabled by toggling features introduced by optional dependencies. In theory it may break build for some people, but it would've happened eventually either way. Also I do not forward std to rand_core in this case since doing it correctly requires weak feature activation.

@newpavlov newpavlov requested a review from tarcieri July 10, 2021 23:37
Copy link
Copy Markdown
Member

@tarcieri tarcieri left a comment

Choose a reason for hiding this comment

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

Also I do not forward std to rand_core in this case since doing it correctly requires weak feature activation.

The nice thing about doing this is it makes it possible to access OsRng without an additional crate dependency.

Perhaps as an alternative, we can add a getrandom feature which enables rand_core/getrandom instead?

@newpavlov
Copy link
Copy Markdown
Member Author

We still will pull rand_core, even though the preview feature is not enabled. But on second though, forwarding std could be fine. Since if I am not missing something, rand_core does not become part of the crate's public API unless rand-preview is enabled. It would mean that we pull a dependency which will not be used in any way, but I guess it should be more or less fine.

BTW such complications is one of the reasons why I am not a huge fan of preview features and generally err on the side of caution with 1.0 releases.

@tarcieri
Copy link
Copy Markdown
Member

tarcieri commented Jul 11, 2021

I'll leave it up to you how you want to handle signature. If need be we can revisit it in another PR.

@newpavlov
Copy link
Copy Markdown
Member Author

newpavlov commented Jul 11, 2021

I think I will leave it as is then. Mostly because std is enabled by default in signature, so forwarding it would mean that users not dependent on rand-preview and who do care about using default-features = false will pull 2 dependencies (rand_core and getrandom) not used in any way by this crate. Yes, they are really common crates and quite small ones, but still it does not look great. Either way, we always can add such forwarding later.

@newpavlov newpavlov merged commit f4ce223 into master Jul 11, 2021
@newpavlov newpavlov deleted the rand_reexport branch July 11, 2021 00:31
@tarcieri tarcieri mentioned this pull request Jul 12, 2021
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.

Improving OsRng access

2 participants