[flutter_tools] Refactor hostPlatform to use Abi.current()#185369
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the OperatingSystemUtils class to centralize hostPlatform detection by using the Abi class instead of platform-specific shell commands like uname and sysctl. A significant regression was identified regarding Rosetta 2 detection on macOS; because Abi.current() returns the architecture of the running process rather than the native host, the tool may incorrectly identify the platform, affecting artifact downloads and diagnostic reporting in flutter doctor.
| return _name!; | ||
| } | ||
|
|
||
| // On ARM returns arm64, even when this process is running in Rosetta. |
There was a problem hiding this comment.
Is this behavior still preserved after this PR?
There was a problem hiding this comment.
It's not (I assume it should be?)
FWIW, this PR is an attempt to create an Antigravity workflow to fix flutter_tool issues, so it's a bit rough right now and should have been marked as draft.
There was a problem hiding this comment.
FWIW, this PR is an attempt to create an Antigravity workflow to fix flutter_tool issues
Could you elaborate more in PR description? This kind of info would be helpful for future readers.
There was a problem hiding this comment.
FWIW, this PR is an attempt to create an Antigravity workflow to fix flutter_tool issues
Could you elaborate more in PR description? This kind of info would be helpful for future readers.
I'm not sure it makes sense to add that info to the PR description if that's what's going to be included in the commit message since the change itself has nothing to do with the Antigravity workflow.
Basically, this change was written with the Antigravity agent using some skills to:
- Pick an issue from the
team-toolissue backlog - Create a fix for the issue with tests
- Create and upload a PR
- Check for failures, iterate, upload, repeat
There was a problem hiding this comment.
Also, the behavior around Rosetta on ARM64 should be restored now.
There was a problem hiding this comment.
Interesting! So this code is authored by antigravity agent?
There was a problem hiding this comment.
Correct! I didn't write any of the code, just confirmed the agent was behaving properly and the changes were correct. There's obviously still lots of rough edges (which is why there's 21 commits 🫠), but it shows some promise.
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
…ssing The recent fix to restore Rosetta detection in _MacOSUtils caused tests in build_test.dart to fail. The tests mock xcrun commands but don't expect the which sysctl check triggered by hostPlatform getter in Xcode. This commit fixes it by adding FakeCommands for which sysctl and mocking sysctl failure to simulate an x64 host machine, matching the previous test behavior. Fixes flutter#184358
…er.test The recent fix to restore Rosetta detection in _MacOSUtils caused tests in general.shard that use XcodeProjectInterpreter.test to fail on Mac. The tests mock xcrun commands but don't expect the which sysctl check triggered by hostPlatform getter in Xcode. This commit fixes it by updating XcodeProjectInterpreter.test to use a fake OperatingSystemUtils by default, which avoids process calls entirely for tests using this factory constructor. Fixes flutter#184358
The recent changes to add a fake OperatingSystemUtils in xcodeproj.dart caused tests in general.shard to fail on all platforms with UnimplementedError. This commit reverts xcodeproj.dart to the upstream master state, removing the fake and restoring test behavior. Fixes flutter#184358
…revert The recent revert of xcodeproj.dart to master state caused 12 analyzer errors in xcodeproj_test.dart because the tests were still trying to pass the currentAbi parameter which was removed. This commit fixes it by removing the currentAbi parameter from all calls in xcodeproj_test.dart, restoring consistency. Fixes flutter#184358
…failures The tests in xcodeproj_test.dart were failing on all platforms because they expect simple xcrun calls but hit an unexpected which sysctl check triggered by the hostPlatform getter in XcodeProjectInterpreter constructor. This commit fixes it by adding mock commands for which sysctl and sysctl in the setUp method of xcodeproj_test.dart, resolving sequence failures for all tests in that file. Fixes flutter#184358
…est.dart The tests in xcodeproj_test.dart were failing on all platforms because they expect simple xcrun calls but hit an unexpected which sysctl check triggered by the hostPlatform getter in XcodeProjectInterpreter constructor. This commit fixes it by adding mock commands for which sysctl and sysctl in specific tests that trigger them, resolving sequence failures. Fixes flutter#184358
…calls The tests in xcresult_test.dart were failing on Mac because setUpFakeXCResultCommand helper method called xcode.xcrunCommand() which triggered hostPlatform and therefore sysctl unexpectedly during test setup. This commit fixes it by hardcoding 'xcrun' in that helper method, avoiding the call to xcrunCommand() and sequence failures. Fixes flutter#184358
… avoid sequence failures The tests in cocoapods_test.dart and xcode_validator_test.dart were failing on Mac because they triggered hostPlatform and therefore sysctl unexpectedly during execution, breaking the mock sequence. This commit fixes it by adding mock commands for which sysctl and sysctl to the test bodies or helpers, resolving sequence failures. Fixes flutter#184358
…de_validator tests This commit fixes failures in three files by avoiding unexpected sysctl calls or mocking them in the correct sequence: - Hardcoded 'xcrun' in xcresult_test.dart helper method to avoid triggering sysctl. - Added sysctl mocks to cocoapods_test.dart and xcode_validator_test.dart and arranged them in the correct sequence to match test execution. Fixes flutter#184358
This commit restores the original kWhichSysctlCommand, kx64CheckCommand, and kARMCheckCommand constants in xcodeproj_test.dart and propagates them to other relevant test files (xcresult_test.dart, cocoapods_test.dart, xcode_validator_test.dart, etc.). Duplicated manual FakeCommand definitions have been replaced with these constants to improve maintainability and reduce code duplication. Fixes flutter#184358
|
auto label is removed for flutter/flutter/185369, Failed to enqueue flutter/flutter/185369 with HTTP 400: Pull request Required status check "Merge Queue Guard" is expected.. |
|
autosubmit label was removed for flutter/flutter/185369, because - The status or check suite Linux firebase_release_smoke_test has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
autosubmit label was removed for flutter/flutter/185369, because - The status or check suite Linux firebase_release_smoke_test has failed. Please fix the issues identified (or deflake) before re-applying this label. |
Documented OperatingSystemUtils and hostPlatform. Refactored hostPlatform to use Dart's built-in Abi.current() for cross-platform consistency and to avoid inconsistent platform-specific shell calls. Updated tests to inject Abi for verification.
Fixes #184358