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 to allow building with latest Rust version #31

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

BigBadE
Copy link

@BigBadE BigBadE commented Sep 14, 2024

Rust changing features broke proc, which broke any crates or dependencies using it. This PR simply updates to the latest version of each dependency to fix this issue.

Changes:

  • Swapped to bincode 2.0.0, switching encode/decode methods to their 2.0.0 serde equivalent
  • Fixed tests to use modern rust assert_eq and its variants
  • swapped u32::max_value() to the modern u32::MAX suggested equivalent

I ran all tests, no errors.

Copy link
Member

@cecton cecton left a comment

Choose a reason for hiding this comment

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

That is awesome thanks a lot! Can you fix the remaining issue? The CI must be green I can't just leave a ❎ like that

@@ -18,7 +18,7 @@ include = ["src/**/*.rs", "tests/fixtures/*.img", "README.md", "LICENSE.Apache-2

[dependencies]
bitvec = "1.0.1"
bincode = "1.0.1"
serde = { version = "1.0.116", features = ["derive"] }
serde-big-array = ">= 0.4.1, < 0.6"
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this shouldn't be upgraded now. Can you check the different versions to see which ones work?

Copy link
Author

Choose a reason for hiding this comment

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

Why do you think it shouldn't be upgraded?

Copy link
Member

Choose a reason for hiding this comment

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

No I meant the opposite. Should this be upgraded too, to 0.6 or up idk

Copy link
Author

Choose a reason for hiding this comment

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

There is no 0.6, 0.5.1 is the latest (https://docs.rs/serde-big-array/latest/serde_big_array/).

@BigBadE
Copy link
Author

BigBadE commented Sep 17, 2024

There we go, clippy warnings fixed

@BigBadE
Copy link
Author

BigBadE commented Sep 17, 2024

Ah, so that's why. Up to @cecton whether to bump the MSRV or to go back down to a 1.x bincode version, which I assume would work on the latest Rust still.

@cecton
Copy link
Member

cecton commented Sep 18, 2024

Ah, so that's why. Up to @cecton whether to bump the MSRV or to go back down to a 1.x bincode version, which I assume would work on the latest Rust still.

Yes I guess since bincode 2 is still just a release candidate, we can't really make a stable release of mbrman like that. Once they finally release it then we can

@BigBadE
Copy link
Author

BigBadE commented Sep 18, 2024

It seems fine for a stable release, given it's just legacy bincode 1 code, it would just require upping the MSRV to match bincode 2's MSRV

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants