-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Turn off bitcode in existing iOS Xcode projects #112828
Conversation
Actually, instead of removing the build setting, it will be safer to set |
Done. |
// Only print for the first discovered change found. | ||
logger.printWarning('Disabling deprecated bitcode Xcode build setting. See https://github.com/flutter/flutter/issues/107887 for additional details.'); | ||
} | ||
return line.replaceAll('ENABLE_BITCODE = YES', 'ENABLE_BITCODE = NO'); |
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.
is the spacing here guaranteed to be a single space on both sides of the =
?
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 mean it's totally undocumented but it's been that way for as long as I've been mucking with Xcode projects.
if (_xcodeProjectInfoFile.existsSync()) { | ||
processFileLines(_xcodeProjectInfoFile); | ||
} else { | ||
logger.printTrace('Xcode project not found, skipping removing bitcode migration.'); |
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.
isn't migrate()
supposed to return false if the migration was skipped?
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.
At least how it works now is that returning false
means something catastrophic happened and the build fails:
flutter/packages/flutter_tools/lib/src/linux/build_linux.dart
Lines 50 to 52 in 5f5d480
if (!migration.run()) { | |
throwToolExit('Unable to migrate project files'); | |
} |
flutter/packages/flutter_tools/lib/src/ios/mac.dart
Lines 134 to 136 in 70aaff1
if (!migration.run()) { | |
return XcodeBuildResult(success: false); | |
} |
etc
flutter/packages/flutter_tools/lib/src/base/project_migrator.dart
Lines 85 to 90 in c77e4cd
if (!migrator.migrate()) { | |
// Migration failures should be more robust, with transactions and fallbacks. | |
// See https://github.com/flutter/flutter/issues/12573 and | |
// https://github.com/flutter/flutter/issues/40460 | |
return false; | |
} |
So this comment is just wrong:
flutter/packages/flutter_tools/lib/src/base/project_migrator.dart
Lines 19 to 20 in c77e4cd
/// Returns whether migration was successful or was skipped. | |
bool migrate(); |
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.
Will fix in follow-up PR.
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.
two nit questions, otherwise LGTM
https://developer.apple.com/documentation/xcode-release-notes/xcode-14-release-notes
Ensure Flutter apps have
ENABLE_BITCODE
set toNO
. We never turned bitcode on by default in our template, but it's possible the user turned it on manually. It it is on, then give the user a warning and remove the build setting.Confirmed this worked and removed the build setting in the test project where I manually turned it on:
DEVELOPMENT_TEAM = S8QB4VV633; - ENABLE_BITCODE = YES; INFOPLIST_FILE = Runner/Info.plist;
Fixes #111776
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.