Add CI builder for windows-arm64. by p-b-o · Pull Request #38394 · flutter-team-archive/engine

p-b-o · 2022-12-19T09:47:31Z

This PR adds support for windows-arm64 artifacts, using new infrastructure that will be used from January 2023, and replacing https://flutter.googlesource.com/recipes/.

I checked locally that targets are correctly built and expected zips are there 👍.

All packages can be cross compiled from an x64 machine.
Unittests are disabled, as they require an arm64 machine.

Solves this issue: flutter/flutter#115983

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 and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

p-b-o · 2022-12-19T09:48:02Z

@godofredoc @loic-sharma This is the PR for adding windows-arm64 artifacts.

p-b-o · 2022-12-22T16:56:16Z

Integrated change to .ci.yaml and rebased.

auto-submit · 2022-12-22T17:03:42Z

auto label is removed for flutter/engine, pr: 38394, due to - Please get at least one approved review if you are already a member or two member reviews if you are not a member before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

auto-submit · 2022-12-22T17:03:43Z

auto label is removed for flutter/engine, pr: 38394, due to Validations Fail.

godofredoc · 2022-12-22T17:08:48Z

@loic-sharma can you please help us with a second review to be able to land this change?

p-b-o · 2022-12-22T17:23:20Z

@godofredoc Once this PR gets merged, will artifacts be available within hours, or later in January?

godofredoc · 2022-12-23T02:10:52Z

@godofredoc Once this PR gets merged, will artifacts be available within hours, or later in January?

This change is safe to land but we need a second review. The artifacts will be generated as soon as this PR is landed and uploaded to a different bucket.

After coming from vacation I can help with the engine.py change for you to get access to the archives from within the flutter sdk.

p-b-o · 2022-12-23T08:52:25Z

Is the engine.py modification still needed? From what I understood, this will be deprecated in January and replaced by current approach. But maybe I missed something. The final goal is to add support to flutter.bat.

p-b-o · 2022-12-28T08:51:06Z

Fix name of archive generated to reflect target architecture.

p-b-o · 2022-12-28T08:51:52Z

@godofredoc Is there something to add to get arm_host_engine workflow enabled in github CI for this PR? That would help to ensure everything is working as expected before merging this.

All packages can be cross compiled from an x64 machine. Unittests are disabled, as they require an arm64 machine.

p-b-o · 2023-01-04T17:39:25Z

Thanks for your help @loic-sharma

I ran a full build to check locally all zips are correct:

python3 ./flutter/tools/gn --runtime-mode debug --full-dart-sdk --no-lto --no-goma --windows-cpu arm64
python3 ./flutter/tools/gn --runtime-mode profile --no-goma --windows-cpu arm64
python3 ./flutter/tools/gn --runtime-mode release --no-goma --windows-cpu arm64
ninja -C out/host_debug_arm64 flutter/build/archives:artifacts flutter/build/archives:embedder flutter/tools/font-subset flutter/build/archives:dart_sdk_archive flutter/shell/platform/windows/client_wrapper:client_wrapper_archive flutter/build/archives:windows_flutter flutter/web_sdk || exit 1
ninja -C out/host_profile_arm64 windows gen_snapshot flutter/build/archives:windows_flutter
ninja -C out/host_release_arm64 windows gen_snapshot flutter/build/archives:windows_flutter

And I get this list of zip files:

./src/out/host_debug_arm64/zip_archives/dart-sdk-windows-arm64.zip
./src/out/host_debug_arm64/zip_archives/flutter-web-sdk-windows-arm64.zip
./src/out/host_debug_arm64/zip_archives/windows-arm64/artifacts.zip
./src/out/host_debug_arm64/zip_archives/windows-arm64/flutter-cpp-client-wrapper.zip
./src/out/host_debug_arm64/zip_archives/windows-arm64/font-subset.zip
./src/out/host_debug_arm64/zip_archives/windows-arm64/windows-arm64-embedder.zip
./src/out/host_debug_arm64/zip_archives/windows-arm64-debug/windows-arm64-flutter.zip
./src/out/host_profile_arm64/zip_archives/windows-arm64-profile/windows-arm64-flutter.zip
./src/out/host_release_arm64/zip_archives/windows-arm64-release/windows-arm64-flutter.zip

I should have check everything manually, sorry. I was expecting CI to do its job for me on this.

chinmaygarde · 2023-01-05T21:10:18Z

From Triage: Please wait for @zanderso s review on this one.

zanderso

lgtm

loic-sharma

LGTM! Nice work 🥳

p-b-o · 2023-01-06T08:17:30Z

Thanks for your precious help @loic-sharma!

This reverts commit 5a39a88.

loic-sharma · 2023-01-09T22:56:38Z

@pbo-linaro This change had to be reverted as it broke the Mac mac_host_engine build with:

Step('Copy gs://flutter_archives_v2/flutter_infra_release/flutter/5a39a8846621408403e5ccb3b078f2e4327ae1eb/flutter-web-sdk-darwin-x64.zip') failed 'ENOENT' with: [Errno 2] No such file or directory: '/Volumes/Work/s/w/ir/cache/builder/src/out/host_debug/zip_archives/flutter-web-sdk-darwin-x64.zip'

My guess is this change caused a mismatch between:

https://github.com/flutter/engine/blob/264aa032cf754f6a010ecb52f248a4df648bb0c6/web_sdk/BUILD.gn#L551-L554

And:

https://github.com/flutter/engine/blob/264aa032cf754f6a010ecb52f248a4df648bb0c6/ci/builders/mac_host_engine.json#L12

One hacky solution may be to use the target platform only on Windows hosts when building the web SDK? Or is there a better solution? /cc @godofredoc @eyebrowsoffire

…e#38394)

* 5a39a88 Add CI builder for windows-arm64. (flutter-team-archive/engine#38394) * 264aa03 Revert "Add CI builder for windows-arm64. (#38394)" (flutter-team-archive/engine#38729)

p-b-o · 2023-01-10T08:05:20Z

Probably adding an if for windows platform is the easiest thing to do.

Just a question: What does the whole CI check regarding artifacts?
I expected missing or wrongly named packages to be found before anything gets merged in trunk.

Are we sure it's the only package that was broken?

p-b-o · 2023-01-10T09:15:16Z

Created a new PR adding a special case for web-sdk artifacts: #38739

* Generate zip archives for Windows following target platform. * Add CI builder for windows-arm64. All packages can be cross compiled from an x64 machine. Unittests are disabled, as they require an arm64 machine. * Add windows-arm64 to CI.

…8208) * 5a39a88 Add CI builder for windows-arm64. (flutter-team-archive/engine#38394) * 264aa03 Revert "Add CI builder for windows-arm64. (flutter#38394)" (flutter-team-archive/engine#38729)

godofredoc reviewed Dec 22, 2022

View reviewed changes

Comment thread ci/builders/windows_arm_host_engine.json Resolved

godofredoc approved these changes Dec 22, 2022

View reviewed changes

godofredoc added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 22, 2022

auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Dec 22, 2022

godofredoc requested a review from loic-sharma December 22, 2022 17:07

loic-sharma reviewed Dec 27, 2022

View reviewed changes

Comment thread ci/builders/windows_arm_host_engine.json Resolved

loic-sharma requested review from cbracken and yaakovschectman January 3, 2023 18:42

loic-sharma reviewed Jan 3, 2023

View reviewed changes

Comment thread ci/builders/windows_arm_host_engine.json Resolved

Pierrick Bouvier added 3 commits January 4, 2023 18:34

Generate zip archives for Windows following target platform.

cb384c3

Add CI builder for windows-arm64.

06a24bc

All packages can be cross compiled from an x64 machine. Unittests are disabled, as they require an arm64 machine.

Add windows-arm64 to CI.

61f7bce

chinmaygarde requested a review from zanderso January 5, 2023 21:09

zanderso approved these changes Jan 5, 2023

View reviewed changes

loic-sharma approved these changes Jan 6, 2023

View reviewed changes

loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 9, 2023

auto-submit Bot merged commit 5a39a88 into flutter-team-archive:main Jan 9, 2023

loic-sharma added a commit that referenced this pull request Jan 9, 2023

Revert "Add CI builder for windows-arm64. (#38394)"

a081e1e

This reverts commit 5a39a88.

loic-sharma mentioned this pull request Jan 9, 2023

Revert "Add CI builder for windows-arm64." #38729

Merged

auto-submit Bot mentioned this pull request Jan 9, 2023

Review request for Revert PR flutter/engine#38729 flutter/flutter#118206

Closed

auto-submit Bot pushed a commit that referenced this pull request Jan 9, 2023

Revert "Add CI builder for windows-arm64. (#38394)" (#38729)

264aa03

This reverts commit 5a39a88.

engine-flutter-autoroll mentioned this pull request Jan 9, 2023

Roll Flutter Engine from ee0c4d26b0fa to 264aa032cf75 (2 revisions) flutter/flutter#118208

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 9, 2023

5a39a88 Add CI builder for windows-arm64. (flutter-team-archive/engin…

a38d091

…e#38394)

loic-sharma mentioned this pull request Jan 10, 2023

Add CI builder for windows-arm64. #38739

Merged

7 of 8 tasks

fluttermirroringbot pushed a commit to flutter/flutter that referenced this pull request Jan 16, 2023

Conversation

p-b-o commented Dec 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

p-b-o commented Dec 19, 2022

Uh oh!

Uh oh!

p-b-o commented Dec 22, 2022

Uh oh!

auto-submit Bot commented Dec 22, 2022

Uh oh!

auto-submit Bot commented Dec 22, 2022

Uh oh!

godofredoc commented Dec 22, 2022

Uh oh!

p-b-o commented Dec 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

godofredoc commented Dec 23, 2022

Uh oh!

p-b-o commented Dec 23, 2022

Uh oh!

Uh oh!

p-b-o commented Dec 28, 2022

Uh oh!

p-b-o commented Dec 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

p-b-o commented Jan 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chinmaygarde commented Jan 5, 2023

Uh oh!

zanderso left a comment

Choose a reason for hiding this comment

Uh oh!

loic-sharma left a comment

Choose a reason for hiding this comment

Uh oh!

p-b-o commented Jan 6, 2023

Uh oh!

loic-sharma commented Jan 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

p-b-o commented Jan 10, 2023

Uh oh!

p-b-o commented Jan 10, 2023

Uh oh!

p-b-o commented Dec 19, 2022 •

edited

Loading

p-b-o commented Dec 22, 2022 •

edited

Loading

p-b-o commented Dec 28, 2022 •

edited

Loading

p-b-o commented Jan 4, 2023 •

edited

Loading

loic-sharma commented Jan 9, 2023 •

edited

Loading