chore(ci): drop duplicate bundle install in Android jobs#7257
Conversation
… install The Android composite actions ran `bundle install` twice per job: first the implicit one from `ruby/setup-ruby@v1` (against the root iOS-flavored Gemfile, ~125 gems including cocoapods/xcodeproj that Android never uses), then an explicit `bundle install --path gems` for `android/Gemfile`. Setting `working-directory: android` on `setup-ruby` makes `bundler-cache: true` target the right Gemfile directly, so the explicit step becomes redundant and the deprecated `--path` flag goes away. Saves ~40s per Android build/upload job.
WalkthroughThree GitHub Actions workflows in the Android build pipeline have been simplified by removing explicit Bundler installation steps ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
Android Build Available Rocket.Chat 4.72.0.108668 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNQosBSttb-N64t4S6tIshvsoCyD5HVh7XwhM-DXUjUYuxF6rJBA8BQ-3pxJCbg5aqux1k7hmZa2-CKq9rv5 |
|
Android Build Available Rocket.Chat Experimental 4.72.0.108669 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNTghqCZKS2ZGxvOpOx4YIeCJNzzfzDVZeuA1A4NuCpIImlj87Gc-z7ffZ5tVFMCt2EDNO5RA1GJZizDxWZ9 |
diegolmello
left a comment
There was a problem hiding this comment.
Review — #7257 — 2026-04-27T21:18:19Z
Findings: 2 (critical: 0, major: 0, minor: 1, nit: 1, spec-mismatch: 0)
Verdict: APPROVE
Skipped (duplicate of existing thread): 0
Summary
This PR achieves its stated goal: it eliminates the duplicate bundle install previously running on every Android CI job. ruby/setup-ruby@v1 with working-directory: android + bundler-cache: true performs exactly one resolve+install against android/Gemfile, writes android/.bundle/config, and the downstream bundle exec fastlane … calls (which already use working-directory: android) resolve gems via that config. The three composite actions (build-android, upload-android, upload-internal-android) receive identical patches, and a repo-wide grep for android/gems returned no references — nothing else in the repo depends on the deleted install path.
Estimated savings: ~40s per Android job today (one per build-android × 2 = ~80s/run unconditional, up to ~4 min/run when uploads run); up to ~67s per job once the bundler cache warms, since the previous setup's cache key was rooted at the wrong lockfile and never hit on Android jobs.
minor: gem path migration from android/gems/ to android/vendor/bundle
Path: .github/actions/build-android/action.yml, .github/actions/upload-android/action.yml, .github/actions/upload-internal-android/action.yml
Category: design
Suggested fix: none — flagged for awareness.
The deleted explicit step used bundle install --path gems, which placed gems under android/gems/. setup-ruby's default install path is vendor/bundle, so installs now land at android/vendor/bundle/. A repo-wide grep for android/gems shows zero hits in CI scripts, Gemfiles, gradle, fastlane, or .gitignore, so the migration is safe. Worth noting if any external tooling or developer workflow caches the old path.
nit: bundler-cache precondition
Path: .github/actions/build-android/action.yml:47-52, .github/actions/upload-android/action.yml:38-43, .github/actions/upload-internal-android/action.yml:38-43
Category: docs
Suggested fix: none.
bundler-cache: true requires android/Gemfile.lock to exist and be authoritative for cache keying. It does, and the lockfile is committed — just calling out that it is now load-bearing for cache hits.
Verified negatives (not findings)
- Ruby version pin (
2.7.7) untouched — no smuggled upgrade. - No secret-handling changes — FASTLANE/Google service account flow is unaffected.
- Three composite actions patched identically — symmetry confirmed.
- No workflow YAML or shell script references the old
android/gems/path. - Subsequent
bundle exec fastlane …invocations useworking-directory: androidand will pick up the new.bundle/config.
Positive observations
- Surgical CI change, net -15 lines, three-file symmetry.
- Uses official action behavior (
bundler-cache+working-directory) rather than a custom step. - Cache path under
android/vendor/bundleis the documented setup-ruby convention; should improve cache hit rates compared to the priorgems/directory.
Proposed changes
Every Android build/upload job in CI ran
bundle installtwice. The first install was triggered implicitly byruby/setup-ruby@v1withbundler-cache: trueand resolved against the rootGemfile— which carries iOS-only deps (cocoapods,xcodeproj, ~125 gems). Android jobs threw all of that away and then ran a second explicitbundle install --path gemsagainstandroid/Gemfile.Pointing
setup-rubyatandroid/viaworking-directory: androidmakesbundler-cache: truetarget the correct Gemfile from the start, so the explicit "Install Fastlane" step becomes redundant. The deprecated--pathflag goes away as a bonus.Affects:
.github/actions/build-android/action.yml.github/actions/upload-android/action.yml.github/actions/upload-internal-android/action.ymlSaves roughly ~40s per Android build/upload job (and enables proper cache hits, which the previous setup also missed because the cache key was rooted at the wrong lockfile).
Issue(s)
https://rocketchat.atlassian.net/browse/CORE-2139
How to test or reproduce
Trigger any Android build job (this PR's own checks are sufficient). In the
Set up Ruby and Bundlerstep, confirm:bundle installruns (againstandroid/Gemfile.lock, ~88 gems)[DEPRECATED] The --path flag is deprecatedwarningSubsequent runs on the same branch should hit the bundler cache.
Screenshots
N/A — CI-only change.
Types of changes
Checklist
Further comments
bundler-cache: truealready runsbundle installautomatically; the prior setup was effectively running it twice with two different lockfiles. Verified against run 25008420199, where both installs were visible back-to-back (~68s combined).Summary by CodeRabbit