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

Documentation says varint is the default, but it isn't unless you use Options #348

Closed
joshtriplett opened this issue Aug 31, 2020 · 3 comments · Fixed by #373
Closed

Documentation says varint is the default, but it isn't unless you use Options #348

joshtriplett opened this issue Aug 31, 2020 · 3 comments · Fixed by #373

Comments

@joshtriplett
Copy link

The documentation at https://docs.rs/bincode/1.3.1/bincode/config/trait.Options.html says that the default int encoding is varint. If you use DefaultOptions, that's true: bincode::DefaultOptions::new().serialize_into(...) will use varint encoding. However, the top-level functions like bincode::serialize_into say they use "the default configuration", but they appear to use fixint encoding.

@pythonesque
Copy link

Also (piggybacking on this issue since it's related), if the default configuration were to change to varint, it should be a breaking change--while it's true that the interface is still the same, trying to read data serialized using the fixed int encoding (before 1.3) will otherwise be incompatible with the 1.3 release by default, which is a problem for a serialization crate since messages in the old format can be stored in files.

So I suggest performing a major version bump before actually making this the default (I also didn't check whether the Options interface was new to 1.3, but if it was then the upgrade to 1.3 probably should have been a major version update regardless).

@ZoeyR
Copy link
Collaborator

ZoeyR commented Sep 11, 2020

@pythonesque 1.3 introduced a new options interface, this was not a breaking change since the old interface was left in tact and still works the same way. For a similar reason the basic bincode::serialize methods were left with the old configuration. Changing their behavior would be a breaking change and I am committed not to break the crate without a very good reason. The documentation should be clearer though so I will work on updating that.

@joshtriplett
Copy link
Author

joshtriplett commented Sep 15, 2020 via email

epilys added a commit to meli/meli that referenced this issue Nov 8, 2020
Previous commit changed bincode deserializes in maildir and sqlite3.rs
from bincode::deserialize_from to using bincode::config::DefaultOptions
and bincode::Options trait's method deserialize_from.

However, these two different deserializes use a different default
settings: bincode-org/bincode#348

Specifically, varint encoding for integers is the default for
DefaultOptions but not when using bincode::{de,}serialize_* functions.
That means that serialized structs were not able to be deserialized.
This commit makes all {de,}serializations use the DefaultOptions
interface rather than the top level functions.
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 a pull request may close this issue.

3 participants