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

cli panics with out of bounds error #2679

Closed
kuviman opened this issue Sep 13, 2021 · 6 comments · Fixed by #2680
Closed

cli panics with out of bounds error #2679

kuviman opened this issue Sep 13, 2021 · 6 comments · Fixed by #2680
Labels

Comments

@kuviman
Copy link

kuviman commented Sep 13, 2021

Describe the Bug

Trying to just compile my project to the web, I stumbled upon this panic message.
I think this may happen because of version mismatch between the cli app and the dependency.
After running cargo update everything works fine.

Steps to Reproduce

I can not really show the code. I may try to provide a simplified example if needed

OS: Windows
Rust version: rustc 1.55.0 (c8dfcfe04 2021-09-06)
wasm-bindgen-cli version: 0.2.77
wasm-bindgen dependency version: 0.2.76

Running command: wasm-bindgen --target=web --no-typescript --out-dir tmp target/wasm32-unknown-unknown/release/theproject.wasm

Expected Behavior

wasm-bindgen does its magic without any errors

Actual Behavior

wasm-bindgen panics with following message:

thread 'main' panicked at 'index out of bounds: the len is 0 but the index is 0', C:\Users\myusername\.cargo\registry\src\github.aaakk.us.kg-1ecc6299db9ec823\wasm-bindgen-cli-support-0.2.77\src\descriptor.rs:205:15

Additional Context

As I said, looks like problems with different versions. I remember wasm-bindgen would just panic instantly with message saying that versions should match. I guess it is not the case anymore? So this took me a while to even think about running cargo update

@kuviman kuviman added the bug label Sep 13, 2021
@alexcrichton
Copy link
Contributor

Yes recently the version-match was removed because it was hoped to get caught by other things and/or not change much. I believe that's the cause of this.

@chinedufn
Copy link
Contributor

Uh oh!

Yeah I worked on this in #2546 with the hope of removing this requirement.

It looks like something is going wrong.. Alex if you already know the issue definitely let me know and I can get it fixed it.

Otherwise I can take a look into this later tonight and figure out what's going wrong!

IIRC this should have actually worked in theory.. so it's probably a minor oversight or something.. hopefully..!

@alexcrichton
Copy link
Contributor

Oh I'm not sure what actually caused this, I'd assume that something about the schema probably changed and the version number of the schema was forgotten to get bumped, but I would have to track things down to be sure.

@chinedufn
Copy link
Contributor

Ahh got it.

Yeah, just checked and this test is failing:

#[test]
fn schema_version() {
assert_eq!(env!("SCHEMA_FILE_HASH"), APPROVED_SCHEMA_FILE_HASH)
}

Seems like we just need to start running it in CI.

@chinedufn
Copy link
Contributor

I'm on it one sec.

chinedufn added a commit to chinedufn/wasm-bindgen that referenced this issue Sep 14, 2021
@chinedufn
Copy link
Contributor

@kuviman (and anyone else that comes here) sorry about this.

In short.. we currently need to manually change the schema version whenever one of wasm-bindgen's internal data structures changes.

This is used to ensure that you get a nice error when trying to use two versions of wasm-bindgen with different schemas.

There's a test that fails when we forget to change this schema version.

But this test wasn't running in CI so no one noticed / remembered to change the schema version.

#2680 runs this test in CI so we shouldn't forget again!

In the future.. we could automatic this entirely using a proc macro that hashes the tokens and generated the schema version variable or something.. but since the schema doesn't change often we probably won't need to go that route.

Sorry!

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

Successfully merging a pull request may close this issue.

3 participants