Skip to content

Commit 70aea54

Browse files
hellohuanlinhellohuanlin
authored
[ios][tools] do not print out bonjour key not found in non-verbose mode (flutter#174001)
<!-- Thanks for filing a pull request! Reviewers are typically assigned within a week of filing a request. To learn more about code review, see our documentation on Tree Hygiene: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md --> This tries flutter#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: flutter#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 flutter#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. <!-- Links --> [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
1 parent edc8a52 commit 70aea54

3 files changed

Lines changed: 221 additions & 156 deletions

File tree

‎packages/flutter_tools/bin/xcode_backend.dart‎

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,22 @@ class Context {
102102

103103
Directory directoryFromPath(String path) => Directory(path);
104104

105-
/// Run given command in a synchronous subprocess.
105+
/// Run given command ([bin]) in a synchronous subprocess.
106+
///
107+
/// If [allowFail] is true, an exception will not be thrown even if the process returns a
108+
/// non-zero exit code. Also, `error:` will not be prefixed to the output to prevent Xcode
109+
/// complication failures.
110+
///
111+
/// If [skipErrorLog] is true, `stderr` from the process will not be output unless in [verbose]
112+
/// mode. If in [verbose], pipes `stderr` to `stdout`.
106113
///
107114
/// Will throw [Exception] if the exit code is not 0.
108115
ProcessResult runSync(
109116
String bin,
110117
List<String> args, {
111118
bool verbose = false,
112119
bool allowFail = false,
120+
bool skipErrorLog = false,
113121
String? workingDirectory,
114122
}) {
115123
if (verbose) {
@@ -122,22 +130,29 @@ class Context {
122130
final String resultStderr = result.stderr.toString().trim();
123131
if (resultStderr.isNotEmpty) {
124132
final errorOutput = StringBuffer();
125-
// If allowFail, do not fail Xcode build. An example is on macOS 26,
126-
// plutil reports NSBonjourServices key not found via stderr (rather than
127-
// stdout on older macOS), and it should not cause compile failure.
128133
if (!allowFail && result.exitCode != 0) {
129134
// "error:" prefix makes this show up as an Xcode compilation error.
130135
errorOutput.write('error: ');
131136
}
132137
errorOutput.write(resultStderr);
133-
echoError(errorOutput.toString());
134-
138+
if (skipErrorLog) {
139+
// Even if skipErrorLog, we still want to write to stdout if verbose.
140+
if (verbose) {
141+
echo(errorOutput.toString());
142+
}
143+
} else {
144+
echoError(errorOutput.toString());
145+
}
135146
// Stream stderr to the Flutter build process.
136147
// When in verbose mode, `echoError` above will show the logs. So only
137148
// stream if not in verbose mode to avoid duplicate logs.
138149
// Also, only stream if exitCode is 0 since errors are handled separately
139150
// by the tool on failure.
140-
if (!verbose && exitCode == 0) {
151+
// Also check for `skipErrorLog`, because some errors should not be printed
152+
// out. For example, on macOS 26, plutil reports NSBonjourServices key not
153+
// found as an error. However, logging it in non-verbose mode would be
154+
// confusing, since not having the key is one of the expected states.
155+
if (!verbose && exitCode == 0 && !skipErrorLog) {
141156
streamOutput(errorOutput.toString());
142157
}
143158
}
@@ -424,16 +439,17 @@ class Context {
424439
return;
425440
}
426441

442+
final bool verbose = (environment['VERBOSE_SCRIPT_LOGGING'] ?? '').isNotEmpty;
443+
427444
// If there are already NSBonjourServices specified by the app (uncommon),
428445
// insert the vmService service name to the existing list.
429-
ProcessResult result = runSync('plutil', <String>[
430-
'-extract',
431-
'NSBonjourServices',
432-
'xml1',
433-
'-o',
434-
'-',
435-
builtProductsPlist,
436-
], allowFail: true);
446+
ProcessResult result = runSync(
447+
'plutil',
448+
<String>['-extract', 'NSBonjourServices', 'xml1', '-o', '-', builtProductsPlist],
449+
verbose: verbose,
450+
allowFail: true,
451+
skipErrorLog: true,
452+
);
437453
if (result.exitCode == 0) {
438454
runSync('plutil', <String>[
439455
'-insert',
@@ -458,14 +474,13 @@ class Context {
458474
// specified (uncommon). This text will appear below the "Your app would
459475
// like to find and connect to devices on your local network" permissions
460476
// popup.
461-
result = runSync('plutil', <String>[
462-
'-extract',
463-
'NSLocalNetworkUsageDescription',
464-
'xml1',
465-
'-o',
466-
'-',
467-
builtProductsPlist,
468-
], allowFail: true);
477+
result = runSync(
478+
'plutil',
479+
<String>['-extract', 'NSLocalNetworkUsageDescription', 'xml1', '-o', '-', builtProductsPlist],
480+
verbose: verbose,
481+
allowFail: true,
482+
skipErrorLog: true,
483+
);
469484
if (result.exitCode != 0) {
470485
runSync('plutil', <String>[
471486
'-insert',

‎packages/flutter_tools/test/general.shard/xcode_backend_test.dart‎

Lines changed: 144 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -390,123 +390,153 @@ void main() {
390390
);
391391
});
392392

393-
test('Missing NSBonjourServices key in Info.plist should not fail Xcode compilation', () {
394-
final Directory buildDir = fileSystem.directory('/path/to/builds')
395-
..createSync(recursive: true);
396-
final File infoPlist = buildDir.childFile('Info.plist')..createSync();
397-
final context = TestContext(
398-
<String>['test_vm_service_bonjour_service'],
399-
<String, String>{
400-
'CONFIGURATION': 'Debug',
401-
'BUILT_PRODUCTS_DIR': buildDir.path,
402-
'INFOPLIST_PATH': 'Info.plist',
403-
},
404-
commands: <FakeCommand>[
405-
FakeCommand(
406-
command: <String>[
407-
'plutil',
408-
'-extract',
409-
'NSBonjourServices',
410-
'xml1',
411-
'-o',
412-
'-',
413-
infoPlist.path,
414-
],
415-
exitCode: 1,
416-
stderr: 'No value at that key path or invalid key path: NSBonjourServices',
417-
),
418-
FakeCommand(
419-
command: <String>[
420-
'plutil',
421-
'-insert',
422-
'NSBonjourServices',
423-
'-json',
424-
'["_dartVmService._tcp"]',
425-
infoPlist.path,
393+
for (final verbose in <bool>[true, false]) {
394+
test(
395+
'Missing NSBonjourServices key in Info.plist should not fail Xcode compilation under ${verbose ? 'verbose' : 'non-verbose'} mode',
396+
() {
397+
final Directory buildDir = fileSystem.directory('/path/to/builds')
398+
..createSync(recursive: true);
399+
final File infoPlist = buildDir.childFile('Info.plist')..createSync();
400+
const plutilErrorMessage =
401+
'Could not extract value, error: No value at that key path or invalid key path: NSBonjourServices';
402+
final File pipe = fileSystem.file('/tmp/pipe')..createSync(recursive: true);
403+
final context = TestContext(
404+
<String>['test_vm_service_bonjour_service'],
405+
<String, String>{
406+
'CONFIGURATION': 'Debug',
407+
'BUILT_PRODUCTS_DIR': buildDir.path,
408+
'INFOPLIST_PATH': 'Info.plist',
409+
if (verbose) 'VERBOSE_SCRIPT_LOGGING': 'YES',
410+
},
411+
commands: <FakeCommand>[
412+
FakeCommand(
413+
command: <String>[
414+
'plutil',
415+
'-extract',
416+
'NSBonjourServices',
417+
'xml1',
418+
'-o',
419+
'-',
420+
infoPlist.path,
421+
],
422+
exitCode: 1,
423+
stderr: plutilErrorMessage,
424+
),
425+
FakeCommand(
426+
command: <String>[
427+
'plutil',
428+
'-insert',
429+
'NSBonjourServices',
430+
'-json',
431+
'["_dartVmService._tcp"]',
432+
infoPlist.path,
433+
],
434+
),
435+
FakeCommand(
436+
command: <String>[
437+
'plutil',
438+
'-extract',
439+
'NSLocalNetworkUsageDescription',
440+
'xml1',
441+
'-o',
442+
'-',
443+
infoPlist.path,
444+
],
445+
),
426446
],
427-
),
428-
FakeCommand(
429-
command: <String>[
430-
'plutil',
431-
'-extract',
432-
'NSLocalNetworkUsageDescription',
433-
'xml1',
434-
'-o',
435-
'-',
436-
infoPlist.path,
447+
fileSystem: fileSystem,
448+
scriptOutputStreamFile: pipe,
449+
)..run();
450+
451+
expect(context.stderr, isNot(startsWith('error: ')));
452+
expect(pipe.readAsStringSync(), isNot(contains(plutilErrorMessage)));
453+
expect(context.stderr, isNot(contains(plutilErrorMessage)));
454+
if (verbose) {
455+
expect(context.stdout, contains(plutilErrorMessage));
456+
} else {
457+
expect(context.stdout, isNot(contains(plutilErrorMessage)));
458+
}
459+
},
460+
);
461+
462+
test(
463+
'Missing NSLocalNetworkUsageDescription in Info.plist should not fail Xcode compilation under ${verbose ? 'verbose' : 'non-verbose'} mode',
464+
() {
465+
final Directory buildDir = fileSystem.directory('/path/to/builds')
466+
..createSync(recursive: true);
467+
final File infoPlist = buildDir.childFile('Info.plist')..createSync();
468+
const plutilErrorMessage =
469+
'Could not extract value, error: No value at that key path or invalid key path: NSLocalNetworkUsageDescription';
470+
final File pipe = fileSystem.file('/tmp/pipe')..createSync(recursive: true);
471+
final context = TestContext(
472+
<String>['test_vm_service_bonjour_service'],
473+
<String, String>{
474+
'CONFIGURATION': 'Debug',
475+
'BUILT_PRODUCTS_DIR': buildDir.path,
476+
'INFOPLIST_PATH': 'Info.plist',
477+
if (verbose) 'VERBOSE_SCRIPT_LOGGING': 'YES',
478+
},
479+
commands: <FakeCommand>[
480+
FakeCommand(
481+
command: <String>[
482+
'plutil',
483+
'-extract',
484+
'NSBonjourServices',
485+
'xml1',
486+
'-o',
487+
'-',
488+
infoPlist.path,
489+
],
490+
),
491+
FakeCommand(
492+
command: <String>[
493+
'plutil',
494+
'-insert',
495+
'NSBonjourServices.0',
496+
'-string',
497+
'_dartVmService._tcp',
498+
infoPlist.path,
499+
],
500+
),
501+
FakeCommand(
502+
command: <String>[
503+
'plutil',
504+
'-extract',
505+
'NSLocalNetworkUsageDescription',
506+
'xml1',
507+
'-o',
508+
'-',
509+
infoPlist.path,
510+
],
511+
exitCode: 1,
512+
stderr: plutilErrorMessage,
513+
),
514+
FakeCommand(
515+
command: <String>[
516+
'plutil',
517+
'-insert',
518+
'NSLocalNetworkUsageDescription',
519+
'-string',
520+
'Allow Flutter tools on your computer to connect and debug your application. This prompt will not appear on release builds.',
521+
infoPlist.path,
522+
],
523+
),
437524
],
438-
),
439-
],
440-
fileSystem: fileSystem,
441-
)..run();
442-
expect(context.stderr, isNot(contains('error: ')));
443-
});
525+
fileSystem: fileSystem,
526+
scriptOutputStreamFile: pipe,
527+
)..run();
444528

445-
test(
446-
'Missing NSLocalNetworkUsageDescription in Info.plist should not fail Xcode compilation',
447-
() {
448-
final Directory buildDir = fileSystem.directory('/path/to/builds')
449-
..createSync(recursive: true);
450-
final File infoPlist = buildDir.childFile('Info.plist')..createSync();
451-
final context = TestContext(
452-
<String>['test_vm_service_bonjour_service'],
453-
<String, String>{
454-
'CONFIGURATION': 'Debug',
455-
'BUILT_PRODUCTS_DIR': buildDir.path,
456-
'INFOPLIST_PATH': 'Info.plist',
457-
},
458-
commands: <FakeCommand>[
459-
FakeCommand(
460-
command: <String>[
461-
'plutil',
462-
'-extract',
463-
'NSBonjourServices',
464-
'xml1',
465-
'-o',
466-
'-',
467-
infoPlist.path,
468-
],
469-
),
470-
FakeCommand(
471-
command: <String>[
472-
'plutil',
473-
'-insert',
474-
'NSBonjourServices.0',
475-
'-string',
476-
'_dartVmService._tcp',
477-
infoPlist.path,
478-
],
479-
),
480-
FakeCommand(
481-
command: <String>[
482-
'plutil',
483-
'-extract',
484-
'NSLocalNetworkUsageDescription',
485-
'xml1',
486-
'-o',
487-
'-',
488-
infoPlist.path,
489-
],
490-
exitCode: 1,
491-
stderr:
492-
'No value at that key path or invalid key path: NSLocalNetworkUsageDescription',
493-
),
494-
FakeCommand(
495-
command: <String>[
496-
'plutil',
497-
'-insert',
498-
'NSLocalNetworkUsageDescription',
499-
'-string',
500-
'Allow Flutter tools on your computer to connect and debug your application. This prompt will not appear on release builds.',
501-
infoPlist.path,
502-
],
503-
),
504-
],
505-
fileSystem: fileSystem,
506-
)..run();
507-
expect(context.stderr, isNot(contains('error: ')));
508-
},
509-
);
529+
expect(context.stderr, isNot(startsWith('error: ')));
530+
expect(pipe.readAsString(), isNot(contains(plutilErrorMessage)));
531+
expect(context.stderr, isNot(contains(plutilErrorMessage)));
532+
if (verbose) {
533+
expect(context.stdout, contains(plutilErrorMessage));
534+
} else {
535+
expect(context.stdout, isNot(contains(plutilErrorMessage)));
536+
}
537+
},
538+
);
539+
}
510540
});
511541

512542
for (final platform in platforms) {

0 commit comments

Comments
 (0)