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

trait impl disappeared without bumping major version #94

Closed
awortman-fastly opened this issue Aug 19, 2019 · 4 comments · Fixed by #96
Closed

trait impl disappeared without bumping major version #94

awortman-fastly opened this issue Aug 19, 2019 · 4 comments · Fixed by #96

Comments

@awortman-fastly
Copy link

Sorry to be the bearer of versioning news, 0.1.10 contains a backwards-incompatible change with respect to 0.1.8 and so on - the feature flags around error_impls changed, and had been accidentally(?) including error_impls for specific OSes regardless of std being selected or not.

The change around feature flags makes sense, but without bumping major versions, version 0.1.10 can cause breakages: see the end of the log here. The missing impl is because minisign depends on getrandom = "0.1.3", where 0.1.10 should be a backwards compatible update, but is not due to the missing trait. We can work around that by specifying std, but I wanted to make sure y'all knew about this.

@newpavlov
Copy link
Member

We could make std feature enabled by default. Strictly speaking it also can be viewed as a breaking change for no_std targets (especially considering that rand_core does not use default-features = "false" for getranom right now), but since no one has complained about about compile_error! on them, I guess it can be considered a lesser evil in this unpleasant situation.

@josephlr
Copy link
Member

josephlr commented Aug 19, 2019

@newpavlov I agree, we should enable the std feature by default to fix this issue and have rand_core pass default-features = false so that rand_core keeps working on no_std targets (really, we should do that regardless). EDIT: we should also make sure default-features = false is passed for the libstd patch as well.

We should prioritize not breaking std users first and foremost.

EDIT2: It looks like rand and rand_core only use getrandom iff std is used, so they are OK. The only concern would be rand_os, which would be broken by changing the defaults. This is wrong, see rust-random/rand#870

@dhardy
Copy link
Member

dhardy commented Aug 23, 2019

I think the solution here is to yank 0.1.10 and prepare 0.2.0?

@josephlr
Copy link
Member

I think the solution here is to yank 0.1.10 and prepare 0.2.0?

@dhardy I agree. My idea is as follows:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants