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

Fix building issue #105

Closed
wants to merge 5 commits into from
Closed

Conversation

BaoshanPang
Copy link

@BaoshanPang BaoshanPang commented Sep 18, 2019

I saw this error when building rust for vxWorks targets:
/folk/prj-rust-dev/bpang/ala-diab-pb19l/latest/latest/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/rand_core-0.5.1/src/err
or.rs:150:28
|
150 | Error { inner: Box::new(error) }
| ^^^^^^^^^^^^^^^ the trait std::error::Error is not implemented for getrandom::error::Error
|
= note: required for the cast to the object type dyn std::error::Error + std::marker::Send + std::marker::Sync

And after adding "mod error_impls;" the issue disappears.

r? @newpavlov
cc @n-salim

@josephlr
Copy link
Member

josephlr commented Sep 18, 2019

@BaoshanPang thanks for opening this. This is an error caused by my change. Essentially when feature = "std" and target_os = "vxworks", we don't add error_impls when we should.

Ideally we would fix the cfg_if declaration so that mod error_impls is included when feature = "std" is set, instead of this change, but that's going to make things messier. So I guess this change is OK.

Note that we're going to remove the automatic error_impls inclusion in 0.2. However, this shouldn't affect you as you're only using getrandom though rand/rand_core which sets the features correct regardless.

Can you rebase this change onto the current master, then I can approve and merge it.

@josephlr josephlr self-requested a review September 18, 2019 02:46
@josephlr
Copy link
Member

This should just be part of #86, as that PR hasn't merged yet. However, this is an important point, and should be accounted for in review.

@josephlr josephlr closed this Sep 18, 2019
@josephlr josephlr removed their request for review September 18, 2019 03:18
@josephlr josephlr mentioned this pull request Sep 18, 2019
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.

4 participants