Fix duplicate devices from xcdevice with iOS 17 (#128802)

This PR fixes issue of duplicate entries from `xcdevice list` cause devices to not show in `flutter devices`, `flutter run`, etc. When a duplicate entry is found, use the entry without errors as the authority. If both have errors, use the one with the higher SDK as the authority. Fixes https://github.com/flutter/flutter/issues/128719.
2023-06-20 13:16:27 -05:00 · 2023-06-20 13:16:27 -05:00 · 25e98b54d7
commit 25e98b54d7
parent fadcaee842
4 changed files with 341 additions and 12 deletions
--- a/packages/flutter_tools/lib/src/ios/devices.dart
+++ b/packages/flutter_tools/lib/src/ios/devices.dart
@ -16,6 +16,7 @@ import '../base/logger.dart';
 import '../base/os.dart';
 import '../base/platform.dart';
 import '../base/utils.dart';
+import '../base/version.dart';
 import '../build_info.dart';
 import '../convert.dart';
 import '../device.dart';
@ -295,10 +296,13 @@ class IOSDevice extends Device {
  final IMobileDevice _iMobileDevice;
  final IProxy _iproxy;

+  Version? get sdkVersion {
+    return Version.parse(_sdkVersion);
+  }
+
  /// May be 0 if version cannot be parsed.
  int get majorSdkVersion {
-    final String? majorVersionString = _sdkVersion?.split('.').first.trim();
-    return majorVersionString != null ? int.tryParse(majorVersionString) ?? 0 : 0;
+    return sdkVersion?.major ?? 0;
  }

  @override
--- a/packages/flutter_tools/lib/src/macos/xcdevice.dart
+++ b/packages/flutter_tools/lib/src/macos/xcdevice.dart
@ -12,6 +12,7 @@ import '../base/io.dart';
 import '../base/logger.dart';
 import '../base/platform.dart';
 import '../base/process.dart';
+import '../base/version.dart';
 import '../build_info.dart';
 import '../cache.dart';
 import '../convert.dart';
@ -493,7 +494,7 @@ class XCDevice {
    //  },
    // ...

-    final List<IOSDevice> devices = <IOSDevice>[];
+    final Map<String, IOSDevice> deviceMap = <String, IOSDevice>{};
    for (final Object device in allAvailableDevices) {
      if (device is Map<String, Object?>) {
        // Only include iPhone, iPad, iPod, or other iOS devices.
@ -531,33 +532,57 @@ class XCDevice {
          }
        }

-        String? sdkVersion = _sdkVersion(device);
+        String? sdkVersionString = _sdkVersion(device);

-        if (sdkVersion != null) {
+        if (sdkVersionString != null) {
          final String? buildVersion = _buildVersion(device);
          if (buildVersion != null) {
-            sdkVersion = '$sdkVersion $buildVersion';
+            sdkVersionString = '$sdkVersionString $buildVersion';
          }
        }

-        devices.add(IOSDevice(
+        // Duplicate entries started appearing in Xcode 15, possibly due to
+        // Xcode's new device connectivity stack.
+        // If a duplicate entry is found in `xcdevice list`, don't overwrite
+        // existing entry when the existing entry indicates the device is
+        // connected and the current entry indicates the device is not connected.
+        // Don't overwrite if current entry's sdkVersion is null.
+        // Don't overwrite if both entries indicate the device is not
+        // connected and the existing entry has a higher sdkVersion.
+        if (deviceMap.containsKey(identifier)) {
+          final IOSDevice deviceInMap = deviceMap[identifier]!;
+          if ((deviceInMap.isConnected && !isConnected) || sdkVersionString == null) {
+            continue;
+          }
+
+          final Version? sdkVersion = Version.parse(sdkVersionString);
+          if (!deviceInMap.isConnected &&
+              !isConnected &&
+              sdkVersion != null &&
+              deviceInMap.sdkVersion != null &&
+              deviceInMap.sdkVersion!.compareTo(sdkVersion) > 0) {
+            continue;
+          }
+        }
+
+        deviceMap[identifier] = IOSDevice(
          identifier,
          name: name,
          cpuArchitecture: _cpuArchitecture(device),
          connectionInterface: _interfaceType(device),
          isConnected: isConnected,
-          sdkVersion: sdkVersion,
+          sdkVersion: sdkVersionString,
          iProxy: _iProxy,
          fileSystem: globals.fs,
          logger: _logger,
          iosDeploy: _iosDeploy,
          iMobileDevice: _iMobileDevice,
          platform: globals.platform,
-          devModeEnabled: devModeEnabled
-        ));
+          devModeEnabled: devModeEnabled,
+        );
      }
    }
-    return devices;
+    return deviceMap.values.toList();
  }

  /// Despite the name, com.apple.platform.iphoneos includes iPhone, iPads, and all iOS devices.
--- a/packages/flutter_tools/test/general.shard/ios/devices_test.dart
+++ b/packages/flutter_tools/test/general.shard/ios/devices_test.dart
@ -13,6 +13,7 @@ import 'package:flutter_tools/src/base/io.dart';
 import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/base/os.dart';
 import 'package:flutter_tools/src/base/platform.dart';
+import 'package:flutter_tools/src/base/version.dart';
 import 'package:flutter_tools/src/build_info.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/device.dart';
@ -177,6 +178,121 @@ void main() {
      ).majorSdkVersion, 0);
    });

+    testWithoutContext('parses sdk version', () {
+      Version? sdkVersion = IOSDevice(
+        'device-123',
+        iProxy: IProxy.test(logger: logger, processManager: FakeProcessManager.any()),
+        fileSystem: fileSystem,
+        logger: logger,
+        platform: macPlatform,
+        iosDeploy: iosDeploy,
+        iMobileDevice: iMobileDevice,
+        name: 'iPhone 1',
+        cpuArchitecture: DarwinArch.arm64,
+        sdkVersion: '13.3.1',
+        connectionInterface: DeviceConnectionInterface.attached,
+        isConnected: true,
+        devModeEnabled: true,
+      ).sdkVersion;
+      Version expectedVersion = Version(13, 3, 1, text: '13.3.1');
+      expect(sdkVersion, isNotNull);
+      expect(sdkVersion!.toString(), expectedVersion.toString());
+      expect(sdkVersion.compareTo(expectedVersion), 0);
+
+      sdkVersion = IOSDevice(
+        'device-123',
+        iProxy: IProxy.test(logger: logger, processManager: FakeProcessManager.any()),
+        fileSystem: fileSystem,
+        logger: logger,
+        platform: macPlatform,
+        iosDeploy: iosDeploy,
+        iMobileDevice: iMobileDevice,
+        name: 'iPhone 1',
+        cpuArchitecture: DarwinArch.arm64,
+        sdkVersion: '13.3.1 (20ADBC)',
+        connectionInterface: DeviceConnectionInterface.attached,
+        isConnected: true,
+        devModeEnabled: true,
+      ).sdkVersion;
+      expectedVersion = Version(13, 3, 1, text: '13.3.1 (20ADBC)');
+      expect(sdkVersion, isNotNull);
+      expect(sdkVersion!.toString(), expectedVersion.toString());
+      expect(sdkVersion.compareTo(expectedVersion), 0);
+
+      sdkVersion = IOSDevice(
+        'device-123',
+        iProxy: IProxy.test(logger: logger, processManager: FakeProcessManager.any()),
+        fileSystem: fileSystem,
+        logger: logger,
+        platform: macPlatform,
+        iosDeploy: iosDeploy,
+        iMobileDevice: iMobileDevice,
+        name: 'iPhone 1',
+        cpuArchitecture: DarwinArch.arm64,
+        sdkVersion: '16.4.1(a) (20ADBC)',
+        connectionInterface: DeviceConnectionInterface.attached,
+        isConnected: true,
+        devModeEnabled: true,
+      ).sdkVersion;
+      expectedVersion = Version(16, 4, 1, text: '16.4.1(a) (20ADBC)');
+      expect(sdkVersion, isNotNull);
+      expect(sdkVersion!.toString(), expectedVersion.toString());
+      expect(sdkVersion.compareTo(expectedVersion), 0);
+
+      sdkVersion = IOSDevice(
+        'device-123',
+        iProxy: IProxy.test(logger: logger, processManager: FakeProcessManager.any()),
+        fileSystem: fileSystem,
+        logger: logger,
+        platform: macPlatform,
+        iosDeploy: iosDeploy,
+        iMobileDevice: iMobileDevice,
+        name: 'iPhone 1',
+        cpuArchitecture: DarwinArch.arm64,
+        sdkVersion: '0',
+        connectionInterface: DeviceConnectionInterface.attached,
+        isConnected: true,
+        devModeEnabled: true,
+      ).sdkVersion;
+      expectedVersion = Version(0, 0, 0, text: '0');
+      expect(sdkVersion, isNotNull);
+      expect(sdkVersion!.toString(), expectedVersion.toString());
+      expect(sdkVersion.compareTo(expectedVersion), 0);
+
+      sdkVersion = IOSDevice(
+        'device-123',
+        iProxy: IProxy.test(logger: logger, processManager: FakeProcessManager.any()),
+        fileSystem: fileSystem,
+        logger: logger,
+        platform: macPlatform,
+        iosDeploy: iosDeploy,
+        iMobileDevice: iMobileDevice,
+        name: 'iPhone 1',
+        cpuArchitecture: DarwinArch.arm64,
+        connectionInterface: DeviceConnectionInterface.attached,
+        isConnected: true,
+        devModeEnabled: true,
+      ).sdkVersion;
+      expect(sdkVersion, isNull);
+
+      sdkVersion = IOSDevice(
+        'device-123',
+        iProxy: IProxy.test(logger: logger, processManager: FakeProcessManager.any()),
+        fileSystem: fileSystem,
+        logger: logger,
+        platform: macPlatform,
+        iosDeploy: iosDeploy,
+        iMobileDevice: iMobileDevice,
+        name: 'iPhone 1',
+        cpuArchitecture: DarwinArch.arm64,
+        sdkVersion: 'bogus',
+        connectionInterface: DeviceConnectionInterface.attached,
+        isConnected: true,
+        devModeEnabled: true,
+      ).sdkVersion;
+      expect(sdkVersion, isNull);
+    });
+
    testWithoutContext('has build number in sdkNameAndVersion', () async {
      final IOSDevice device = IOSDevice(
        'device-123',
--- a/packages/flutter_tools/test/general.shard/macos/xcode_test.dart
+++ b/packages/flutter_tools/test/general.shard/macos/xcode_test.dart
@ -791,7 +791,7 @@ void main() {
    "available" : true,
    "platform" : "com.apple.platform.iphoneos",
    "modelCode" : "iPhone8,1",
-    "identifier" : "d83d5bc53967baa0ee18626ba87b6254b2ab5418",
+    "identifier" : "43ad2fda7991b34fe1acbda82f9e2fd3d6ddc9f7",
    "architecture" : "BOGUS",
    "modelName" : "Future iPad",
    "name" : "iPad"
@ -865,6 +865,190 @@ void main() {
          Platform: () => macPlatform,
        });

+        testUsingContext('use connected entry when filtering out duplicates', () async {
+          const String devicesOutput = '''
+[
+  {
+    "simulator" : false,
+    "operatingSystemVersion" : "13.3 (17C54)",
+    "interface" : "usb",
+    "available" : false,
+    "platform" : "com.apple.platform.iphoneos",
+    "modelCode" : "iPhone8,1",
+    "identifier" : "c4ca6f7a53027d1b7e4972e28478e7a28e2faee2",
+    "architecture" : "arm64",
+    "modelName" : "iPhone 6s",
+    "name" : "iPhone"
+  },
+  {
+    "simulator" : false,
+    "operatingSystemVersion" : "13.3 (17C54)",
+    "interface" : "usb",
+    "available" : false,
+    "platform" : "com.apple.platform.iphoneos",
+    "modelCode" : "iPhone8,1",
+    "identifier" : "c4ca6f7a53027d1b7e4972e28478e7a28e2faee2",
+    "architecture" : "arm64",
+    "modelName" : "iPhone 6s",
+    "name" : "iPhone",
+    "error" : {
+      "code" : -13,
+      "failureReason" : "",
+      "description" : "iPhone iPad is not connected",
+      "recoverySuggestion" : "Xcode will continue when iPhone is connected and unlocked.",
+      "domain" : "com.apple.platform.iphoneos"
+    }
+  }
+]
+''';
+
+          fakeProcessManager.addCommand(const FakeCommand(
+            command: <String>['xcrun', 'xcdevice', 'list', '--timeout', '2'],
+            stdout: devicesOutput,
+          ));
+          final List<IOSDevice> devices = await xcdevice.getAvailableIOSDevices();
+          expect(devices, hasLength(1));
+
+          expect(devices[0].id, 'c4ca6f7a53027d1b7e4972e28478e7a28e2faee2');
+          expect(devices[0].name, 'iPhone');
+          expect(await devices[0].sdkNameAndVersion, 'iOS 13.3 17C54');
+          expect(devices[0].cpuArchitecture, DarwinArch.arm64);
+          expect(devices[0].connectionInterface, DeviceConnectionInterface.attached);
+          expect(devices[0].isConnected, true);
+
+          expect(fakeProcessManager, hasNoRemainingExpectations);
+        }, overrides: <Type, Generator>{
+          Platform: () => macPlatform,
+          Artifacts: () => Artifacts.test(),
+        });
+
+        testUsingContext('use entry with sdk when filtering out duplicates', () async {
+          const String devicesOutput = '''
+[
+  {
+    "simulator" : false,
+    "interface" : "usb",
+    "available" : false,
+    "platform" : "com.apple.platform.iphoneos",
+    "modelCode" : "iPhone8,1",
+    "identifier" : "c4ca6f7a53027d1b7e4972e28478e7a28e2faee2",
+    "architecture" : "arm64",
+    "modelName" : "iPhone 6s",
+    "name" : "iPhone_1",
+    "error" : {
+      "code" : -13,
+      "failureReason" : "",
+      "description" : "iPhone iPad is not connected",
+      "recoverySuggestion" : "Xcode will continue when iPhone is connected and unlocked.",
+      "domain" : "com.apple.platform.iphoneos"
+    }
+  },
+  {
+    "simulator" : false,
+    "operatingSystemVersion" : "13.3 (17C54)",
+    "interface" : "usb",
+    "available" : false,
+    "platform" : "com.apple.platform.iphoneos",
+    "modelCode" : "iPhone8,1",
+    "identifier" : "c4ca6f7a53027d1b7e4972e28478e7a28e2faee2",
+    "architecture" : "arm64",
+    "modelName" : "iPhone 6s",
+    "name" : "iPhone_2",
+    "error" : {
+      "code" : -13,
+      "failureReason" : "",
+      "description" : "iPhone iPad is not connected",
+      "recoverySuggestion" : "Xcode will continue when iPhone is connected and unlocked.",
+      "domain" : "com.apple.platform.iphoneos"
+    }
+  }
+]
+''';
+
+          fakeProcessManager.addCommand(const FakeCommand(
+            command: <String>['xcrun', 'xcdevice', 'list', '--timeout', '2'],
+            stdout: devicesOutput,
+          ));
+          final List<IOSDevice> devices = await xcdevice.getAvailableIOSDevices();
+          expect(devices, hasLength(1));
+
+          expect(devices[0].id, 'c4ca6f7a53027d1b7e4972e28478e7a28e2faee2');
+          expect(devices[0].name, 'iPhone_2');
+          expect(await devices[0].sdkNameAndVersion, 'iOS 13.3 17C54');
+          expect(devices[0].cpuArchitecture, DarwinArch.arm64);
+          expect(devices[0].connectionInterface, DeviceConnectionInterface.attached);
+          expect(devices[0].isConnected, false);
+
+          expect(fakeProcessManager, hasNoRemainingExpectations);
+        }, overrides: <Type, Generator>{
+          Platform: () => macPlatform,
+          Artifacts: () => Artifacts.test(),
+        });
+
+        testUsingContext('use entry with higher sdk when filtering out duplicates', () async {
+          const String devicesOutput = '''
+[
+  {
+    "simulator" : false,
+    "operatingSystemVersion" : "14.3 (17C54)",
+    "interface" : "usb",
+    "available" : false,
+    "platform" : "com.apple.platform.iphoneos",
+    "modelCode" : "iPhone8,1",
+    "identifier" : "c4ca6f7a53027d1b7e4972e28478e7a28e2faee2",
+    "architecture" : "arm64",
+    "modelName" : "iPhone 6s",
+    "name" : "iPhone_1",
+    "error" : {
+      "code" : -13,
+      "failureReason" : "",
+      "description" : "iPhone iPad is not connected",
+      "recoverySuggestion" : "Xcode will continue when iPhone is connected and unlocked.",
+      "domain" : "com.apple.platform.iphoneos"
+    }
+  },
+  {
+    "simulator" : false,
+    "operatingSystemVersion" : "13.3 (17C54)",
+    "interface" : "usb",
+    "available" : false,
+    "platform" : "com.apple.platform.iphoneos",
+    "modelCode" : "iPhone8,1",
+    "identifier" : "c4ca6f7a53027d1b7e4972e28478e7a28e2faee2",
+    "architecture" : "arm64",
+    "modelName" : "iPhone 6s",
+    "name" : "iPhone_2",
+    "error" : {
+      "code" : -13,
+      "failureReason" : "",
+      "description" : "iPhone iPad is not connected",
+      "recoverySuggestion" : "Xcode will continue when iPhone is connected and unlocked.",
+      "domain" : "com.apple.platform.iphoneos"
+    }
+  }
+]
+''';
+
+          fakeProcessManager.addCommand(const FakeCommand(
+            command: <String>['xcrun', 'xcdevice', 'list', '--timeout', '2'],
+            stdout: devicesOutput,
+          ));
+          final List<IOSDevice> devices = await xcdevice.getAvailableIOSDevices();
+          expect(devices, hasLength(1));
+
+          expect(devices[0].id, 'c4ca6f7a53027d1b7e4972e28478e7a28e2faee2');
+          expect(devices[0].name, 'iPhone_1');
+          expect(await devices[0].sdkNameAndVersion, 'iOS 14.3 17C54');
+          expect(devices[0].cpuArchitecture, DarwinArch.arm64);
+          expect(devices[0].connectionInterface, DeviceConnectionInterface.attached);
+          expect(devices[0].isConnected, false);
+
+          expect(fakeProcessManager, hasNoRemainingExpectations);
+        }, overrides: <Type, Generator>{
+          Platform: () => macPlatform,
+          Artifacts: () => Artifacts.test(),
+        });
+
        testUsingContext('handles bad output',()  async {
          fakeProcessManager.addCommand(const FakeCommand(
            command: <String>['xcrun', 'xcdevice', 'list', '--timeout', '2'],