-
Notifications
You must be signed in to change notification settings - Fork 29.7k
[ios26]Do not report error for Info.plist key not found #172913
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ios26]Do not report error for Info.plist key not found #172913
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request prevents xcode_backend.dart from reporting a command failure as an Xcode build error when the failure is expected. The change is correct and minimal. I've added one suggestion for a potential future improvement to make the failure handling more robust.
| if (resultStderr.isNotEmpty) { | ||
| final errorOutput = StringBuffer(); | ||
| if (result.exitCode != 0) { | ||
| if (result.exitCode != 0 && !allowFail) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this change correctly uses allowFail to suppress the error prefix, the allowFail flag is a bit broad. It will suppress the error prefix for any non-zero exit code as long as the command produces some output on stderr.
For the case mentioned in the PR title (Info.plist key not found), plutil exits with code 1 and empty stderr. The surrounding if (resultStderr.isNotEmpty) block handles this by not printing anything, which is correct.
However, for other commands or other failure modes of plutil (e.g., a malformed plist), allowFail would also suppress the error prefix, which might hide real problems from developers.
A more robust approach for the future could be to replace the allowFail boolean with a callback that can inspect the ProcessResult. For example:
typedef bool FailureValidator(ProcessResult result);
... runSync(..., FailureValidator? allowedToFail) ...
// in _echoQueryResult
final bool shouldReportAsError = result.exitCode != 0 && !(allowedToFail?.call(result) ?? false);This would allow callers to be more specific about which failures are acceptable, for instance:
allowedToFail: (result) => result.exitCode == 1 && result.stderr.isEmpty
This is a suggestion for future improvement and not a blocker for this PR, as it would require a larger refactoring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this change correctly uses allowFail to suppress the error prefix, the allowFail flag is a bit broad.
Bad Gemini. If we want Xcode compile to fail, we should set allowFail = false; if we want Xcode compile to continue, then set allowFail = true.
| final errorOutput = StringBuffer(); | ||
| if (result.exitCode != 0) { | ||
| if (result.exitCode != 0 && !allowFail) { | ||
| // "error:" prefix makes this show up as an Xcode compilation error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// "error:" prefix makes this show up as an Xcode compilation error.
Comment
This feels so fragile. Wondering if it's documented somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You know it ain't (that I'm aware of).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment explaining this is needed to prevent the build from failing?
Can you also add a unit test to general.shard/xcode_backend_test.dart to ensure it doesn't stream the error logs when allowFail is true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure I can add some comment.
Just to clarify, this is to fix the logic error, which happens to be surfaced by macOS 26. In another word, even without this incident, if I randomly come across this code, I would fix it too.
So strictly speaking, it's more like "this is needed, so that the code is doing what it's intended to do, i.e., don't cause Xcode failure if allowFail=true". That being said, I can refer to this incident as an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re the streaming part - the streaming logic doesn't seem to be related to allowFail:
if (!verbose && exitCode == 0) {
streamOutput(errorOutput.toString());
}
I will leave it there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, I took a look at the whole xcode_backend.dart file and its tests. Looks like there's a design issue. Let me file a ticket which should clear things up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you suggesting to write the same test again, but in unit test with mock commands?
Yes, see my comment above:
"With only an integration test, if plutil changes again, we'll no longer be validating this logic -- which is why I think it'd be good to also have a unit test."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With only an integration test, if plutil changes again, we'll no longer be validating this logic
To clarify, if plutil changes again, the best tests to catch any breakage would be the integration test, not the unit test. This is because integration test runs the actual command, while unit test runs fake command.
which is why I think it'd be good to also have a unit test.
That being said, I fully agree we should also have a unit test, which provides another level of confidence. The unit test can either directly test runSync, or better, test another function that calls runSync function.
However, I think there's a design flaw in our setup that needs to be addressed before being able to do so - runSync itself is currently not testable - our unit test does not run this code at all since it's stubbed out (more details here #173133).
I am OK with either:
(1) land this PR first, and fix the design flaw as a follow up; or
(2) fix the design flaw first (at least the "short term fix" section), and then rebase this PR. I can do this when I come back next Wednesday.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to read the addVmServiceBonjourService function to figure out what this change is doing and why.
@hellohuanlin can you update the PR description to explain in particular what allowFail has to do with the fix.
Remember we need to be able to read these commits and understand what they mean years from now 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jmagman updated description! let me know if it's not clear
a2735ea to
ac48243
Compare
jmagman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why isn't it found though? NSBonjourServices etc needs to be there for the port to be published. A race condition?
Victoria explained it to me, the key is supposed to be missing, the new behavior is that it errors during the check. |
@jmagman Sorry just saw this. I did more testing today - it turns out that it's because the change in On old macOS, plutil uses But on macOS 26, plutil uses |
| </plist> | ||
| '''); | ||
|
|
||
| expect(result.stderr, isNot(startsWith("error:"))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without the fix, this would fail on macOS 26. I manually tested it since our CI doesn't run macOS 26 yet.
e96b3c9 to
b16b0cb
Compare
|
friendly ping @vashworth @jmagman (as the github refresh button is gone for some reason) |
| List<String> args, { | ||
| String? workingDirectory, | ||
| }) { | ||
| return Process.runSync(bin, args, workingDirectory: workingDirectory); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vashworth @jmagman This is the short term workaround to make bonjour unit testable. It follows the existing design pattern (inheritance). I will just go with it first since it's a quick fix and we are fire fighting.
Before the change, the whole implementation of runSync is stubbed out, which defeats the purpose of unit tests. For unit test, we should stub out the dependencies of the test subject, not the implementation of test subject itself.
In the long term, however, i'd like to fix the design flaw (inheritance -> composition) for better testability and maintainability. See also: #173133 and #173138
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense to me, thanks for figuring it out!
| ); | ||
| }); | ||
|
|
||
| test('Missing NSBonjourServices key in Info.plist should not fail Xcode compilation', () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: While these tests technically covers the expected behavior, we could make them even more specific by calling runSync directly. This would better confirm that error: is not added when allowFail is true and would also allow us to add a test case for when allowFail is false. This isn't a blocking issue, but it would improve the test coverage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
runSync is our private helper function which is the implementation details though. Unit tests should verify the behavior, not the implementation.
that being said, in the future if runSync is used elsewhere, I don't object refactoring it into a separate component, then it would become the API of it, which should be unit tested
vashworth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
autosubmit label was removed for flutter/flutter/172913, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
a933a2f to
44ae288
Compare
|
@hellohuanlin You have analyze errors: To fix, run |
In xcode_backend's `runSync` function, there's a flag parameter `allowFail`. The purpose of this flag is that, when enabled, even if command fails, we should not fail Xcode. However, the current implementation has a bug, that even when `allowFail = true`, we still prefix `"error:"` string to `stderr`, which causes Xcode compilation error. Before macOS 26, plutil used `stdout` rather than `stderr`, so the bug was not surfaced. On macOS 26, pltuil uses `stderr` instead (which makes sense), so the bug is surfaced now. Note: even if plutil doesn't change behavior on macOS 26, if I randomly come across this code, I'd still fix the logic error. *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. - [x] 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 --------- Co-authored-by: Jenn Magder <[email protected]>
flutter/flutter@92a6bfb...3821790 2025-08-08 [email protected] Use LLDB as the default debugging method for iOS 17+ and Xcode 26+ (flutter/flutter#173443) 2025-08-08 [email protected] Roll Fuchsia Linux SDK from i4vsuEGyP8Xeb5tiy... to HclTm0V8hgSpfqmtG... (flutter/flutter#173462) 2025-08-08 [email protected] Support launching a HTTPS URL (flutter/flutter#164720) 2025-08-08 [email protected] Roll Dart SDK from c48772a79e1f to 4b7b565eb468 (1 revision) (flutter/flutter#173454) 2025-08-08 [email protected] Roll Dart SDK from ba58b96a80d7 to c48772a79e1f (3 revisions) (flutter/flutter#173451) 2025-08-07 [email protected] Web dev proxy (flutter/flutter#172175) 2025-08-07 [email protected] Roll ICU from b929596baebf to 1b2e3e8a421e (7 revisions) (flutter/flutter#173436) 2025-08-07 [email protected] [A11y] TextField prefix icon and suffix icon create a sibling node' (flutter/flutter#173312) 2025-08-07 [email protected] [Android templates] Remove jetifier usage (flutter/flutter#173431) 2025-08-07 [email protected] Remove a couple of asserts from display_list_unittest (flutter/flutter#173381) 2025-08-07 [email protected] Fix ScaffoldGeometry null scale with noAnimation FAB (flutter/flutter#172914) 2025-08-07 [email protected] Prepare for iOS debugging with lldb and devicectl (flutter/flutter#173417) 2025-08-07 [email protected] Fix `ReorderableList` proxy animation for partial drag-back (flutter/flutter#172380) 2025-08-07 [email protected] [ios26]Do not report error for Info.plist key not found (flutter/flutter#172913) 2025-08-07 [email protected] Manual roll to 3.10.0-75.1.beta (flutter/flutter#173423) 2025-08-07 [email protected] [web] add --static-assets-url argument to build web (flutter/flutter#171638) 2025-08-07 [email protected] Adds deprecation for impeller opt out on android (flutter/flutter#173375) 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
…173569) The previous PR #172913 avoids compilation error by not having the "error: " prefix. However, it's still very confusing to have "key not found" printed out in the terminal (either stderr or stdout), despite that it doesn't cause compilation error anymore. This PR introduces a new flag "skipErrorLog" which skips logging the `stderr` if set to true. We set it for plist extraction, because "not having bonjour key" should be one of the expected "normal" states <s> We don't want to pipe it to `stdout` either, because it would be confusing too. I figured people would still file issue if they see it in terminal, even if it's a completely normal state. But not a strong opinion, so let me know if you disagree. </s> However, under verbose mode, we would pipe it to `stdout`. This will be consistent with our previous behavior before macOS 26. *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 #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. - [x] 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
…lutter#173569) The previous PR flutter#172913 avoids compilation error by not having the "error: " prefix. However, it's still very confusing to have "key not found" printed out in the terminal (either stderr or stdout), despite that it doesn't cause compilation error anymore. This PR introduces a new flag "skipErrorLog" which skips logging the `stderr` if set to true. We set it for plist extraction, because "not having bonjour key" should be one of the expected "normal" states <s> We don't want to pipe it to `stdout` either, because it would be confusing too. I figured people would still file issue if they see it in terminal, even if it's a completely normal state. But not a strong opinion, so let me know if you disagree. </s> However, under verbose mode, we would pipe it to `stdout`. This will be consistent with our previous behavior before macOS 26. *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. - [x] 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
…lutter#173569) The previous PR flutter#172913 avoids compilation error by not having the "error: " prefix. However, it's still very confusing to have "key not found" printed out in the terminal (either stderr or stdout), despite that it doesn't cause compilation error anymore. This PR introduces a new flag "skipErrorLog" which skips logging the `stderr` if set to true. We set it for plist extraction, because "not having bonjour key" should be one of the expected "normal" states <s> We don't want to pipe it to `stdout` either, because it would be confusing too. I figured people would still file issue if they see it in terminal, even if it's a completely normal state. But not a strong opinion, so let me know if you disagree. </s> However, under verbose mode, we would pipe it to `stdout`. This will be consistent with our previous behavior before macOS 26. *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. - [x] 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
…lutter#173569) The previous PR flutter#172913 avoids compilation error by not having the "error: " prefix. However, it's still very confusing to have "key not found" printed out in the terminal (either stderr or stdout), despite that it doesn't cause compilation error anymore. This PR introduces a new flag "skipErrorLog" which skips logging the `stderr` if set to true. We set it for plist extraction, because "not having bonjour key" should be one of the expected "normal" states <s> We don't want to pipe it to `stdout` either, because it would be confusing too. I figured people would still file issue if they see it in terminal, even if it's a completely normal state. But not a strong opinion, so let me know if you disagree. </s> However, under verbose mode, we would pipe it to `stdout`. This will be consistent with our previous behavior before macOS 26. *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. - [x] 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
In xcode_backend's `runSync` function, there's a flag parameter `allowFail`. The purpose of this flag is that, when enabled, even if command fails, we should not fail Xcode. However, the current implementation has a bug, that even when `allowFail = true`, we still prefix `"error:"` string to `stderr`, which causes Xcode compilation error. Before macOS 26, plutil used `stdout` rather than `stderr`, so the bug was not surfaced. On macOS 26, pltuil uses `stderr` instead (which makes sense), so the bug is surfaced now. Note: even if plutil doesn't change behavior on macOS 26, if I randomly come across this code, I'd still fix the logic error. *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. - [x] 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 --------- Co-authored-by: Jenn Magder <[email protected]>
…lutter#173569) The previous PR flutter#172913 avoids compilation error by not having the "error: " prefix. However, it's still very confusing to have "key not found" printed out in the terminal (either stderr or stdout), despite that it doesn't cause compilation error anymore. This PR introduces a new flag "skipErrorLog" which skips logging the `stderr` if set to true. We set it for plist extraction, because "not having bonjour key" should be one of the expected "normal" states <s> We don't want to pipe it to `stdout` either, because it would be confusing too. I figured people would still file issue if they see it in terminal, even if it's a completely normal state. But not a strong opinion, so let me know if you disagree. </s> However, under verbose mode, we would pipe it to `stdout`. This will be consistent with our previous behavior before macOS 26. *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. - [x] 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
In xcode_backend's `runSync` function, there's a flag parameter `allowFail`. The purpose of this flag is that, when enabled, even if command fails, we should not fail Xcode. However, the current implementation has a bug, that even when `allowFail = true`, we still prefix `"error:"` string to `stderr`, which causes Xcode compilation error. Before macOS 26, plutil used `stdout` rather than `stderr`, so the bug was not surfaced. On macOS 26, pltuil uses `stderr` instead (which makes sense), so the bug is surfaced now. Note: even if plutil doesn't change behavior on macOS 26, if I randomly come across this code, I'd still fix the logic error. *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. - [x] 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 --------- Co-authored-by: Jenn Magder <[email protected]>
…lutter#173569) The previous PR flutter#172913 avoids compilation error by not having the "error: " prefix. However, it's still very confusing to have "key not found" printed out in the terminal (either stderr or stdout), despite that it doesn't cause compilation error anymore. This PR introduces a new flag "skipErrorLog" which skips logging the `stderr` if set to true. We set it for plist extraction, because "not having bonjour key" should be one of the expected "normal" states <s> We don't want to pipe it to `stdout` either, because it would be confusing too. I figured people would still file issue if they see it in terminal, even if it's a completely normal state. But not a strong opinion, so let me know if you disagree. </s> However, under verbose mode, we would pipe it to `stdout`. This will be consistent with our previous behavior before macOS 26. *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. - [x] 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
In xcode_backend's `runSync` function, there's a flag parameter `allowFail`. The purpose of this flag is that, when enabled, even if command fails, we should not fail Xcode. However, the current implementation has a bug, that even when `allowFail = true`, we still prefix `"error:"` string to `stderr`, which causes Xcode compilation error. Before macOS 26, plutil used `stdout` rather than `stderr`, so the bug was not surfaced. On macOS 26, pltuil uses `stderr` instead (which makes sense), so the bug is surfaced now. Note: even if plutil doesn't change behavior on macOS 26, if I randomly come across this code, I'd still fix the logic error. *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. - [x] 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 --------- Co-authored-by: Jenn Magder <[email protected]>
…lutter#173569) The previous PR flutter#172913 avoids compilation error by not having the "error: " prefix. However, it's still very confusing to have "key not found" printed out in the terminal (either stderr or stdout), despite that it doesn't cause compilation error anymore. This PR introduces a new flag "skipErrorLog" which skips logging the `stderr` if set to true. We set it for plist extraction, because "not having bonjour key" should be one of the expected "normal" states <s> We don't want to pipe it to `stdout` either, because it would be confusing too. I figured people would still file issue if they see it in terminal, even if it's a completely normal state. But not a strong opinion, so let me know if you disagree. </s> However, under verbose mode, we would pipe it to `stdout`. This will be consistent with our previous behavior before macOS 26. *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. - [x] 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
In xcode_backend's `runSync` function, there's a flag parameter `allowFail`. The purpose of this flag is that, when enabled, even if command fails, we should not fail Xcode. However, the current implementation has a bug, that even when `allowFail = true`, we still prefix `"error:"` string to `stderr`, which causes Xcode compilation error. Before macOS 26, plutil used `stdout` rather than `stderr`, so the bug was not surfaced. On macOS 26, pltuil uses `stderr` instead (which makes sense), so the bug is surfaced now. Note: even if plutil doesn't change behavior on macOS 26, if I randomly come across this code, I'd still fix the logic error. *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. - [x] 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 --------- Co-authored-by: Jenn Magder <[email protected]>
…lutter#173569) The previous PR flutter#172913 avoids compilation error by not having the "error: " prefix. However, it's still very confusing to have "key not found" printed out in the terminal (either stderr or stdout), despite that it doesn't cause compilation error anymore. This PR introduces a new flag "skipErrorLog" which skips logging the `stderr` if set to true. We set it for plist extraction, because "not having bonjour key" should be one of the expected "normal" states <s> We don't want to pipe it to `stdout` either, because it would be confusing too. I figured people would still file issue if they see it in terminal, even if it's a completely normal state. But not a strong opinion, so let me know if you disagree. </s> However, under verbose mode, we would pipe it to `stdout`. This will be consistent with our previous behavior before macOS 26. *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. - [x] 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
In xcode_backend's
runSyncfunction, there's a flag parameterallowFail. The purpose of this flag is that, when enabled, even if command fails, we should not fail Xcode.However, the current implementation has a bug, that even when
allowFail = true, we still prefix"error:"string tostderr, which causes Xcode compilation error.Before macOS 26, plutil used
stdoutrather thanstderr, so the bug was not surfaced. On macOS 26, pltuil usesstderrinstead (which makes sense), so the bug is surfaced now.Note: even if plutil doesn't change behavior on macOS 26, if I randomly come across this code, I'd still fix the logic error.
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 #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
///).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. Comments from the
gemini-code-assistbot 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.