-
Notifications
You must be signed in to change notification settings - Fork 190
[ISSUE #5006]♻️Refactor CI workflow: unify system dependencies, streamline caching, and simplify build/test matrix for all platforms #5007
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| name: Rocketmq Rust CI | ||
| name: RocketMQ Rust CI | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
@@ -14,130 +14,178 @@ env: | |
| RUST_BACKTRACE: full | ||
| CI: true | ||
|
|
||
| # ===== RocksDB / build cache ===== | ||
| CARGO_TARGET_DIR: target | ||
| ROCKSDB_DISABLE_JEMALLOC: 1 | ||
|
|
||
| jobs: | ||
| # Format and lint checks | ||
| # ========================= | ||
| # Format + Clippy | ||
| # ========================= | ||
| check: | ||
| name: Check (fmt + clippy) | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Install dependencies (ubuntu-latest) | ||
| - name: Install system dependencies (Ubuntu) | ||
| run: | | ||
| sudo apt-get install protobuf-compiler | ||
| sudo apt-get update | ||
| sudo apt-get install -y \ | ||
| clang llvm libclang-dev \ | ||
| cmake make ninja-build pkg-config \ | ||
| protobuf-compiler \ | ||
| libsnappy-dev liblz4-dev libzstd-dev zlib1g-dev | ||
|
|
||
| echo "CC=clang" >> $GITHUB_ENV | ||
| echo "CXX=clang++" >> $GITHUB_ENV | ||
|
|
||
| LIBCLANG_DIR=$(ls -d /usr/lib/llvm-*/lib | head -n 1) | ||
| echo "LIBCLANG_PATH=$LIBCLANG_DIR" >> $GITHUB_ENV | ||
|
|
||
| - name: Set up Rust nightly | ||
| uses: dtolnay/rust-toolchain@nightly | ||
| with: | ||
| components: rustfmt, clippy | ||
|
|
||
| - name: Cache Rust dependencies | ||
| - name: Cache Rust + RocksDB | ||
| uses: Swatinem/rust-cache@v2 | ||
| with: | ||
| shared-key: rust-ci | ||
|
|
||
| - name: Install dependencies | ||
| run: | | ||
| rustup component add rustfmt | ||
| rustup component add clippy | ||
| shared-key: rocketmq-rust-ubuntu | ||
| workspaces: | | ||
| . | ||
| env-vars: | | ||
| CC | ||
| CXX | ||
| LIBCLANG_PATH | ||
| ROCKSDB_DISABLE_JEMALLOC | ||
|
|
||
| - name: Format check | ||
| run: cargo fmt --all -- --check | ||
|
|
||
| - name: Clippy check (other packages) | ||
| run: cargo clippy --all-targets --workspace --exclude rocketmq-controller --all-features -- -D warnings | ||
| - name: Clippy (all features) | ||
| run: cargo clippy --workspace --all-targets --all-features -- -D warnings | ||
|
|
||
| - name: Clippy check (controller without rocksdb) | ||
| run: cargo clippy --all-targets -p rocketmq-controller --no-default-features --features storage-file,metrics,debug -- -D warnings | ||
|
|
||
| # Build and test on multiple platforms | ||
| # ========================= | ||
| # Build + Test | ||
| # ========================= | ||
| build-test: | ||
| name: Build & Test (${{ matrix.os }}) | ||
| runs-on: ${{ matrix.os }} | ||
| needs: check | ||
| runs-on: ${{ matrix.os }} | ||
|
|
||
| strategy: | ||
| fail-fast: true | ||
| fail-fast: false | ||
| matrix: | ||
| os: [ ubuntu-latest, macos-latest, windows-latest ] | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - if: matrix.os == 'ubuntu-latest' | ||
| name: Install dependencies (ubuntu-latest) | ||
| # ---------- Ubuntu ---------- | ||
| - name: Install system dependencies (Ubuntu) | ||
| if: matrix.os == 'ubuntu-latest' | ||
| run: | | ||
| sudo apt-get install protobuf-compiler | ||
| - if: matrix.os == 'macos-latest' | ||
| name: Install dependencies (macos-latest) | ||
| sudo apt-get update | ||
| sudo apt-get install -y \ | ||
| clang llvm libclang-dev \ | ||
| cmake make ninja-build pkg-config \ | ||
| protobuf-compiler \ | ||
| libsnappy-dev liblz4-dev libzstd-dev zlib1g-dev | ||
|
|
||
| echo "CC=clang" >> $GITHUB_ENV | ||
| echo "CXX=clang++" >> $GITHUB_ENV | ||
| LIBCLANG_DIR=$(ls -d /usr/lib/llvm-*/lib | head -n 1) | ||
| echo "LIBCLANG_PATH=$LIBCLANG_DIR" >> $GITHUB_ENV | ||
|
|
||
| # ---------- macOS ---------- | ||
| - name: Install system dependencies (macOS) | ||
| if: matrix.os == 'macos-latest' | ||
| run: | | ||
| FILENAME="protoc-3.20.3-osx-x86_64.zip" | ||
| URL=https://github.com/protocolbuffers/protobuf/releases/download/v3.20.3/protoc-3.20.3-osx-x86_64.zip | ||
| echo "Detected arch: $ARCH, downloading $FILENAME" | ||
| curl -LO "$URL" | ||
|
|
||
| sudo unzip -o "$FILENAME" -d /usr/local | ||
| protoc --version | ||
|
|
||
| - if: matrix.os == 'windows-latest' | ||
| name: Install dependencies (windows-latest) | ||
| brew update | ||
| brew install cmake protobuf snappy lz4 zstd | ||
|
|
||
| # ---------- Windows ---------- | ||
| - name: Install system dependencies (Windows) | ||
| if: matrix.os == 'windows-latest' | ||
| run: | | ||
| choco install protoc | ||
| choco install cmake ninja protoc -y | ||
|
|
||
| - name: Set up Rust nightly | ||
| uses: dtolnay/rust-toolchain@nightly | ||
|
|
||
| - name: Cache Rust dependencies | ||
| - name: Cache Rust + RocksDB | ||
| uses: Swatinem/rust-cache@v2 | ||
| with: | ||
| shared-key: rust-ci | ||
|
|
||
| - name: Build (other packages) | ||
| run: cargo build --verbose --workspace --exclude rocketmq-controller --all-features | ||
|
|
||
| - name: Build (controller without rocksdb) | ||
| run: cargo build --verbose -p rocketmq-controller --no-default-features --features storage-file,metrics,debug | ||
|
|
||
| - name: Run tests (other packages) | ||
| run: cargo test --verbose --workspace --exclude rocketmq-controller --all-features | ||
|
|
||
| - name: Run tests (controller without rocksdb) | ||
| run: cargo test --verbose -p rocketmq-controller --no-default-features --features storage-file,metrics,debug | ||
|
|
||
| # Code coverage (only on Ubuntu) | ||
| shared-key: rocketmq-rust-${{ matrix.os }} | ||
| workspaces: | | ||
| . | ||
| env-vars: | | ||
| CC | ||
| CXX | ||
| LIBCLANG_PATH | ||
| ROCKSDB_DISABLE_JEMALLOC | ||
|
Comment on lines
+120
to
+130
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cache env-vars include variables not set on all platforms. The cache configuration lists
This can lead to suboptimal cache behavior, as the cache key will differ when these variables are unset vs. set, potentially causing cache misses or inconsistencies across platforms. Recommended fix: Either set these variables consistently across all platforms, or make the cache 🤖 Prompt for AI Agents |
||
|
|
||
| - name: Build (all features) | ||
| run: cargo build --workspace --all-features --verbose | ||
|
|
||
| - name: Test (all features) | ||
| run: cargo test --workspace --all-features --verbose | ||
|
|
||
| # ========================= | ||
| # Coverage (Ubuntu only) | ||
| # ========================= | ||
| coverage: | ||
| name: Code Coverage | ||
| runs-on: ubuntu-latest | ||
| needs: check | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Install dependencies (ubuntu-latest) | ||
| run: sudo apt-get install protobuf-compiler | ||
| - name: Install system dependencies (Ubuntu) | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y \ | ||
| clang llvm libclang-dev \ | ||
| cmake make ninja-build pkg-config \ | ||
| protobuf-compiler \ | ||
| libsnappy-dev liblz4-dev libzstd-dev zlib1g-dev | ||
|
|
||
| echo "CC=clang" >> $GITHUB_ENV | ||
| echo "CXX=clang++" >> $GITHUB_ENV | ||
| LIBCLANG_DIR=$(ls -d /usr/lib/llvm-*/lib | head -n 1) | ||
| echo "LIBCLANG_PATH=$LIBCLANG_DIR" >> $GITHUB_ENV | ||
|
Comment on lines
+150
to
+162
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Third instance of duplicated dependency installation logic. This Ubuntu dependency installation block is identical to the ones in the check job (lines 33-46) and build-test job (lines 88-103). Consider extracting this into a reusable composite action to reduce duplication and ensure consistency. Benefits:
Example composite action structure: Create name: 'Setup Ubuntu Dependencies'
description: 'Install system dependencies for RocketMQ Rust on Ubuntu'
runs:
using: 'composite'
steps:
- name: Install system dependencies
shell: bash
run: |
sudo apt-get update
sudo apt-get install -y \
clang llvm libclang-dev \
cmake make ninja-build pkg-config \
protobuf-compiler \
libsnappy-dev liblz4-dev libzstd-dev zlib1g-dev
echo "CC=clang" >> $GITHUB_ENV
echo "CXX=clang++" >> $GITHUB_ENV
LIBCLANG_DIR=$(ls -d /usr/lib/llvm-*/lib | sort -V | tail -n 1)
echo "LIBCLANG_PATH=$LIBCLANG_DIR" >> $GITHUB_ENVThen replace each Ubuntu installation block with: - name: Setup Ubuntu dependencies
uses: ./.github/actions/setup-ubuntu-deps</review_comment_end> 🤖 Prompt for AI Agents |
||
|
|
||
| - name: Set up Rust nightly | ||
| uses: dtolnay/rust-toolchain@nightly | ||
|
|
||
| - name: Cache Rust dependencies | ||
| - name: Cache Rust + RocksDB | ||
| uses: Swatinem/rust-cache@v2 | ||
| with: | ||
| shared-key: rust-ci | ||
| shared-key: rocketmq-rust-coverage | ||
| workspaces: | | ||
| . | ||
| env-vars: | | ||
| CC | ||
| CXX | ||
| LIBCLANG_PATH | ||
| ROCKSDB_DISABLE_JEMALLOC | ||
|
|
||
| - name: Install cargo-llvm-cov | ||
| uses: taiki-e/install-action@cargo-llvm-cov | ||
|
|
||
| - name: Generate code coverage (other packages) | ||
| run: | | ||
| cargo llvm-cov --workspace --exclude rocketmq-controller --all-features --lcov --output-path lcov-other.info | ||
| cargo llvm-cov -p rocketmq-controller --no-default-features --features storage-file,metrics,debug --lcov --output-path lcov-controller.info | ||
| cat lcov-other.info lcov-controller.info > lcov.info | ||
| - name: Generate coverage (all features) | ||
| run: cargo llvm-cov --workspace --all-features --lcov --output-path lcov.info | ||
|
|
||
| - name: Upload coverage to Codecov | ||
| - name: Upload to Codecov | ||
| uses: codecov/codecov-action@v5 | ||
| with: | ||
| token: ${{ secrets.CODECOV_TOKEN }} | ||
| files: lcov.info | ||
| fail_ci_if_error: false | ||
| verbose: true | ||
| verbose: true | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing compiler and LIBCLANG_PATH configuration on macOS.
Unlike Ubuntu (lines 99-102), the macOS step does not set
CC,CXX, orLIBCLANG_PATHenvironment variables. This creates inconsistency across platforms and may lead to:LIBCLANG_PATH🔎 Suggested fix to align macOS configuration
- name: Install system dependencies (macOS) if: matrix.os == 'macos-latest' run: | brew update brew install cmake protobuf snappy lz4 zstd + + echo "CC=clang" >> $GITHUB_ENV + echo "CXX=clang++" >> $GITHUB_ENV + # macOS ships with LLVM/clang in a standard location + echo "LIBCLANG_PATH=/usr/local/opt/llvm/lib" >> $GITHUB_ENVNote: Verify the actual LLVM path on
macos-latestrunners if using Homebrew LLVM.