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

Better error if binary does not match version #20

Open
drahnr opened this issue Feb 4, 2021 · 8 comments
Open

Better error if binary does not match version #20

drahnr opened this issue Feb 4, 2021 · 8 comments
Labels
P2 Medium priority

Comments

@drahnr
Copy link
Contributor

drahnr commented Feb 4, 2021

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: InvalidBoolEncoding(2)', src/checker/nlprules.rs:22:70

static TOKENIZER_BYTES: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/tokenizer.bin"));
static RULES_BYTES: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/rules.bin"));

lazy_static::lazy_static! {
    static ref TOKENIZER: Tokenizer = Tokenizer::from_reader(&mut &*TOKENIZER_BYTES).unwrap();
    static ref RULES: Rules = Rules::from_reader(&mut &*RULES_BYTES).unwrap(); /// <<< this one errors, and it's always this one
}
@drahnr
Copy link
Contributor Author

drahnr commented Feb 4, 2021

@bminixhofer
Copy link
Owner

Which binaries are you using? This looks like an issue with an outdated binary.

At the moment I don't want to make any stability guarantees of the same binaries being usable across versions because things in the core are still prone to change quickly. So the binary for version 0.3.0 does not work with the latest commit to master.

I think the right course of action here is a better error message that informs which version of the binary you are using and which version of nlprule; that requires some sort of compatibility to read the version but that's doable.

In the meantime you can download the unreleased binaries for the latest commit to master at the corresponding GH action output: https://github.com/bminixhofer/nlprule/actions/runs/531964158

@drahnr
Copy link
Contributor Author

drahnr commented Feb 4, 2021

If that's the case, then the helper API would really make sense, so issues like these won't pop up again, but compatibility is guaranteed by API.

@bminixhofer
Copy link
Owner

I agree 100%. Still it should also have a better error message and i. e. read the version as the first x bytes of the binary and if it does not match stop right away.

At the moment I'm working on #15 but a build.rs and from_lang_code (or equivalent) API will be part of the next release.

@bminixhofer bminixhofer changed the title Latest git causes content issues Better error if binary does not match version Feb 4, 2021
@bminixhofer
Copy link
Owner

I'll implement this soon. I think using the first 64 (or 128, 256, but I think 64 is enough) bytes of each binary to store the version encoded as UTF8 is the best way. I'm not too sure if there isn't maybe a more standard way so I'd appreciate a second opinion @drahnr :)

@bminixhofer bminixhofer added the P2 Medium priority label Feb 21, 2021
@drahnr
Copy link
Contributor Author

drahnr commented Feb 22, 2021

I think kiss is the best approach here. Reserving 8 bytes and using them as big endian u64 for a running number and using a static var to reference this. As alternative, one could make the files part of an archive instead of just compressing them and adding two more entries/files, VERSION and GITSHA. The first one is leaner, but I'd prefer the latter, since the resulting format is inspectable with common tooling available.

@bminixhofer
Copy link
Owner

one could make the files part of an archive instead of just compressing them and adding two more entries/files, VERSION and GITSHA

Oh that's a good idea! I have to check how much overhead there is from tarring / untarring (also w.r.t dependencies) but if it's reasonably small then that's definitely a better approach.

@drahnr
Copy link
Contributor Author

drahnr commented Feb 22, 2021

The rust cookbook contains a few examples which might be helpful https://rust-lang-nursery.github.io/rust-cookbook/compression/tar.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Medium priority
Projects
None yet
Development

No branches or pull requests

2 participants