-
Notifications
You must be signed in to change notification settings - Fork 478
Refine CI workflows and fix macOS matrix gap #557
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
Conversation
- Rename workflow file from android.yml to build.yml (best practice)
- Support 3 OS platforms: Windows, macOS, Ubuntu
- Support 2 NDK versions: r26d (stable), r27c (latest)
- Support 2 build systems: CMake and ndk-build
- Support 4 product variants: ±FFmpeg × ±16KB page size
- Total 48 build configurations (3×2×2×2×2)
Trigger strategy:
- Master branch: Full build (48 jobs)
- Pull requests: Simplified build (18 jobs)
- Windows: 1 job (r26d, CMake, FFmpeg, 16KB)
- macOS: 1 job (r27c, ndk-build, no-FFmpeg, 4KB)
- Ubuntu: 16 jobs (full matrix)
- Other branches: No build (save resources)
Features:
- Structured artifact naming: apk-{OS}-{NDK}-{BuildSystem}-{FFmpeg}-{PageSize}
- Conditional job execution based on branch
- Cross-platform build tool installation
- Separate lint job for code quality (master only)
- Added comprehensive workflow documentation
Co-authored-by: wysaid <1430725+wysaid@users.noreply.github.com>
Co-authored-by: wysaid <1430725+wysaid@users.noreply.github.com>
WalkthroughReplaces a legacy Android CI with matrix-driven workflows: adds Ubuntu, Windows, and macOS build workflows plus a release workflow and a CI README; removes the old single-file Android workflow. Workflows drive NDK/JDK/Ninja setup, matrix builds (cmake/ndk-build, FFmpeg, page size), artifact naming and uploads, and tag-based releases. Changes
Sequence Diagram(s)sequenceDiagram
participant Trigger as Git Event (push / PR / tag)
participant Orchestrator as Matrix Orchestrator
participant Runner as CI Runner (ubuntu / windows / macos)
participant Env as Env Setup (NDK / JDK / Ninja)
participant Builder as Build Script (`./tasks.sh`)
participant Store as Artifact Storage (upload)
participant Release as GitHub Release
Trigger->>Orchestrator: start workflow (matrix params)
Orchestrator->>Runner: spawn job for matrix combination
Runner->>Env: checkout + install NDK/JDK/Ninja
Env->>Builder: set BUILD_SYSTEM_FLAG / FFMPEG_FLAG / PAGE_SIZE_FLAG
Builder->>Builder: build APK (cmake or ndk-build)
Builder->>Store: locate, rename & upload APK
alt tag-triggered release
Orchestrator->>Release: collect APK variants
Release->>Release: publish APKs to GitHub Release
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@Auggie review |
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.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In @.github/workflows/macos-build.yml:
- Around line 164-169: The workflow step named "Setup JDK 17" is using the
outdated actions/setup-java@v3; update the action reference to
actions/setup-java@v4 (i.e., change uses: actions/setup-java@v3 to uses:
actions/setup-java@v4) to move to the actively maintained major release while
keeping the existing inputs (java-version: '17', distribution: 'temurin', cache:
gradle) unchanged.
- Around line 146-147: The job-level if condition uses matrix.run-on which is
not available at job evaluation time; change the logic so the matrix context is
only referenced where it exists: either move the conditional into step-level if
expressions (so reference matrix.run-on in individual steps), or instead filter
matrix combinations via strategy.matrix.include/exclude to produce only desired
run-on values, or split into separate jobs (one guarded by github.ref ==
'refs/heads/master' and another for matrix PR runs). Update the line containing
the job-level "if:" that references matrix.run-on and github.ref accordingly and
ensure any step-level conditions use matrix.run-on while job-level conditions
use only contexts available at job evaluation.
In @.github/workflows/README.md:
- Around line 107-110: The README's "Lint 检查" section is inaccurate: it states
lint runs only on master pushes while the workflow file ubuntu-build.yml
triggers lint on PRs too; update the README text under "Lint 检查" to reflect the
actual triggers (e.g., mention that lint runs on pull requests and master branch
pushes) or alternatively adjust ubuntu-build.yml so lint runs only on
master—edit the line in README and any descriptive text referencing the workflow
to match the behavior of ubuntu-build.yml (or vice versa) so the documentation
and workflow are consistent.
- Around line 98-100: Update the fenced code block that contains the artifact
name "apk-{OS}-{NDK}-{BuildSystem}-{FFmpeg}-{PageSize}.apk" to include a
language identifier (e.g., change the opening fence from ``` to ```text) so
markdownlint MD040 is satisfied; leave the block contents unchanged and only
modify the opening fence line.
In @.github/workflows/release.yml:
- Around line 71-76: The workflow step named "Setup JDK 17" is using
actions/setup-java@v3 (Node16) which is deprecated; update the uses reference to
actions/setup-java@v4 to ensure Node20 compatibility and GitHub Actions
acceptance, leaving the existing with keys (java-version: '17', distribution:
'temurin', cache: gradle) intact so the JDK setup behavior is unchanged.
In @.github/workflows/ubuntu-build.yml:
- Around line 137-138: The job-level if uses the invalid matrix context
"matrix.run-on" (matrix is not available at job evaluation); remove the
"matrix.run-on == 'always' || " part from the condition so the if becomes just
"if: github.ref == 'refs/heads/master'", ensuring you no longer reference
matrix.run-on in the job-level if and leave the github.ref check intact.
In @.github/workflows/windows-build.yml:
- Around line 139-140: The job-level if using "matrix.run-on == 'always' ||
github.ref == 'refs/heads/master'" is invalid because the matrix context is
unavailable at job evaluation; update the workflow by removing the job-level use
of matrix.run-on and instead either (A) move the conditional into individual
steps (use step-level if expressions that can reference matrix.run-on and
github.ref) for the job that needs gating, or (B) restructure the matrix
generation so non-master combinations are excluded (e.g., create a dynamic
matrix in a preparatory job and pass it via fromJSON to the job's
strategy.matrix), ensuring any references to matrix.run-on happen only where the
matrix context is valid.
- Around line 157-162: The workflow uses actions/setup-java@v3 in the "Setup JDK
17" step which is deprecated; update the action to actions/setup-java@v4
(replace the reference to setup-java@v3 with setup-java@v4) while keeping the
existing inputs (java-version: '17', distribution: 'temurin', cache: gradle) so
the step name "Setup JDK 17" and its behavior remain the same but with the
Node.js-20-compatible v4 action.
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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/macos-build.yml:
- Around line 233-240: The "Find and rename APK" step currently assumes APK_FILE
is found; add validation after computing APK_FILE (the APK_FILE variable in the
"Find and rename APK" step) to check if it's empty or missing and fail early
(echo a clear error and exit 1) or skip copying, before running mkdir/cp; ensure
the cp to "artifacts/cgeDemo-${{ env.ARTIFACT_NAME }}.apk" only runs when
APK_FILE is non-empty so the workflow doesn't try to copy a non-existent file.
In @.github/workflows/ubuntu-build.yml:
- Around line 224-231: The workflow's APK copy step uses APK_FILE without
validating it, so if find returns nothing the cp will fail; update the "Find and
rename APK" run script to check that APK_FILE (the variable set by
APK_FILE=$(find ...)) is non-empty and points to an existing file before
mkdir/cp, and if it is missing emit a clear error and exit non-zero (so
steps.gate.outputs.should_run remains meaningful); reference the APK_FILE
variable and the "Find and rename APK" step when making this change.
In @.github/workflows/windows-build.yml:
- Around line 222-229: The current job uses APK_FILE=$(find ... ) and then
blindly cp "$APK_FILE", which will fail silently if no APK is found; update the
step (the block gated by if: steps.gate.outputs.should_run == 'true') to
validate APK_FILE after assignment by checking if it's empty or non-existent,
and if so echo a clear error like "No release APK found in cgeDemo/build" and
exit non‑zero (fail the job) before attempting cp; otherwise proceed to mkdir -p
artifacts and cp the verified APK to artifacts/cgeDemo-${{ env.ARTIFACT_NAME
}}.apk so the failure is explicit and informative.
♻️ Duplicate comments (4)
.github/workflows/windows-build.yml (1)
166-172: Updateactions/setup-javato v4.
actions/setup-java@v3is deprecated as part of GitHub's Node.js 16 → 20 transition. Update to v4.Proposed fix
- name: Setup JDK 17 if: steps.gate.outputs.should_run == 'true' - uses: actions/setup-java@v3 + uses: actions/setup-java@v4 with: java-version: '17' distribution: 'temurin' cache: gradle.github/workflows/ubuntu-build.yml (2)
164-170: Updateactions/setup-javato v4 in the build job.
actions/setup-java@v3is deprecated. Update to v4 for Node.js 20 compatibility.Proposed fix
- name: Setup JDK 17 if: steps.gate.outputs.should_run == 'true' - uses: actions/setup-java@v3 + uses: actions/setup-java@v4 with: java-version: '17' distribution: 'temurin' cache: gradle
252-257: Updateactions/setup-javato v4 in the lint job.Same deprecation issue applies to the lint job.
Proposed fix
- name: Setup JDK 17 - uses: actions/setup-java@v3 + uses: actions/setup-java@v4 with: java-version: '17' distribution: 'temurin' cache: gradle.github/workflows/macos-build.yml (1)
173-179: Updateactions/setup-javato v4.
actions/setup-java@v3is deprecated. Update to v4.Proposed fix
- name: Setup JDK 17 if: steps.gate.outputs.should_run == 'true' - uses: actions/setup-java@v3 + uses: actions/setup-java@v4 with: java-version: '17' distribution: 'temurin' cache: gradle
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/ubuntu-build.yml:
- Around line 239-244: Update the "Setup JDK 17" GitHub Actions step to use
actions/setup-java@v4 (replace uses: actions/setup-java@v3 with `@v4`) so it
matches the build job and the current maintained version; ensure the existing
inputs (java-version: '17', distribution: 'temurin', cache: gradle) remain
unchanged and the step name ("Setup JDK 17") continues to identify the correct
step.
🧹 Nitpick comments (2)
.github/workflows/ubuntu-build.yml (2)
24-30: Consider removing unusedrun-onfield from matrix entries.The
run-on: alwaysfield appears in all 16 matrix entries but is not referenced anywhere in the workflow. This appears to be leftover from the removed job-levelifcondition. Removing it would reduce configuration noise.
159-161: Redundantrunner.os != 'Windows'condition.This workflow only runs on
ubuntu-latest(all matrix entries specify this), so the Windows check is unnecessary. Consider simplifying by removing the condition.♻️ Suggested simplification
- name: Grant execute permission for scripts (Unix) - if: runner.os != 'Windows' run: chmod +x gradlew tasks.sh
|
已将 |
|
macOS 任务的 NDK 安装已改为 |
* Refactor CI/CD workflow: multi-platform, multi-NDK, multi-config builds
- Rename workflow file from android.yml to build.yml (best practice)
- Support 3 OS platforms: Windows, macOS, Ubuntu
- Support 2 NDK versions: r26d (stable), r27c (latest)
- Support 2 build systems: CMake and ndk-build
- Support 4 product variants: ±FFmpeg × ±16KB page size
- Total 48 build configurations (3×2×2×2×2)
Trigger strategy:
- Master branch: Full build (48 jobs)
- Pull requests: Simplified build (18 jobs)
- Windows: 1 job (r26d, CMake, FFmpeg, 16KB)
- macOS: 1 job (r27c, ndk-build, no-FFmpeg, 4KB)
- Ubuntu: 16 jobs (full matrix)
- Other branches: No build (save resources)
Features:
- Structured artifact naming: apk-{OS}-{NDK}-{BuildSystem}-{FFmpeg}-{PageSize}
- Conditional job execution based on branch
- Cross-platform build tool installation
- Separate lint job for code quality (master only)
- Added comprehensive workflow documentation
* Initial plan
* Initial plan
* Split workflows and add release automation
Co-authored-by: wysaid <1430725+wysaid@users.noreply.github.com>
* Add explicit workflow permissions
Co-authored-by: wysaid <1430725+wysaid@users.noreply.github.com>
* Fix CI triggers for better_workflow and simplify OS-specific steps
* Trigger CI for better_workflow PRs
* Fix workflow triggers and macOS matrix
* Fix matrix gating in workflows
* Reduce PR matrix and fix macOS NDK arch
* Address PR review comments
* Use macos-latest runner for macOS CI
* Update lint job to setup-java v4
* Install NDK via sdkmanager on macOS
* Normalize NDK flavor parsing on macOS
* Handle sdkmanager pipefail on macOS
* Run full workflows on workflow branches and add test release mode
---------
Co-authored-by: wangyang (wysaid) <wysaid@gmail.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: wysaid <1430725+wysaid@users.noreply.github.com>
Summary
Testing
Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.