From ccd1912edeb9c690bd0b61edab6c9fe0d0e91d1e Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Fri, 1 Sep 2023 15:16:29 -0400 Subject: [PATCH 1/9] ci: replace actions-rs with dtolnay/rust-toolchain The former is not maintained, and produces warnings in GitHub Actions due to node-js deprecations. The latter is used by the other Rustls ecosystems projects and actively maintained. --- .github/workflows/ci.yml | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3c585be1..0950ec26 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,16 +30,13 @@ jobs: - uses: actions/checkout@v3 with: persist-credentials: false - - uses: actions-rs/toolchain@v1 + - uses: dtolnay/rust-toolchain@master with: - profile: minimal toolchain: ${{ matrix.rust }} components: clippy - name: Clippy (${{ matrix.os }}) - uses: actions-rs/cargo@v1 - with: - command: clippy-ci + run: cargo clippy-ci - name: Clippy (Android) if: matrix.os == 'ubuntu-latest' @@ -74,15 +71,11 @@ jobs: - uses: actions/checkout@v3 with: persist-credentials: false - - uses: actions-rs/toolchain@v1 - with: - profile: minimal - toolchain: stable - + + - uses: dtolnay/rust-toolchain@stable + - name: Test (${{ matrix.os }}) - uses: actions-rs/cargo@v1 - with: - command: test + run: cargo test - name: Setup Android test environment uses: actions/setup-java@v2 @@ -137,15 +130,10 @@ jobs: - uses: actions/checkout@v3 with: persist-credentials: false - - uses: actions-rs/toolchain@v1 + - uses: dtolnay/rust-toolchain@stable with: - profile: minimal - toolchain: stable components: rustfmt - - uses: actions-rs/cargo@v1 - with: - command: fmt - args: --all -- --check + - run: cargo fmt --all -- --check android_fmt: name: Ktlint From 8b3e6230c3341aa3b2d36f48b93421be9182b366 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Thu, 7 Sep 2023 09:51:57 -0400 Subject: [PATCH 2/9] ci: consistent spacing between steps --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0950ec26..7e6b8aad 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,6 +30,7 @@ jobs: - uses: actions/checkout@v3 with: persist-credentials: false + - uses: dtolnay/rust-toolchain@master with: toolchain: ${{ matrix.rust }} @@ -130,9 +131,11 @@ jobs: - uses: actions/checkout@v3 with: persist-credentials: false + - uses: dtolnay/rust-toolchain@stable with: components: rustfmt + - run: cargo fmt --all -- --check android_fmt: From 5edaaf4ee085602bbe1c8ec5a7ca8f0be2d2d939 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Thu, 7 Sep 2023 09:53:51 -0400 Subject: [PATCH 3/9] ci: split out MSRV clippy job Previously there was one job that ran clippy across all supported operating systems for both the latest stable, and the MSRV Rust toolchains. This commit splits that job into two: one for stable and one for the MSRV toolchain. This will make it easier to customize MSRV specific settings in a subsequent commit. --- .github/workflows/ci.yml | 47 +++++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7e6b8aad..3fc20232 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,18 +22,13 @@ jobs: - ubuntu-latest - macos-latest - windows-latest - rust: - - stable - # MSRV - - 1.56.0 steps: - uses: actions/checkout@v3 with: persist-credentials: false - - uses: dtolnay/rust-toolchain@master + - uses: dtolnay/rust-toolchain@stable with: - toolchain: ${{ matrix.rust }} components: clippy - name: Clippy (${{ matrix.os }}) @@ -59,6 +54,46 @@ jobs: # rustup target add wasm32-unknown-unknown # cargo clippy-ci --target wasm32-unknown-unknown + clippy-msrv: + name: Clippy (MSRV) + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: + - ubuntu-latest + - macos-latest + - windows-latest + steps: + - uses: actions/checkout@v3 + with: + persist-credentials: false + + - uses: dtolnay/rust-toolchain@master + with: + toolchain: "1.56.0" # MSRV + components: clippy + + - name: Install cargo-ndk. + run: | + cargo install cargo-ndk + rustup target add aarch64-linux-android + + - name: Clippy (${{ matrix.os }}) + run: cargo clippy-ci + + - name: Clippy (Android) + if: matrix.os == 'ubuntu-latest' + run: | + cargo ndk -t arm64-v8a clippy-ci + + - name: Clippy (iOS) + if: matrix.os == 'macos-latest' + run: | + rustup target add x86_64-apple-ios + cargo clippy-ci --target x86_64-apple-ios + + # TODO: Consider WASM. See note on "clippy" job. + test: name: Test runs-on: ${{ matrix.os }} From 6a39133247172c5d330dbf7c9751a1fcfa9a492c Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Thu, 7 Sep 2023 09:55:57 -0400 Subject: [PATCH 4/9] ci: add clippy-msrv-ci alias This commit adds a variation of the existing `clippy-ci` alias that only tests the `--lib` target. This is intended to be used when linting with the MSRV, where we don't want dev dependencies and other non-essential code being linted with the MSRV toolchain. --- .cargo/config.toml | 1 + .github/workflows/ci.yml | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index e3432467..42bbb13f 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,2 +1,3 @@ [alias] clippy-ci = "clippy --all-features --all-targets --all" +clippy-msrv-ci = "clippy --all-features --lib --all" # Only check --lib target w/ MSRV toolchain. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3fc20232..988f3521 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,18 +79,18 @@ jobs: rustup target add aarch64-linux-android - name: Clippy (${{ matrix.os }}) - run: cargo clippy-ci + run: cargo clippy-msrv-ci - name: Clippy (Android) if: matrix.os == 'ubuntu-latest' run: | - cargo ndk -t arm64-v8a clippy-ci + cargo ndk -t arm64-v8a clippy-msrv-ci - name: Clippy (iOS) if: matrix.os == 'macos-latest' run: | rustup target add x86_64-apple-ios - cargo clippy-ci --target x86_64-apple-ios + cargo clippy-msrv-ci --target x86_64-apple-ios # TODO: Consider WASM. See note on "clippy" job. From 351179cca02e0840462008b0af75a3fc4843d707 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Thu, 7 Sep 2023 09:58:45 -0400 Subject: [PATCH 5/9] project: MSRV 1.56 -> 1.64 Unfortunately Rust 1.64.0 is the oldest toolchain version that we can get working with cargo-ndk, which is required for building *ring* for the Android targets. The prior MSRV was not being tested correctly in CI and so wasn't actually compatible with the crate as it stands today. --- .github/workflows/ci.yml | 2 +- Cargo.toml | 2 +- README.md | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 988f3521..08d36b49 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -70,7 +70,7 @@ jobs: - uses: dtolnay/rust-toolchain@master with: - toolchain: "1.56.0" # MSRV + toolchain: "1.64.0" # MSRV components: clippy - name: Install cargo-ndk. diff --git a/Cargo.toml b/Cargo.toml index 6d50acc2..09d2378e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ keywords = ["tls", "certificate", "verification", "os", "native"] repository = "https://github.com/1Password/rustls-platform-verifier" license = "MIT OR Apache-2.0" edition = "2021" -rust-version = "1.56" +rust-version = "1.64.0" exclude = [ "android/.run", diff --git a/README.md b/README.md index 6b6a9c22..c3626b0e 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ [![crates.io version](https://img.shields.io/crates/v/rustls-platform-verifier.svg)](https://crates.io/crates/rustls-platform-verifier) [![crate documentation](https://docs.rs/rustls-platform-verifier/badge.svg)](https://docs.rs/rustls-platform-verifier) -![MSRV](https://img.shields.io/badge/rustc-1.56+-blue.svg) +![MSRV](https://img.shields.io/badge/rustc-1.64+-blue.svg) [![crates.io downloads](https://img.shields.io/crates/d/rustls-platform-verifier.svg)](https://crates.io/crates/rustls-platform-verifier) ![CI](https://github.com/1Password/rustls-platform-verifier/workflows/CI/badge.svg) @@ -159,4 +159,4 @@ Licensed under either of Apache License, Version Unless you explicitly state otherwise, any contribution intentionally submitted for inclusion in this crate by you, as defined in the Apache-2.0 license, shall be dual licensed as above, without any additional terms or conditions. - \ No newline at end of file + From 2751c09da521f6be071be7871bf2951fb263cc28 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Thu, 7 Sep 2023 10:00:28 -0400 Subject: [PATCH 6/9] ci: use cargo-ndk 2.12.7 for MSRV tests With an MSRV of 1.64.0 we can use the latest cargo-ndk release in the v2.12.x stream when doing our MSRV testing for the Android target. The 3.0.x and 3.1.x streams require a more aggressive MSRV than we can support at this time. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 08d36b49..84687399 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,7 +75,7 @@ jobs: - name: Install cargo-ndk. run: | - cargo install cargo-ndk + cargo install cargo-ndk --version 2.12.7 rustup target add aarch64-linux-android - name: Clippy (${{ matrix.os }}) From cbe72912fd96a57da91f11eae1fd10f38b59ce1a Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Thu, 7 Sep 2023 10:12:09 -0400 Subject: [PATCH 7/9] verification: fix unnecessary lazy evaluations findings Clippy is flagging a couple instances of this: ``` error: unnecessary closure used with `bool::then` --> src/verification/android.rs:251:18 | 251 | .map(|o| (!o.is_null()).then(|| o)) | ^^^^^^^^^^^^^^^---------- | | | help: use `then_some(..)` instead: `then_some(o)` | ``` This commit applies the recommended fix. --- src/verification/android.rs | 2 +- src/verification/windows.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/verification/android.rs b/src/verification/android.rs index 5bda06a5..90239095 100644 --- a/src/verification/android.rs +++ b/src/verification/android.rs @@ -248,7 +248,7 @@ fn extract_result_info(env: &JNIEnv<'_>, result: JObject<'_>) -> (VerifierStatus let msg = env .get_field(result, "message", "Ljava/lang/String;") .and_then(|m| m.l()) - .map(|o| (!o.is_null()).then(|| o)) + .map(|o| (!o.is_null()).then_some(o)) .and_then(|s| s.map(|s| JavaStr::from_env(env, s.into())).transpose()) .unwrap(); diff --git a/src/verification/windows.rs b/src/verification/windows.rs index cddab88c..23952bfc 100644 --- a/src/verification/windows.rs +++ b/src/verification/windows.rs @@ -184,7 +184,7 @@ impl Certificate { CERT_SET_PROPERTY_IGNORE_PERSIST_ERROR_FLAG, prop_data, ) == TRUE) - .then(|| ()) + .then_some(()) }) } } From 27899a0f96702a482f44a07da090e7e8c84a7909 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Thu, 7 Sep 2023 10:28:58 -0400 Subject: [PATCH 8/9] ci: add cron schedule This commit adds a cron schedule to the CI workflow so we can catch things like new clippy breakages sooner. --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 84687399..232b371e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,6 +4,8 @@ on: - main - "*_dev" pull_request: + schedule: + - cron: '0 18 * * *' name: CI permissions: From 8489d3a2c6015e159e6dcc5e49694404762d8141 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Thu, 7 Sep 2023 10:29:46 -0400 Subject: [PATCH 9/9] ci: add merge_group trigger This will allow using the merge queue feature. --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 232b371e..ec17adce 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,6 +4,7 @@ on: - main - "*_dev" pull_request: + merge_group: schedule: - cron: '0 18 * * *'