From 70aea543b32e44bf736bb2f88a2cd05c2b68febd Mon Sep 17 00:00:00 2001 From: hellohuanlin <41930132+hellohuanlin@users.noreply.github.com> Date: Wed, 20 Aug 2025 14:00:09 -0700 Subject: [PATCH] [ios][tools] do not print out bonjour key not found in non-verbose mode (#174001) This tries https://github.com/flutter/flutter/pull/173569 again That PR was reverted, because I didn't know that `streamOutput` was the one used during non-verbose mode The logging logic is a bit convoluted, see: https://github.com/flutter/flutter/issues/173887 So this PR simply changes the condition from ``` if (!verbose && exitCode == 0) ``` To ``` if (!verbose && exitCode == 0 && !skipErrorLog) ``` So that if we skipErrorLog (will be true for bonjour features), it doesn't call `streamOutput`. *List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.* Fixes https://github.com/flutter/flutter/issues/172627 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --- packages/flutter_tools/bin/xcode_backend.dart | 61 ++-- .../general.shard/xcode_backend_test.dart | 260 ++++++++++-------- .../integration.shard/xcode_backend_test.dart | 56 ++-- 3 files changed, 221 insertions(+), 156 deletions(-) diff --git a/packages/flutter_tools/bin/xcode_backend.dart b/packages/flutter_tools/bin/xcode_backend.dart index 857fa90de2f..1357baa8867 100644 --- a/packages/flutter_tools/bin/xcode_backend.dart +++ b/packages/flutter_tools/bin/xcode_backend.dart @@ -102,7 +102,14 @@ class Context { Directory directoryFromPath(String path) => Directory(path); - /// Run given command in a synchronous subprocess. + /// Run given command ([bin]) in a synchronous subprocess. + /// + /// If [allowFail] is true, an exception will not be thrown even if the process returns a + /// non-zero exit code. Also, `error:` will not be prefixed to the output to prevent Xcode + /// complication failures. + /// + /// If [skipErrorLog] is true, `stderr` from the process will not be output unless in [verbose] + /// mode. If in [verbose], pipes `stderr` to `stdout`. /// /// Will throw [Exception] if the exit code is not 0. ProcessResult runSync( @@ -110,6 +117,7 @@ class Context { List args, { bool verbose = false, bool allowFail = false, + bool skipErrorLog = false, String? workingDirectory, }) { if (verbose) { @@ -122,22 +130,29 @@ class Context { final String resultStderr = result.stderr.toString().trim(); if (resultStderr.isNotEmpty) { final errorOutput = StringBuffer(); - // If allowFail, do not fail Xcode build. An example is on macOS 26, - // plutil reports NSBonjourServices key not found via stderr (rather than - // stdout on older macOS), and it should not cause compile failure. if (!allowFail && result.exitCode != 0) { // "error:" prefix makes this show up as an Xcode compilation error. errorOutput.write('error: '); } errorOutput.write(resultStderr); - echoError(errorOutput.toString()); - + if (skipErrorLog) { + // Even if skipErrorLog, we still want to write to stdout if verbose. + if (verbose) { + echo(errorOutput.toString()); + } + } else { + echoError(errorOutput.toString()); + } // Stream stderr to the Flutter build process. // When in verbose mode, `echoError` above will show the logs. So only // stream if not in verbose mode to avoid duplicate logs. // Also, only stream if exitCode is 0 since errors are handled separately // by the tool on failure. - if (!verbose && exitCode == 0) { + // Also check for `skipErrorLog`, because some errors should not be printed + // out. For example, on macOS 26, plutil reports NSBonjourServices key not + // found as an error. However, logging it in non-verbose mode would be + // confusing, since not having the key is one of the expected states. + if (!verbose && exitCode == 0 && !skipErrorLog) { streamOutput(errorOutput.toString()); } } @@ -424,16 +439,17 @@ class Context { return; } + final bool verbose = (environment['VERBOSE_SCRIPT_LOGGING'] ?? '').isNotEmpty; + // If there are already NSBonjourServices specified by the app (uncommon), // insert the vmService service name to the existing list. - ProcessResult result = runSync('plutil', [ - '-extract', - 'NSBonjourServices', - 'xml1', - '-o', - '-', - builtProductsPlist, - ], allowFail: true); + ProcessResult result = runSync( + 'plutil', + ['-extract', 'NSBonjourServices', 'xml1', '-o', '-', builtProductsPlist], + verbose: verbose, + allowFail: true, + skipErrorLog: true, + ); if (result.exitCode == 0) { runSync('plutil', [ '-insert', @@ -458,14 +474,13 @@ class Context { // specified (uncommon). This text will appear below the "Your app would // like to find and connect to devices on your local network" permissions // popup. - result = runSync('plutil', [ - '-extract', - 'NSLocalNetworkUsageDescription', - 'xml1', - '-o', - '-', - builtProductsPlist, - ], allowFail: true); + result = runSync( + 'plutil', + ['-extract', 'NSLocalNetworkUsageDescription', 'xml1', '-o', '-', builtProductsPlist], + verbose: verbose, + allowFail: true, + skipErrorLog: true, + ); if (result.exitCode != 0) { runSync('plutil', [ '-insert', diff --git a/packages/flutter_tools/test/general.shard/xcode_backend_test.dart b/packages/flutter_tools/test/general.shard/xcode_backend_test.dart index 4548261e607..24c602551ac 100644 --- a/packages/flutter_tools/test/general.shard/xcode_backend_test.dart +++ b/packages/flutter_tools/test/general.shard/xcode_backend_test.dart @@ -390,123 +390,153 @@ void main() { ); }); - test('Missing NSBonjourServices key in Info.plist should not fail Xcode compilation', () { - final Directory buildDir = fileSystem.directory('/path/to/builds') - ..createSync(recursive: true); - final File infoPlist = buildDir.childFile('Info.plist')..createSync(); - final context = TestContext( - ['test_vm_service_bonjour_service'], - { - 'CONFIGURATION': 'Debug', - 'BUILT_PRODUCTS_DIR': buildDir.path, - 'INFOPLIST_PATH': 'Info.plist', - }, - commands: [ - FakeCommand( - command: [ - 'plutil', - '-extract', - 'NSBonjourServices', - 'xml1', - '-o', - '-', - infoPlist.path, + for (final verbose in [true, false]) { + test( + 'Missing NSBonjourServices key in Info.plist should not fail Xcode compilation under ${verbose ? 'verbose' : 'non-verbose'} mode', + () { + final Directory buildDir = fileSystem.directory('/path/to/builds') + ..createSync(recursive: true); + final File infoPlist = buildDir.childFile('Info.plist')..createSync(); + const plutilErrorMessage = + 'Could not extract value, error: No value at that key path or invalid key path: NSBonjourServices'; + final File pipe = fileSystem.file('/tmp/pipe')..createSync(recursive: true); + final context = TestContext( + ['test_vm_service_bonjour_service'], + { + 'CONFIGURATION': 'Debug', + 'BUILT_PRODUCTS_DIR': buildDir.path, + 'INFOPLIST_PATH': 'Info.plist', + if (verbose) 'VERBOSE_SCRIPT_LOGGING': 'YES', + }, + commands: [ + FakeCommand( + command: [ + 'plutil', + '-extract', + 'NSBonjourServices', + 'xml1', + '-o', + '-', + infoPlist.path, + ], + exitCode: 1, + stderr: plutilErrorMessage, + ), + FakeCommand( + command: [ + 'plutil', + '-insert', + 'NSBonjourServices', + '-json', + '["_dartVmService._tcp"]', + infoPlist.path, + ], + ), + FakeCommand( + command: [ + 'plutil', + '-extract', + 'NSLocalNetworkUsageDescription', + 'xml1', + '-o', + '-', + infoPlist.path, + ], + ), ], - exitCode: 1, - stderr: 'No value at that key path or invalid key path: NSBonjourServices', - ), - FakeCommand( - command: [ - 'plutil', - '-insert', - 'NSBonjourServices', - '-json', - '["_dartVmService._tcp"]', - infoPlist.path, - ], - ), - FakeCommand( - command: [ - 'plutil', - '-extract', - 'NSLocalNetworkUsageDescription', - 'xml1', - '-o', - '-', - infoPlist.path, - ], - ), - ], - fileSystem: fileSystem, - )..run(); - expect(context.stderr, isNot(contains('error: '))); - }); + fileSystem: fileSystem, + scriptOutputStreamFile: pipe, + )..run(); - test( - 'Missing NSLocalNetworkUsageDescription in Info.plist should not fail Xcode compilation', - () { - final Directory buildDir = fileSystem.directory('/path/to/builds') - ..createSync(recursive: true); - final File infoPlist = buildDir.childFile('Info.plist')..createSync(); - final context = TestContext( - ['test_vm_service_bonjour_service'], - { - 'CONFIGURATION': 'Debug', - 'BUILT_PRODUCTS_DIR': buildDir.path, - 'INFOPLIST_PATH': 'Info.plist', - }, - commands: [ - FakeCommand( - command: [ - 'plutil', - '-extract', - 'NSBonjourServices', - 'xml1', - '-o', - '-', - infoPlist.path, - ], - ), - FakeCommand( - command: [ - 'plutil', - '-insert', - 'NSBonjourServices.0', - '-string', - '_dartVmService._tcp', - infoPlist.path, - ], - ), - FakeCommand( - command: [ - 'plutil', - '-extract', - 'NSLocalNetworkUsageDescription', - 'xml1', - '-o', - '-', - infoPlist.path, - ], - exitCode: 1, - stderr: - 'No value at that key path or invalid key path: NSLocalNetworkUsageDescription', - ), - FakeCommand( - command: [ - 'plutil', - '-insert', - 'NSLocalNetworkUsageDescription', - '-string', - 'Allow Flutter tools on your computer to connect and debug your application. This prompt will not appear on release builds.', - infoPlist.path, - ], - ), - ], - fileSystem: fileSystem, - )..run(); - expect(context.stderr, isNot(contains('error: '))); - }, - ); + expect(context.stderr, isNot(startsWith('error: '))); + expect(pipe.readAsStringSync(), isNot(contains(plutilErrorMessage))); + expect(context.stderr, isNot(contains(plutilErrorMessage))); + if (verbose) { + expect(context.stdout, contains(plutilErrorMessage)); + } else { + expect(context.stdout, isNot(contains(plutilErrorMessage))); + } + }, + ); + + test( + 'Missing NSLocalNetworkUsageDescription in Info.plist should not fail Xcode compilation under ${verbose ? 'verbose' : 'non-verbose'} mode', + () { + final Directory buildDir = fileSystem.directory('/path/to/builds') + ..createSync(recursive: true); + final File infoPlist = buildDir.childFile('Info.plist')..createSync(); + const plutilErrorMessage = + 'Could not extract value, error: No value at that key path or invalid key path: NSLocalNetworkUsageDescription'; + final File pipe = fileSystem.file('/tmp/pipe')..createSync(recursive: true); + final context = TestContext( + ['test_vm_service_bonjour_service'], + { + 'CONFIGURATION': 'Debug', + 'BUILT_PRODUCTS_DIR': buildDir.path, + 'INFOPLIST_PATH': 'Info.plist', + if (verbose) 'VERBOSE_SCRIPT_LOGGING': 'YES', + }, + commands: [ + FakeCommand( + command: [ + 'plutil', + '-extract', + 'NSBonjourServices', + 'xml1', + '-o', + '-', + infoPlist.path, + ], + ), + FakeCommand( + command: [ + 'plutil', + '-insert', + 'NSBonjourServices.0', + '-string', + '_dartVmService._tcp', + infoPlist.path, + ], + ), + FakeCommand( + command: [ + 'plutil', + '-extract', + 'NSLocalNetworkUsageDescription', + 'xml1', + '-o', + '-', + infoPlist.path, + ], + exitCode: 1, + stderr: plutilErrorMessage, + ), + FakeCommand( + command: [ + 'plutil', + '-insert', + 'NSLocalNetworkUsageDescription', + '-string', + 'Allow Flutter tools on your computer to connect and debug your application. This prompt will not appear on release builds.', + infoPlist.path, + ], + ), + ], + fileSystem: fileSystem, + scriptOutputStreamFile: pipe, + )..run(); + + expect(context.stderr, isNot(startsWith('error: '))); + expect(pipe.readAsString(), isNot(contains(plutilErrorMessage))); + expect(context.stderr, isNot(contains(plutilErrorMessage))); + if (verbose) { + expect(context.stdout, contains(plutilErrorMessage)); + } else { + expect(context.stdout, isNot(contains(plutilErrorMessage))); + } + }, + ); + } }); for (final platform in platforms) { 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 55afb30b11f..b30cb08924e 100644 --- a/packages/flutter_tools/test/integration.shard/xcode_backend_test.dart +++ b/packages/flutter_tools/test/integration.shard/xcode_backend_test.dart @@ -132,27 +132,47 @@ void main() { }); for (final buildConfiguration in ['Debug', 'Profile']) { - test('add keys in $buildConfiguration', () async { - infoPlist.writeAsStringSync(emptyPlist); + for (final verbose in [true, false]) { + test( + 'add keys in $buildConfiguration under ${verbose ? 'verbose' : 'non-verbose'} mode', + () async { + infoPlist.writeAsStringSync(emptyPlist); + final File pipe = fileSystem.file('/tmp/pipe')..createSync(recursive: true); - final ProcessResult result = await Process.run( - xcodeBackendPath, - ['test_vm_service_bonjour_service'], - environment: { - 'CONFIGURATION': buildConfiguration, - 'BUILT_PRODUCTS_DIR': buildDirectory.path, - 'INFOPLIST_PATH': 'Info.plist', + final ProcessResult result = await Process.run( + xcodeBackendPath, + ['test_vm_service_bonjour_service'], + environment: { + 'CONFIGURATION': buildConfiguration, + 'BUILT_PRODUCTS_DIR': buildDirectory.path, + 'INFOPLIST_PATH': 'Info.plist', + if (verbose) 'VERBOSE_SCRIPT_LOGGING': 'YES', + 'SCRIPT_OUTPUT_STREAM_FILE': pipe.path, + }, + ); + + final String actualInfoPlist = infoPlist.readAsStringSync(); + expect(actualInfoPlist, contains('NSBonjourServices')); + expect(actualInfoPlist, contains('dartVmService')); + expect(actualInfoPlist, contains('NSLocalNetworkUsageDescription')); + + // Make sure no Xcode compilation error. + expect(result.stderr, isNot(startsWith('error:'))); + + const plutilErrorMessage = + 'Could not extract value, error: No value at that key path or invalid key path: NSBonjourServices'; + expect(pipe.readAsStringSync(), isNot(contains(plutilErrorMessage))); + expect(result.stderr, isNot(contains(plutilErrorMessage))); + if (verbose) { + expect(result.stdout, contains(plutilErrorMessage)); + } else { + expect(result.stdout, isNot(contains(plutilErrorMessage))); + } + + expect(result, const ProcessResultMatcher()); }, ); - - final String actualInfoPlist = infoPlist.readAsStringSync(); - expect(actualInfoPlist, contains('NSBonjourServices')); - expect(actualInfoPlist, contains('dartVmService')); - expect(actualInfoPlist, contains('NSLocalNetworkUsageDescription')); - - expect(result.stderr, isNot(startsWith('error:'))); - expect(result, const ProcessResultMatcher()); - }); + } } test(