-
Notifications
You must be signed in to change notification settings - Fork 37
chore: update Android SDK tools to version 35 and adjust README for API levels 31-35 #53
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
WalkthroughUpdated Dockerfile to upgrade Android command-line tools and build-tools, refactored SDK installation (platforms and extras), added Gradle and package installs, and fixed ENV PATH syntax; README updated to reference Android 15 (API 31–35) and Java 17.0.12. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Docker as Docker Build
participant Apt as Package Manager
participant Files as FS (unpack/move)
participant SDK as sdkmanager
Dev->>Docker: docker build
Docker->>Apt: apt update && apt install (wget, curl, unzip, maven, ant, gradle ...)
Docker->>Docker: Download and unpack commandlinetools (13114758)
Docker->>Files: reorganize cmdline-tools into cmdline-tools/latest
Docker->>Docker: Install Gradle 8.12 and create symlink
Docker->>SDK: sdkmanager --install platforms (31–33,34–35), build-tools 35.0.0, extras/add-ons
Docker->>Docker: set ENV (ANDROID_SDK_ROOT, PATH=...)
Docker-->>Dev: Built image with updated toolchain
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🧹 Nitpick comments (3)
Dockerfile (3)
16-18
: Tighten apt install (add unzip, minimize image size, avoid version drift).
- The build unzips the SDK but doesn’t install
unzip
here; if the base image doesn’t include it, builds will fail. Addunzip
.- Use
--no-install-recommends
and clean apt lists in the same layer to actually keep the image small.- Installing
maven
,ant
, andgradle
via apt may override the pinned versions implied by README (Gradle 4.4.1, Maven 3.6.3). If the base image already provides these, prefer not reinstalling to avoid version drift.Proposed minimal diff (keeps only the tools needed here):
-RUN apt-get -qq update && \ - apt-get -qq install -y wget curl maven ant gradle +RUN apt-get -qq update && \ + apt-get -qq install -y --no-install-recommends wget curl unzip && \ + rm -rf /var/lib/apt/lists/*If you do require Maven/Ant/Gradle here, consider pinning versions and still using
--no-install-recommends
, plus inline cleanup.
21-25
: Harden the “move into latest/” step to avoid ls/grep/xargs pitfalls.
Using ls | grep | xargs can break with odd filenames and may fail if no matches are found. Prefer find with -exec.Apply:
- cd cmdline-tools && \ - mkdir latest && \ - ls | grep -v latest | xargs mv -t latest + cd cmdline-tools && \ + mkdir -p latest && \ + find . -mindepth 1 -maxdepth 1 ! -name latest -exec mv -t latest {} +
34-40
: Image-size cleanup placement.
The apt lists cleanup occurs in a later layer, which doesn’t reclaim space from the earlier apt update/install layer. Move apt cleanup into the same RUN as the install (see earlier diff). The remaining cleanup here is fine.Additionally, printing tool versions at build time is helpful for provenance; keeping
mvn -v && gradle -v && java -version && ant -version
is fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
Dockerfile
(3 hunks)README.md
(2 hunks)
🔇 Additional comments (6)
README.md (3)
11-11
: Docs align with Dockerfile targets (API 31–35) — looks good.
Title correctly reflects Android 15 and supported API levels (31–35) as configured in the Dockerfile.
15-15
: Confirm the base image actually ships Java 17.0.12.
README states Java 17.0.12, but the version comes from the base image (beevelop/java). Please verify the current tag resolves to 17.0.12; otherwise, update either the base tag or the README.You can verify locally by building the image and checking the tail of the build logs (the Dockerfile runs
java -version
). Alternatively, run:docker run --rm beevelop/android:<this-tag> java -version
.
44-44
: Minor wording/whitespace polish — OK.
Licenses section reads well and matches the project stance of not pre-accepting licenses inside the image.Dockerfile (3)
12-12
: Canonical PATH assignment — good change.
Switching toENV PATH=$PATH:...
avoids accidental PATH resets and ensures newly installed SDK tool paths are active in subsequent layers.
29-30
: Platforms 31–35 installation — aligned with README.
Targets for 31–35 look correct for Android 12–15. Good split to keep the list readable.
27-33
: Consider explicit license acceptance step or confirm intentional license removal.
You pipe “y” into sdkmanager per command (fine), and later remove$ANDROID_SDK_ROOT/licenses
. If the intent is to avoid distributing accepted licenses (policy-friendly), keep as-is. If you expect downstream users to run sdkmanager in CI, they will need to re-accept licenses.Do you want the image to contain accepted licenses? If yes, add:
yes | sdkmanager --licenses --sdk_root=$ANDROID_SDK_ROOT
after installs,- and avoid deleting
$ANDROID_SDK_ROOT/licenses
.
If no, keeping deletion is correct and the README’s “accept licenses” step remains necessary.
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: 0
♻️ Duplicate comments (1)
Dockerfile (1)
4-5
: Re-check cmdline-tools archive 13114758; consider pinning to a manifest-listed build.This URL has been flagged before as potentially not present in the official repository manifest anymore. Prefer a cmdline-tools ZIP that’s listed in dl.google.com’s repository2 XML to reduce risk of 404s in future builds.
Run this to verify the ZIP and build-tools 35.0.0 exist in the official manifest:
#!/bin/bash set -euo pipefail echo "Checking for commandlinetools-linux-13114758_latest.zip in official manifests..." found=0 for i in {1..9}; do url="https://dl.google.com/android/repository/repository2-$i.xml" if curl -fsSL "$url" | rg -n 'commandlinetools-linux-13114758_latest\.zip' -C2; then echo "Found in $url" found=1 break fi done if [[ $found -eq 0 ]]; then echo "Not found in repository2 manifests." fi echo echo "Checking for build-tools;35.0.0..." for i in {1..9}; do url="https://dl.google.com/android/repository/repository2-$i.xml" curl -fsSL "$url" | rg -n 'build-tools;35\.0\.0' -C2 && { echo "Found in $url"; break; } || true done
🧹 Nitpick comments (5)
README.md (2)
11-11
: Clarify heading to avoid implying all listed API levels are “Android 15”.The current heading can be misread. Suggest making it explicit that the image supports multiple API levels.
-# Android 15 (API levels 31 - 35) +# Android SDK (API levels 31 - 35)Alternative: “Android 12–15 (API levels 31–35)”.
44-44
: Punctuate and optionally add a quick command hint.Minor grammar polish and a helpful hint for users accepting licenses.
-The usage of the Android SDK requires you to accept the licenses +The usage of the Android SDK requires you to accept the licenses.Optionally append a snippet right below:
yes | sdkmanager --licenses --sdk_root=$ANDROID_SDK_ROOT
Dockerfile (3)
22-26
: Verify Gradle download integrity and remove the ZIP to reduce image size.Add checksum verification (official .sha256) and clean up artifacts.
-RUN wget https://services.gradle.org/distributions/gradle-8.12-bin.zip && \ - unzip -d /usr/share/ gradle-8.12-bin.zip && \ - mv /usr/share/gradle-8.12 /usr/share/gradle && \ - ln -s /usr/share/gradle/bin/gradle /usr/bin/gradle +RUN curl -fsSLo gradle-8.12-bin.zip https://services.gradle.org/distributions/gradle-8.12-bin.zip && \ + curl -fsSLo gradle-8.12-bin.zip.sha256 https://services.gradle.org/distributions/gradle-8.12-bin.zip.sha256 && \ + sha256sum -c gradle-8.12-bin.zip.sha256 && \ + unzip gradle-8.12-bin.zip -d /usr/share/ && \ + mv /usr/share/gradle-8.12 /usr/share/gradle && \ + ln -s /usr/share/gradle/bin/gradle /usr/bin/gradle && \ + rm -f gradle-8.12-bin.zip gradle-8.12-bin.zip.sha256
43-47
: Keep SDK licenses to avoid downstream build prompts; add explicit acceptance.Removing
$ANDROID_SDK_ROOT/licenses
can cause Gradle/AGP to complain during app builds or when additional components are fetched at build time.- rm -rf /opt/android/licenses && \ + # Keep accepted SDK licenses to avoid Gradle/AGP prompts at build time + # rm -rf /opt/android/licenses && \Consider adding a dedicated acceptance step right after installing packages (outside this hunk):
RUN yes | sdkmanager --licenses --sdk_root=$ANDROID_SDK_ROOT
If you intentionally remove licenses to save space, please confirm that downstream images won’t need to run
sdkmanager
at build time.
38-39
: Collapse SDKmanager calls into one and confirm API 35 availabilityWe’ve verified that all requested platform packages (
android-31
throughandroid-35
) are published in the official Android SDK repository (found inrepository2-1.xml
). You can merge the twosdkmanager
invocations into a single call to reduce Docker layers and simplify build logs:- while true; do echo 'y'; sleep 2; done | sdkmanager "platforms;android-31" "platforms;android-32" "platforms;android-33" && \ - while true; do echo 'y'; sleep 2; done | sdkmanager "platforms;android-34" "platforms;android-35" && \ + while true; do echo 'y'; sleep 2; done | sdkmanager \ + "platforms;android-31" "platforms;android-32" "platforms;android-33" \ + "platforms;android-34" "platforms;android-35" && \
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
Dockerfile
(1 hunks)README.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~16-~16: There might be a mistake here.
Context: ...0.12- Gradle
8.12(Groovy:
3.0.22) - Apache Maven
3.6.3- Ant
1.10.12` ##...
(QB_NEW_EN)
🔇 Additional comments (4)
README.md (1)
15-16
: Java/Gradle versions look consistent with the toolchain updates.
These align with the Dockerfile changes and modern AGP requirements.Dockerfile (3)
12-12
: ENV PATH fix is correct.
UsingENV PATH=...
ensures the layer sets PATH as intended.
16-21
: Apt install block is fine and lean.
Uses--no-install-recommends
and cleans lists to keep the image slim.
33-35
: Layout of cmdline-tools → latest is OK.
The move intocmdline-tools/latest
matches expected SDK layout for PATH usage.
I use docker-cordova to build android apps. I need to update that to cordova 14, which uses SDK Platform 35 and SDK Build Tools 35.0.0.
Summary by CodeRabbit
New Features
Chores
Documentation