Skip to content

Upgrade bincode to released version 2 and fix compilation when disabled #21

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

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

Conversation

markbt
Copy link

@markbt markbt commented Apr 9, 2025

Currently the build of plexi-core fails when the bincode feature is disabled:

$ cargo check --no-default-features
error[E0004]: non-exhaustive patterns: `Ciphersuite::BincodeEd25519` not covered
   --> plexi_core/src/lib.rs:487:15
    |
487 |         match self.version {
    |               ^^^^^^^^^^^^ pattern `Ciphersuite::BincodeEd25519` not covered
    |
note: `Ciphersuite` defined here
   --> plexi_core/src/lib.rs:50:10
    |
50  | pub enum Ciphersuite {
    |          ^^^^^^^^^^^
51  |     ProtobufEd25519 = 0x0001,
52  |     BincodeEd25519 = 0x0002,
    |     -------------- not covered
    = note: the matched value is of type `Ciphersuite`

This is because the bincode arm is compiled out if the feature is disabled. Instead, keep the match arm and change it to a runtime error if bincode is not enabled.

Bincode 2 has also been released, so we can upgrade to it. There is a small difference in how the Decode and BorrowDecode traits are defined which we must adapt to.

markbt added 2 commits April 9, 2025 13:48
Disabling the `bincode` feature causes a compilation error because the
`Ciphersuite::BincodeEd25519` variant of the verify `match` is compiled
out.  Instead, make it a runtime error.
Bincode 2 has now been released.  The difference from rc3 is that the `Decode`
and `BorrowDecode` traits now have a generic parameter (`Context`) which we
must also be generic over in our implementations.
@thibmeu
Copy link
Collaborator

thibmeu commented Apr 16, 2025

@markbt Thank you for the commit. Overall looks ok.

The builds have been fixed to work with rust 1.86 in #22. This is required by some of the dependencies you are updating. You should be able to rebase on main to fix them.

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.

3 participants