Fix Android license detection for cmdline-tools 23.0+ - #191554
Conversation
Android SDK cmdline-tools 23.0 deprecated 'sdkmanager --licenses' in favor of the new 'android' CLI. Running 'sdkmanager --licenses' now only prints a deprecation warning ('Warning: The --licenses option is no longer needed.') instead of the parseable license summary Flutter previously relied on, so none of the existing regexes match and flutter doctor incorrectly reports Android license status as unknown.
This adds a narrow fallback: when sdkmanager's output is recognized as this specific new-CLI deprecation message (and only then, not for arbitrary unparseable output), licensesAccepted checks the SDK's licenses directory on disk directly instead. This was verified against a real cmdline-tools 23.0 install, including confirming that the deprecation warning is printed identically regardless of whether licenses are actually accepted, which is why the fallback checks the licenses directory rather than trusting the warning text itself.
Note: this fallback can only distinguish 'some license files are present' from 'no license files at all', so it cannot reproduce the previous LicensesAccepted.some state for the new CLI.
Fixes flutter#191487
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
github-actions
Bot
added
tool
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a fallback mechanism for checking Android SDK licenses when the sdkmanager --licenses output is unparseable due to deprecation warnings in newer Android CLI tools. If the warning is detected, the validator checks the licenses directory on disk to determine if licenses have been accepted. Tests have been added to verify this fallback behavior. Feedback was provided to handle potential FileSystemException errors when calling listSync() on the licenses directory to prevent crashes.
| final Directory licensesDir = _androidSdk.directory.childDirectory('licenses'); | ||
| final bool hasAcceptedLicense = licensesDir.listSync().whereType<File>().any( | ||
| (File file) => file.lengthSync() > 0, | ||
| ); | ||
| return hasAcceptedLicense ? LicensesAccepted.all : LicensesAccepted.none; |
There was a problem hiding this comment.
Calling listSync() on a directory can throw a FileSystemException if the directory does not exist, is actually a file, or if there are permission issues. To prevent potential crashes in flutter doctor, we should check if the directory exists and handle any FileSystemException gracefully.
final Directory licensesDir = _androidSdk.directory.childDirectory('licenses');
if (!licensesDir.existsSync()) {
return LicensesAccepted.none;
}
try {
final bool hasAcceptedLicense = licensesDir.listSync().whereType<File>().any(
(File file) => file.lengthSync() > 0,
);
return hasAcceptedLicense ? LicensesAccepted.all : LicensesAccepted.none;
} on FileSystemException {
return LicensesAccepted.none;
}Addresses review feedback: listSync() on the licenses directory can throw a FileSystemException if the directory doesn't exist, is actually a file, or there are permission issues (e.g. a race between the licensesAvailable check and use). Wrap the disk check in a try/on FileSystemException and return LicensesAccepted.unknown in that case instead of letting flutter doctor crash. Added a regression test simulating this scenario.
1 participant
Description
Fixes #191487
Android SDK cmdline-tools 23.0 deprecated
sdkmanager --licensesin favor of the newandroidCLI. Runningsdkmanager --licensesnow only prints a deprecation warning instead of the parseable license summary Flutter previously relied on:None of the existing regexes in
licensesAcceptedmatch this output, soflutter doctorincorrectly reportsAndroid license status unknowneven when licenses are actually accepted.Approach
I initially considered treating the new deprecation message as
LicensesAccepted.all, but testing against a real cmdline-tools 23.0 install showed this message prints identically whether or not the licenses directory exists — so it can't be trusted as a signal of actual license state. I verified this by temporarily renaming my<sdk>/licensesdirectory and re-runningsdkmanager --licenses; the output was byte-for-byte identical either way.I also checked the new
androidCLI's--helpoutput (sdk,sdk install,info) — there's no license-status subcommand or flag exposed in this version, so there's no better structured signal to query instead.Since the SDK's actual license acceptance state is still tracked on disk under
<sdk>/licenses/(this is presumably what Gradle checks directly, since Gradle builds still succeed after this cmdline-tools update), this PR adds a narrow, additive fallback:sdkmanager's output matches the specific new-CLI deprecation message (and only then — not for arbitrary unparseable/garbage output),licensesAcceptedchecks thelicenses/directory on disk directly instead of trusting stdout.sdkmanagerversions is untouched.Known limitation
This fallback can only distinguish "some license files are present and non-empty" (→
all) from "no license files at all" (→none). It cannot reproduce the previousLicensesAccepted.somestate for the new CLI, since that information is no longer exposed by the tool in any form I could find. Happy to adjust the approach if maintainers have a preference here.Tests
Added two regression tests reproducing the exact real-world output captured from a Windows machine running cmdline-tools 23.0:
licensesAccepted falls back to the licenses directory when sdkmanager output is unparseable (new Android CLI, licenses present)licensesAccepted falls back to none when sdkmanager output is unparseable and no licenses are present (new Android CLI, licenses missing)All existing tests in
android_workflow_test.dartcontinue to pass (35/35).Also manually verified end-to-end:
flutter doctor -von the affected machine now correctly reportsAll Android licenses accepted.instead ofAndroid license status unknown.