From 0383d8ba9eb16ffd801311742dd40dd985d68fc1 Mon Sep 17 00:00:00 2001 From: Victoria Ashworth <15619084+vashworth@users.noreply.github.com> Date: Tue, 17 Oct 2023 14:09:08 -0500 Subject: [PATCH] Skip injecting Bonjour settings when port publication is disabled (#136562) Some of our tests in CI are triggering the `NSLocalNetworkUsageDescription` dialog when they're not supposed to (https://github.com/flutter/flutter/issues/129836) since it's disabled via flags (`--no-publish-port` for flutter/flutter and `--disable-vm-service-publication` for flutter/engine). Normally, we inject `NSLocalNetworkUsageDescription` (and other bonjour settings) to the Info.plist during the project build for debug and profile mode since by default they will publish the VM Service port over mDNS. To help diagnose the issue, though, this PR changes it so that we don't inject `NSLocalNetworkUsageDescription` (and other bonjour settings) when port publication is disabled since it shouldn't be needed. Hopefully, this will give us better error messages or cause the app to crash and end the test early (rather than timeout after 30 minutes). --- dev/devicelab/lib/tasks/perf_tests.dart | 1 + packages/flutter_tools/bin/xcode_backend.dart | 6 ++ .../lib/src/commands/build_ios.dart | 7 ++- .../flutter_tools/lib/src/ios/devices.dart | 1 + packages/flutter_tools/lib/src/ios/mac.dart | 7 +++ .../hermetic/build_ios_test.dart | 62 +++++++++++++++++++ .../integration.shard/xcode_backend_test.dart | 25 ++++++++ 7 files changed, 108 insertions(+), 1 deletion(-) diff --git a/dev/devicelab/lib/tasks/perf_tests.dart b/dev/devicelab/lib/tasks/perf_tests.dart index 48bf20dd0e..aca0742b78 100644 --- a/dev/devicelab/lib/tasks/perf_tests.dart +++ b/dev/devicelab/lib/tasks/perf_tests.dart @@ -886,6 +886,7 @@ class StartupTest { '-v', '--profile', '--target=$target', + '--no-publish-port', ]); final String buildRoot = path.join(testDirectory, 'build'); applicationBinaryPath = _findDarwinAppInBuildDirectory(buildRoot); diff --git a/packages/flutter_tools/bin/xcode_backend.dart b/packages/flutter_tools/bin/xcode_backend.dart index 6e3fb7174a..70e856405e 100644 --- a/packages/flutter_tools/bin/xcode_backend.dart +++ b/packages/flutter_tools/bin/xcode_backend.dart @@ -256,6 +256,12 @@ class Context { // Add the vmService publisher Bonjour service to the produced app bundle Info.plist. void addVmServiceBonjourService() { + // Skip adding Bonjour service settings when DISABLE_PORT_PUBLICATION is YES. + // These settings are not needed if port publication is disabled. + if (environment['DISABLE_PORT_PUBLICATION'] == 'YES') { + return; + } + final String buildMode = parseFlutterBuildMode(); // Debug and profile only. diff --git a/packages/flutter_tools/lib/src/commands/build_ios.dart b/packages/flutter_tools/lib/src/commands/build_ios.dart index 117414e89f..8dab363c3f 100644 --- a/packages/flutter_tools/lib/src/commands/build_ios.dart +++ b/packages/flutter_tools/lib/src/commands/build_ios.dart @@ -27,7 +27,11 @@ import 'build.dart'; /// Builds an .app for an iOS app to be used for local testing on an iOS device /// or simulator. Can only be run on a macOS host. class BuildIOSCommand extends _BuildIOSSubCommand { - BuildIOSCommand({ required super.logger, required super.verboseHelp }) { + BuildIOSCommand({ + required super.logger, + required bool verboseHelp, + }) : super(verboseHelp: verboseHelp) { + addPublishPort(verboseHelp: verboseHelp); argParser ..addFlag('config-only', help: 'Update the project configuration without performing a build. ' @@ -658,6 +662,7 @@ abstract class _BuildIOSSubCommand extends BuildSubCommand { configOnly: configOnly, buildAction: xcodeBuildAction, deviceID: globals.deviceManager?.specifiedDeviceId, + disablePortPublication: usingCISystem && await disablePortPublication, ); xcodeBuildResult = result; diff --git a/packages/flutter_tools/lib/src/ios/devices.dart b/packages/flutter_tools/lib/src/ios/devices.dart index d07542d436..56cfa87ae6 100644 --- a/packages/flutter_tools/lib/src/ios/devices.dart +++ b/packages/flutter_tools/lib/src/ios/devices.dart @@ -501,6 +501,7 @@ class IOSDevice extends Device { targetOverride: mainPath, activeArch: cpuArchitecture, deviceID: id, + disablePortPublication: debuggingOptions.usingCISystem && debuggingOptions.disablePortPublication, ); if (!buildResult.success) { _logger.printError('Could not build the precompiled application for the device.'); diff --git a/packages/flutter_tools/lib/src/ios/mac.dart b/packages/flutter_tools/lib/src/ios/mac.dart index 8819b5cfe1..6865b3689f 100644 --- a/packages/flutter_tools/lib/src/ios/mac.dart +++ b/packages/flutter_tools/lib/src/ios/mac.dart @@ -135,6 +135,7 @@ Future buildXcodeProject({ String? deviceID, bool configOnly = false, XcodeBuildAction buildAction = XcodeBuildAction.build, + bool disablePortPublication = false, }) async { if (!upgradePbxProjWithFlutterAssets(app.project, globals.logger)) { return XcodeBuildResult(success: false); @@ -382,6 +383,12 @@ Future buildXcodeProject({ _kResultBundleVersion, ]); + // Adds a setting which xcode_backend.dart will use to skip adding Bonjour + // service settings to the Info.plist. + if (disablePortPublication) { + buildCommands.add('DISABLE_PORT_PUBLICATION=YES'); + } + // Don't log analytics for downstream Flutter commands. // e.g. `flutter build bundle`. buildCommands.add('FLUTTER_SUPPRESS_ANALYTICS=true'); diff --git a/packages/flutter_tools/test/commands.shard/hermetic/build_ios_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/build_ios_test.dart index 8f0ae0d5b3..88f4212064 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/build_ios_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/build_ios_test.dart @@ -134,6 +134,7 @@ void main() { bool verbose = false, bool simulator = false, bool customNaming = false, + bool disablePortPublication = false, String? deviceId, int exitCode = 0, String? stdout, @@ -177,6 +178,8 @@ void main() { ], '-resultBundlePath', _xcBundleDirectoryPath, '-resultBundleVersion', '3', + if (disablePortPublication) + 'DISABLE_PORT_PUBLICATION=YES', 'FLUTTER_SUPPRESS_ANALYTICS=true', 'COMPILER_INDEX_STORE_ENABLE=NO', ], @@ -281,6 +284,65 @@ void main() { XcodeProjectInterpreter: () => FakeXcodeProjectInterpreterWithBuildSettings(), }); + testUsingContext('ios build invokes xcode build with disable port publication setting', () async { + final BuildCommand command = BuildCommand( + androidSdk: FakeAndroidSdk(), + buildSystem: TestBuildSystem.all(BuildResult(success: true)), + fileSystem: MemoryFileSystem.test(), + logger: BufferLogger.test(), + osUtils: FakeOperatingSystemUtils(), + ); + createMinimalMockProjectFiles(); + + await createTestCommandRunner(command).run( + const ['build', 'ios', '--no-pub', '--no-publish-port', '--ci'] + ); + expect(testLogger.statusText, contains('build/ios/iphoneos/Runner.app')); + }, overrides: { + FileSystem: () => fileSystem, + ProcessManager: () => FakeProcessManager.list([ + xattrCommand, + setUpFakeXcodeBuildHandler( + disablePortPublication: true, + onRun: () { + fileSystem.directory('build/ios/Release-iphoneos/Runner.app').createSync(recursive: true); + }, + ), + setUpRsyncCommand(), + ]), + Platform: () => macosPlatform, + XcodeProjectInterpreter: () => FakeXcodeProjectInterpreterWithBuildSettings(), + }); + + testUsingContext('ios build invokes xcode build without disable port publication setting when not in CI', () async { + final BuildCommand command = BuildCommand( + androidSdk: FakeAndroidSdk(), + buildSystem: TestBuildSystem.all(BuildResult(success: true)), + fileSystem: MemoryFileSystem.test(), + logger: BufferLogger.test(), + osUtils: FakeOperatingSystemUtils(), + ); + createMinimalMockProjectFiles(); + + await createTestCommandRunner(command).run( + const ['build', 'ios', '--no-pub', '--no-publish-port'] + ); + expect(testLogger.statusText, contains('build/ios/iphoneos/Runner.app')); + }, overrides: { + FileSystem: () => fileSystem, + ProcessManager: () => FakeProcessManager.list([ + xattrCommand, + setUpFakeXcodeBuildHandler( + onRun: () { + fileSystem.directory('build/ios/Release-iphoneos/Runner.app').createSync(recursive: true); + }, + ), + setUpRsyncCommand(), + ]), + Platform: () => macosPlatform, + XcodeProjectInterpreter: () => FakeXcodeProjectInterpreterWithBuildSettings(), + }); + testUsingContext('ios build invokes xcode build with renamed xcodeproj and xcworkspace', () async { final BuildCommand command = BuildCommand( androidSdk: FakeAndroidSdk(), diff --git a/packages/flutter_tools/test/integration.shard/xcode_backend_test.dart b/packages/flutter_tools/test/integration.shard/xcode_backend_test.dart index d34d242625..01d3be850a 100644 --- a/packages/flutter_tools/test/integration.shard/xcode_backend_test.dart +++ b/packages/flutter_tools/test/integration.shard/xcode_backend_test.dart @@ -183,5 +183,30 @@ void main() { '''); expect(result, const ProcessResultMatcher()); }); + + test('does not add bonjour settings when port publication is disabled', () async { + infoPlist.writeAsStringSync(''' + + + + + +'''); + + final ProcessResult result = await Process.run( + xcodeBackendPath, + ['test_vm_service_bonjour_service'], + environment: { + 'CONFIGURATION': 'Debug', + 'BUILT_PRODUCTS_DIR': buildDirectory.path, + 'INFOPLIST_PATH': 'Info.plist', + 'DISABLE_PORT_PUBLICATION': 'YES', + }, + ); + + expect(infoPlist.readAsStringSync().contains('NSBonjourServices'), isFalse); + expect(infoPlist.readAsStringSync().contains('NSLocalNetworkUsageDescription'), isFalse); + expect(result, const ProcessResultMatcher()); + }); }, skip: !io.Platform.isMacOS); // [intended] requires macos toolchain. }