.travis.yml: Test no_std platform, WASM, all feature combos, --release #21
Conversation
This uses a CI config more like the one at: https://github.com/RustCrypt/signatures It runs all tests with the `--release` flag to ensure that crates aren't broken by release-dependent functionality after being tested in debug mode (we can add back debug mode testing if it's helpful, but it doesn't look like we have anything presently gated on debug builds). I now see @newpavlov also requested it on #20. It also tests on a `no_std` platform (thumbv7) which @newpavlov requested in #18. Finally, it tests with `--all-features`. This should uncover some lingering breakages (it won't in this commit, but it appears some of the conditional compilation is incorrectly gated and I intend to fix it in a follow-up commit).
The code presently isn't rustfmt'd. Some of these clippy failures really look like they should be addressed, but for the purposes of getting more CI checks to pass, we don't need to enforce clippy for now.
Each install step cooresponds to only one entry in the build matrix, so split it up that way. This allows tests on Rust 1.27.0 to continue to pass, and prevents installing irrelevant components on Rust versions that don't need them.
@newpavlov suggested we either do a full-blown WASM test on Node or don't build at all. I agree that if it builds on another `no_std` platform, it will probably build on WASM too, so building on both probably isn't helpful.
I think these might be worth keeping as I think when running tests in a workspace it doesn't do a docs test by default, but we can circle back on that later.
The `--all-features` build doesn't work on 1.27.0 because `zeroize` is a 2018 edition crate and 1.27.0 is a pre-2018-edition release.
|
@newpavlov regarding c57a8cd, any thoughts on bumping these to 2018 edition crates? (and therefore 1.31+) |
|
@newpavlov okay, well it's green and the The clippy warnings seem really bad, and in general a lot of the code seems to have warnings (and cargo features that simply don't work, but I'll save that for another PR), but this seems like a reasonable baseline of CI checks to me. |
|
After a cursory look I don't see any serious issues in clippy warnings, one questionable place is this one from ptr::read_unaligned(nonce as *const Nonce<Self> as *const [u64; 2])Clippy does not like casting into a pointer with a stricter aligment, but if I am not mistaken it should not be an issue since we use resulting pointer with Also maybe it's worth to use |
Several things were gated under `cargo_feature` attributes, which don't appear to do anything. Because of it, much of it was broken/untested.
tarcieri
force-pushed
the
ci-config-improvements
branch
from
4e37486
to
a973884
Compare
It was previously missing a needed generic parameter
It appears `std` was being imported to access a trait from `core` (`Drop`). This removes the `std` import and imports `Drop` from `core` instead.
|
@newpavlov okay, I'd say this is ready. Re:
I agree with that, but how about separate PRs? I think the fix for the current warning deserves to be split out because it's a little weird and I want to make sure it the fix for it I want to do doesn't unintentionally impact the current logic and I don't want that change to get lost in this much shuffle. |
|
I opened an issue to discuss the current warning in I suppose I can add |
Have CI fail on warnings. There's one warning across the workspace in the `salsa20-crate`. I opened an issue about it here: #23 I've gone ahead and thrown an `allow` attribute on the relevant code so CI passes, which should at least prevent additional warnings from going uncaught on CI.
|
@newpavlov okay, 6d0d83b adds |
|
Unfortunately it seems on Rust 1.27.0, there are several https://travis-ci.org/RustCrypto/stream-ciphers/jobs/571083202 I'll see if I can disable |
There are several `generic-array`-related warnings on 1.27.0: https://travis-ci.org/RustCrypto/stream-ciphers/jobs/571083202 We should probably phase 1.27.0 out anyway (#24), so ignore them.
|
Disabling the warnings for 1.27.0 worked. Now on to the PR I actually wanted to write in the first place 😉 |
This uses a CI config more like the one at:
https://github.com/RustCrypt/signatures
It runs all tests with the
--releaseflag to ensure that crates aren't broken by release-dependent functionality after being tested in debug mode (we can add back debug mode testing if it's helpful, but it doesn't look like we have anything presently gated on debug builds). I now see @newpavlov also requested it on #20.It also tests on a
no_stdplatform (thumbv7) which @newpavlov requested in #18.Finally, it tests with
--all-features. This should uncover some lingering breakages (it won't in this commit, but it appears some of the conditional compilation is incorrectly gated and I intend to fix it in a follow-up commit).