Skip to content

Prepare rand_core 0.6.0#1072

Merged
dhardy merged 5 commits into
rust-random:masterfrom
dhardy:work
Dec 14, 2020
Merged

Prepare rand_core 0.6.0#1072
dhardy merged 5 commits into
rust-random:masterfrom
dhardy:work

Conversation

@dhardy

@dhardy dhardy commented Dec 7, 2020

Copy link
Copy Markdown
Member

Unfinished:

  • rand_chacha
  • rand_hc
  • rand_pcg

rand_chacha: #1023 re-enabled a dependency on the simd feature of ppv-lite86 since at the time that wasn't enabled-by-default, but this should be fixed now: cryptocorrosion/cryptocorrosion#36. So I think we should remove this feature dependency again since it is deprecated.

@dhardy

dhardy commented Dec 7, 2020

Copy link
Copy Markdown
Member Author

Hmm, we could force a dependency on ppv-lite >= 0.2.10 to properly resolve the issue, or we could be lax and let cargo update do its thing (but inevitably some users will be left out). I'd prefer the stricter dependency requirement. CC @kazcw

@dhardy

dhardy commented Dec 7, 2020

Copy link
Copy Markdown
Member Author

I disabled the cargo deadlinks check since we can't fix those errors right now. See also rust-lang/rust#77200

Changelogs have today's date, but I'm not certain the release will happen today (esp. if review is required).

@dhardy dhardy requested a review from vks December 7, 2020 17:10
@kazcw

kazcw commented Dec 7, 2020

Copy link
Copy Markdown
Contributor

@dhardy Either approach to proactively removing the simd flag from the ppv-lite dependency could degrade the experience of some rand users. When I meant by deprecating the simd flag is that new code has no reason to use it, but it wouldn't hurt anything to leave it for continued compatibility without older versions of the crate.

@jyn514

jyn514 commented Dec 7, 2020

Copy link
Copy Markdown

@dhardy the deadlinks failure isn't related to rustdoc at all, it's deadlinks/cargo-deadlinks#120. You can ignore it for now with --ignore-fragments, which will still check the links exist, just not the URL fragments.

@jyn514

jyn514 commented Dec 7, 2020

Copy link
Copy Markdown

Also, FWIW you can simplify the build script by running cargo +nightly deadlinks -- --all-features --all instead of running cargo doc first. That will also respect doc = false if you have it set somewhere.

@dhardy

dhardy commented Dec 7, 2020

Copy link
Copy Markdown
Member Author

@kazcw I tested with ppv-lite86 0.2.9 and 0.2.10 and clearly the fix works. That said we've no reason to do this (until ppv-lite86 0.3) so I guess you're right that removing the change is better.

@jyn514 the link failure is real (from POV of rand_distr which is part of this repo), so I don't think it is that?

@jyn514

jyn514 commented Dec 7, 2020

Copy link
Copy Markdown

@dhardy this is the file I see for target/doc/rand/rng/trait.Rng.html:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta http-equiv="refresh" content="0;URL=../../rand/trait.Rng.html">
</head>
<body>
    <p>Redirecting to <a href="../../rand/trait.Rng.html">../../rand/trait.Rng.html</a>...</p>
    <script>location.replace("../../rand/trait.Rng.html" + location.search + location.hash);</script>
</body>
</html>

What do you see? It exists, because the error reported is about a broken fragment, not a missing file.

@kazcw

kazcw commented Dec 7, 2020

Copy link
Copy Markdown
Contributor

@dhardy I have no doubt that 0.2.10 works correctly. I was referring to the cost to some users of forcing a dependency upgrade

@vks

vks commented Dec 7, 2020

Copy link
Copy Markdown
Contributor

I was referring to the cost to some users of forcing a dependency upgrade

I'm not sure what the cost of upgrading from ppv-lite86 0.2.8 to 0.2.10 is. The difference is only some patch releases, so cargo update should be enough?

Comment thread rand_chacha/CHANGELOG.md Outdated
## [0.3.0] - 2020-12-07
- Bump `rand_core` version to 0.6.0
- Bump MSRV to 1.36 (#1011)
- Remove usage of deprecated feature "simd" of `ppv-lite86` (#979), revert

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a bit confusing. What exactly is reverted?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The previous change (removal of feature usage)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe write 'Use deprecated feature "simd" of ppv-lite86 for improved compatibility'?

@dhardy

dhardy commented Dec 8, 2020

Copy link
Copy Markdown
Member Author

@jyn514 aha... I didn't realise doc/rand_distr/struct.Uniform.html links to <a href="../rand/rng/trait.Rng.html#method.gen_range">. Thanks for the help.

dhardy added a commit to dhardy/rand that referenced this pull request Dec 8, 2020
@dhardy

dhardy commented Dec 8, 2020

Copy link
Copy Markdown
Member Author

Force update:

  • update dates
  • remove the change on ppv-lite86 dependency feature
  • use --ignore-fragments on cargo-deadlinks

@dhardy

dhardy commented Dec 8, 2020

Copy link
Copy Markdown
Member Author

So much for fixing up the Travis CI. We don't get much notice do we: https://www.reddit.com/r/programming/comments/k8v6u2/travis_ci_is_no_longer_providing_ci_minutes_for/

Comment thread .travis.yml Outdated

@vks vks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good!

@dhardy

dhardy commented Dec 14, 2020

Copy link
Copy Markdown
Member Author

Yeah, but I want to rebase after porting CI. Travis dropped our jobs just before validating this one. 😒

@dhardy dhardy merged commit 98e220c into rust-random:master Dec 14, 2020
@dhardy

dhardy commented Dec 14, 2020

Copy link
Copy Markdown
Member Author

Published!

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