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

Enable Wasm CI #265

Closed
janhohenheim opened this issue May 16, 2023 · 10 comments
Closed

Enable Wasm CI #265

janhohenheim opened this issue May 16, 2023 · 10 comments
Labels
dev tooling Involves tools with which the game is produced wasm Has something to do with the Wasm build in particular

Comments

@janhohenheim
Copy link
Owner

janhohenheim commented May 16, 2023

See #255 and #264
@TimJentzsch do you have any idea how we can keep the target dir from being invalidated on Wasm builds?

@janhohenheim janhohenheim added dev tooling Involves tools with which the game is produced wasm Has something to do with the Wasm build in particular labels May 16, 2023
@TimJentzsch
Copy link
Collaborator

I'll try to take a look tomorrow :)

@TimJentzsch
Copy link
Collaborator

So one thing that strikes me is that this is not only a problem for WASM -- the test-native job also appears to take a very long time.
Take a look at this job for example: https://github.com/janhohenheim/foxtrot/actions/runs/4997406397/jobs/8951737694

Here, the cache has a hit, but the test is still recompiling the dependencies.
This means that cargo depends on something that is not considered in the cache key.

My first guess is that it's the rustc version, which is still an open issue for the cache action: Leafwing-Studios/cargo-cache#12

I assume that cargo doesn't use any of the cached stuff if it was compiled with an older rust version, which makes sense for performance and other stuff.
So in that case, the cache won't be updated, but can never be used for compilation.

Here's the catch though: We currently use the latest nightly version in CI. That means that, even if we fix the cache action to consider the rustc version, it will be outdated every day and we'll basically render the cache useless.

So we should probably do two things:

  • Add the rust version to the cache key in the action
  • Use a fixed Rust version in CI (we could also consider using stable Rust if it's possible for foxtrot)

I'm not 100% certain that this will fix the issue, but it's worth a shot.

@TimJentzsch
Copy link
Collaborator

We can also try this tip from the Cargo book https://doc.rust-lang.org/cargo/faq.html#why-is-cargo-rebuilding-my-code to get more info on why cargo is rebuilding stuff:

$ CARGO_LOG=cargo::core::compiler::fingerprint=info cargo build

So basically setting the CARGO_LOG variable in CI.
Unfortunately that won't help with debugging why the WASM stuff is rebuilding, as trunk doesn't appear to be forwarding that env variable to the build command as far as I could see.

But it could still be helpful if the native builds aren't caching properly.

@janhohenheim
Copy link
Owner Author

janhohenheim commented May 17, 2023

Your reasoning sound sound. I'm very open to setting a fixed nightly version for the CI and periodically updating it (say, at every new Bevy release).
The reason Foxtrot uses nightly is because of the if let chain feature, which makes some code much much easier to write, and, ironically, gains in compile time, which in my experience can definitely be felt when compiling on my own machine.

@TimJentzsch
Copy link
Collaborator

Fair point, I'm really looking forward to if let hitting stable. It really is a great feature

@janhohenheim
Copy link
Owner Author

@TimJentzsch looks like pinning the nightly version didn't change anything 🤔

@TimJentzsch
Copy link
Collaborator

@TimJentzsch looks like pinning the nightly version didn't change anything 🤔

Did you clear the caches afterwards? Since the version is not yet included in the cache hash, they need to be regenerated or it will always start with a different nightly version

@TimJentzsch
Copy link
Collaborator

@janhohenheim It looks like the caches were still from the old nightly setup, I cleared them just now.
So your next PR will first have to regenerate the caches, but afterwards it should (hopefully) be faster again!

@janhohenheim
Copy link
Owner Author

Thanks!! You're the best :D

@TimJentzsch
Copy link
Collaborator

Small update: The Rust version is now included in the cache key for cargo-cache, so at least that should not be causing trouble anymore

@janhohenheim janhohenheim closed this as not planned Won't fix, can't repro, duplicate, stale Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev tooling Involves tools with which the game is produced wasm Has something to do with the Wasm build in particular
Projects
None yet
Development

No branches or pull requests

2 participants