-
Notifications
You must be signed in to change notification settings - Fork 64
refactor: Replace bincode with postcard #99
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #99 +/- ##
==========================================
- Coverage 67.41% 67.25% -0.16%
==========================================
Files 18 18
Lines 2145 2138 -7
==========================================
- Hits 1446 1438 -8
- Misses 699 700 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi! Thanks for filing your merge request. I appreciate you taking a crack at removing the advisory warning! As you might have seen from my comment on the issue, I am more after a patch like this, without any runtime deserialization at all: diff --git a/build.rs b/build.rs
index 44840cd..0512c95 100644
--- a/build.rs
+++ b/build.rs
@@ -1,5 +1,6 @@
use std::env;
use std::fs::File;
+use std::io::Write;
use std::io::{BufReader, BufWriter};
use std::path::Path;
@@ -17,15 +18,17 @@ fn main() {
File::open(pnm_path).expect("could not open metadata file"),
))
.expect("failed to load metadata");
+ let metadata = metadata.as_slice();
+
+ // TODO: remove
+ let metadata = &metadata[..4];
+
println!("cargo:rerun-if-changed={pnm_path}");
let mut out = BufWriter::new(
- File::create(Path::new(&env::var("OUT_DIR").unwrap()).join("database.bin"))
+ File::create(Path::new(&env::var("OUT_DIR").unwrap()).join("database.rs"))
.expect("could not create database file"),
);
- bincode::options()
- .with_varint_encoding()
- .serialize_into(&mut out, &metadata)
- .expect("failed to serialize database");
+ write!(out, "pub const DATABASE: &[Metadata] = &{:?};", metadata).expect("write database");
}
diff --git a/src/metadata/database.rs b/src/metadata/database.rs
index 46ae55e..24dca0c 100644
--- a/src/metadata/database.rs
+++ b/src/metadata/database.rs
@@ -15,8 +15,6 @@
use crate::error;
use crate::metadata::loader;
use crate::Metadata;
-use bincode;
-use bincode::Options;
use fnv::FnvHashMap;
use once_cell::sync::Lazy;
use regex_cache::{CachedRegex, CachedRegexBuilder, RegexCache};
@@ -27,18 +25,17 @@ use std::io::{BufReader, Cursor};
use std::path::Path;
use std::sync::{Arc, Mutex};
-const DATABASE: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/database.bin"));
+mod database {
+ use crate::metadata::loader::*;
+ // `database.rs` contains
+ // pub const DATABASE: &[Metadata] = &[ .. ];
+ include!(concat!(env!("OUT_DIR"), "/database.rs"));
+}
+
+use database::DATABASE;
/// The Google provided metadata database, used as default.
-pub static DEFAULT: Lazy<Database> = Lazy::new(|| {
- Database::from(
- bincode::options()
- .with_varint_encoding()
- .deserialize(DATABASE)
- .unwrap(),
- )
- .unwrap()
-});
+pub static DEFAULT: Lazy<Database> = Lazy::new(|| Database::from(DATABASE.to_vec()).unwrap());
/// Representation of a database of metadata for phone number.
#[derive(Clone, Debug)]
This has quite some implications regarding borrowing (preferably we'd be on |
|
My suggestion requires quite some refactoring though: the |
|
I spent a bit of time seeing how far it trickles down... Probably this really warrants a complete rewrite of the database loading system as a whole, splitting up the loader into a separate crate for use in the main crate's Could you have a look at why the |
fb99979 to
593759f
Compare
|
Would you like me to open a PR to update the version so this can be released? |
|
Nah, is fine, I'll do it in a few minutes :-) |
I saw #98 and figured I would take a crack at moving away from
bincode. I went withpostcardas the replacement, as it is essentially a drop-in replacement forbincodegiven howphonenumberused it.postcardalso uses varint encoding by default, which is a nice plus.The only major difference (from what I can tell) is that the serialized format is different from bincode, but that shouldn't matter.