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

support Rust 1.70 as MSRV #1206

Merged
merged 2 commits into from
Feb 26, 2024
Merged

support Rust 1.70 as MSRV #1206

merged 2 commits into from
Feb 26, 2024

Conversation

davidhewitt
Copy link
Contributor

@davidhewitt davidhewitt commented Feb 26, 2024

Change Summary

This PR sets Rust 1.70 as the minimum supported version. To go lower, we would need to replace use of OnceLock with external once_cell::sync::OnceCell both here and in jiter.

I think going below 1.65 would be hard because we use both GATs and let-else from that release.

Related issue number

Related to #1176

Checklist

  • Unit tests for the changes exist
  • Documentation reflects the changes where applicable
  • Pydantic tests pass with this pydantic-core (except for expected changes)
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @samuelcolvin

Copy link

codspeed-hq bot commented Feb 26, 2024

CodSpeed Performance Report

Merging #1206 will degrade performances by 23.75%

Comparing dh/rust-msrv (63aad1a) with main (47aff70)

Summary

❌ 3 regressions
✅ 145 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main dh/rust-msrv Change
test_core_future 47.7 µs 55.3 µs -13.76%
test_core_future_str 27 µs 35.5 µs -23.75%
test_small_class_core_dict 31.9 µs 38.8 µs -17.77%

@davidhewitt
Copy link
Contributor Author

please review

Copy link
Member

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

Looks good overall, just left one question

@@ -24,6 +24,7 @@ include = [
"!tests/.pytest_cache",
"!*.so",
]
rust-version = "1.70"
Copy link
Member

Choose a reason for hiding this comment

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

Does setting this set the min version, or the required version?

Copy link
Contributor Author

@davidhewitt davidhewitt Feb 26, 2024

Choose a reason for hiding this comment

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

A minimum version, and at the moment all this will do is cause Cargo to fail the build if the installed rust version is lower.

Future Rust versions might do more here (e.g. fail the build if we accidentally use stuff newer than this version).

@davidhewitt davidhewitt merged commit 7779206 into main Feb 26, 2024
27 of 28 checks passed
@davidhewitt davidhewitt deleted the dh/rust-msrv branch February 26, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants