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

update dependencies and bump MSRV #22

Merged
merged 1 commit into from
Aug 19, 2022
Merged

update dependencies and bump MSRV #22

merged 1 commit into from
Aug 19, 2022

Conversation

decathorpe
Copy link
Contributor

The current MSRV of 1.36 is increasingly outdated, as are some of the dependencies. This PR updates all dependencies to the latest available versions, and bumps the documented MSRV to 1.51 (which is now ~18 months old):

  • serde-big-array 0.4: MSRV 1.51
  • proptest 1: MSRV 1.50
  • byteorder 1.4.x: MSRV 1.41.1
  • bitflags 1.3.x: MSRV 1.46

All linux distributions that I checked should work with Rust 1.51 - except maybe debian stable, which has both Rust 1.48 and 1.51 available.

@vks
Copy link
Owner

vks commented Aug 17, 2022

Thanks, this makes sense! Note however that average is a dev dependency of Rand, so we need to be careful about raising the MSRV.

There are some issues with the tests:

  • The pipeline still needs to be adapted to run Rust 1.51.
  • The minimal version of serde-big-array might need to be stricter to fix the minimal versions test.

Do we even need serde-big-array now that we can use const generics?

@decathorpe
Copy link
Contributor Author

Thanks, this makes sense! Note however that average is a dev dependency of Rand, so we need to be careful about raising the MSRV.

Right, I had not thought about this. Looks like the only "rand project" crate that depens on average is rand_distr, it currently has an MSRV of 1.36. But do dev-dependencies even impact MSRV? They wouldn't even be downloaded when compiling something that depends on rand_distr.

There are some issues with the tests:

* The pipeline still needs to be adapted to run Rust 1.51.

Right, I hadn't considered that the CI would need to be adapted. I can attempt to change the settings.

* The minimal version of `serde-big-array` might need to be stricter to fix the minimal versions test.

The test output looks like a (temporary?) floating point accuracy issue rather than a problem with serde-big-array:

---- moments::simple_serde stdout ----
thread 'moments::simple_serde' panicked at 'assertion failed: `(left == right)`
  left: `"{\"n\":5,\"avg\":3.0,\"m\":[10.0,1.7763568394002506e-15,34.00000000000001]}"`,
 right: `"{\"n\":5,\"avg\":3.0,\"m\":[10.0,1.7763568394002505e-15,34.00000000000001]}"`', tests/integration/moments.rs:58:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

(The only difference between these two strings seems to be the least significant digit, i.e. 6 vs. 5.)

Do we even need serde-big-array now that we can use const generics?

Apparently yes, according to the projects's readme, serde does not support const generics yet.

@vks
Copy link
Owner

vks commented Aug 17, 2022

But do dev-dependencies even impact MSRV?

At least for Rand they do, because we run the tests with the MSRV. However, Rand is going to bump the MSRV to 1.51 with the upcoming 0.9 release as well, so this should be fine to merge.

The test output looks like a (temporary?) floating point accuracy issue rather than a problem with serde-big-array

Not sure it's temporary, floating-point math is still deterministic. Could also be related to the compiler version, but I think the test was passing before, so what else could it be?

@decathorpe
Copy link
Contributor Author

decathorpe commented Aug 17, 2022

But do dev-dependencies even impact MSRV?

At least for Rand they do, because we run the tests with the MSRV. However, Rand is going to bump the MSRV to 1.51 with the upcoming 0.9 release as well, so this should be fine to merge.

Yeah, I suspected as much. But if MSRV is going to be 1.51 with the next release, that's good news 🎉

The test output looks like a (temporary?) floating point accuracy issue rather than a problem with serde-big-array

Not sure it's temporary, floating-point math is still deterministic. Could also be related to the compiler version, but I think the test was passing before, so what else could it be?

Not necessarily ... some results of floating-point operations might depend on the specific CPU model, or the underlying libc implementation, or the implementation in Rust's std. And in this case, floating-point → string conversion also might play a role. I looked at the dependencies, but ryu (which was recently updated) isn't used, and the version of dtoa is really old, so I'm nor really sure what else could be the culprit. Do you know whether CI still passes on the current master branch with current nightly Rust, i.e. without my changes?

@decathorpe
Copy link
Contributor Author

Hum, it looks like the "minimal_versions" test has been failing for a while, even without this PR. I can't look at the logs though, because they're been removed by now.

@vks
Copy link
Owner

vks commented Aug 19, 2022

In any case, if it fails on master, it should not block this PR.

Thanks for contributing!

@vks vks merged commit c70070b into vks:master Aug 19, 2022
@decathorpe
Copy link
Contributor Author

Thanks for merging :)

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.

2 participants