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

Drop rustc-serialize dependency #8

Closed
dtolnay opened this issue May 21, 2017 · 6 comments
Closed

Drop rustc-serialize dependency #8

dtolnay opened this issue May 21, 2017 · 6 comments
Assignees

Comments

@dtolnay
Copy link

dtolnay commented May 21, 2017

It has been deprecated: announcement.

Is there any functionality you would require from another library before this would be possible?

@tatsuya6502 tatsuya6502 self-assigned this May 21, 2017
@tatsuya6502
Copy link
Member

tatsuya6502 commented May 22, 2017

Thanks for the heads-up. This seems an easy change. I will give it a try.

Here are some background:

  • We use rmp-serialize crate to serialize/deserialize key-value metadata stored in RocksDB. rmp-serialize is the rustc-serialize binding of rmp (Rust MessagePack) crate.
    • -> rmp has a serde flavor rmp-serde from the same author, so we will switch to it.
  • We also use ToHex trait in rustc-serialize to convert a [u8] value into a hexadecimal String.
    • -> If rmp-serialize's license permits, we will copy the source file (hex.rs) to this repository.
    • -> EDIT: We will use hex crate.
    • -> EDIT2: We used data-encoding crate.

@tatsuya6502
Copy link
Member

tatsuya6502 commented May 22, 2017

Note: This project depends on rust-crypto, which also depends rustc-serialize. I will check if they are planning to drop the dependency.

EDIT: I found this pull request: rust-crypto - Remove rustc-serialize usage #415

@dtolnay
Copy link
Author

dtolnay commented May 22, 2017

Yes, here is the PR to drop rustc-serialize in rust-crypto DaGenix/rust-crypto#415. But there have been 2 commits to rust-crypto in the past 12 months so it looks abandoned to me. Maybe consider a different library?

@tatsuya6502
Copy link
Member

tatsuya6502 commented May 22, 2017

But there have been 2 commits to rust-crypto in the past 12 months so it looks abandoned to me. Maybe consider a different library?

Uh, ok. We can use any crate that provides md5 digest calculation. We use it to verify the data in storage file is not corrupted. (I am not sure if md5 is the best. I need a fast and reliable digest algorithm to check data corruption)

@tatsuya6502
Copy link
Member

(I am not sure if md5 is the best. I need a fast and reliable digest algorithm to check data corruption)

Opened #9 "Replace MD5 checksum on blob with a safe hasher".

tatsuya6502 added a commit that referenced this issue May 22, 2017
Drop rustc-serialize dependency (fix #8)
@tatsuya6502
Copy link
Member

tatsuya6502 commented May 22, 2017

Conclusion

  • For RMP (Rust MessagePack): replaced rmp-serialize (rustc-serialize binding) with rmp-serde (Serde binding). This crate is used to serialize/deserialize key and metadata stored in RocksDB.
  • Replaced rust-crypto crate with md-5 and data-encoding generic-array crates. These crates are used to calculate MD5 checksum for blob.
  • Replaced direct dependency to rustc-serialize with data-encoding. This crate used to convert raw digest bytes to hexadecimal String.

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

No branches or pull requests

2 participants