From c79b6619e2689edbd092bea1356ac1ea8359d7f5 Mon Sep 17 00:00:00 2001 From: dhilipsiva Date: Thu, 18 Jul 2024 22:48:37 +0530 Subject: [PATCH] ci: fix clippy actions workflow and add cargo-fmt action (#353) * ci: Improve workflow configuration and code formatting * chore: cargo fmt and install protoc * chore: fix rust-clippy and rust-fmt * ci: Refactor GitHub actions for rust-fmt workflow - Renamed the job from "rust-clippy-analyze" to "rust-fmt-analyze" - Updated the name of the job to "Run rust-fmt analyzing" - Removed unused permissions for "security-events" and "actions" - Added the step to configure the cache for cargo - Updated the command to run "cargo fmt" * test: break clippy on purpose to test GH Actions * Refactor GitHub workflows and protoc compilation * chore: Update GitHub workflows for Rust code analysis - Update the configuration for running rust-clippy in GitHub Actions - Update the version of the Actions used in the workflow - Refactor the rust-fmt job in GitHub Actions to remove the continue-on-error flag * ci: Improve GitHub actions for Rust codebase * ci: fail clippy while piping, if need be * ci: Update GitHub Actions workflow for rust-clippy * ci: fix clippy warnings * ci: fix last error * ci: restore comments --- .github/workflows/rust-clippy.yml | 43 ++++++++++++------- .github/workflows/rust-fmt.yml | 39 +++++++++++++++++ .github/workflows/rust.yml | 13 +++++- bin/src/http.rs | 4 +- .../packet_selector/video_vp8_sim.rs | 1 + .../packet_selector/video_vp9_svc.rs | 1 + packages/media_record/src/storage/disk.rs | 4 +- packages/media_record/src/storage/memory.rs | 8 ++-- packages/media_utils/src/f16.rs | 2 +- packages/protocol/build.rs | 1 + packages/transport_webrtc/src/media/vp8.rs | 2 +- 11 files changed, 90 insertions(+), 28 deletions(-) create mode 100644 .github/workflows/rust-fmt.yml diff --git a/.github/workflows/rust-clippy.yml b/.github/workflows/rust-clippy.yml index ebcfb2ff..3068840e 100644 --- a/.github/workflows/rust-clippy.yml +++ b/.github/workflows/rust-clippy.yml @@ -1,7 +1,3 @@ -# This workflow uses actions that are not certified by GitHub. -# They are provided by a third-party and are governed by -# separate terms of service, privacy policy, and support -# documentation. # rust-clippy is a tool that runs a bunch of lints to catch common # mistakes in your Rust code and help improve your Rust code. # More details at https://github.com/rust-lang/rust-clippy @@ -18,6 +14,13 @@ on: schedule: - cron: '29 19 * * 2' +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +env: + CARGO_TERM_COLOR: always + jobs: rust-clippy-analyze: name: Run rust-clippy analyzing @@ -30,23 +33,31 @@ jobs: - name: Checkout code uses: actions/checkout@v4 - - name: Install Rust toolchain - uses: actions-rs/toolchain@16499b5e05bf2e26879000db0c1d13f7e13fa3af #@v1 + - uses: actions/cache@v3 + id: cache-cargo + with: + path: | + ~/.cargo/registry + ~/.cargo/git + target + key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + restore-keys: ${{ runner.os }}-cargo- + + - name: Install Protoc + uses: arduino/setup-protoc@v3 with: - profile: minimal - toolchain: stable - components: clippy - override: true + version: "25.1" + repo-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Run rust-clippy + run: cargo clippy --all-targets --all-features -- -D warnings - name: Install required cargo run: cargo install clippy-sarif sarif-fmt - - name: Run rust-clippy - run: - cargo clippy - --all-features - --message-format=json | clippy-sarif | tee rust-clippy-results.sarif | sarif-fmt - continue-on-error: true + - name: Run rust-sarif + run: cargo clippy --all-features --message-format=json | + clippy-sarif | tee rust-clippy-results.sarif | sarif-fmt - name: Upload analysis results to GitHub uses: github/codeql-action/upload-sarif@v3 diff --git a/.github/workflows/rust-fmt.yml b/.github/workflows/rust-fmt.yml new file mode 100644 index 00000000..1fd3b5b6 --- /dev/null +++ b/.github/workflows/rust-fmt.yml @@ -0,0 +1,39 @@ +name: rust-fmt analyze + +on: + push: + branches: [ "master" ] + pull_request: + # The branches below must be a subset of the branches above + branches: [ "master" ] + schedule: + - cron: '29 19 * * 2' + +concurrency: + # One build per PR, branch or tag + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +env: + CARGO_TERM_COLOR: always + +jobs: + rust-fmt-analyze: + name: Run rust-fmt analyzing + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - uses: actions/cache@v3 + with: + path: | + ~/.cargo/bin/ + ~/.cargo/registry/index/ + ~/.cargo/registry/cache/ + ~/.cargo/git/db/ + target/ + key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + + - name: cargo fmt + run: cargo fmt --all -- --check diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index f0822714..676388da 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -25,8 +25,17 @@ jobs: run: | sudo apt-get update sudo apt install -y libsoxr-dev libopus-dev libssl-dev libfdk-aac-dev - - name: Install Rust - run: rustup update stable + + - uses: actions/cache@v3 + with: + path: | + ~/.cargo/bin/ + ~/.cargo/registry/index/ + ~/.cargo/registry/cache/ + ~/.cargo/git/db/ + target/ + key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + - name: Install Protoc uses: arduino/setup-protoc@v3 with: diff --git a/bin/src/http.rs b/bin/src/http.rs index 8d4fb10f..dd3d1083 100644 --- a/bin/src/http.rs +++ b/bin/src/http.rs @@ -8,8 +8,6 @@ use media_server_protocol::protobuf::cluster_connector::MediaConnectorServiceCli use media_server_protocol::rpc::quinn::{QuinnClient, QuinnStream}; use media_server_protocol::transport::{RpcReq, RpcRes}; use media_server_secure::{MediaEdgeSecure, MediaGatewaySecure}; -#[cfg(feature = "embed_static")] -use utils::EmbeddedFilesEndpoint; #[cfg(not(feature = "embed_static"))] use poem::endpoint::StaticFilesEndpoint; use poem::{listener::TcpListener, middleware::Cors, EndpointExt, Route, Server}; @@ -17,6 +15,8 @@ use poem_openapi::types::{ToJSON, Type}; use poem_openapi::OpenApiService; use poem_openapi::{types::ParseFromJSON, Object}; use tokio::sync::mpsc::Sender; +#[cfg(feature = "embed_static")] +use utils::EmbeddedFilesEndpoint; mod api_connector; mod api_console; diff --git a/packages/media_core/src/endpoint/internal/local_track/packet_selector/video_vp8_sim.rs b/packages/media_core/src/endpoint/internal/local_track/packet_selector/video_vp8_sim.rs index 2efddf1b..a9cba716 100644 --- a/packages/media_core/src/endpoint/internal/local_track/packet_selector/video_vp8_sim.rs +++ b/packages/media_core/src/endpoint/internal/local_track/packet_selector/video_vp8_sim.rs @@ -254,6 +254,7 @@ mod tests { res } + #[allow(clippy::too_many_arguments)] fn video_pkt(seq: u16, ts: u32, key: bool, layers: Option<&[[u16; 3]]>, spatial: u8, temporal: u8, layer_sync: bool, tl0idx: u8, pic_id: u16) -> MediaPacket { MediaPacket { ts, diff --git a/packages/media_core/src/endpoint/internal/local_track/packet_selector/video_vp9_svc.rs b/packages/media_core/src/endpoint/internal/local_track/packet_selector/video_vp9_svc.rs index 90d64484..d6365a96 100644 --- a/packages/media_core/src/endpoint/internal/local_track/packet_selector/video_vp9_svc.rs +++ b/packages/media_core/src/endpoint/internal/local_track/packet_selector/video_vp9_svc.rs @@ -268,6 +268,7 @@ mod tests { res } + #[allow(clippy::too_many_arguments)] fn video_pkt(seq: u16, ts: u32, key: bool, layers: Option<&[[u16; 3]]>, spatial: u8, temporal: u8, switching_point: bool, end_frame: bool, pic_id: u16) -> MediaPacket { MediaPacket { ts, diff --git a/packages/media_record/src/storage/disk.rs b/packages/media_record/src/storage/disk.rs index 6180de62..e90aa7b0 100644 --- a/packages/media_record/src/storage/disk.rs +++ b/packages/media_record/src/storage/disk.rs @@ -154,8 +154,8 @@ mod test { let storage = DiskStorage::new("/tmp/"); let mut mem_file = MemoryFile::default(); - mem_file.write(&[1, 2, 3, 4]).await.expect("should write"); - mem_file.write(&[5, 6, 7, 8, 9]).await.expect("should write"); + mem_file.write_all(&[1, 2, 3, 4]).await.expect("should write"); + mem_file.write_all(&[5, 6, 7, 8, 9]).await.expect("should write"); let mut file = storage.copy_from_mem(mem_file).await.expect("Should create file"); diff --git a/packages/media_record/src/storage/memory.rs b/packages/media_record/src/storage/memory.rs index 4f31ee0c..5f43013d 100644 --- a/packages/media_record/src/storage/memory.rs +++ b/packages/media_record/src/storage/memory.rs @@ -159,8 +159,8 @@ mod test { use std::io::Write; let mut file = MemoryFile::default(); - file.write(&[1, 2, 3, 4]).expect("should write"); - file.write(&[5, 6, 7, 8, 9]).expect("should write"); + file.write_all(&[1, 2, 3, 4]).expect("should write"); + file.write_all(&[5, 6, 7, 8, 9]).expect("should write"); let mut buf = [0; 10]; let len = file.read(&mut buf).await.unwrap(); @@ -177,8 +177,8 @@ mod test { use tokio::io::AsyncWriteExt; let mut file = MemoryFile::default(); - file.write(&[1, 2, 3, 4]).await.expect("should write"); - file.write(&[5, 6, 7, 8, 9]).await.expect("should write"); + file.write_all(&[1, 2, 3, 4]).await.expect("should write"); + file.write_all(&[5, 6, 7, 8, 9]).await.expect("should write"); let mut buf = [0; 10]; let len = file.read(&mut buf).await.unwrap(); diff --git a/packages/media_utils/src/f16.rs b/packages/media_utils/src/f16.rs index 6a51f9c7..e56e81ab 100644 --- a/packages/media_utils/src/f16.rs +++ b/packages/media_utils/src/f16.rs @@ -32,7 +32,7 @@ pub struct F16i(i16); impl From for f32 { fn from(val: F16i) -> Self { - val.0 as f32 / 100.0 + val.0 as f32 / 100.0 } } diff --git a/packages/protocol/build.rs b/packages/protocol/build.rs index 2c1543c0..0d698937 100644 --- a/packages/protocol/build.rs +++ b/packages/protocol/build.rs @@ -28,6 +28,7 @@ fn main() -> Result<()> { .type_attribute("cluster_connector.PeerEvent.LocalTrack", "#[derive(serde::Serialize)]") .type_attribute("cluster_connector.PeerEvent.LocalTrackAttach", "#[derive(serde::Serialize)]") .type_attribute("cluster_connector.PeerEvent.LocalTrackDetach", "#[derive(serde::Serialize)]") + .protoc_arg("--experimental_allow_proto3_optional") .compile_protos( &[ "./proto/shared.proto", diff --git a/packages/transport_webrtc/src/media/vp8.rs b/packages/transport_webrtc/src/media/vp8.rs index 9d43a29e..e3c8ac6b 100644 --- a/packages/transport_webrtc/src/media/vp8.rs +++ b/packages/transport_webrtc/src/media/vp8.rs @@ -99,7 +99,6 @@ pub fn parse_rtp(packet: &[u8], rid: Option) -> Option { } } -#[allow(unused_assignments)] pub fn rewrite_rtp(payload: &mut [u8], sim: &Vp8Sim) { let mut payload_index = 0; @@ -130,6 +129,7 @@ pub fn rewrite_rtp(payload: &mut [u8], sim: &Vp8Sim) { } } + #[allow(unused_assignments)] if l == 1 { payload[payload_index] = sim.tl0_pic_idx.unwrap_or(0); payload_index += 1;