Skip to content

Recent build failures from wasm-bindgen ecosystem #2508

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

Closed
code-ape opened this issue Mar 29, 2021 · 1 comment · Fixed by #2510
Closed

Recent build failures from wasm-bindgen ecosystem #2508

code-ape opened this issue Mar 29, 2021 · 1 comment · Fixed by #2510
Labels

Comments

@code-ape
Copy link

code-ape commented Mar 29, 2021

Describe the Bug

Our CI started randomly failing today on wasm-bindgen projects with no dependency changes. This appears to be a failure with the wasm-bindgen macro. Since our project was a library we didn't have a pinned Cargo.lock I suspect this isn't the fault of wasm-bindgen but of one of its dependencies, but I wasn't sure were else to report it.

UPDATE: We just found that forcing rollback of syn from 1.0.66 to 1.0.65 appears to fix this issue. cargo update -p syn --precise 1.0.65. Leaving this open for wasm-bindgen to do with as they see fit.

Steps to Reproduce

Attempt to run cargo check on urbdyn/petgraph-wasm, commit c4e3db9 with syn version 1.0.66.

Expected Behavior

cargo check exits fine.

Actual Behavior

Recieve hundreds of errors relating to #[wasm_bindgen(...)] and WebAssembly when attempting to build dependency js-sys-0.3.49 in cargo check process.

Additional Context

Last successful build was ~5 hours prior to time of filing this issue. No changes to source code since then. Blowing away my local Cargo.lock duplicated the issue. Sadly I failed to back it up before hand to diff with new one.

Link to last successful build for time reference: https://github.com/urbdyn/petgraph-wasm/actions/runs/696142253

Example from GitHub Actions output, full run viewable at: https://github.com/urbdyn/petgraph-wasm/pull/12/checks?check_run_id=2215306539#step:5:68

# Omitting non-wasm-bindgen related libraries.
Compiling wasm-bindgen-shared v0.2.72
Compiling serde_derive v1.0.125
Compiling serde v1.0.125
Compiling serde_json v1.0.64
Compiling wasm-bindgen v0.2.72
Compiling wasm-bindgen-test-macro v0.3.22
Compiling wasm-bindgen-backend v0.2.72
Compiling wasm-bindgen-macro-support v0.2.72
Compiling wasm-bindgen-macro v0.2.72
Checking js-sys v0.3.49
Checking console_error_panic_hook v0.1.6

error: expected `,`
Error:    --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/js-sys-0.3.49/src/lib.rs:639:39
    |
639 |         #[wasm_bindgen(js_namespace = Atomics, catch)]
    |                                       ^^^^^^^

error: expected `,`
Error:    --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/js-sys-0.3.49/src/lib.rs:649:39
    |
649 |         #[wasm_bindgen(js_namespace = Atomics, catch)]
    |                                       ^^^^^^^

@afilini
Copy link
Contributor

afilini commented Mar 29, 2021

I opened this issue in the syn repo before noticing this thread. I believe in a way this bug is actually here in wasm-bindgen, but nobody had noticed it before because syn also had a different "bug", or at least it was behaving differently when trying to parse an ExprArray which was not an ExprArray (it was still consuming tokens from the parse buffer).

I guess the fix here would be to just avoid forking the ParseBuffer?

let input_before_parse = input.fork();

afilini added a commit to afilini/wasm-bindgen that referenced this issue Mar 29, 2021
`syn` < 1.0.66 would consume the `AnyIdent` anyway while trying to parse
the `ExprArray`. Now due to a refactoring they did, they (correctly)
leave the `ParseBuffer` untouched if the input is not a valid `ExprArray`,
so we should avoid forking the input buffer and just use the same for both
tries, otherwise the unwanted tokens left in there would make `Punctuated`
break soon after.

Fixes rustwasm#2508
afilini added a commit to afilini/wasm-bindgen that referenced this issue Mar 29, 2021
`syn` < 1.0.66 would consume the `AnyIdent` anyway while trying to parse
the `ExprArray`.

Now due to a refactoring they did, they (correctly) leave the `ParseBuffer`
untouched if the input is not a valid `ExprArray`, so we should avoid forking
the input buffer and just use the same for both tries, otherwise the unwanted
tokens left in there would make `Punctuated` break soon after.

Fixes rustwasm#2508
sanpii added a commit to sanpii/oxfeed that referenced this issue Mar 29, 2021
Tamschi added a commit to Tamschi/lignin that referenced this issue Mar 29, 2021
Tamschi added a commit to Tamschi/lignin that referenced this issue Mar 29, 2021
fornwall added a commit to fornwall/advent-of-code that referenced this issue Mar 29, 2021
fornwall added a commit to fornwall/advent-of-code that referenced this issue Mar 29, 2021
alexcrichton added a commit that referenced this issue Mar 29, 2021
* Correctly consume tokens when parsing `js_namespace`

`syn` < 1.0.66 would consume the `AnyIdent` anyway while trying to parse
the `ExprArray`.

Now due to a refactoring they did, they (correctly) leave the `ParseBuffer`
untouched if the input is not a valid `ExprArray`, so we should avoid forking
the input buffer and just use the same for both tries, otherwise the unwanted
tokens left in there would make `Punctuated` break soon after.

Fixes #2508

* Update nightly used on CI

* Update syn version dependency

Co-authored-by: Alex Crichton <alex@alexcrichton.com>
VincentJousse pushed a commit to VincentJousse/wgpu-rs that referenced this issue Mar 29, 2021
bors bot added a commit to gfx-rs/wgpu-rs that referenced this issue Mar 29, 2021
818: Update wasm-bindgen dependencies. r=kvark a=VincentFTS

Because of rustwasm/wasm-bindgen#2508

Co-authored-by: Vincent Jousse <contact@ftsoftware.fr>
bors bot pushed a commit to bevyengine/bevy that referenced this issue Mar 30, 2021
This is a temporary fix for the CI (and anyone building for wasm) break until `wgpu` can update

* `syn` released a version that fixed a bug in how they parsed attributes
* `wasm_bindgen` released a version that uses that fix
* but we're stuck with old `wasm_bindgen` as `wgpu` uses a fixed version: https://github.com/gfx-rs/wgpu-rs/blob/c5ee9cd98310aee66fb49bc98f4f65590304e4aa/Cargo.toml#L118

So, to fix this, either we update everyone to latest version of `wasm_bindgen` or we keep using old version of `syn`.

On Bevy side, it should be faster to fix the version of `syn` to one that works.

More details: rustwasm/wasm-bindgen#2510 & rustwasm/wasm-bindgen#2508
sanpii added a commit to sanpii/oxfeed that referenced this issue Apr 13, 2021
paytonrules added a commit to PacktPublishing/Game-Development-with-Rust-and-WebAssembly that referenced this issue Jun 19, 2021
This dependency needs to be updated to avoid the following gnarly bug:

rustwasm/wasm-bindgen#2508. It's gnarly when
it happens, and it's intermittent.
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.

2 participants