Skip to content
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

Remove multi-arch check in iOS builds #37407

Merged
merged 2 commits into from
Aug 1, 2019
Merged

Remove multi-arch check in iOS builds #37407

merged 2 commits into from
Aug 1, 2019

Conversation

liamappelbe
Copy link
Contributor

@liamappelbe liamappelbe commented Aug 1, 2019

Description

As of flutter/engine#10010, iOS builds targeting armv7 are built using a 64-bit version of gen_snapshot that can produce 32-bit arm code. So this multi-arch special case is no longer necessary (and actually breaks the build).

Related Issues

#22598

Tests

I built the flutter examples in this mode using Xcode 10 and verified that they work on an iPhone 4S.

Manual roll

git log b41c172..9fca3c7 --no-merges --oneline
flutter/engine@9fca3c744 Use simarm_x64 when targeting arm (flutter/engine#10010)
flutter/engine@bf9288597 [fuchsia] Add kernel_compiler target in build_fuchsia script (flutter/engine#10403)

@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 1, 2019
@fluttergithubbot
Copy link
Contributor

It looks like this pull request may not have tests. Please make sure to add tests before merging. While there are exceptions to this rule, if this patch modifies code it is probably not an exception.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

/cc @dnfield

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM stamp from a Japanese personal seal

@cbracken
Copy link
Member

cbracken commented Aug 1, 2019

Surprised we didn't have any tests for how we used to call gen_snapshot for iOS. They're no longer necessary in any case :)

@liamappelbe liamappelbe merged commit 5809219 into flutter:master Aug 1, 2019
@chinmaygarde
Copy link
Member

This roll was made without waiting for the bots to generate assets. I am reverting it as ToT is broken.

chinmaygarde added a commit that referenced this pull request Aug 1, 2019
chinmaygarde added a commit that referenced this pull request Aug 1, 2019
@tvolkert
Copy link
Contributor

tvolkert commented Aug 1, 2019

It looks like when this was merged, the engine binaries weren't done building on LUCI yet, and the pre-commit checks were correspondingly red. Please don't merge commits until the pre-commit checks are green.

@chinmaygarde
Copy link
Member

chinmaygarde commented Aug 1, 2019

I think the nuance in submitting breaking changes that was missed was that the red on the engine presubmit has to be ignored (because it checks against ToT framework) but the framework presubmits must always be green.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants