-
Notifications
You must be signed in to change notification settings - Fork 369
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
1.5.2 breaks serialization for formats like "bincode" #414
Comments
I'll revert this change tonight, and then we can consider whether there's a better way to accomplish the same thing. Do you have time to put together a small program that reproduces the break you're seeing? |
Changes since 1.5.2: - Revert the serialization change. It was intended to be backwards compatible, but that didn't hold for non-self-describing serialization formats like bincode. See #414.
I've published v1.5.3 with the revert. Let me know if you can confirm that it fixes your system. |
Yes it does fix it! I think a better way of doing this is probably to check whether the format is human readable, which might be a good proxy for whether it's self-describing. But I think probably the best way is to release 2.0 instead. |
CBOR, JSONB and MessagePack would be missed by that check, right? |
This mostly reverts commits 8416b16 and dd0afd6. Changing the serialization of Hash can only be backwards-compatible in self-describing formats like CBOR. In non-self-describing formats like bincode, the deserializer has to know in advance which serialization format was used. Fixes BLAKE3-team#414. Reopens BLAKE3-team#412.
Changes since 1.5.2: - Revert the serialization change. It was intended to be backwards compatible, but that didn't hold for non-self-describing serialization formats like bincode. See BLAKE3-team#414.
This is not backwards compatible for formats like "bincode", where it's not possible to first try to deserialize as an array and then try deserialization as a bytestring. Previously serialized and saved
Hash
es will now fail on deserialization.This has actually broken a large production system I run that currently must be pegged to 1.5.1 as a result. Please at least make this feature optional, or enable it only for human-readable serde formats like JSON.
The text was updated successfully, but these errors were encountered: