Allow for gradle downloading missing SDK assets #28097

dnfield · 2019-02-18T09:21:10Z

It looks like when we moved to a gradle based project structure (#7902), we never updated our checks for this - but support for this has been there since versions of the plugin before when we converted over, so this change shouldn't break anyone. Gradle is fully capable of downloading whatever necessary SDK components when using a command line build, which is what build apk does, as long as the SDK licenses have been accepted (which can be done in a number of ways, per https://developer.android.com/studio/intro/update#download-with-gradle).

I suspect we still need the validation logic in there e.g. if someone ends up corrupting their SDK installation, or for other commands (like listing devices). Tagged @devoncarew to validate that this won't break anything for Android Studio. and @DanTup to validate for VSCode.

I've updated Flutter doctor to report on this scenario. /cc @gspencergoog because this changes a test you wrote to fix a crasher (#22676) - we'd no longer crash on a missing adb, here, but would crash if ADB couldn't be executed.

In local tests, a build where I only have my licenses present with a broadband connection:

dnfield@mbp:fresh$ flutter build apk 
Running "flutter packages get" in fresh...                          0.4s
Initializing gradle...                                              0.7s
Resolving dependencies...                                          26.6s
Running Gradle task 'assembleRelease'...                                
Running Gradle task 'assembleRelease'... Done                      24.1s
Built build/app/outputs/apk/release/app-release.apk (4.9MB).

Second run (after flutter clean):

dnfield@mbp:fresh$ flutter build apk 
Initializing gradle...                                              0.8s
Resolving dependencies...                                           1.4s
Running Gradle task 'assembleRelease'...                                
Running Gradle task 'assembleRelease'... Done                      21.7s
Built build/app/outputs/apk/release/app-release.apk (4.9MB).

I've seen the resolving dependencies bit take longer though, up to 90ish seconds, when downloading SDK bits.

…resent.

dnfield · 2019-02-18T09:28:05Z

This has some unfortunate paths right now - gradle doesn't download the SDK manager if you don't already have it, so this path is bad:

flutter doctor with licenses only: Reports that you're missing some SDK bits and gradle should fix it.
flutter build apk: gradle gets the missing bits
flutter doctor: reports that you're missing the SDK manager and exits.

My gut says the right fix for this is to download the SDK manager for them if it's missing instead. I'll open a separate PR for that.

dotdoom · 2019-02-18T13:42:16Z

@dnfield thank you for this PR, and a super fast turnaround! Here's an issue number for flutter doctor vs missing sdkmanager, in case you'd be interested: #25119

dnfield · 2019-02-18T16:36:20Z

Great, thanks for the link to the other issue. I think the best work around for that right now is to just say license status is unknown if we can't access the Android SDK manager rather thant throwing, and printing some instructions about how to get it.

tvolkert

Generally LGTM, but other reviewers should add their review as well.

tvolkert · 2019-02-19T03:35:41Z

packages/flutter_tools/lib/src/android/android_sdk.dart

  List<AndroidSdkVersion> _sdkVersions;
  AndroidSdkVersion _latestVersion;

+  final bool licensesOnly;


The licenses directory is always present, right? Should this be called missingPlatformTools?

Either way, can you document the member to describe the supported sdk structures and how this member denotes which one is present here?

It's not present if you haven't accepted the licenses - in which case gradle wouldn't download the assets for you.

That said, this would probably be more useful as a getter called platformToolsAvailable or missingPlatformTools. I'll look at that and make sure it's documented.

packages/flutter_tools/lib/src/android/android_workflow.dart

DanTup · 2019-02-19T08:03:51Z

and @DanTup to validate for VSCode.

I wouldn't expect this to change anything in VS Code at all since it all looks internal (we don't make any assumptions about the Android SDK dependencies, we just run Flutter and let it complain - or not). I did try kicking off Dart Code's integration tests against your branch on Travis/AppVeyor but they failed because your fork doesn't have all the tags (so it thinks Flutter is 0.3.6-pre.2578 and fails on flutter packages get in the test projects). If you want me to retry them, you'll need to git push --tags to update your fork.

jonahwilliams

LGTM

This is a great change and I really appreciate the additional documentation added

DanTup · 2019-02-19T18:08:19Z

@dnfield I pulled your fork and did a quick test with VS Code (since my tests use flutter-tester and don't exercise Android building!).

With the changes and my normal SDK, everything was fine.

When I wiped out the SDK except for licences, Flutter doctor reported the expected text (first build will be slower), however trying to run (flutter_gallery) resulted in this:

Launching lib/main.dart on XT1562 in debug mode...
Unable to locate the Android SDK; please run 'flutter doctor'.
No application found for TargetPlatform.android_arm.
Is your project missing an android/app/src/main/AndroidManifest.xml?
Consider running "flutter create ." to create one.

The file android/app/src/main/AndroidManifest.xml does exist in the project. I was trying to run on a physical device (Moto X Play).

So, I tried from the terminal with -v and it worked fine. So I tried again from IDE, also fine. So I ran flutter clean, cleared the SDK and tried again from VS Code, this time a different error:

Built build/app/outputs/apk/debug/app-debug.apk.
Unable to locate the Android SDK; please run 'flutter doctor'.
NoSuchMethodError: The getter 'name' was called on null.
Receiver: null
Tried calling: name
Exited (sigterm)

Tried a second time, worked fine.

Repeated the whole thing again - same result. First build attempt failed (with the "getter 'name' called on null" error), second run worked.

So, one last go from the command line with -v, this time immediately after cleaning the SDK. It also failed, though this one ended with:

[ +369 ms] Running Gradle task 'assembleDebug'... (completed in 16.5s)
[  +58 ms] calculateSha: LocalDirectory: '/Users/dantup/Dev/Google/flutter/examples/flutter_gallery/build/app/outputs/apk'/app.apk
[ +778 ms] Built build/app/outputs/apk/debug/app-debug.apk.
[   +2 ms] Unable to locate the Android SDK; please run 'flutter doctor'.
[   +4 ms] "flutter run" took 49,173ms.
[        ] "flutter run" took 49,173ms.

Oops; flutter has exited unexpectedly.
Sending crash report to Google.
Failed to send crash report. Server responded with HTTP status code 400
Crash report written to /Users/dantup/Dev/Google/flutter/examples/flutter_gallery/flutter_03.log;
please let us know at https://github.com/flutter/flutter/issues.

It's the same error as mentioned above, though this time it pointed me at the log with a stack trace:

NoSuchMethodError: NoSuchMethodError: The getter 'name' was called on null.
Receiver: null
Tried calling: name

#0      Object.noSuchMethod (dart:core/runtime/libobject_patch.dart:50:5)
#1      AndroidDevice.startApp (package:flutter_tools/src/android/android_device.dart:388:41)
<asynchronous suspension>
#2      FlutterDevice.runHot (package:flutter_tools/src/resident_runner.dart:309:54)
<asynchronous suspension>
#3      HotRunner.run (package:flutter_tools/src/run_hot.dart:300:39)
<asynchronous suspension>
#4      RunCommand.runCommand (package:flutter_tools/src/commands/run.dart:412:37)
<asynchronous suspension>
#5      FlutterCommand.verifyThenRunCommand (package:flutter_tools/src/runner/flutter_command.dart:545:18)
#6      _asyncThenWrapperHelper.<anonymous closure> (dart:async/runtime/libasync_patch.dart:77:64)
#7      _rootRunUnary (dart:async/zone.dart:1132:38)
#8      _CustomZone.runUnary (dart:async/zone.dart:1029:19)
#9      _FutureListener.handleValue (dart:async/future_impl.dart:126:18)
#10     Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:639:45)
#11     Future._propagateToListeners (dart:async/future_impl.dart:668:32)
#12     Future._complete (dart:async/future_impl.dart:473:7)
#13     _SyncCompleter.complete (dart:async/future_impl.dart:51:12)
#14     _AsyncAwaitCompleter.complete (dart:async/runtime/libasync_patch.dart:28:18)
#15     _completeOnAsyncReturn (dart:async/runtime/libasync_patch.dart:294:13)
#16     RunCommand.usageValues (package:flutter_tools/src/commands/run.dart)
#17     _asyncThenWrapperHelper.<anonymous closure> (dart:async/runtime/libasync_patch.dart:77:64)
#18     _rootRunUnary (dart:async/zone.dart:1132:38)
#19     _CustomZone.runUnary (dart:async/zone.dart:1029:19)
#20     _FutureListener.handleValue (dart:async/future_impl.dart:126:18)
#21     Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:639:45)
#22     Future._propagateToListeners (dart:async/future_impl.dart:668:32)
#23     Future._complete (dart:async/future_impl.dart:473:7)
#24     _SyncCompleter.complete (dart:async/future_impl.dart:51:12)
#25     _AsyncAwaitCompleter.complete (dart:async/runtime/libasync_patch.dart:28:18)
#26     _completeOnAsyncReturn (dart:async/runtime/libasync_patch.dart:294:13)
#27     AndroidDevice.targetPlatform (package:flutter_tools/src/android/android_device.dart)
#28     _asyncThenWrapperHelper.<anonymous closure> (dart:async/runtime/libasync_patch.dart:77:64)
#29     _rootRunUnary (dart:async/zone.dart:1132:38)
#30     _CustomZone.runUnary (dart:async/zone.dart:1029:19)
#31     _FutureListener.handleValue (dart:async/future_impl.dart:126:18)
#32     Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:639:45)
#33     Future._propagateToListeners (dart:async/future_impl.dart:668:32)
#34     Future._complete (dart:async/future_impl.dart:473:7)
#35     _SyncCompleter.complete (dart:async/future_impl.dart:51:12)
#36     _AsyncAwaitCompleter.complete.<anonymous closure> (dart:async/runtime/libasync_patch.dart:33:20)
#37     _rootRun (dart:async/zone.dart:1124:13)
#38     _CustomZone.run (dart:async/zone.dart:1021:19)
#39     _CustomZone.bindCallback.<anonymous closure> (dart:async/zone.dart:947:23)
#40     _microtaskLoop (dart:async/schedule_microtask.dart:41:21)
#41     _startMicrotaskLoop (dart:async/schedule_microtask.dart:50:5)
#42     _runPendingImmediateCallback (dart:isolate/runtime/libisolate_patch.dart:115:13)
#43     _RawReceivePortImpl._handleMessage (dart:isolate/runtime/libisolate_patch.dart:172:5)

dotdoom · 2019-02-20T06:25:59Z

I'm not an expert in Android SDK internals, but I am guessing that emulator is part of SDK. However since emulator is not required to build a project, Gradle may not pull it together with dependencies.

In other words, I expect flutter build apk to work, but flutter run to fail without first installing emulator via sdkmanager or Studio.

dnfield · 2019-02-20T06:37:53Z

I have some more tests to create and paths to update to make sure that's covered.

DanTup · 2019-02-20T08:35:27Z

@dnfield LMK if you can't repro the above and I'll do some debugging (it's using a physical device, so I don't think it's related to the last comments above).

dotdoom · 2019-02-20T09:00:08Z

@DanTup oh true, unrelated to physical device, my bad. Although it's unclear whether Flutter uses adb to deploy to device, and is it installed by Gradle as a part of SDK, or separately.

DanTup · 2019-02-20T09:27:36Z

Although it's unclear whether Flutter uses adb to deploy to device, and is it installed by Gradle as a part of SDK, or separately.

It does use ADB, and it seems Gradle is able to install it - since after the first failure the subsequent runs work fine (without me downloading anything myself). My guess is that there's something running prior to the Gradle step that relies on something from the SDK (but doesn't fail nicely, so we continue on and instead fail later). I haven't dug into it, but if Dan can't repro I'll have a look.

dnfield · 2019-02-20T09:30:45Z

Yes, flutter run tries to list all the available devices, which uses adb for Android.

dnfield · 2019-02-20T09:39:38Z

I'm not able to reproduce your issue @DanTup - flutter run does indeed fail, but it just tells me there are no devices available. Do you have another adb hanging out somewhere on your system path?

DanTup · 2019-02-20T09:47:05Z

Oh, apparently I do :/

dantup-macbookpro:Dart-Code dantup$ which adb
/usr/local/bin/adb

It's not a symlink either, it's a binary. I'm not sure how I ended up with one there as well as one in the SDK folder. I think I got everything I had through Android Studio, but I could be wrong.

dnfield · 2019-02-20T09:47:56Z

Ah! Do you have $ANDROID_HOME and/or $ANDROID_SDK_ROOT set?

The error is strange. We probably should have an assert in there to check that the project isn't null by that step. But I think your couldn't find android SDK issue is from that.

dnfield · 2019-02-20T09:49:33Z

Scratch that - sorry for the spam. I see where the problem is now. We're using aapt to find the bundle identifier in this path and for some reason you don't seem to have it....

DanTup · 2019-02-20T09:50:13Z

Yep, ANDROID_SDK_ROOT is set to /Users/dantup/Library/Android/sdk (which is the location I made a licence-only SDK for testing).

DanTup · 2019-02-20T09:51:02Z

We're using aapt to find the bundle identifier in this path and for some reason you don't seem to have it

I don't know what aapt is, but you're right that I don't seem to have it (on PATH, at least).

dnfield · 2019-02-20T09:53:54Z

I'm able to reproduce this by putting adb back on my path. I'll see about fixing it.

dnfield · 2019-02-20T21:37:52Z

Right now, flutter run ends up going through a code path where, to find the APK, it relies on being able to invoke aapt.

This is a really weird path. It can only happen under the following circumstances:

A user has adb on their path, but is missing the platform tools from the Android SDK. This means flutter run won't fail when it tries to enumerate devices/emulators.
There is a previously-built version of the APK around. This means flutter run won't re-run the gradle build process, and won't download the missing SDK assets.
flutter run tries to use aapt from build-tools to get information about the existing APK and fails.

It's fixable though :) Just writing a test for it.

DanTup · 2019-02-21T09:25:05Z

@dnfield with a licence-only SDK and adb on PATH it builds with no issues now :-)

But... I did see this a couple of times while testing - when the app launched on the device it crashed and I got this in the output:

Built build/app/outputs/apk/debug/app-debug.apk.
F/libc    ( 8400): Fatal signal 11 (SIGSEGV), code 128, fault addr 0x0 in tid 8427 (er.demo.gallery)
Lost connection to device.

I'm not sure whether it's related to this change.

dnfield · 2019-02-21T19:13:56Z

@DanTup I'm pretty sure that's related to other problems that happened in the midst of this. Using engine from master should fix that.

This reverts commit 6f5e88a.

…8295) This reverts commit 6f5e88a.

…8355) * Allow for gradle downloading missing SDK assets if SDK licenses are present. * Improvements for windows testing

Allow for gradle downloading missing SDK assets if SDK licenses are p…

8b18046

…resent.

dnfield added tool Affects the "flutter" command-line tool. See also t: labels. t: gradle "flutter build" and "flutter run" on Android labels Feb 18, 2019

dnfield requested review from DanTup, devoncarew, gspencergoog, jonahwilliams and tvolkert February 18, 2019 09:21

googlebot added the cla: yes label Feb 18, 2019

dnfield added the waiting for confirmation before landing label Feb 18, 2019

dnfield removed the waiting for confirmation before landing label Feb 18, 2019

Fix license path for missing sdkmanager

8573afc

dnfield force-pushed the auto_sdk branch from c3dea42 to 8573afc Compare February 18, 2019 16:36

dnfield added 2 commits February 18, 2019 12:59

Cirrus re-run...

608d062

Fix condition check

25cde27

tvolkert reviewed Feb 19, 2019

View reviewed changes

dnfield added 2 commits February 18, 2019 20:30

rename props, add docs, rename method

3059c30

fix tests after param changes

644d066

jonahwilliams approved these changes Feb 19, 2019

View reviewed changes

gspencergoog approved these changes Feb 19, 2019

View reviewed changes

Fix weird path for flutter run and add tests

5ad79d6

dnfield force-pushed the auto_sdk branch from 9c1ed8c to 5ad79d6 Compare February 21, 2019 04:47

remove print, fix tests

c41e07b

Merge remote-tracking branch 'upstream/master' into auto_sdk

0dcb9f0

dnfield merged commit 6f5e88a into flutter:master Feb 21, 2019

dnfield deleted the auto_sdk branch February 21, 2019 23:57

dnfield restored the auto_sdk branch February 22, 2019 00:45

dnfield added a commit that referenced this pull request Feb 22, 2019

Revert "Allow for gradle downloading missing SDK assets (#28097)"

4072747

This reverts commit 6f5e88a.

dnfield mentioned this pull request Feb 22, 2019

Revert "Allow for gradle downloading missing SDK assets" #28295

Merged

dnfield added a commit that referenced this pull request Feb 22, 2019

Revert "Allow for gradle downloading missing SDK assets (#28097)" (#2…

74a9086

…8295) This reverts commit 6f5e88a.

dnfield mentioned this pull request Feb 22, 2019

Reland "Allow for gradle downloading missing SDK assets" (#28097) #28355

Merged

dnfield added a commit that referenced this pull request Feb 23, 2019

Reland "Allow for gradle downloading missing SDK assets" (#28097) (#2…

15f2119

…8355) * Allow for gradle downloading missing SDK assets if SDK licenses are present. * Improvements for windows testing

Allow for gradle downloading missing SDK assets #28097

Allow for gradle downloading missing SDK assets #28097

Uh oh!

Conversation

dnfield commented Feb 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dnfield commented Feb 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dotdoom commented Feb 18, 2019

Uh oh!

dnfield commented Feb 18, 2019

Uh oh!

tvolkert left a comment

Choose a reason for hiding this comment

Uh oh!

tvolkert Feb 19, 2019

Choose a reason for hiding this comment

Uh oh!

dnfield Feb 19, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DanTup commented Feb 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

DanTup commented Feb 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dotdoom commented Feb 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dnfield commented Feb 20, 2019

Uh oh!

DanTup commented Feb 20, 2019

Uh oh!

dotdoom commented Feb 20, 2019

Uh oh!

DanTup commented Feb 20, 2019

Uh oh!

dnfield commented Feb 20, 2019

Uh oh!

dnfield commented Feb 20, 2019

Uh oh!

DanTup commented Feb 20, 2019

Uh oh!

dnfield commented Feb 20, 2019

Uh oh!

dnfield commented Feb 20, 2019

Uh oh!

DanTup commented Feb 20, 2019

Uh oh!

DanTup commented Feb 20, 2019

Uh oh!

dnfield commented Feb 20, 2019

Uh oh!

dnfield commented Feb 20, 2019

Uh oh!

DanTup commented Feb 21, 2019

Uh oh!

dnfield commented Feb 21, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

dnfield commented Feb 18, 2019 •

edited

Loading

dnfield commented Feb 18, 2019 •

edited

Loading

DanTup commented Feb 19, 2019 •

edited

Loading

DanTup commented Feb 19, 2019 •

edited

Loading

dotdoom commented Feb 20, 2019 •

edited

Loading