Skip to content

trait impl disappeared without bumping major version #94

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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:

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