Skip to content

Enable v0.1 std-trait workaround for additional targets #169

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

Merged
merged 6 commits into from
Dec 7, 2020

Conversation

josephlr
Copy link
Member

Fixes #168

The first commit cleans up how we handle our various helper modules (similar to what we did for v0.2), but does not make any functional changes.

The second commit enables our std::error::Error implementations for windows, target_os = "fuchsia", target_os = "ios". These are the additional windows and unix targets we supported in v0.1.6. We don't enable the workaround for target_arch = "wasm32" as that would be a breaking change (from newer version) for no_std wasm32 targets.

Here is a history of when we've enabled std::error::Error implementations:

  • v0.1.0 - v0.1.6:
    • Initially, we enabled the std feature on most targets
    #[cfg(any(
      feature = "std",
      windows, unix,
      target_os = "redox",
      target_arch = "wasm32",
    ))]
    
  • v0.1.7 - v0.1.8:
    • We later restricted the implementations to certain unix targets that needed util_libc
    #[cfg(any(
      feature = "std",
      target_os = "android",
      target_os = "dragonfly",
      target_os = "emscripten",
      target_os = "freebsd",
      target_os = "haiku",
      target_os = "illumos",
      target_os = "linux",
      target_os = "macos",
      target_os = "netbsd",
      target_os = "openbsd",
      target_os = "redox",
      target_os = "solaris",
    ))]
    
  • v0.1.9 - v0.1.10:
    • We further restricted things to only the "std" feature
    #[cfg(feature = "std")]
    
  • v0.1.11 - v0.1.15
    • We rolled back our v0.1.9 changes (as they were a breaking change)
    #[cfg(any(
      feature = "std",
      target_os = "android",
      target_os = "dragonfly",
      target_os = "emscripten",
      target_os = "freebsd",
      target_os = "haiku",
      target_os = "illumos",
      target_os = "linux",
      target_os = "macos",
      target_os = "netbsd",
      target_os = "openbsd",
      target_os = "redox",
      target_os = "solaris",
    ))]
    

Verified

This commit was signed with the committer’s verified signature.
josephlr Joe Richey
This makes it easier to see what targets use which internal modules.
It also makes our std-traits workaround more clear.

Signed-off-by: Joe Richey <joerichey@google.com>

Verified

This commit was signed with the committer’s verified signature.
josephlr Joe Richey
From `v0.1.0` to `v0.1.6` we had this enabled for windows/unix, and we
inadvertantly removed it.

Signed-off-by: Joe Richey <joerichey@google.com>
@josephlr josephlr requested a review from newpavlov October 28, 2020 20:26

Verified

This commit was signed with the committer’s verified signature.
josephlr Joe Richey
Signed-off-by: Joe Richey <joerichey@google.com>
@josephlr
Copy link
Member Author

@pie-flavor can you verify that this fixes your problem.

@newpavlov
Copy link
Member

newpavlov commented Nov 12, 2020

Though I do not understand why the UWP build fails in the CI job.

UPD: It looks like for some reason among UWP targets only thumbv7a-uwp-windows-msvc has officially built std, so we may have to use all(windows, not(getrandom_uwp)) which enables std::Error impl. IIRC UWP support was added after the v0.1.8 release, so it will stay backward compatible.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@newpavlov newpavlov merged commit b33dcdd into rust-random:0.1 Dec 7, 2020
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.

None yet

2 participants