Skip to content

Conversation

@zanderso
Copy link
Member

@zanderso zanderso commented Feb 3, 2020

Description

getprop can come back without an API version. In that case the getter will be null. Previously this was used only for filling out a String, but recently a change also used it to decide what flags to pass to logcat. This change makes the flag logic handle the null case.

Related Issues

Most frequent tool crasher on dev.

Tests

I added the following tests:

Added tests to android_device_test.dart.

Breaking Change

Did any tests fail when you ran them? Please read [Handling breaking changes].

  • No, no existing tests failed, so this is not a breaking change.

@zanderso zanderso added tool Affects the "flutter" command-line tool. See also t: labels. work in progress; do not review labels Feb 3, 2020
final int apiVersion = (String v) {
// If the API version string isn't found, conservatively assume that the
// version is less recent than the one we're looking for.
return v == null ? kLollipopVersionCode - 1 : int.tryParse(v);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know anything about the situations where apiVersion is showing up as null?
Is something going wrong when the tool tries to run adb getprop, or is the ro.build.version.sdk property actually missing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's most likely that this property is just missing. If adb getprop had failed more severely, then the tool would not have gotten this far. In particular, it wouldn't have been able to figure out the device.targetPlatform:

final TargetPlatform targetPlatform = await device.targetPlatform;

which boils down to a getprop call on Android:

switch (await _getProperty('ro.product.cpu.abi')) {

@blasten
Copy link

blasten commented Feb 3, 2020

LGTM + @jason-simmons comment

@fluttergithubbot fluttergithubbot merged commit ef62d53 into flutter:master Feb 3, 2020
@zanderso zanderso deleted the fix-tryParse-crash branch February 3, 2020 23:27
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants