Conversation
TomasArrachea
added
no changelog
SantiagoPittella
left a comment
SantiagoPittella
left a comment
There was a problem hiding this comment.
Can we do the same that we did with the node for the remote prover? it takes a couple minutes to compile it
| - name: Build note-transport | ||
| if: steps.cache-note-transport.outputs.cache-hit != 'true' | ||
| run: | | ||
| git clone --depth=1 -b main https://github.com/0xMiden/miden-note-transport .tmp/miden-note-transport |
There was a problem hiding this comment.
If I'm not mistaken, temporary directories are created with the runner.temp variable, something like so:
| git clone --depth=1 -b main https://github.com/0xMiden/miden-note-transport .tmp/miden-note-transport | |
| git clone --depth=1 -b main https://github.com/0xMiden/miden-note-transport ${{ runner.temp }}/miden-note-transport |
Here's the reference documentation where I got that from:
There was a problem hiding this comment.
I simplified this to use cargo install --git instead of having a clone of the note-transport repo. I think it's cleaner this way and also we avoid the .tmp dir 8d0d217
There was a problem hiding this comment.
I simplified this to use cargo install --git instead of having a clone of the note-transport repo. I think it's cleaner this way and also we avoid the
.tmpdir 8d0d217
Great idea! You cane even specify a specific directory to install the binaries in with cargo install --root <destination directory>. It doesn't seem necessary in this case, just thought I'd mention it.
| - name: Add Rust Cache | ||
| uses: Swatinem/rust-cache@v2 | ||
| with: | ||
| shared-key: rust-build-wasm | ||
| prefix-key: ${{ env.RUST_CACHE_KEY }} | ||
| save-if: ${{ github.ref == 'refs/heads/next' }} |
There was a problem hiding this comment.
Is there a benefit to keeping these but with save-if: false?
There was a problem hiding this comment.
Yes, save-if: false means the job still restores from the cache saved by the build job (which runs with save-if: ${{ github.ref == 'refs/heads/next' }}) but avoids redundant saves that waste time and could evict the canonical cache entry.
There was a problem hiding this comment.
The rust-wasm cache entry gets written by the Build Web Client job, that compiles the wasm client with a different set of Rust flags and features (defined in the rolup config file) as the make build-wasm command. So IIUC that requires a recompilation anyway.
There was a problem hiding this comment.
The same applies to the other cases, for the cache to make sense the jobs have to compile the Rust code with the same profile.
There was a problem hiding this comment.
Could we look into standardizing those flags so that we can reuse the cache? Probably not doable, but wonder if you looked into it.
4 participants
This PR introduces:
So for a single push to next, the resulting Rust cache entries would be:
rust-release, size: 750 MB, used by: build, test, integration-tests, integration-tests-remote-proverrust-wasm, size: 490 MB, used by: build-wasm, clippy-wasm, wasm-bindgen-types, build-web-clientnode-builder, size: 15 MB, used by: integration-tests, integration-tests-web-client, integration-tests-remote-provernote-transport, size: 4 MB, used by: integration-testsremote-prover, size: 16 MB, used by integration-tests-remote-prover-web-client