-
Notifications
You must be signed in to change notification settings - Fork 20
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add rust-version field to Cargo.toml
- Loading branch information
Showing
1 changed file
with
2 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
241eaed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason for this change? Libs depending on this package will require the latest rust version and fail as it takes a while for that version to end up on default build nodes.
https://github.com/francisdb/vpin/actions/runs/9534568985/job/26280157347
Adding github actions builds to this project would indicate this issue.
241eaed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for this change is that the latest version of Rust allows for building read/write method variants which can catch overreads/overwrites much sooner than before. For example, this will compile:
but it always fails at runtime even though the number of bits being read is always the same.
However, using a new read method variant which takes a constant number of bits:
this is an error that will be caught at compile-time using assertions in the new const blocks which just stabilized.
And catching possible errors at compile-time rather than at run-time seems rather helpful in the long run.
241eaed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Makes sense. I guess the issue is then that this is not a major version change or one of the libs I use having too loose dependency versions to bitstream-io. Anyway, should fix itself once GitHub catches up.
241eaed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requiring a newer Rust version is not a breaking change, so this is all working as intended. The main problem is that cargo does not implement Rust-version-dependent dependency resolution yet, see rust-lang/cargo#13873 etc.
This means that currently cargo will (for now by default) happily pull in newer versions of dependencies even if an older Rust version is used.