Run build tests on both x64 and arm64. #141206

godofredoc · 2024-01-09T21:52:38Z

Most of the builds for mac_x64 can actually run on any available architecture. This is required to use capacity more efficiently.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

godofredoc · 2024-01-09T23:00:30Z

These builds should be using the regular mac top level configuration but keeping mac_x64 for now for easy reverting.

keyonghan · 2024-01-09T23:02:59Z

.ci.yaml

This makes mac_x64 the same as mac. Switching these mac_x64 targets to mac?

Never mind. I missed the lastest comment.

Considering we have only 5 mac_x64 targets, do we want to led to validate they pass to avoid potential revert if this causes tree red? We are missing them in pre-submit.

Or alternatively, we can update those mac_x64 targets to mac with brigup: true, which is more safer.

I'll send a second PR for that once we ensure all the tests pass correctly and we don't need to revert.

I guess my point here is: do we want to land this PR and revert if it causes tree red, or do we want to validate them in staging pool without potentially breaking the tree. I would prefer the former option.

We have been doing similarly when switching targets from linux_android to linux_pixel_7pro:
https://github.com/flutter/flutter/pull/140389/files
https://github.com/flutter/flutter/pull/140438/files

Updated them to use bringup.

jmagman

The mac_x64 is used to deliberately run the tools test on x86_64 machines to validate Flutter runs on those host Macs. tool_host_cross_arch_tests for example validates that the tool can build a valid and well-formed iOS and macOS app on either host machine. verify_binaries_codesigned validates the engine artifacts are codesigned once they make it to the tool. hot_mode_dev_cycle_macos_target__benchmark validates a macOS app on either platform can be hot-reloaded.

Until the tool deprecates support for Flutter developers using x64 Macs we should keep these tests. cc @christopherfujino

christopherfujino · 2024-01-10T02:52:16Z

Agree with Jenn, I think we should audit the existing x64 targets one by one, and at least the following legitimately should use x64, IMO:

Mac_x64 tool_host_cross_arch_tests we are explicitly testing the host architecture (although I notice you hard-coded the cpu here)
Mac_x64 verify_binaries_codesigned which artifacts we download is dependent on host architecture, so we want to test this on x64
Mac_x64 hot_mode_dev_cycle_macos_target__benchmark I'm guessing the performance of a benchmark would be very different between M1 and intel mac
Mac_x64 hot_mode_dev_cycle_ios_simulator ditto

I'm not sure about:

Mac_x64 macos_chrome_dev_mode
Mac_x64 ios_app_with_extensions_test

jmagman · 2024-01-10T04:19:49Z

I think we should audit the existing x64 targets one by one

Check out go/flutter-mac-test-architectures

Most of the builds for mac_x64 can actually run on any available architecture. This is required to use capacity more efficiently.

godofredoc · 2024-01-11T20:56:37Z

@jmagman @christopherfujino and @keyonghan please take another look at this PR I updated to run build_tests on arm and x64 as recommended by go/flutter-mac-test-architectures. I'll follow with a few more PRs to update the other tests in the document list.

keyonghan

LGTM

christopherfujino

LGTM

jmagman

If we have capacity issues this may be a good one to re-evaluate to run on either.

flutter/flutter@def6af0...f77f824 2024-01-18 [email protected] Roll Flutter Engine from 49fa2cb9024f to b75d6d80d813 (1 revision) (flutter/flutter#141771) 2024-01-18 [email protected] Roll Flutter Engine from 49c6ca211aa4 to 49fa2cb9024f (1 revision) (flutter/flutter#141762) 2024-01-18 [email protected] Roll Flutter Engine from 873449c27d5a to 49c6ca211aa4 (1 revision) (flutter/flutter#141760) 2024-01-18 [email protected] Roll Flutter Engine from bfdc0c5b2826 to 873449c27d5a (1 revision) (flutter/flutter#141759) 2024-01-18 [email protected] Catch UnsupportedError thrown when user provides an asset directory path containing invalid characters (flutter/flutter#141214) 2024-01-18 [email protected] Roll Flutter Engine from 48f89ac064ac to bfdc0c5b2826 (1 revision) (flutter/flutter#141752) 2024-01-18 [email protected] Roll Flutter Engine from 924c17245a78 to 48f89ac064ac (2 revisions) (flutter/flutter#141751) 2024-01-18 [email protected] Roll Flutter Engine from 98c16b430e6b to 924c17245a78 (1 revision) (flutter/flutter#141749) 2024-01-18 [email protected] Roll Flutter Engine from 73a2de5da53f to 98c16b430e6b (16 revisions) (flutter/flutter#141744) 2024-01-18 [email protected] Move mac pixel 7 pro test to presubmit: false (flutter/flutter#141747) 2024-01-18 [email protected] [web] prepare layers_test.dart for flutter/engine#49786 (flutter/flutter#141731) 2024-01-17 [email protected] Remove non-needed bot and increase time out for leak_tracking. (flutter/flutter#141712) 2024-01-17 [email protected] Add `headerHeight` for `SearchAnchor` (flutter/flutter#141223) 2024-01-17 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.23.0 to 3.23.1 (flutter/flutter#141715) 2024-01-17 [email protected] Make test file systems/platforms used in asset_bundle_test.dart less dependent on the host platform (flutter/flutter#141657) 2024-01-17 [email protected] Native assets: roll deps (flutter/flutter#141684) 2024-01-17 [email protected] Run build tests on both x64 and arm64. (flutter/flutter#141206) 2024-01-17 [email protected] Update tests to Xcode 15 (flutter/flutter#141706) 2024-01-17 [email protected] [web] prepare for flutter/engine#49786 (flutter/flutter#141700) 2024-01-17 [email protected] Marks Windows framework_tests_misc_leak_tracking to be unflaky (flutter/flutter#141676) 2024-01-17 [email protected] Label "flutter_localizations" PRs with "framework" (flutter/flutter#141654) 2024-01-17 [email protected] Fix Tooltip show delay when mouse moves to one Tooltip from another (flutter/flutter#141656) 2024-01-17 [email protected] Roll Packages from 7dd0fcb to 1a2b780 (6 revisions) (flutter/flutter#141683) 2024-01-17 [email protected] Fix the --empty flag to not try working with non-app templates (flutter/flutter#141632) 2024-01-17 [email protected] Revert "Roll Flutter Engine from 73a2de5da53f to c7e328518bc0 (5 revisions)" (flutter/flutter#141691) 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 Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: 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

It shouldn't be very common for x64 example projects to not build if they built on arm64. We have other tool tests that would probably catch those issues in presubmit. Instead run them on postsubmit only to free up limited x64 staging resources. Follow up from #141206

godofredoc requested review from jmagman and keyonghan January 9, 2024 22:59

keyonghan reviewed Jan 9, 2024

View reviewed changes

jmagman requested changes Jan 10, 2024

View reviewed changes

godofredoc added 6 commits January 11, 2024 12:29

Run mac_x64 builds on any architecture available.

4dff4fe

Most of the builds for mac_x64 can actually run on any available architecture. This is required to use capacity more efficiently.

Set mac_x64 builds to bringup.

17d9660

Run build_tests on both arm and x64 bots.

8cf8deb

Re-add x86 cpu parameter to x64 top level config.

ecc52e1

Limit changes to build tests.

4bf91d0

Restore other changes.

4b60bf6

godofredoc changed the title ~~Run mac_x64 builds on any architecture available.~~ Run build tests on both x64 and arm64. Jan 11, 2024

godofredoc added 2 commits January 11, 2024 16:19

Merge branch 'master' into remove_arch_ho

0337dfa

Merge branch 'master' into remove_arch_ho

d59fc21

keyonghan approved these changes Jan 12, 2024

View reviewed changes

christopherfujino approved these changes Jan 12, 2024

View reviewed changes

jmagman approved these changes Jan 17, 2024

View reviewed changes

godofredoc added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 17, 2024

auto-submit bot merged commit 66f5e88 into flutter:master Jan 17, 2024

godofredoc deleted the remove_arch_ho branch January 17, 2024 21:24

engine-flutter-autoroll mentioned this pull request Jan 18, 2024

Roll Flutter from def6af0b8e6e to f77f82435a97 (25 revisions) flutter/packages#5924

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 18, 2024

66f5e88 Run build tests on both x64 and arm64. (flutter/flutter#141206)

e83f76c

jmagman mentioned this pull request Jan 26, 2024

Run Mac x64 build tests in postsubmit only #142334

Merged

8 tasks

Run build tests on both x64 and arm64. #141206

Run build tests on both x64 and arm64. #141206

Uh oh!

Conversation

godofredoc commented Jan 9, 2024

Pre-launch Checklist

Uh oh!

godofredoc commented Jan 9, 2024

Uh oh!

keyonghan Jan 9, 2024

Choose a reason for hiding this comment

Uh oh!

keyonghan Jan 9, 2024

Choose a reason for hiding this comment

Uh oh!

godofredoc Jan 9, 2024

Choose a reason for hiding this comment

Uh oh!

keyonghan Jan 9, 2024

Choose a reason for hiding this comment

Uh oh!

godofredoc Jan 10, 2024

Choose a reason for hiding this comment

Uh oh!

jmagman left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

christopherfujino commented Jan 10, 2024

Uh oh!

jmagman commented Jan 10, 2024

Uh oh!

godofredoc commented Jan 11, 2024

Uh oh!

keyonghan left a comment

Choose a reason for hiding this comment

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jmagman left a comment •

edited

Loading