search

atom-archive / xray

An experimental next-generation Electron-based text editor
MIT License
8.48k stars 235 forks source link

Don't cache cargo on Travis and install `wasm-bindgen-cli` on CI #105

Closed pranaygp closed 6 years ago

pranaygp commented 6 years ago

I ran into some issues when building this locally and think it might have to do with a broken wasm-bindgen nightly build (look at https://github.com/rustwasm/wasm-bindgen/issues/235) but was confused how the tests worked in CI.

~This is just a PR to test if it's still broken in CI (by making trivial code changes to trigger CI)~

I discovered that, if you run the current master according to the steps, it will actually fail building but only worked in CI because of Travis' caching. For more complete tests and to avoid this scenario where it works on CI but not in a new setup, I think we should disable caching on CI (for now)

pranaygp commented 6 years ago

So it seems like, when you disable caching, it fails on Travis CI too. I think this has something to do with how rust nightly interacts with building wasm-bindgen-cli. If so, it makes sense to not cache the built deps because there can actually be an issue when something updates and we won't catch it (like the successful Travis builds before disabling caching).

Also, remember that because we're using rust nightly in CI, our CI runs are not idempotent 😢

pranaygp commented 6 years ago

There we go! This is the exact error I've run into when setting it up locally (from the latest Travis Build). Look for https://travis-ci.org/atom/xray/builds/389110655#L1166

I've been digging around and narrowed it down further to be an issue with wasm-bindgen (or with the latest Rust nightly ¯\(ツ)/¯). I opened an issue upstream about it https://github.com/rustwasm/wasm-bindgen/issues/235

nathansobo commented 6 years ago

I've managed to reproduce this locally on a newer version of wasm-bindgen. Still investigating how things have changed.

pranaygp commented 6 years ago

Good to know I'm not going crazy. This seems to me like a regression on wasm-bindgen's side, but I'd love to see what you come up with.

I'm brand new to wasm and Rust (this is the first time I'm playing with it)

nathansobo commented 6 years ago

This seems to have been caused by you running the build with a different version of cargo-bindgen-cli than we had installed as a dependency of xray_wasm. I'm not sure how this happened, since /script/build explicitly ensures that a specific version of cargo-bindgen-cli is installed on your system. We went ahead and updated to the latest version of Rust nightly and cargo-bindgen while we were in here, however. Thanks.

pranaygp commented 6 years ago

Goddammit, I know why the script didn't install the right version for me. I just checked out the script script/build (https://github.com/atom/xray/blob/master/xray_wasm/script/build).

I'm running fish. Yet another reason I should stop doing that.

Oh well, thanks for helping me out @nathansobo 🙌

nathansobo commented 6 years ago

😢 🐟