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

Replace libstd with spin crate in cpu::cache_detected_features. #759

Merged
merged 1 commit into from
Jan 31, 2019

Conversation

briansmith
Copy link
Owner

@briansmith briansmith commented Jan 15, 2019

Eliminate one of the two remaining problems with #![no_std] support
and reduce platform variance.

@elichai
Copy link

elichai commented Jan 17, 2019

It seems that the failure of the test here is because of a network problem, I would just try to rerun it.
https://travis-ci.org/briansmith/ring/jobs/479666488

@elichai
Copy link

elichai commented Jan 17, 2019

@briansmith If you could use version 0.4.10 instead it would be great, because 0.5.0 doesn't have feature(min_const_fn) and it's still an unstable feature in nightly-2018-10-01, so it doesn't compile at that version.
Then this would work with my PR to support sgx: #764

Cargo.toml Outdated
@@ -297,6 +297,9 @@ name = "ring"
[dependencies]
untrusted = "0.6.2"

[target.'cfg(not(target_os = "ios"))'.dependencies]
spin = { version = "0.5.0", default-features = false }
Copy link

Choose a reason for hiding this comment

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

Suggested change
spin = { version = "0.5.0", default-features = false }
spin = "0.4.10"

This would support @baidu/rust-sgx-sdk and there are no features in spin so that disabling default-features isn't needed

Copy link
Owner Author

Choose a reason for hiding this comment

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

With this:

[target.'cfg(not(target_os = "ios"))'.dependencies]
spin = { version = "0.4.10", features = ["once"] }

I get:

error[E0554]: #![feature] may not be used on the stable release channel
#![cfg_attr(feature = "const_fn", feature(const_fn))]

So we have to use spin 0.5 in order for it to work on the stable channel.

IMO it would be much better to have the Baidu SGX SDK update the version of Rust it uses to a newer version. Is that not going to happen any time soon?

Copy link

Choose a reason for hiding this comment

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

That's too bad.
I hope it will be soon
apache/incubator-teaclave-sgx-sdk#48

Copy link

Choose a reason for hiding this comment

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

So this isn't relevant anymore, Sorry for the trouble.
I still don't think the default-features = false, I see no features in the toml: https://github.com/mvdnes/spin-rs/blob/master/Cargo.toml

Copy link
Owner Author

Choose a reason for hiding this comment

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

I still don't think the default-features = false, I see no features in the toml: https://github.com/mvdnes/spin-rs/blob/master/Cargo.toml

Thanks! I updated the PR to remove default-features = false.

@briansmith
Copy link
Owner Author

So, we can go ahead and do this, but if we do, this would be the first and only non-rust-lang crate that ring depends on. OTOH lazy_static does depend on spin itself it any crate has activated the spin_no_std feature, and we already depend on lazy_static (though we don't active spin_no_std ourselves).

Eliminate one of the two remaining problems with `#![no_std]` support
and reduce platform variance.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 93.627% when pulling 4925653 on b/no_std_cpu-2 into c7f0ead on master.

4 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 93.627% when pulling 4925653 on b/no_std_cpu-2 into c7f0ead on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 93.627% when pulling 4925653 on b/no_std_cpu-2 into c7f0ead on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 93.627% when pulling 4925653 on b/no_std_cpu-2 into c7f0ead on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 93.627% when pulling 4925653 on b/no_std_cpu-2 into c7f0ead on master.

@coveralls
Copy link

coveralls commented Jan 29, 2019

Coverage Status

Coverage increased (+0.01%) to 93.627% when pulling 4925653 on b/no_std_cpu-2 into c7f0ead on master.

@briansmith briansmith merged commit 154ca29 into master Jan 31, 2019
@briansmith briansmith deleted the b/no_std_cpu-2 branch January 31, 2019 09:18
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