-
Notifications
You must be signed in to change notification settings - Fork 6
ci: use actions/cache for Android NDK instead of broken local-cache #393
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
base: master
Are you sure you want to change the base?
Conversation
The nttld/setup-ndk local-cache option has a known bug where symlinks break after cache restore (see nttld/setup-ndk#547). This causes NDK to be re-downloaded on every run despite caching. Switch to manual caching with actions/cache which properly handles the NDK directory. The cache key is simple (OS + NDK version) since the NDK content is static. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThe pull request adds Android NDK caching to two CI workflows ( Changes
Sequence Diagram(s)sequenceDiagram
participant GH as "GitHub Actions"
participant Cache as "NDK Cache"
participant Setup as "Setup Android NDK"
participant Build as "Rust Build Step"
GH->>Cache: restore ndk-cache key
alt cache hit
Cache-->>GH: cache hit
GH->>Build: run build (NDK_HOME=/home/runner/.setup-ndk/r27c)
else cache miss
Cache-->>GH: cache miss
GH->>Setup: install NDK r27c
Setup-->>GH: NDK installed at /home/runner/.setup-ndk/r27c
GH->>Build: run build (NDK_HOME=/home/runner/.setup-ndk/r27c)
GH->>Cache: save ndk-cache
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Deploying maple with
|
| Latest commit: |
aa851da
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://084b35bf.maple-ca8.pages.dev |
| Branch Preview URL: | https://fix-ndk-cache-manual.maple-ca8.pages.dev |
Greptile OverviewGreptile SummaryImplemented manual NDK caching using Key Changes:
Minor Consideration:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant GHA as GitHub Actions
participant Cache as actions/cache
participant NDK as setup-ndk
participant Build as Rust Build
GHA->>Cache: Check for cached NDK at /home/runner/.setup-ndk/r27c
alt Cache Hit
Cache->>GHA: Restore NDK from cache
Note over NDK: Setup NDK step skipped (if condition false)
GHA->>Build: Use cached NDK at /home/runner/.setup-ndk/r27c
else Cache Miss
Cache->>GHA: No cache found
GHA->>NDK: Run setup-ndk (download ~1.5GB)
NDK->>GHA: NDK installed to /home/runner/.setup-ndk/r27c
GHA->>Cache: Save NDK to cache
GHA->>Build: Use newly installed NDK at /home/runner/.setup-ndk/r27c
end
Build->>Build: Export NDK_HOME and build Rust library
|
Without local-cache: true, setup-ndk puts NDK in /opt/hostedtoolcache/ instead of ~/.setup-ndk/r27c where the build expects it.
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.
2 files reviewed, 2 comments
| with: | ||
| ndk-version: r27c | ||
| add-to-path: true | ||
| local-cache: true |
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.
local-cache: true still enabled but PR description says this option has symlink bugs. Consider removing since you're using actions/cache now.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/android-build.yml
Line: 142:142
Comment:
`local-cache: true` still enabled but PR description says this option has symlink bugs. Consider removing since you're using `actions/cache` now.
How can I resolve this? If you propose a fix, please make it concise.| with: | ||
| ndk-version: r27c | ||
| add-to-path: true | ||
| local-cache: true |
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.
local-cache: true still enabled but PR description says this option has symlink bugs. Consider removing since you're using actions/cache now.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release.yml
Line: 267:267
Comment:
`local-cache: true` still enabled but PR description says this option has symlink bugs. Consider removing since you're using `actions/cache` now.
How can I resolve this? If you propose a fix, please make it concise.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/release.yml:
- Around line 255-267: The workflow hardcodes the NDK install path and thus
mismatches the dynamic path returned by the setup action; update the cache and
environment usage to consume the setup action output instead: read the NDK path
from the setup action's output (steps.setup-ndk.outputs.ndk-path) returned by
uses: nttld/setup-ndk@v1 (the step currently referenced as setup-ndk), replace
the hardcoded /home/runner/.setup-ndk/r27c used in the actions/cache step (id:
ndk-cache) and any NDK-related env vars (NDK_HOME, ANDROID_NDK_HOME, etc.) with
that output reference so the cache key/path and environment variables
consistently use ${{ steps.setup-ndk.outputs.ndk-path }}.
♻️ Duplicate comments (1)
.github/workflows/android-build.yml (1)
187-190: Consider DRY-ing NDK path/version (same as release.yml).
The same drift/sanity-check suggestion applies here too.
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
312-316: Centralize NDK_VERSION/NDK_HOME and add a quick sanity check.
The hard-coded path appears in multiple places; definingNDK_VERSION/NDK_HOMEonce reduces drift when upgrading and a quicktest -xguard makes cache corruption fail fast.♻️ Proposed refactor
jobs: android-rust-build: needs: android-frontend-build runs-on: ubuntu-latest-8-cores + env: + NDK_VERSION: r27c + NDK_HOME: /home/runner/.setup-ndk/r27c @@ - name: Cache Android NDK uses: actions/cache@v4 id: ndk-cache with: - path: /home/runner/.setup-ndk/r27c - key: ${{ runner.os }}-ndk-r27c + path: ${{ env.NDK_HOME }} + key: ${{ runner.os }}-ndk-${{ env.NDK_VERSION }} @@ - name: Setup Android NDK if: steps.ndk-cache.outputs.cache-hit != 'true' uses: nttld/setup-ndk@v1 with: - ndk-version: r27c + ndk-version: ${{ env.NDK_VERSION }} @@ - name: Build Rust library (${{ matrix.rust_target }}) run: | - export NDK_HOME=/home/runner/.setup-ndk/r27c + export NDK_HOME=${NDK_HOME} + test -x "$NDK_HOME/toolchains/llvm/prebuilt/linux-x86_64/bin/clang" || { echo "NDK missing/corrupt"; exit 1; } export PATH=$NDK_HOME/toolchains/llvm/prebuilt/linux-x86_64/bin:$PATH
| - name: Cache Android NDK | ||
| uses: actions/cache@v4 | ||
| id: ndk-cache | ||
| with: | ||
| path: /home/runner/.setup-ndk/r27c | ||
| key: ${{ runner.os }}-ndk-r27c | ||
|
|
||
| - name: Setup Android NDK | ||
| if: steps.ndk-cache.outputs.cache-hit != 'true' | ||
| uses: nttld/setup-ndk@v1 | ||
| id: setup-ndk | ||
| with: | ||
| ndk-version: r27c | ||
| add-to-path: true | ||
| local-cache: true | ||
|
|
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.
🧩 Analysis chain
🌐 Web query:
nttld/setup-ndk v1 default install directory path
💡 Result:
nttld/setup-ndk@v1 does not expose a single fixed global path — it downloads/extracts the requested NDK for the runner/workflow and returns the actual install location as the action output steps..outputs.ndk-path. Use that output (examples show ANDROID_NDK_HOME: ${{ steps.setup-ndk.outputs.ndk-path }}). [1]
Source: action marketplace page / usage examples. [1]
🌐 Web query:
GitHub Actions cache size limit per repository
💡 Result:
Short answer
- Default per-repository cache quota: 10 GB. [1][2]
- Unused caches (not accessed) are eligible for eviction after 7 days; when the repo exceeds its quota, least‑recently‑used caches are evicted. [1][3]
- You can raise the per‑repo limit (paid, admin-configurable / pay‑as‑you‑go). Enterprise/org admins can set higher maxima — Enterprise Cloud docs note configurable limits up to 10,000 GB per repo (and GitHub announced support for >10 GB with billing). [2][3]
Sources
- actions/cache README (cache limits & eviction). [1]
- GitHub changelog: cache can exceed 10 GB / pay-as-you-go (Nov 20, 2025). [2]
- GitHub Docs — managing Actions cache settings / Enterprise Cloud limits (configurable maxes, eviction/retention). [3]
🏁 Script executed:
# Check if there are any other references to NDK paths in the workflow
rg "ndk|NDK" .github/workflows/release.yml -nRepository: OpenSecretCloud/Maple
Length of output: 1264
NDK cache and setup need critical fixes to work reliably.
The nttld/setup-ndk@v1 action returns the NDK install path dynamically via steps.ndk-cache.outputs.ndk-path — it does not guarantee installation to /home/runner/.setup-ndk/r27c. The workflow currently hardcodes this path in both the cache configuration (line 259) and environment variables (lines 315–328), creating a mismatch: the cache may miss if the action installs elsewhere, and the hardcoded NDK_HOME may point to the wrong location.
Fix: Capture the NDK path from the setup action output and use it consistently. Replace hardcoded paths with ${{ steps.setup-ndk.outputs.ndk-path }}.
GitHub Actions cache limits (10 GB default per repo) are not a concern for the NDK size.
🤖 Prompt for AI Agents
In @.github/workflows/release.yml around lines 255 - 267, The workflow hardcodes
the NDK install path and thus mismatches the dynamic path returned by the setup
action; update the cache and environment usage to consume the setup action
output instead: read the NDK path from the setup action's output
(steps.setup-ndk.outputs.ndk-path) returned by uses: nttld/setup-ndk@v1 (the
step currently referenced as setup-ndk), replace the hardcoded
/home/runner/.setup-ndk/r27c used in the actions/cache step (id: ndk-cache) and
any NDK-related env vars (NDK_HOME, ANDROID_NDK_HOME, etc.) with that output
reference so the cache key/path and environment variables consistently use ${{
steps.setup-ndk.outputs.ndk-path }}.
The
nttld/setup-ndklocal-cacheoption has a known bug where symlinks break after cache restore (see nttld/setup-ndk#547). This causes the NDK to be re-downloaded on every run despite caching attempts.Changes:
actions/cachewhich properly handles the NDK directoryLinux-ndk-r27c) since NDK content is static for a given versionsetup-ndkaction entirely when cache hitsExpected behavior:
This should eliminate the 5-15 minute waits you've been seeing when Google's CDN is slow.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.