embassy-rs
Improvements to the CI coverage
I noticed, while doing #474 that not all corners of the code base got the same testing by CI. This PR is based on the assumptions that:
- all features are equal; if a source file is built/tested in CI, it should be done so with all (necessary) feature combinations
Note: The above is ideal (not realistic). But it can be seen as something to aim for. In particular, the PR adds test coverage for the security feature.
I would also like to reorder the ci.sh commands, lifting some cargo commands from below cargo-batch section to the tip. The main reason for this is to fail fast if there are any formatting glitches (cargo fmt). I believe many PRs might have them; testing them doesn't require any downloads, so it would be a fast sanity check up-front. This, however, needs an edit in the .github/workflows/ci.yaml, adding (perhaps) the - uses: dtolnay/rust-toolchain@nightly to build. It's already used in binary-size. Without that, cargo is not available until it's been installed by the commands in ci.sh.
Furthermore, what is this cargo batch?
Looking at https://github.com/embassy-rs/cargo-batch, it seems a little home-grown. Would like to see a comment in ci.sh as to what gap it fills that standard cargo couldn't cater to.
I'd really like to run the ci.sh locally, but it turned out too difficult. Thanks to cargo-batch.
I noticed, while doing #474 that not all corners of the code base got the same testing by CI. This PR is based on the assumptions that:
- all features are equal; if a source file is built/tested in CI, it should be done so with all (necessary) feature combinations
Note: The above is ideal (not realistic). But it can be seen as something to aim for. In particular, the PR adds test coverage for the
securityfeature.I would also like to reorder the
ci.shcommands, lifting somecargocommands from belowcargo-batchsection to the tip. The main reason for this is to fail fast if there are any formatting glitches (cargo fmt). I believe many PRs might have them; testing them doesn't require any downloads, so it would be a fast sanity check up-front. This, however, needs an edit in the.github/workflows/ci.yaml, adding (perhaps) the- uses: dtolnay/rust-toolchain@nightlytobuild. It's already used inbinary-size. Without that,cargois not available until it's been installed by the commands inci.sh.
Good idea on doing format and clippy up front.
Furthermore, what is this
cargo batch?
It allows running parallell builds for multiple target and feature combinations, greatly saving time in CI.
Looking at https://github.com/embassy-rs/cargo-batch, it seems a little home-grown. Would like to see a comment in
ci.shas to what gap it fills that standardcargocouldn't cater to.
It is a bit home grown, but it's used by embassy as well, and the improvements it brings far outweighs the downsides.
I'd really like to run the
ci.shlocally, but it turned out too difficult. Thanks tocargo-batch.
It should be automatically downloading it if it can't find it, what issue are you seeing?
It should be automatically downloading it if it can't find it, what issue are you seeing?
It's linux only? I cannot run it too, cannot execute binary file was reported
I'll add changes on Monday, most likely:
Good idea on doing format and clippy up front.
Ok. Will change the .github/workflows/ci.yaml to make this possible (as discussed above).
It allows running parallell builds for multiple target and feature combinations, greatly saving time in CI.
Thanks. As long as it has benefits. Would this information have been available anywhere? Didn't find it - cargo batch README could state why it's different from mainline Cargo. But... that's out of focus, here!
It should be automatically downloading it if it can't find it, what issue are you seeing?
It was just taking way, way too long to build it. I work on a Linux VM with 2-3 cores. Waited until progress was "300/1604" or something and chose to quit.
Edit: I saw 300/1640 and thought it's only building cargo-batch. In fact, it's (likely) building all the pieces needed by the parameters.
Compiling defmt-parser v1.0.0
Compiling defmt-macros v1.0.1
Compiling phf v0.11.3
Compiling doxygen-rs v0.4.2
Compiling zerocopy-derive v0.8.27
Building [=> ] 149/1640: defmt-macros, zerocopy-derive, cortex-m-rt-macros
Will let it run, now.
It should be automatically downloading it if it can't find it, what issue are you seeing?
It's linux only? I cannot run it too,
cannot execute binary filewas reported
Ah, right! In that case, you can compile and install it like this:
cargo install --git https://github.com/embassy-rs/cargo-batch cargo --bin cargo-batch --locked
I'll add changes on Monday, most likely:
Good idea on doing format and clippy up front.
Ok. Will change the
.github/workflows/ci.yamlto make this possible (as discussed above).It allows running parallell builds for multiple target and feature combinations, greatly saving time in CI.
Thanks. As long as it has benefits. Would this information have been available anywhere? Didn't find it - cargo batch README could state why it's different from mainline Cargo. But... that's out of focus, here!
It should be automatically downloading it if it can't find it, what issue are you seeing?
It was just taking way, way too long to build it. I work on a Linux VM with 2-3 cores. Waited until progress was "300/1604" or something and chose to quit.
You should be able to use the pre-compiled binary from the cargo-batch release then I think?
My latest addition seems to have blown the bank exceeded the limits of the CI VM:
= note: /usr/bin/ld: final link failed: No space left on device
collect2: error: ld returned 1 exit status
Please check the changes and comment them. Moving the fmt and clippy to front was the cause for this; perhaps I did something wrong with .github.
In the mean time, the current CI setup e.g. grows ~/.cargo/git without limits, I think.
Maybe https://github.com/marketplace/actions/maximize-build-disk-space would work?