Revert "[ios][tools]do not log "bonjour not found" at all (unless verbose)" #173879

hellohuanlin · 2025-08-15T20:59:10Z

Sorry I have to revert this one as it didn't work.

Why didn't work

From the comment, it looks like echoError is only used for verbose mode, and streamOutput is used for non-verbose mode (I am still looking into how this is setup)

flutter/packages/flutter_tools/bin/xcode_backend.dart

Line 151 in e4f27cd

// When in verbose mode, `echoError` above will show the logs. So only

How to fix

Possible Fix 1

I tested this works, by changing this line:

https://github.com/flutter/flutter/blob/master/packages/flutter_tools/bin/xcode_backend.dart#L159

into this:

if (!verbose && exitCode == 0 && !skipErrorLog)

Possible fix 2:

Surprisingly this also works:

if (!verbose && exitCode == 0) {
  streamOutput(errorOutput.string().replaceAll('error', '');
}

Although we made sure errorOutput doesn't start with error:, there's an error: in the middle of the string.

Possibly because of this code in mac.dart:

flutter/packages/flutter_tools/lib/src/ios/mac.dart

Lines 431 to 433 in e4f27cd

    
           if (!globals.logger.isVerbose) { 
        
             if (line.contains('error:')) { 
        
               globals.printError(line);

Why we missed it?

I tested my initial code code and assumed updated code would work, because I incorrectly thought echoError was the one triggered the output in non-verbose mode. I should have verified it.

…bose) (#…" This reverts commit b581cda.

gemini-code-assist

Code Review

This pull request reverts a previous change that attempted to suppress certain error logs. The revert removes the skipErrorLog parameter and associated logic from runSync in xcode_backend.dart, and simplifies the related tests.

My review has identified a critical issue where an undefined variable is used, which will cause a runtime error. I've also pointed out missing documentation that violates the project's style guide. Please address these points.

gemini-code-assist · 2025-08-15T21:00:28Z

packages/flutter_tools/bin/xcode_backend.dart

+  /// Run given command in a synchronous subprocess.
  ///
  /// Will throw [Exception] if the exit code is not 0.


The documentation for the verbose and allowFail parameters is missing. According to the Flutter Style Guide, all public members should have documentation.¹

Please add documentation for these parameters to explain their purpose.

/// Run given command in a synchronous subprocess. /// /// If [verbose] is true, the command being run and its stdout will be printed. /// /// 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. /// /// Will throw [Exception] if the exit code is not 0.

Style Guide References

Footnotes

All public members should have documentation. (link) ↩

…_not_log_at_all

hellohuanlin · 2025-08-15T22:40:51Z

From the comment, it looks like echoError is only used for verbose mode, and streamOutput is used for non-verbose mode (I am still looking into how this is setup)

I think I get it now.

For echoError: mac.dart calls _DefaultProcessUtils::run, which does _logger.printTrace(runResult). So in echoError is only print out in verbose mode!

For streamOutput: mac.dart::listenToScriptOutputLine loops through lines like this:

if !verbose {
  only print lines containing "error:" and "warning:"
} else {
  print line
}

This is so confusing...

I think I will go with the fix 1 in the PR description, since fix 2's call path is a bit convoluted

…_not_log_at_all

jmagman

Clean revert LGTM.
@hellohuanlin can you file a new issue or open the old one and document your findings and suggested fixes?

hellohuanlin · 2025-08-15T23:23:13Z

@hellohuanlin can you file a new issue or open the old one and document your findings and suggested fixes?

re-opened the old issue.

…unless verbose)" (flutter/flutter#173879)

flutter/flutter@52af7a5...0a2906b 2025-08-16 [email protected] Improve `SweepGradient` angle and `TileMode` documentation (flutter/flutter#172406) 2025-08-16 [email protected] Roll Skia from 1e148cada9d4 to 16dbd908dcab (1 revision) (flutter/flutter#173901) 2025-08-16 [email protected] Roll Skia from 91ad1f21ca61 to 1e148cada9d4 (3 revisions) (flutter/flutter#173890) 2025-08-16 [email protected] Roll Dart SDK from 9277d6303da5 to 67ca79475db6 (1 revision) (flutter/flutter#173886) 2025-08-15 [email protected] Blocks exynos9820 chip from vulkan (flutter/flutter#173807) 2025-08-15 [email protected] Revert "[ios][tools]do not log "bonjour not found" at all (unless verbose)" (flutter/flutter#173879) 2025-08-15 [email protected] Roll `package:analyzer` forward to `8.1.1` (flutter/flutter#173867) 2025-08-15 [email protected] Roll Skia from 2f66be8a593a to 91ad1f21ca61 (3 revisions) (flutter/flutter#173877) 2025-08-15 [email protected] [a11y] : set isFocused will update isFocusable to true (flutter/flutter#170935) 2025-08-15 [email protected] Reland predictive back route transitions by default (flutter/flutter#173860) 2025-08-15 [email protected] Roll Fuchsia Linux SDK from zWRpLglb48zC1vZLv... to H1kVA85LyQsK8EDp2... (flutter/flutter#173874) 2025-08-15 [email protected] Add onHover callback support for TableRowInkWell (flutter/flutter#173373) 2025-08-15 [email protected] Roll Skia from 5654ac32ede0 to 2f66be8a593a (6 revisions) (flutter/flutter#173866) 2025-08-15 [email protected] Roll Dart SDK from cc008dc8e7aa to 9277d6303da5 (2 revisions) (flutter/flutter#173864) 2025-08-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Implements the Android native stretch effect as a fragment shader (Impeller-only). (#169293)" (flutter/flutter#173865) 2025-08-15 [email protected] Re-add `Linux_android_emu *` tests that had KVM issues, now resolved (flutter/flutter#173812) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…bose)" (flutter#173879) Reverts flutter#173569 Sorry I have to revert this one as it didn't work. # Why didn't work From the comment, it looks like `echoError` is only used for verbose mode, and `streamOutput` is used for non-verbose mode (I am still looking into how this is setup) https://github.com/flutter/flutter/blob/e4f27cd09734db6c6ed94e104ab5333c48dfc544/packages/flutter_tools/bin/xcode_backend.dart#L151 # How to fix ## Possible Fix 1 I tested this works, by changing this line: https://github.com/flutter/flutter/blob/master/packages/flutter_tools/bin/xcode_backend.dart#L159 into this: ``` if (!verbose && exitCode == 0 && !skipErrorLog) ``` ## Possible fix 2: Surprisingly this also works: ``` if (!verbose && exitCode == 0) { streamOutput(errorOutput.string().replaceAll('error', ''); } ``` Although we made sure `errorOutput` doesn't start with `error:`, there's an `error:` in the middle of the string. Possibly because of this code in `mac.dart`: https://github.com/flutter/flutter/blob/e4f27cd09734db6c6ed94e104ab5333c48dfc544/packages/flutter_tools/lib/src/ios/mac.dart#L431-L433 # Why we missed it? I tested my initial code code and assumed updated code would work, because I incorrectly thought `echoError` was the one triggered the output in non-verbose mode. I should have verified it.

…r#9836) flutter/flutter@52af7a5...0a2906b 2025-08-16 [email protected] Improve `SweepGradient` angle and `TileMode` documentation (flutter/flutter#172406) 2025-08-16 [email protected] Roll Skia from 1e148cada9d4 to 16dbd908dcab (1 revision) (flutter/flutter#173901) 2025-08-16 [email protected] Roll Skia from 91ad1f21ca61 to 1e148cada9d4 (3 revisions) (flutter/flutter#173890) 2025-08-16 [email protected] Roll Dart SDK from 9277d6303da5 to 67ca79475db6 (1 revision) (flutter/flutter#173886) 2025-08-15 [email protected] Blocks exynos9820 chip from vulkan (flutter/flutter#173807) 2025-08-15 [email protected] Revert "[ios][tools]do not log "bonjour not found" at all (unless verbose)" (flutter/flutter#173879) 2025-08-15 [email protected] Roll `package:analyzer` forward to `8.1.1` (flutter/flutter#173867) 2025-08-15 [email protected] Roll Skia from 2f66be8a593a to 91ad1f21ca61 (3 revisions) (flutter/flutter#173877) 2025-08-15 [email protected] [a11y] : set isFocused will update isFocusable to true (flutter/flutter#170935) 2025-08-15 [email protected] Reland predictive back route transitions by default (flutter/flutter#173860) 2025-08-15 [email protected] Roll Fuchsia Linux SDK from zWRpLglb48zC1vZLv... to H1kVA85LyQsK8EDp2... (flutter/flutter#173874) 2025-08-15 [email protected] Add onHover callback support for TableRowInkWell (flutter/flutter#173373) 2025-08-15 [email protected] Roll Skia from 5654ac32ede0 to 2f66be8a593a (6 revisions) (flutter/flutter#173866) 2025-08-15 [email protected] Roll Dart SDK from cc008dc8e7aa to 9277d6303da5 (2 revisions) (flutter/flutter#173864) 2025-08-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Implements the Android native stretch effect as a fragment shader (Impeller-only). (#169293)" (flutter/flutter#173865) 2025-08-15 [email protected] Re-add `Linux_android_emu *` tests that had KVM issues, now resolved (flutter/flutter#173812) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…bose)" (flutter#173879) Reverts flutter#173569 Sorry I have to revert this one as it didn't work. # Why didn't work From the comment, it looks like `echoError` is only used for verbose mode, and `streamOutput` is used for non-verbose mode (I am still looking into how this is setup) https://github.com/flutter/flutter/blob/e4f27cd09734db6c6ed94e104ab5333c48dfc544/packages/flutter_tools/bin/xcode_backend.dart#L151 # How to fix ## Possible Fix 1 I tested this works, by changing this line: https://github.com/flutter/flutter/blob/master/packages/flutter_tools/bin/xcode_backend.dart#L159 into this: ``` if (!verbose && exitCode == 0 && !skipErrorLog) ``` ## Possible fix 2: Surprisingly this also works: ``` if (!verbose && exitCode == 0) { streamOutput(errorOutput.string().replaceAll('error', ''); } ``` Although we made sure `errorOutput` doesn't start with `error:`, there's an `error:` in the middle of the string. Possibly because of this code in `mac.dart`: https://github.com/flutter/flutter/blob/e4f27cd09734db6c6ed94e104ab5333c48dfc544/packages/flutter_tools/lib/src/ios/mac.dart#L431-L433 # Why we missed it? I tested my initial code code and assumed updated code would work, because I incorrectly thought `echoError` was the one triggered the output in non-verbose mode. I should have verified it.

…unless verbose)" (flutter/flutter#173879)

…bose)" (flutter#173879) Reverts flutter#173569 Sorry I have to revert this one as it didn't work. # Why didn't work From the comment, it looks like `echoError` is only used for verbose mode, and `streamOutput` is used for non-verbose mode (I am still looking into how this is setup) https://github.com/flutter/flutter/blob/e4f27cd09734db6c6ed94e104ab5333c48dfc544/packages/flutter_tools/bin/xcode_backend.dart#L151 # How to fix ## Possible Fix 1 I tested this works, by changing this line: https://github.com/flutter/flutter/blob/master/packages/flutter_tools/bin/xcode_backend.dart#L159 into this: ``` if (!verbose && exitCode == 0 && !skipErrorLog) ``` ## Possible fix 2: Surprisingly this also works: ``` if (!verbose && exitCode == 0) { streamOutput(errorOutput.string().replaceAll('error', ''); } ``` Although we made sure `errorOutput` doesn't start with `error:`, there's an `error:` in the middle of the string. Possibly because of this code in `mac.dart`: https://github.com/flutter/flutter/blob/e4f27cd09734db6c6ed94e104ab5333c48dfc544/packages/flutter_tools/lib/src/ios/mac.dart#L431-L433 # Why we missed it? I tested my initial code code and assumed updated code would work, because I incorrectly thought `echoError` was the one triggered the output in non-verbose mode. I should have verified it.

Revert "[ios][tools]do not log "bonjour not found" at all (unless ver…

9a8c511

…bose) (#…" This reverts commit b581cda.

hellohuanlin requested a review from a team as a code owner August 15, 2025 20:59

github-actions bot added tool Affects the "flutter" command-line tool. See also t: labels. team-ios Owned by iOS platform team labels Aug 15, 2025

hellohuanlin requested review from jmagman and vashworth August 15, 2025 21:00

gemini-code-assist bot reviewed Aug 15, 2025

View reviewed changes

Merge branch 'master' into revert-173569-xcode_plist_key_not_found_do…

e71014a

…_not_log_at_all

Merge branch 'master' into revert-173569-xcode_plist_key_not_found_do…

5541c84

…_not_log_at_all

jmagman approved these changes Aug 15, 2025

View reviewed changes

hellohuanlin mentioned this pull request Aug 15, 2025

[ios/tools] Simplify mac.dart / xcode_backend.dart logging logic #173887

Closed

hellohuanlin added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 15, 2025

auto-submit bot added this pull request to the merge queue Aug 15, 2025

Merged via the queue into master with commit eeb3a40 Aug 16, 2025
150 of 151 checks passed

auto-submit bot deleted the revert-173569-xcode_plist_key_not_found_do_not_log_at_all branch August 16, 2025 00:16

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 16, 2025

engine-flutter-autoroll mentioned this pull request Aug 16, 2025

Roll Flutter from 52af7a504025 to 0a2906b81d5e (16 revisions) flutter/packages#9836

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2025

eeb3a40cb Revert "[ios][tools]do not log "bonjour not found" at all (…

98f517c

…unless verbose)" (flutter/flutter#173879)

hellohuanlin mentioned this pull request Aug 19, 2025

[ios][tool] a logging bug in mac.dart when checking the "error:" prefix #174080

Open

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025

eeb3a40cb Revert "[ios][tools]do not log "bonjour not found" at all (…

a8c60b7

…unless verbose)" (flutter/flutter#173879)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Revert "[ios][tools]do not log "bonjour not found" at all (unless verbose)" #173879

Revert "[ios][tools]do not log "bonjour not found" at all (unless verbose)" #173879

Uh oh!

hellohuanlin commented Aug 15, 2025 •

edited

Loading

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Aug 15, 2025

Uh oh!

hellohuanlin commented Aug 15, 2025 •

edited

Loading

Uh oh!

jmagman left a comment

Uh oh!

hellohuanlin commented Aug 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	if (!globals.logger.isVerbose) {
	if (line.contains('error:')) {
	globals.printError(line);

Revert "[ios][tools]do not log "bonjour not found" at all (unless verbose)" #173879

Revert "[ios][tools]do not log "bonjour not found" at all (unless verbose)" #173879

Uh oh!

Conversation

hellohuanlin commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why didn't work

How to fix

Possible Fix 1

Possible fix 2:

Why we missed it?

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Aug 15, 2025

Choose a reason for hiding this comment

Style Guide References

Footnotes

Uh oh!

hellohuanlin commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

hellohuanlin commented Aug 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hellohuanlin commented Aug 15, 2025 •

edited

Loading

hellohuanlin commented Aug 15, 2025 •

edited

Loading