[google_maps_flutter] Restore the arm64 workaround #5915
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
test-exempt: removing the workarounds from the Xcode project is a test for the actual change |
Is this because |
|
Ah, right. That cleanup is the least important, so I'll just revert it, and make a note to do a separate PR for it later. Looks like I'll need to keep some workarounds in the unit test target as well. |
stuartmorgan
added
the
autosubmit
flutter/packages@1a2b780...83c2c4d 2024-01-18 [email protected] [google_maps_flutter] Restore the arm64 workaround (flutter/packages#5915) 2024-01-17 [email protected] [pigeon] Separates message call code generation into separate methods in the KotlinGenerator (flutter/packages#5891) 2024-01-17 [email protected] [webview_flutter_wkebview] Verify JavaScriptChannels have a unique name (flutter/packages#5904) 2024-01-17 [email protected] [pigeon] Separates message call code generation into separate methods in the `DartGenerator` (flutter/packages#5859) 2024-01-17 [email protected] Roll Flutter from 8e94423 to def6af0 (23 revisions) (flutter/packages#5911) 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-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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
Follows up on #5915 to remove the example-app-level workaround for the lack of arm64 slice, since the `flutter` build process will handle it automatically with current versions (now that the plugin-level workaround has been restored). Also fixes another problem I noticed while removing those, which is that when I federated this plugin I never removed the RunnerTests and RunnerUITests targets from this project, even though all of the files had moved, leaving missing file references and targets that would not build. This hadn't been noticed because our tooling doesn't even try to run native tests on plugins that indicate that their implementation is in a different package (and we rarely manually open the native projects for the app-facing package).
|
I am not sure if this is the correct place to post, so happy to delete this comment if necessary. Similar to one of the comments above, I had the same issue below:
Since your latest commit, this issue has now been resolved (much appreciated), but for some reason the app crashes every time I do a hot restart. Just wondering if anyone else is experiencing this issue now or if it's just me. |
@IttaiBarkai can you file a new issue and completely fill out the template, including reproduction steps? https://github.com/flutter/flutter/issues/new?assignees=&labels=&projects=&template=2_bug.yml |
|
Hi @jmagman, no problem, I will try and do it this weekend if I have time. Thanks for the quick response. |
Restores the workaround to exclude arm64 when building for simulator, which is still necessary for apps that support back to iOS 12. Also removes all the example-app-level workarounds that manually excluded arm64 simulator, which were masking the failure in the first place, so that we don't accidentally break this again. The exception is the exclusions for the unit test targets, which need them in order to match the Flutter-tool-generated exclusion to the Runner, otherwise the unit tests can't build. See flutter/flutter#94491
Follows up on flutter#5915 to remove the example-app-level workaround for the lack of arm64 slice, since the `flutter` build process will handle it automatically with current versions (now that the plugin-level workaround has been restored). Also fixes another problem I noticed while removing those, which is that when I federated this plugin I never removed the RunnerTests and RunnerUITests targets from this project, even though all of the files had moved, leaving missing file references and targets that would not build. This hadn't been noticed because our tooling doesn't even try to run native tests on plugins that indicate that their implementation is in a different package (and we rarely manually open the native projects for the app-facing package).
Restores the workaround to exclude arm64 when building for simulator, which is still necessary for apps that support back to iOS 12.
Also removes all the example-app-level workarounds that manually excluded arm64 simulator, which were masking the failure in the first place, so that we don't accidentally break this again. The exception is the exclusions for the unit test targets, which need them in order to match the Flutter-tool-generated exclusion to the Runner, otherwise the unit tests can't build.
See flutter/flutter#94491
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).