delete some terrible code that was written to work around cargo bugs #2503
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cbeck88
requested review from
jcape,
eranrund,
sugargoat,
nick-mobilecoin and
a team
nick-mobilecoin
approved these changes
Sep 10, 2022
|
Can you please rebase this? |
many years ago, we encountered this bug: rust-lang/cargo#6571 This bug meant that if we wanted to have a single Rng type that worked on all platforms, including SGX, then it could not use the standard library on any platform, because cargo would evaluate dependencies without respect to what platform you are on. However, it was really important for our code that we had such an abstraction layer. This was important for writing enclave impl code that could run in SGX and also in unit tests for instance. The way we solved this issue was that, we took the current version of `ThreadRng` which is the generically recommendable cryptographic RNG type from `rand` crate, and made the smallest possible changes to it until it would build without the standard library. The main change was replacing `std::thread_local!` with the `#[thread_local]` attribute, which turned out to be straightforward. However, it creates an annoying maintenance burden on us, and there has been a lot of churn in the rand crate since then. Since the `resolver = 2` stuff was stabilized, the original problem is no longer the case. It's fine to have crates that pull in `std` in one platform but not another. So we can now just use `ThreadRng` as the no-RDRAND fallback, like we wanted all along.
cbeck88
force-pushed
the
crypto-rand-cleanup
branch
from
0c108d7 to
ce0929f
Compare
eranrund
approved these changes
Sep 13, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
many years ago, we encountered this bug:
rust-lang/cargo#6571
This bug meant that if we wanted to have a single Rng type that worked on all platforms, including SGX, then it could not use the standard library on any platform, because cargo would evaluate dependencies without respect to what platform you are on.
However, it was really important for our code that we had such an abstraction layer. This was important for writing enclave impl code that could run in SGX and also in unit tests for instance.
The way we solved this issue was that, we took the current version of
ThreadRngwhich is the generically recommendable cryptographic RNG type fromrandcrate, and made the smallest possible changes to it until it would build without the standard library. The main change was replacingstd::thread_local!with the#[thread_local]attribute, which turned out to be straightforward. However, it creates an annoying maintenance burden on us, and there has been a lot of churn in the rand crate since then.Since the
resolver = 2stuff was stabilized, the original problem is no longer the case. It's fine to have crates that pull instdin one platform but not another. So we can now just useThreadRngas the no-RDRAND fallback, like we wanted all along.