-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Bump minimum required Xcode version to 15 and recommended to 16 #180531
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
base: master
Are you sure you want to change the base?
Conversation
|
I used 16.2 as the recommended version since the ci hasnt been fully upgraded to 26 |
Can you explain why you picked 16.2? |
I used 16.2 because that seems to be the current xcode version in ci but i agree we should be encouraging users to be using 26.2(the latest) since we're in the process of upgrading anyway. wdyt? |
packages/flutter_tools/test/commands.shard/hermetic/build_ios_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/test/commands.shard/hermetic/build_ipa_test.dart
Outdated
Show resolved
Hide resolved
| command: <String>['xcrun', '--sdk', 'iphonesimulator', '--show-sdk-platform-version'], | ||
| stdout: '17.0', | ||
| ), | ||
| ]); |
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 is this required?
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 added the mock commands because subsequent validation checks e.g EULA, simctl, SDK version
run after the version check and could append their messages to the result because FakeProcessManager.any()
allows these commands to execute and they may return unexpected results, causing some checks to fail
and add error messages.
Since the test asserts on result.messages.last, we need to ensure all
subsequent checks pass so that the last message remains the expected hint about the recommended
Xcode version.
Generally we keep the minimum required and minimum suggested as low (not high) as possible and only bump it up if Apple/Xcode/features we need require. I think you can make both the required and recommended 16, unless there's actually something special about 16.2. You can use flutter/packages/flutter_tools/test/general.shard/macos/xcode_validator_test.dart Line 106 in 30799e1
- }, skip: false); // [intended] Skip this test when minimum and required check versions converge.
+ }, skip: true); // [intended] Skip this test when minimum and required check versions converge. |
|
Actually the issue is to update to Xcode 15, not 16. |
79e3784 to
9085bfb
Compare
7a19002 to
5c91142
Compare
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.
Couple tweaks to tests
packages/flutter_tools/test/commands.shard/hermetic/build_ipa_test.dart
Outdated
Show resolved
Hide resolved
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 once @vashworth is happy with the comment thread.
This PR updates the Flutter tool's Xcode version requirements:
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 #144582
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.