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.

Sign up for GitHub

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

Jump to bottom

Update Github Actions CI for the master branch #192

Merged
merged 6 commits into from
Jan 4, 2021
Merged

Update Github Actions CI for the master branch #192

merged 6 commits into from
Jan 4, 2021

Conversation

josephlr
Copy link
Member

@josephlr josephlr commented Jan 3, 2021 •
edited
Loading

Fixes #181 and adds a bunch of stuff to the CI

  • Use -Z build-std=core instead of xbuild
  • Use Clippy configuration file, instead of manually whitelisting lints
  • Cleanup tests.yml
  • Faster binary downloads
  • Add minimal dependencies check
  • Add tests for custom feature
  • Build/Link for iOS
  • Run cross tests on aarch64 linux and Android
  • Link on Solaris and Netbsd
  • Test wasm code on Node, Chrome, Firefox
  • Test WASI
  • No need for RDRAND feature on VxWorks

The following TODOs will be done in a follow up PR:

  • Actually run the iOS binaries
  • Build/Run aarch64-apple-darwin binaries (aka "Apple Silicon")
  • Add emscripten tests

@josephlr
Copy link
Member Author

josephlr commented Jan 3, 2021

@newpavlov This is ready for review, let me know what you think.

Right now we will run 6 tests on macos-latest (4 for macos, 2 for iOS). Given that those CI jobs are the slowest to schedule, should we try to reduce the number of jobs? Maybe only test stable for macos?

@josephlr josephlr mentioned this pull request Jan 3, 2021
Merged
@josephlr
Copy link
Member Author

josephlr commented Jan 3, 2021

Right now we will run 6 tests on macos-latest (4 for macos, 2 for iOS). Given that those CI jobs are the slowest to schedule, should we try to reduce the number of jobs? Maybe only test stable for macos?

I changed the test matrix to only run macos-latest tests for:

  • stable x86_64-apple-darwin
  • stable aarch64-apple-ios
  • stable x86_64-apple-ios

I don't think there's a huge advantage to testing things like 1.34, beta, etc... on multiple different OSes (as we mostly use these tests for code compatibility, not OS compatibility), so this should be fine.

Our entire CI now runs in < 3 minutes, with the build-std job being the last major bottleneck (as it has to build libcore 4 times in a row).

newpavlov
newpavlov approved these changes Jan 4, 2021
View reviewed changes
Copy link
Member

@newpavlov newpavlov left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Do we still need the .cargo/config file? Also maybe it's worth to move test modules from src/ to tests/?

.github/workflows/tests.yml Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
.github/workflows/tests.yml Show resolved Hide resolved
.github/workflows/tests.yml Show resolved Hide resolved
@josephlr josephlr mentioned this pull request Jan 4, 2021
Merged
2 tasks
@josephlr
Use -Z build-std=core instead of xbuild
3303349 
See: https://github.com/rust-osdev/cargo-xbuild#alternative-the-build-std-feature-of-cargo

Signed-off-by: Joe Richey <joerichey@google.com>
@josephlr
Add Clippy msrv configuration file
d5186e0 
This allows us to automatically ignore lints that are
incompatible with our MSRV.

Signed-off-by: Joe Richey <joerichey@google.com>
@josephlr
Update CI
ea73f5a 
- Cleanup `tests.yml`
- Add better binary downloads
- Add minimal dependancies check
- Add tests for `custom` feature
- Build/Link for iOS
- Run cross tests on aarch64 linux and Android
- Link on Solaris and Netbsd
- Test wasm code on Node, Chrome, Firefox
- Test WASI
- No need for RDRAND feature on VxWorks

Signed-off-by: Joe Richey <joerichey@google.com>
@josephlr
Only test macOS on stable
7687498 
Most of the advantages from testing various Rust versions already come
from running those tests on Linux and Windows. There's very little
gain from also running these tests on macOS, while macOS jobs are the
slowest to schedule.

Signed-off-by: Joe Richey <joerichey@google.com>
@josephlr
Clarify use of .cargo/config
e593f43 
Signed-off-by: Joe Richey <joerichey@google.com>
@josephlr
Copy link
Member Author

josephlr commented Jan 4, 2021 •
edited
Loading

Do we still need the .cargo/config file?

This is what allows us to use cargo run/cargo test normally on wasm32-unknown-unknown and wasm32-wasi. The same effect can be achieved with environment variables, but I thought the .cargo/config was more idiomatic. I added a comment explaining.

Also maybe it's worth to move test modules from src/ to tests/?

Good idea, I did this in a separate PR, see: 28cafca

@josephlr josephlr mentioned this pull request Jan 4, 2021
Closed
6 tasks
@josephlr josephlr merged commit 1c6749e into master Jan 4, 2021
@josephlr josephlr deleted the ci branch January 4, 2021 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@newpavlov newpavlov newpavlov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add CI for WASM targets
2 participants
@josephlr @newpavlov