From 9deae3653a46b7f585942f9824254e6ef132ddea Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 30 Jun 2023 00:33:02 +0100 Subject: [PATCH] chore: unify and optimize Rust CI (#1710) - Instead of having two, very similar jobs, we run our fmt, clippy and tests steps across all crates and operating systems. - We remove the dependency of the android and apple builds on the tests and thus get faster feedback. - We force clippy to fail on any warning. This one is super important IMO. Warnings in Rust are very useful and ignoring them can lead to bugs (think "unused Result" etc). Resolves #1714. --------- Signed-off-by: Thomas Eizinger Co-authored-by: Francesca Lovebloom --- .github/workflows/rust.yml | 93 +++++++----------------- rust/connlib/clients/android/Cargo.toml | 1 + rust/connlib/clients/apple/Cargo.toml | 1 + rust/connlib/clients/apple/build.rs | 5 ++ rust/connlib/libs/client/src/messages.rs | 4 +- 5 files changed, 35 insertions(+), 69 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index a3d9a1c86..fcf60d1d1 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -11,6 +11,10 @@ concurrency: group: "rust-${{ github.workflow }}-${{ github.ref }}" cancel-in-progress: true +defaults: + run: + working-directory: ./rust + jobs: draft-release: runs-on: ubuntu-latest @@ -24,12 +28,18 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - test-relay: - name: Test relay - runs-on: ubuntu-latest - defaults: - run: - working-directory: ./rust + test: + strategy: + fail-fast: false + matrix: + runs-on: + - ubuntu-20.04 + - ubuntu-22.04 + - macos-11 + - macos-12 + - windows-2019 + - windows-2022 + runs-on: ${{ matrix.runs-on }} steps: - uses: actions/checkout@v3 @@ -40,37 +50,8 @@ jobs: - uses: Swatinem/rust-cache@v2 with: workspaces: ./rust - - run: cargo fmt -p relay -- --check - - run: cargo doc -p relay --no-deps --document-private-items - env: - RUSTDOCFLAGS: "-D warnings" - - run: cargo clippy -p relay --all-targets --all-features -- -D warnings - - run: cargo test -p relay + key: v2 - test-connlib: - needs: draft-release - name: Connlib checks - strategy: - matrix: - runs-on: - - ubuntu-20.04 - - ubuntu-22.04 - - macos-11 - - macos-12 - - windows-2019 - - windows-2022 - runs-on: ${{ matrix.runs-on }} - defaults: - run: - working-directory: ./rust - steps: - - name: Checkout - uses: actions/checkout@v3 - - name: Update toolchain - run: rustup show - - uses: Swatinem/rust-cache@v2 - with: - workspaces: ./rust # TODO: Building *ring* from git requires us to install additional tools; # once we're not using a forked *ring* these 2 steps can be removed. - if: ${{ contains(matrix.runs-on, 'windows') }} @@ -88,22 +69,20 @@ jobs: name: Move *ring* build tools run: | mv target/tools/windows/nasm/nasm.exe target/tools/nasm.exe - - name: Run connlib checks and tests - run: | - cargo check --workspace --exclude connlib-apple --exclude relay - cargo clippy --workspace --exclude connlib-apple --exclude relay -- -D clippy::all - cargo test --workspace --exclude connlib-apple --exclude relay + + - run: cargo fmt -- --check + - run: cargo doc --all-features --no-deps --document-private-items + env: + RUSTDOCFLAGS: "-D warnings" + - run: cargo clippy --all-targets --all-features -- -D warnings + - run: cargo test --all-features build-android: needs: - - test-connlib - draft-release runs-on: ubuntu-latest permissions: contents: read - strategy: - matrix: - rust: [stable] steps: - uses: actions/checkout@v3 - uses: Swatinem/rust-cache@v2 @@ -111,18 +90,11 @@ jobs: workspaces: ./rust - name: Update toolchain run: rustup show - - uses: actions/cache@v3 - with: - path: | - ~/rust/connlib/clients/android/.gradle/caches - ~/rust/connlib/clients/android/.gradle/wrapper - key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }} - restore-keys: | - ${{ runner.os }}-gradle- - uses: actions/setup-java@v3 with: java-version: '17' distribution: 'adopt' + cache: gradle - name: Validate Gradle wrapper uses: gradle/wrapper-validation-action@v1 - name: Assemble Release @@ -132,7 +104,7 @@ jobs: build-root-directory: rust/connlib/clients/android - name: Move artifact run: | - mv ./rust/connlib/clients/android/lib/build/outputs/aar/lib-release.aar ./connlib-${{ needs.draft-release.outputs.tag_name }}.aar + mv ./connlib/clients/android/lib/build/outputs/aar/lib-release.aar ./connlib-${{ needs.draft-release.outputs.tag_name }}.aar - uses: actions/upload-artifact@v3 with: name: connlib-android @@ -141,14 +113,10 @@ jobs: build-apple: needs: - - test-connlib - draft-release runs-on: macos-latest permissions: contents: read - strategy: - matrix: - rust: [stable] steps: - uses: actions/checkout@v3 - uses: Swatinem/rust-cache@v2 @@ -156,12 +124,6 @@ jobs: workspaces: ./rust - name: Update toolchain run: rustup show - - name: Run connlib checks and tests - working-directory: ./rust/connlib/clients/apple - run: | - cargo check -p connlib-apple - cargo clippy -p connlib-apple -- -D clippy::all - cargo test -p connlib-apple - name: Setup lipo run: cargo install cargo-lipo - uses: actions/cache@v3 @@ -193,9 +155,6 @@ jobs: cross-relay: # cross is separate from test because cross-compiling yields different artifacts and we cannot reuse the cache. name: Cross compile relay runs-on: ubuntu-latest - defaults: - run: - working-directory: ./rust steps: - uses: actions/checkout@v3 diff --git a/rust/connlib/clients/android/Cargo.toml b/rust/connlib/clients/android/Cargo.toml index 6526daa3e..f1f563c31 100644 --- a/rust/connlib/clients/android/Cargo.toml +++ b/rust/connlib/clients/android/Cargo.toml @@ -12,3 +12,4 @@ android_logger = "0.13" [lib] name = "connlib" crate-type = ["cdylib"] +doc = false diff --git a/rust/connlib/clients/apple/Cargo.toml b/rust/connlib/clients/apple/Cargo.toml index de9bd3cd0..2b126c3dc 100644 --- a/rust/connlib/clients/apple/Cargo.toml +++ b/rust/connlib/clients/apple/Cargo.toml @@ -17,3 +17,4 @@ firezone-client-connlib = { path = "../../libs/client" } [lib] name = "connlib" crate-type = ["staticlib"] +doc = false diff --git a/rust/connlib/clients/apple/build.rs b/rust/connlib/clients/apple/build.rs index 2989be079..5c9c53b22 100644 --- a/rust/connlib/clients/apple/build.rs +++ b/rust/connlib/clients/apple/build.rs @@ -131,6 +131,11 @@ fn link_swift(env: &Env) { } fn main() -> anyhow::Result<()> { + // Early exit build script to avoid errors on non-Apple platforms. + if std::env::var("CARGO_CFG_TARGET_VENDOR").as_deref() != Ok("apple") { + return Ok(()); + } + println!("cargo:rerun-if-env-changed={XCODE_CONFIGURATION_ENV}"); let env = Env::gather(); gen_bridges(&env); diff --git a/rust/connlib/libs/client/src/messages.rs b/rust/connlib/libs/client/src/messages.rs index df8bb3c2f..d2731e764 100644 --- a/rust/connlib/libs/client/src/messages.rs +++ b/rust/connlib/libs/client/src/messages.rs @@ -201,7 +201,7 @@ mod test { "topic": "device" } "#; - let egress_message = serde_json::from_str(&message).unwrap(); + let egress_message = serde_json::from_str(message).unwrap(); assert_eq!(m, egress_message); } @@ -268,7 +268,7 @@ mod test { "status":"ok" } }"#; - let reply_message = serde_json::from_str(&message).unwrap(); + let reply_message = serde_json::from_str(message).unwrap(); assert_eq!(m, reply_message); } }