android: accept licenses listed in settings.android.acceptedLicenses - #17
android: accept licenses listed in settings.android.acceptedLicenses#17成继来 (chengjilai) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for explicitly accepting Android SDK licenses via configuration, so fresh machines/CI can proceed without running interactive sdkmanager --licenses.
Changes:
- Introduces
settings.android.acceptedLicensesin the frontend schema. - Extends the Android SDK license check task to write license hash files for explicitly accepted license IDs, then re-check remaining unaccepted licenses.
- Adds unit tests covering license acceptance and unknown-id behavior.
Reviewed changes
Copilot reviewed 6 out of 50 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| sources/frontend-api/src/org/jetbrains/amper/frontend/schema/androidSettings.kt | Adds settings.android.acceptedLicenses schema setting and documentation. |
| sources/amper-cli/src/org/jetbrains/amper/tasks/android/AndroidSdkLicenseChecker.kt | Adds acceptLicenses() to write license acceptance hash files. |
| sources/amper-cli/src/org/jetbrains/amper/tasks/android/SdkInstallManager.kt | Exposes acceptSdkLicenses() API and refactors license checker access. |
| sources/amper-cli/src/org/jetbrains/amper/tasks/android/CheckAndroidSdkLicenseTask.kt | Applies configured accepted license IDs before failing the build. |
| sources/amper-cli/src/org/jetbrains/amper/tasks/android/taskBuilderAndroid.kt | Plumbs accepted license IDs from module fragments into the license-check task. |
| sources/amper-cli/test/org/jetbrains/amper/tasks/android/AndroidSdkLicenseCheckerTest.kt | Adds tests for accepting listed licenses and ignoring unknown IDs. |
| sources/frontend/schema/junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/all settings read()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| sources/frontend/schema/junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/all settings merge for android()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| sources/frontend/schema/junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/all settings merge for jvm()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| sources/frontend/schema/junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/conflicting template values are reported()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| sources/frontend/schema/junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/context conflicts are not reported if resolved in a more specific context()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| sources/frontend/schema/junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/context conflicts on a scalar()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| sources/frontend/schema/junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/merge with nested templates()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| sources/frontend/schema/junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/merge with templates()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| sources/frontend/schema/junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/no context conflicts if value is the same()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| sources/frontend/schema/junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/read module file with custom properties diagnostics()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| sources/frontend/schema/junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/read module file with custom properties()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| sources/frontend/schema/junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/template diamond with unresolved conflict()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| sources/frontend/schema/junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/template diamond with resolved conflict()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| sources/frontend/schema/junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/template loop()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| sources/frontend/schema/junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/unknown properties with misnomer()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/all settings read()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/all settings merge for android()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/all settings merge for jvm()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/conflicting template values are reported()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/context conflicts on a scalar()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/context conflicts are not reported if resolved in a more specific context()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/merge with nested templates()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/merge with templates()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/no context conflicts if value is the same()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/read module file with custom properties diagnostics()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/read module file with custom properties()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/template diamond with unresolved conflict()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/template diamond with resolved conflict()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/template loop()/test-debug.log | Added test debug log (appears to be a generated artifact). |
| junit-jupiter/org.jetbrains.amper.frontend.tree.TreeTests/unknown properties with misnomer()/test-debug.log | Added test debug log (appears to be a generated artifact). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Hi! Thanks a lot for the PR—having the property in the frontend indeed seems like a reasonable compromise. However, we should be extra careful here because of the legal aspects of auto-accepting licenses. E.g., if the license content changes, it's going to have the same ID, while its hash would change, so having the ID alone might not be sufficient. |
The license check fails on a fresh machine or CI runner until the user runs
the provisioned sdkmanager --licenses, which is interactive and needs a
JAVA_HOME that a kotlin-only machine does not have. Users can now list the
license ids they explicitly accept in module.yaml:
settings:
android:
acceptedLicenses: [android-sdk-license]
When the check finds one of these licenses unaccepted, the toolchain writes
its hash file into the SDK licenses directory itself (the same acceptance
sdkmanager --licenses performs, via License.setAccepted) and the build
continues. Only the listed licenses are accepted, never anything implicit.
Unaccepted licenses not in the list still fail with the sdkmanager hint,
now mentioning the new option.
This is the config-file opt-in discussed in KTC-5695 (YouTrack) and keeps
the legal side explicit: the user names the exact license terms they accept.
79b3ef4 to
2f719eb
Compare
|
Note that, even if we added the content hashes as well, accepting in the module file might still not be possible legally. We have to check if it's OK that other devs building the project automatically accept the license without putting these lines in the module file themselves. |
Fixes the first-run / CI gap discussed in KTC-5695: the license check fails on a fresh machine until the user runs the provisioned
sdkmanager --licenses— which is interactive, and on a machine with only the toolchain (no system JDK) fails outright with "JAVA_HOME is not set" because the command does not point at the provisioned JDK.What this adds
A new
settings.android.acceptedLicensessetting: the user lists the SDK license ids whose terms they explicitly accept, and the toolchain writes the corresponding hash files into the SDK's licenses directory during the license check (the same acceptancesdkmanager --licensesperforms, viaLicense.setAccepted), then the build continues.Only the listed licenses are ever accepted — the toolchain never accepts licenses implicitly (the legal point from the KTC-5695 discussion). Unaccepted licenses not in the list still fail the build, with the sdkmanager hint updated to mention the new option.
Design notes
License.setAccepted(Path), so the written state is exactly what the library considers accepted.Testing
amper-cliandschemamodules compile.AndroidSdkLicenseCheckerTest: 10/10 pass, including two new tests (hash file written for a listed license; unknown ids ignored, unlisted licenses never written).SdkInstallManagerTest: 14/14 pass.TreeTests(frontend schema golden files): 15/15 pass — no golden updates needed since the setting defaults to empty.Live demonstration on a real machine: with the licenses directory removed,
kotlin buildfails with the expected message; after the new logic is in place, a module listingacceptedLicenses: [android-sdk-license]builds to an APK with no sdkmanager step.