From f1e5020cd7a343759142a59a719b00416928f8ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois?= Date: Mon, 2 May 2022 14:58:45 +0000 Subject: [PATCH] CI tool usage (#3876) # Objective - Original objective was to add doc build warning check to the ci local execution - I somewhat deviated and changed other things... ## Solution `cargo run -p ci` can now take more parameters: * `format` - cargo fmt * `clippy` - clippy * `compile-fail` - bevy_ecs_compile_fail_tests tests * `test` - tests but not doc tests and do not build examples * `doc-test` - doc tests * `doc-check` - doc build and warnings * `bench-check` - check that benches build * `example-check` - check that examples build * `lints` - group - run lints and format and clippy * `doc` - group - run doc-test and doc-check * `compile` - group - run compile-fail and bench-check and example-check * not providing a parameter will run everything Ci is using those when possible: * `build` jobs now don't run doc tests and don't build examples. it makes this job faster, but doc tests and examples are not built for each architecture target * `ci` job doesn't run the `compile-fail` part but only format and clippy, taking less time * `check-benches` becomes `check-compiles` and runs the `compile` tasks. It takes longer. I also fixed how it was using cache * `check-doc` job is now independent and also run the doc tests, so it takes longer. I commented out the deadlinks check as it takes 2.5 minutes (to install) and doesn't work --- .github/workflows/ci.yml | 44 +++++++++++++------ CONTRIBUTING.md | 11 +++-- tools/ci/Cargo.toml | 1 + tools/ci/src/main.rs | 95 +++++++++++++++++++++++++++++++--------- 4 files changed, 113 insertions(+), 38 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 428b0e359fc692..61cc9a7a0e22d9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,7 +41,8 @@ jobs: run: sudo apt-get update; sudo apt-get install --no-install-recommends libasound2-dev libudev-dev if: runner.os == 'linux' - name: Build & run tests - run: cargo test --workspace + # See tools/ci/src/main.rs for the commands this runs + run: cargo run -p ci -- test env: CARGO_INCREMENTAL: 0 RUSTFLAGS: "-C debuginfo=0 -D warnings" @@ -68,7 +69,7 @@ jobs: run: sudo apt-get update; sudo apt-get install --no-install-recommends libasound2-dev libudev-dev libwayland-dev libxkbcommon-dev - name: CI job # See tools/ci/src/main.rs for the commands this runs - run: cargo run -p ci -- nonlocal + run: cargo run -p ci -- lints miri: runs-on: ubuntu-latest @@ -102,7 +103,7 @@ jobs: # to raw pointer aliasing rules that we should be trying to uphold. MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-ignore-leaks -Zmiri-tag-raw-pointers - check-benches: + check-compiles: runs-on: ubuntu-latest needs: ci steps: @@ -115,15 +116,17 @@ jobs: ~/.cargo/registry/cache/ ~/.cargo/git/db/ target/ - key: ${{ runner.os }}-cargo-check-benches-${{ hashFiles('**/Cargo.toml') }} + crates/bevy_ecs_compile_fail_tests/target/ + key: ${{ runner.os }}-cargo-check-compiles-${{ hashFiles('**/Cargo.toml') }} - uses: actions-rs/toolchain@v1 with: toolchain: stable override: true - name: Install alsa and udev run: sudo apt-get update; sudo apt-get install --no-install-recommends libasound2-dev libudev-dev - - name: Check Benches - run: cd benches && cargo check --benches + - name: Check Compile + # See tools/ci/src/main.rs for the commands this runs + run: cargo run -p ci -- compile build-wasm: strategy: @@ -292,23 +295,36 @@ jobs: check-doc: runs-on: ubuntu-latest - needs: check-markdown-links - if: always() steps: - uses: actions/checkout@v3 + - uses: actions/cache@v2 + with: + path: | + ~/.cargo/bin/ + ~/.cargo/registry/index/ + ~/.cargo/registry/cache/ + ~/.cargo/git/db/ + target/ + key: ${{ runner.os }}-check-doc-${{ hashFiles('**/Cargo.toml') }} - uses: actions-rs/toolchain@v1 with: toolchain: stable - name: Install alsa and udev run: sudo apt-get update; sudo apt-get install --no-install-recommends libasound2-dev libudev-dev libwayland-dev libxkbcommon-dev if: runner.os == 'linux' - - name: Installs cargo-deadlinks - run: cargo install --force cargo-deadlinks - name: Build and check doc - run: RUSTDOCFLAGS='-D warnings' cargo doc --workspace --all-features --no-deps --document-private-items - - name: Checks dead links - run: cargo deadlinks --dir target/doc/bevy - continue-on-error: true + # See tools/ci/src/main.rs for the commands this runs + run: cargo run -p ci -- doc + env: + CARGO_INCREMENTAL: 0 + RUSTFLAGS: "-C debuginfo=0" + # This currently report a lot of false positives + # Enable it again once it's fixed - https://github.com/bevyengine/bevy/issues/1983 + # - name: Installs cargo-deadlinks + # run: cargo install --force cargo-deadlinks + # - name: Checks dead links + # run: cargo deadlinks --dir target/doc/bevy + # continue-on-error: true check-missing-examples-in-docs: runs-on: ubuntu-latest diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b43e4749327b35..50328836be3569 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -280,10 +280,13 @@ If you're new to Bevy, here's the workflow we use: 1. Fork the `bevyengine/bevy` repository on GitHub. You'll need to create a GitHub account if you don't have one already. 2. Make your changes in a local clone of your fork, typically in its own new branch. 1. Try to split your work into separate commits, each with a distinct purpose. Be particularly mindful of this when responding to reviews so it's easy to see what's changed. -3. To test CI validations locally, run the `cargo run -p ci` command. You can also run sub-commands manually: - 1. `cargo fmt --all -- --check` (remove `--check` to let the command fix found problems) - 2. `cargo clippy --workspace --all-targets --all-features -- -D warnings -A clippy::type_complexity` - 3. `cargo test --all-targets --workspace` +3. To test CI validations locally, run the `cargo run -p ci` command. This will run most checks that happen in CI, but can take some time. You can also run sub-commands to iterate faster depending on what you're contributing: + * `cargo run -p ci -- lints` - to run formatting and clippy + * `cargo run -p ci -- test` - to run tests + * `cargo run -p ci -- doc` - to run doc tests and doc checks + * `cargo run -p ci -- compile` - to check that everything that must compile still does (examples and benches), and that some that shouldn't still don't ([`crates/bevy_ecs_compile_fail_tests`](./crates/bevy_ecs_compile_fail_tests)) + * to get more informations on commands available and what is run, check the [tools/ci crate](./tools/ci) + 4. When working with Markdown (`.md`) files, Bevy's CI will check markdown files (like this one) using [markdownlint](https://github.com/DavidAnson/markdownlint). To locally lint your files using the same workflow as our CI: 1. Install [markdownlint-cli](https://github.com/igorshubovych/markdownlint-cli). diff --git a/tools/ci/Cargo.toml b/tools/ci/Cargo.toml index 3c2a6ee93c3596..509d65db9074be 100644 --- a/tools/ci/Cargo.toml +++ b/tools/ci/Cargo.toml @@ -8,3 +8,4 @@ license = "MIT OR Apache-2.0" [dependencies] xshell = "0.1" +bitflags = "1.3" diff --git a/tools/ci/src/main.rs b/tools/ci/src/main.rs index 8264db7b44c30a..672bcdc640d8a0 100644 --- a/tools/ci/src/main.rs +++ b/tools/ci/src/main.rs @@ -1,44 +1,99 @@ use xshell::{cmd, pushd}; +use bitflags::bitflags; + +bitflags! { + struct Check: u32 { + const FORMAT = 0b00000001; + const CLIPPY = 0b00000010; + const COMPILE_FAIL = 0b00000100; + const TEST = 0b00001000; + const DOC_TEST = 0b00010000; + const DOC_CHECK = 0b00100000; + const BENCH_CHECK = 0b01000000; + const EXAMPLE_CHECK = 0b10000000; + } +} + fn main() { // When run locally, results may differ from actual CI runs triggered by // .github/workflows/ci.yml // - Official CI runs latest stable // - Local runs use whatever the default Rust is locally - // See if any code needs to be formatted - cmd!("cargo fmt --all -- --check") - .run() - .expect("Please run 'cargo fmt --all' to format your code."); + let what_to_run = match std::env::args().nth(1).as_deref() { + Some("format") => Check::FORMAT, + Some("clippy") => Check::CLIPPY, + Some("compile-fail") => Check::COMPILE_FAIL, + Some("test") => Check::TEST, + Some("doc-test") => Check::DOC_TEST, + Some("doc-check") => Check::DOC_CHECK, + Some("bench-check") => Check::BENCH_CHECK, + Some("example-check") => Check::EXAMPLE_CHECK, + Some("lints") => Check::FORMAT | Check::CLIPPY, + Some("doc") => Check::DOC_TEST | Check::DOC_CHECK, + Some("compile") => Check::COMPILE_FAIL | Check::BENCH_CHECK | Check::EXAMPLE_CHECK, + _ => Check::all(), + }; - // See if clippy has any complaints. - // - Type complexity must be ignored because we use huge templates for queries - cmd!("cargo clippy --workspace --all-targets --all-features -- -D warnings -A clippy::type_complexity -W clippy::doc_markdown") + if what_to_run.contains(Check::FORMAT) { + // See if any code needs to be formatted + cmd!("cargo fmt --all -- --check") + .run() + .expect("Please run 'cargo fmt --all' to format your code."); + } + + if what_to_run.contains(Check::CLIPPY) { + // See if clippy has any complaints. + // - Type complexity must be ignored because we use huge templates for queries + cmd!("cargo clippy --workspace --all-targets --all-features -- -A clippy::type_complexity -W clippy::doc_markdown -D warnings") .run() .expect("Please fix clippy errors in output above."); + } - // Run UI tests (they do not get executed with the workspace tests) - // - See crates/bevy_ecs_compile_fail_tests/README.md - { + if what_to_run.contains(Check::COMPILE_FAIL) { + // Run UI tests (they do not get executed with the workspace tests) + // - See crates/bevy_ecs_compile_fail_tests/README.md let _bevy_ecs_compile_fail_tests = pushd("crates/bevy_ecs_compile_fail_tests") .expect("Failed to navigate to the 'bevy_ecs_compile_fail_tests' crate"); - cmd!("cargo test") + cmd!("cargo test --target-dir ../../target") .run() .expect("Compiler errors of the ECS compile fail tests seem to be different than expected! Check locally and compare rust versions."); } - // These tests are already run on the CI - // Using a double-negative here allows end-users to have a nicer experience - // as we can pass in the extra argument to the CI script - let args: Vec = std::env::args().collect(); - if args.get(1) != Some(&"nonlocal".to_string()) { - // Run tests - cmd!("cargo test --workspace") + if what_to_run.contains(Check::TEST) { + // Run tests (except doc tests and without building examples) + cmd!("cargo test --workspace --lib --bins --tests --benches") .run() .expect("Please fix failing tests in output above."); + } + + if what_to_run.contains(Check::DOC_TEST) { + // Run doc tests + cmd!("cargo test --workspace --doc") + .run() + .expect("Please fix failing doc-tests in output above."); + } + + if what_to_run.contains(Check::DOC_CHECK) { + // Check that building docs work and does not emit warnings + std::env::set_var("RUSTDOCFLAGS", "-D warnings"); + cmd!("cargo doc --workspace --all-features --no-deps --document-private-items") + .run() + .expect("Please fix doc warnings in output above."); + } + + if what_to_run.contains(Check::COMPILE_FAIL) { + // Check that benches are building + let _benches = pushd("benches").expect("Failed to navigate to the 'benches' folder"); + cmd!("cargo check --benches --target-dir ../target") + .run() + .expect("Failed to check the benches."); + } - // Run doc tests: these are ignored by `cargo test` - cmd!("cargo test --doc --workspace") + if what_to_run.contains(Check::EXAMPLE_CHECK) { + // Build examples and check they compile + cmd!("cargo check --workspace --examples") .run() .expect("Please fix failing doc-tests in output above."); }