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

travis: Fix wasm32 CI #141

Merged
merged 1 commit into from Apr 26, 2020
Merged

travis: Fix wasm32 CI #141

merged 1 commit into from Apr 26, 2020

Conversation

josephlr
Copy link
Member

@josephlr josephlr commented Apr 25, 2020

Split emscripten stuff into its own target and stop relying on cargo web
to download/manage then emscripten toolchain. We can just get it
ourselves.

We now use stable for all WASM32 tests and are now running the emscripten tests.

Fixes #140

Signed-off-by: Joe Richey joerichey@google.com

Split emscripten stuff into its own target and stop relying on cargo web
to download/manage then emscripten toolchain. We can just get it
ourselves. We also now run the emscripten tests.

Signed-off-by: Joe Richey <joerichey@google.com>
@josephlr
Copy link
Member Author

@newpavlov @dhardy dropping the use of cargo web for emscripten seems to have fixed the CI.

Other improvements:

  • We now actually run the tests (instead of just building them)
  • The CI is faster (one 6-7 minute stage became two 2-2.5 minute stages)

@dhardy
Copy link
Member

dhardy commented Apr 26, 2020

I don't think I can provide any useful feedback here. If someone else (e.g. @zer0x64) can provide feedback then I'll be happy to "approve" (if required).

@josephlr
Copy link
Member Author

josephlr commented Apr 26, 2020

I don't think I can provide any useful feedback here. If someone else (e.g. @zer0x64) can provide feedback then I'll be happy to "approve" (if required).

@dhardy, I can give some context for the exact steps I added to the CI. I replaced cargo web prepare-emscripten with the steps from the emscripten download page. Getting the tests to run was then just a matter of copying what cross does.

Either you or @newpavlov have to approve (I can't approve my own requests).

EDIT: We had to get rid of cargo web prepare-emscripten because of koute/cargo-web#244

@zer0x64
Copy link
Contributor

zer0x64 commented Apr 26, 2020

If I understand correctly, cargo-web is currently broken with wasm32-unknown-emscripten as per this, and while the fixed is merge and released, this change aims compile and run directly this specific build without using cargo web?
Also, the CI step is split into two, which is cool because they can now run concurrently and fail separately.
Makes sense to me!

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

Successfully merging this pull request may close these issues.

None yet

3 participants