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

Fix benchmarks build #995

Merged
merged 4 commits into from
Mar 14, 2023

Conversation

iamnotacake
Copy link
Contributor

@iamnotacake iamnotacake commented Mar 6, 2023

One of dev dependencies pulls rayon 1.7.0, and its dependency rayon-core 1.11.0 now requires rust 1.59+

Freeze rayon so it will pull a bit older rayon-core

See failed job for the cause of this PR

Checklist

  • Change is covered by automated tests
  • Benchmark results are attached (if applicable)
  • The coding guidelines are followed
  • Public API has proper documentation
  • Example projects and code samples are up-to-date (in case of API changes)
  • Changelog is updated (in case of notable or breaking changes)

One of dev dependencies pulls rayon 1.7.0, and its dependency
rayon-core 1.11.0 now requires rust 1.59+
@Lagovas Lagovas mentioned this pull request Mar 6, 2023
6 tasks
@iamnotacake iamnotacake marked this pull request as ready for review March 6, 2023 19:23
Copy link
Contributor

@vixentael vixentael left a comment

Choose a reason for hiding this comment

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

@ilammy wdyt

@ilammy
Copy link
Collaborator

ilammy commented Mar 7, 2023

I don't think this fixes the issue. RustThemis CI jobs didn't run here, but I'm thinking they won't pass.

rust-version in Cargo.toml only instructs Cargo to fail compilation earlier instead of when reaching the dependency. It does not make the new version auto-install or anything like that.

The linked job fails because one of the transitive dependencies requires Rust 1.59 now. I see three ways around it:

  1. Bump our MSRV to 1.59, don't bother.
  2. Pin that rayon/rayon-core dependency to an older version in Cargo.toml of benchmarks. Unpin when we bump MSRV.
  3. Kick the benchmarks out of the top-level workspace, test them separately with only stable Rust.

@Lagovas
Copy link
Collaborator

Lagovas commented Mar 10, 2023

so, lets pin version of the dependency? can we do it, @iamnotacake ?

benches/themis/Cargo.toml Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Add back lines that were previously removed by mistake
@iamnotacake iamnotacake merged commit 0bd5aac into cossacklabs:master Mar 14, 2023
@iamnotacake iamnotacake deleted the fix-benchmarks-build branch March 14, 2023 13:10
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