-
Notifications
You must be signed in to change notification settings - Fork 1
Upgrade to Gradle 9.6.1 and Android Gradle plugin 9 #66
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: main
Are you sure you want to change the base?
Changes from all commits
5d7cfba
4461a1b
eb8a550
ebfddbe
f8c68d4
e8acd1e
80102ba
51f9dbc
d260d8e
f6457e2
ed1ab49
50e3749
75d21ab
04ebfc3
40d7e8a
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 |
|---|---|---|
|
|
@@ -24,3 +24,66 @@ jobs: | |
|
|
||
| - name: Check API Compatibility | ||
| run: ./gradlew :device-sdk:japicmp | ||
|
|
||
| # japicmp only compares classes.jar, so nothing else guards the parts of the | ||
| # AAR that decide who can consume it. AGP derives minCompileSdk, | ||
| # minCompileSdkExtension and minCompileMinorSdk from the library's own | ||
| # compileSdk, so a compileSdk bump can raise the consumer floor without | ||
| # anyone editing this repo. The whole file is compared rather than a few | ||
| # keys, so an added or changed key has to be acknowledged here deliberately. | ||
| # | ||
| # minAndroidGradlePluginVersion=1.0.0 holds only while | ||
| # android.aar.metadata.autoEncodeMinAgpVersion stays off. Its default flips | ||
| # in AGP 10, after which AGP derives the value from compileSdk. If this | ||
| # fails during an AGP upgrade, that is a real consumer-facing change to | ||
| # decide on, not a stale expectation to edit away. | ||
| - name: Check AAR consumer metadata | ||
| if: ${{ !cancelled() }} | ||
| run: | | ||
| set -euo pipefail | ||
|
|
||
| # Build what we inspect: japicmp happens to produce the AAR today, but | ||
| # only as a side effect of its own task graph. | ||
| ./gradlew :device-sdk:bundleReleaseAar | ||
| aar=device-sdk/build/outputs/aar/device-sdk-release.aar | ||
| tmp=$(mktemp -d) | ||
|
|
||
| expected=$(printf '%s\n' \ | ||
| 'aarFormatVersion=1.0' \ | ||
| 'aarMetadataVersion=1.0' \ | ||
| 'coreLibraryDesugaringEnabled=false' \ | ||
| 'minAndroidGradlePluginVersion=1.0.0' \ | ||
| 'minCompileSdk=30' \ | ||
| 'minCompileSdkExtension=0') | ||
| metadata=$(unzip -p "$aar" META-INF/com/android/build/gradle/aar-metadata.properties) | ||
| echo "$metadata" | ||
| if ! diff <(sort <<<"$expected") <(sort <<<"$metadata"); then | ||
| echo "::error::AAR consumer metadata changed (< expected, > actual)" | ||
| exit 1 | ||
| fi | ||
|
|
||
| manifest=$(unzip -p "$aar" AndroidManifest.xml) | ||
| echo "$manifest" | ||
| if ! grep -q 'android:minSdkVersion="27"' <<<"$manifest"; then | ||
| echo "::error::expected minSdkVersion 27 in the AAR manifest" | ||
| exit 1 | ||
| fi | ||
|
Comment on lines
+65
to
+70
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. nit — this branch prints no context on failure, unlike the properties branch which does On failure the log shows only
Member
Author
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. Claude here, replying on behalf of @oschwald. Fixed in
Comment on lines
+65
to
+70
Member
Author
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. Claude here, replying on behalf of @oschwald. This conflates AARs with APKs. So the Switching to Keeping the current check. |
||
|
|
||
| # Direct witness for the pinned Kotlin module name. japicmp catches a | ||
| # change to it only while DeviceTracker still has internal members whose | ||
| # mangled names reach the ABI, so it would stop guarding this after an | ||
| # ordinary refactor. | ||
| unzip -p "$aar" classes.jar >"$tmp/classes.jar" | ||
| entries=$(unzip -l "$tmp/classes.jar") | ||
| if ! grep -q 'META-INF/device-sdk_release\.kotlin_module' <<<"$entries"; then | ||
| echo "::error::expected META-INF/device-sdk_release.kotlin_module in classes.jar" | ||
| grep -i 'kotlin_module' <<<"$entries" || true | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Consumer ProGuard rules have to actually ship. | ||
| unzip -p "$aar" proguard.txt >"$tmp/proguard.txt" | ||
| if [ ! -s "$tmp/proguard.txt" ]; then | ||
| echo "::error::proguard.txt is missing or empty in the AAR" | ||
| exit 1 | ||
| fi | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,10 @@ jobs: | |
| # Use the MIT-licensed basic caching provider instead of the | ||
| # default enhanced caching, which is proprietary. | ||
| cache-provider: basic | ||
| # Checked in as a binary, so verify it against Gradle's published | ||
| # checksums. On by default, but stated explicitly so it cannot vanish | ||
| # in an action upgrade without a visible diff. | ||
| validate-wrappers: true | ||
|
|
||
| - name: Build SDK | ||
| run: ./gradlew :device-sdk:build | ||
|
|
@@ -39,8 +43,33 @@ jobs: | |
| - name: Build sample app | ||
| run: ./gradlew :sample:assembleDebug | ||
|
|
||
| # Nothing else compiles the androidTest sources, and a toolchain upgrade is | ||
| # the likeliest thing to break them. Cheap, so it runs before the R8 build. | ||
| - name: Compile instrumented tests | ||
| if: ${{ !cancelled() }} | ||
| run: ./gradlew :device-sdk:assembleDebugAndroidTest | ||
|
Comment on lines
+46
to
+50
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. minor — no The two new steps close two independent verification gaps and neither depends on the other. On the exact scenario this commit is written for — a toolchain upgrade — an R8 failure means you never learn whether the androidTest sources still compile, so the upgrade gets fixed and re-pushed one failure at a time. Same serial-masking the existing steps have, but this commit is explicitly adding independent checks, so it's worth breaking the pattern. Consider putting the cheaper androidTest compile first, too. Two related notes on what this step actually buys:
Member
Author
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. Claude here, replying on behalf of @oschwald. Fixed in The three related notes are real and none of them are this branch's doing — instrumented tests are compiled but never executed anywhere, |
||
|
|
||
| # device-sdk's release build type and sample's debug build type are the only | ||
| # places isMinifyEnabled is set, both to false, so this is the only step | ||
| # that runs R8 over the SDK's classes at all. It does not reproduce the | ||
| # missing-class failure that motivates minCompileSdk: that needs a consumer | ||
| # compiling below 30, and the sample is at compileSdk 36. The floor itself | ||
| # is pinned by the metadata assertion in api-compat.yml. | ||
| # | ||
| # AGP also wires lintVitalRelease into the release assemble, so a failure | ||
| # here is not necessarily an R8 or keep-rule problem. | ||
| - name: Build minified sample app | ||
| if: ${{ !cancelled() }} | ||
| run: ./gradlew :sample:assembleRelease | ||
|
|
||
| - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 | ||
| if: always() | ||
| with: | ||
|
Comment on lines
65
to
67
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. minor — the artifact upload still collects only When Extend
Member
Author
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. Claude here, replying on behalf of @oschwald. Fixed in |
||
| name: test-results | ||
| path: device-sdk/build/reports/tests/ | ||
| # R8 failures are explained by missing_rules.txt and configuration.txt, | ||
| # not by the test reports, and those are otherwise discarded with the | ||
| # runner. | ||
| path: | | ||
| device-sdk/build/reports/tests/ | ||
| sample/build/outputs/mapping/release/ | ||
| sample/build/reports/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,24 @@ | ||
| # Changelog | ||
|
|
||
| ## 0.4.0 (TBD) | ||
|
|
||
| - The published AAR now declares a `minCompileSdk` of 30. Consumers that minify, | ||
| or that set a Java 9 or later `sourceCompatibility`, already needed | ||
| `compileSdk` 30: `InstallationInfoHelper` references | ||
| `android.content.pm.InstallSourceInfo`, added in API 30, so R8 reported it as | ||
| a missing class, and the Android Gradle plugin separately refuses to configure | ||
| Java 9 or later compilation below 30. For consumers that do neither, this is a | ||
| new requirement rather than a pre-existing one. The API-30 call is guarded by | ||
| an `SDK_INT` check and is safe at runtime, so the floor trades an obscure R8 | ||
| failure for a clear Gradle error. If your build fails with a `minCompileSdk` | ||
| error, raise `compileSdk` to 30 or later, or stay on 0.3.1. Note this is your | ||
| app's `compileSdk` only: the SDK still supports API 27 and later devices, and | ||
| `minSdk` is unchanged. | ||
| - The SDK is now built with Gradle 9.6.1 and the Android Gradle plugin 9.3.1. | ||
| The public API and the published ABI are unchanged, and the artifact still | ||
| imposes no Android Gradle plugin floor on consumers, so the toolchain change | ||
| itself requires no action from consumers. | ||
|
Comment on lines
+17
to
+20
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. nit — the 0.3.1 entry a few lines below now contradicts this release.
Consumers grep the changelog for compatibility statements, and two entries a dozen lines apart now say opposite things about whether a compileSdk floor exists. Scope the historical entry ("as of 0.3.1 the artifact imposed no…") or point it at the 0.4.0 bullet. Separately: "The public API and the published ABI are unchanged" is asserted here but nothing in CI verifies the ABI claim at the Kotlin-metadata level — see the comment on
Member
Author
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. Claude here, replying on behalf of @oschwald. Declining this one. The 0.3.1 entry describes 0.3.1, and it was accurate for that release — the artifact really did impose no The 0.4.0 entry twelve lines above states the floor plainly and now carries the remedy ( |
||
|
|
||
| ## 0.3.1 (2026-07-24) | ||
|
|
||
| - Removed the unused `androidx.lifecycle:lifecycle-runtime-ktx` dependency. In | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,7 +33,7 @@ Follow Kotlin conventions (kotlinlang.org/docs/coding-conventions.html): | |
| # Build SDK library only (fastest for SDK development) | ||
| ./gradlew :device-sdk:assemble | ||
|
|
||
| # Build debug variants (skips minification issues) | ||
| # Build debug variants (no minification, so faster) | ||
| ./gradlew assembleDebug | ||
|
|
||
| # Build SDK library with all variants | ||
|
|
@@ -46,14 +46,12 @@ Follow Kotlin conventions (kotlinlang.org/docs/coding-conventions.html): | |
| ### Testing | ||
|
|
||
| ```bash | ||
| # Run unit tests for SDK | ||
| # Run unit tests for SDK. Under AGP 9 this runs the suite once, against the | ||
| # debug variant: AGP 9 no longer registers testReleaseUnitTest. | ||
| ./gradlew :device-sdk:test | ||
|
|
||
| # Run specific test class | ||
| ./gradlew :device-sdk:test --tests "com.maxmind.device.DeviceTrackerTest" | ||
|
|
||
| # Run tests with coverage (JaCoCo) | ||
| ./gradlew :device-sdk:testDebugUnitTest jacocoTestReport | ||
| ``` | ||
|
Comment on lines
52
to
55
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. nit — AGP 9 halved what this command runs, and that's recorded only in 4461a1b's commit message. AGP 9 no longer registers Practical risk is low — the SDK's release variant sets Separately,
Member
Author
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. Claude here, replying on behalf of @oschwald. Added the note in Took the note rather than re-enabling the release unit-test component: with no Also fixed |
||
|
|
||
| ### Code Quality | ||
|
|
@@ -66,7 +64,7 @@ Follow Kotlin conventions (kotlinlang.org/docs/coding-conventions.html): | |
| ./gradlew ktlintFormat | ||
|
|
||
| # Generate API documentation | ||
| ./gradlew :device-sdk:dokkaHtml | ||
| ./gradlew :device-sdk:dokkaGenerate | ||
| # Output: device-sdk/build/dokka/ | ||
| ``` | ||
|
|
||
|
|
@@ -237,24 +235,22 @@ mise run setup # Accepts licenses, installs platform packages, creates loca | |
|
|
||
| **Manual setup:** | ||
|
|
||
| 1. Java 21 (Android Studio JDK) configured in `gradle.properties`: | ||
|
|
||
| ``` | ||
| org.gradle.java.home=/home/greg/.local/share/android-studio/jbr | ||
| ``` | ||
| 1. Java 21. The version is pinned by `mise.toml`, not by the build scripts — | ||
| there is no `org.gradle.java.home` in `gradle.properties`. Without mise, put | ||
| a Java 21 JDK on `JAVA_HOME`. | ||
|
|
||
| 2. Android SDK with API 36 at `~/Android/Sdk` | ||
| 2. Android SDK with the platform and build-tools matching `compileSdk` in | ||
| `gradle/libs.versions.toml` | ||
|
|
||
| 3. `local.properties` file (gitignored): | ||
| ```properties | ||
|
Comment on lines
245
to
246
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. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Add a blank line before the nested code fence. Markdownlint reports MD031 because the fence immediately follows the list item. 🧰 Tools🪛 markdownlint-cli2 (0.23.0)[warning] 246-246: Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) 🤖 Prompt for AI AgentsSource: Linters/SAST tools
Member
Author
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. Claude here, replying on behalf of @oschwald. Applied in Markdownlint is not this repo's markdown standard. The reason worth acting on is internal consistency:
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. Do you need to push that commit? 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.
|
||
| sdk.dir=/home/greg/Android/Sdk | ||
| sdk.dir=/path/to/your/Android/Sdk | ||
| ``` | ||
|
|
||
| **Java Version Issues:** | ||
|
|
||
| - The project requires Java 17+ (set to use Java 21 from Android Studio) | ||
| - If you see "25" error, Java 25 is being used instead | ||
| - Fix by setting `org.gradle.java.home` in `gradle.properties` | ||
| - The project requires Java 17+; `mise.toml` pins Java 21 | ||
| - If Gradle picks up a different JDK, check `mise current` and `JAVA_HOME` | ||
|
|
||
| ## Common Issues | ||
|
|
||
|
|
@@ -266,31 +262,29 @@ mise run setup # Accepts licenses, installs platform packages, creates loca | |
| ~/Android/Sdk/cmdline-tools/latest/bin/sdkmanager --licenses | ||
| ``` | ||
|
|
||
| **"Resource mipmap/ic_launcher not found"** | ||
|
|
||
| - Sample app has no launcher icons (intentional for simplicity) | ||
| - Build works for `assembleDebug`, may fail on `assemble` (release variant) | ||
|
|
||
| **Detekt/ktlint failures** | ||
|
|
||
| - Skip with: `./gradlew build -x detekt -x ktlintCheck` | ||
| - Auto-fix formatting: `./gradlew ktlintFormat` | ||
|
|
||
| ### Release Build MinifyEnabled | ||
|
|
||
| The sample app has `isMinifyEnabled = true` for release builds, which may cause | ||
| R8 issues. For development: | ||
| The sample app has `isMinifyEnabled = true` for release builds, so | ||
| `:sample:assembleRelease` is the only thing that runs R8 over the SDK's classes. | ||
| CI runs it on every pull request and on pushes to `main`, so it is expected to | ||
| pass; if it starts failing, treat that as a real problem rather than an expected | ||
| quirk of the sample. | ||
|
|
||
| ```bash | ||
| # Build debug variant instead | ||
| ./gradlew :sample:assembleDebug | ||
| ``` | ||
| Note what a green run does and does not prove. It shows R8 completed, not that | ||
| `consumer-rules.pro` still keeps everything kotlinx.serialization needs at | ||
| runtime — that would fail as a `SerializationException` inside a consumer app, | ||
| not as a build failure. AGP also wires `lintVitalRelease` into this task, so a | ||
| failure is not necessarily R8's. | ||
|
|
||
| ## Testing Strategy | ||
|
|
||
| - **Unit tests** in `device-sdk/src/test/` use JUnit 5, MockK, and Robolectric | ||
| - **Android instrumented tests** in `device-sdk/src/androidTest/` | ||
| - Test coverage with JaCoCo | ||
|
|
||
| When adding features, write unit tests that: | ||
|
|
||
|
|
@@ -307,7 +301,7 @@ Uses Gradle version catalog in `gradle/libs.versions.toml`: | |
| - `[plugins]` - Gradle plugins | ||
| - `[bundles]` - Grouped dependencies (e.g., `ktor`, `testing`) | ||
|
|
||
| Access in build files: `libs.ktor.client.core`, `libs.plugins.kotlin.android` | ||
| Access in build files: `libs.ktor.client.core`, `libs.plugins.android.library` | ||
|
|
||
| ## Maven Publishing Configuration | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,26 +93,25 @@ Replace `/path/to/your/Android/Sdk` with your actual SDK location: | |
| ./gradlew :sample:installDebug | ||
|
|
||
| # Generate documentation | ||
| ./gradlew dokkaHtml | ||
| ./gradlew :device-sdk:dokkaGenerate | ||
| ``` | ||
|
|
||
| ## Publishing to Maven Central | ||
|
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. minor — same stale-Gradle-task bug class this commit set out to fix, five lines below this one, in a file the commit already had open.
Publishing is done by the Vanniktech plugin: Point this section at
Member
Author
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. Claude here, replying on behalf of @oschwald. Fixed in While in the file I also swept the same class of staleness in its Troubleshooting section, which told people to point |
||
|
|
||
| The SDK is configured for Maven Central publishing: | ||
| Releases are cut with `./dev-bin/release.sh`. See [README.dev.md](README.dev.md) | ||
| for the full process, including Central Portal credentials and GPG setup. | ||
|
|
||
| The underlying task, if you need it directly: | ||
|
|
||
| ```bash | ||
| ./gradlew :device-sdk:publishReleasePublicationToMavenCentralRepository | ||
| ./gradlew :device-sdk:publishAndReleaseToMavenCentral | ||
| ``` | ||
|
|
||
| Required credentials (set in `local.properties` or environment variables): | ||
|
|
||
| ```properties | ||
| signing.keyId=YOUR_KEY_ID | ||
| signing.password=YOUR_KEY_PASSWORD | ||
| signing.secretKeyRingFile=/path/to/secring.gpg | ||
| mavenCentralUsername=YOUR_USERNAME | ||
| mavenCentralPassword=YOUR_PASSWORD | ||
| ``` | ||
| Credentials come from `~/.m2/settings.xml` (server id `central`), or | ||
| `mavenCentralUsername` / `mavenCentralPassword` as Gradle properties or | ||
| `ORG_GRADLE_PROJECT_*` environment variables. Signing uses the system `gpg` | ||
| command via `signing { useGpgCmd() }`, so `signing.keyId` and | ||
| `signing.secretKeyRingFile` are not used. | ||
|
|
||
| ## Environment Variables | ||
|
|
||
|
|
@@ -139,12 +138,15 @@ $ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager --licenses | |
|
|
||
| ### Wrong Java Version | ||
|
|
||
| The Java version is pinned in `mise.toml`. Check what is actually active: | ||
|
|
||
| ```bash | ||
| # Use Android Studio's JDK | ||
| export JAVA_HOME=~/.local/share/android-studio/jbr | ||
| ./gradlew build | ||
| mise current | ||
| java -version | ||
| ``` | ||
|
|
||
| Without mise, put a JDK matching `mise.toml` on `JAVA_HOME`. | ||
|
|
||
| ### SDK Not Found | ||
|
|
||
| Ensure `local.properties` exists with correct SDK path: | ||
|
|
||
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.
minor — this step consumes an artifact it doesn't produce, and it's skipped exactly when you'd want both signals.
device-sdk-release.aarexists only because the previous step's:device-sdk:japicmphappens to pull inbundleReleaseAar(viaextractCurrentClasses'dependsOnatdevice-sdk/build.gradle.kts:228). Nothing in the workflow states that dependency, and there's noif:, so the step is skipped whenever the japicmp step fails — including when japicmp fails for unrelated reasons like the Maven Central baseline download being unavailable.Two consequences: a future refactor of the japicmp pipeline (comparing
intermediates/classes.jarinstead of unzipping the AAR — a perfectly natural cleanup) removes the AAR from the graph and this guard starts failing withunzip: cannot find or open, which reads like infra flake rather than a real signal. And when an API break and a metadata break land in the same PR, the author only sees the first, fixes it, and gets a second red run.Have the step build what it inspects —
./gradlew :device-sdk:bundleReleaseAaras the first line of the run block, a no-op when japicmp already built it — and considerif: always().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.
Claude here, replying on behalf of @oschwald.
Fixed in
d260d8e. The step now runs./gradlew :device-sdk:bundleReleaseAaras its first line, so it builds what it inspects instead of relying onextractCurrentClasses'dependsOn, and it runs underif: ${{ !cancelled() }}so an unrelated japicmp failure — a baseline download outage, say — no longer hides a metadata break. Both of your consequences applied: a future japicmp refactor comparingintermediates/would have turned this intounzip: cannot find or open.