Build engine for windows_arm on beta and stable#176385
Build engine for windows_arm on beta and stable#176385Gustl22 wants to merge 1 commit intoflutter:masterfrom
Conversation
![gemini-code-assist[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enables the windows_arm_host_engine build for beta and stable branches by removing the enabled_branches restriction. While this aligns with the updated Dart SDK support, enabling a build on release branches without a trial period can be risky. If the build is unstable, it could block releases. I have provided a suggestion to mitigate this risk by using the bringup: true flag, which will allow the build's stability to be monitored without blocking the tree on failures.
|
This is also needed on the "master" channel. If checking out the stable version tags of git e.g. via subosito/flutter-action, it still uses the engine builds of the according stable versions, which do not exist yet and then falls back to x64: flutter/bin/internal/update_dart_sdk.ps1 Line 63 in 908012d There don't exist any arm64 windows engine builds (in contrast to linux-arm64) at the moment. Is there a browsable index of all builds? E.g. for this one? |
|
The patch and the verification of the dependent issues no longer being relevant looks good to me. |
|
@chinmaygarde I think the team-infra label doesn't apply to Pull Requests |
There are! dart compile for flutter: |
|
I created this PR before #176723 was merged, and didn't update. |
|
@jtmcdole Do you want to review this? For the additional channels? |
|
@jtmcdole Gentle ping. |
|
Coming back to this - and looking at the engine's ci.yaml file, there are no other "release" orchestrators for Windows. I believe this is a fairly limited resource; so I'd like:
Beta/Stable engines are build on dart owned infrastructure, which is far more constrained. |
|
Thank you for getting back and leading in the right direction. I also read through this document: flutter/docs/infra/Ci-Best-Practices.md Line 128 in 66b42dd For the third step, I suppose you meant: |
|
I followed up on #181075 - with some minor changes there, we can see the Linux orchestrator take over in our staging environment. This is similar to what Matan did: Once the orchestrator is green, we can remove the Windows one and delete "bringup" from the LInux orchestrator. Then we can promote this by removing the "enabled_branches" |
In order to save resources on windows runners the orchestrator for windows_arm_host_engine is moved to Linux. See: #176385 (comment) Towards: #62597 Ref PR: #168941 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] 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]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: gaaclarke <[email protected]>
… Linux (#183574) In order to save resources on windows runners the orchestrator for windows_arm_host_engine is moved to Linux. See: #176385 (comment) The orchestrator seems to be successful on Linux (last column): https://flutter-dashboard.appspot.com/#/build?showMac=false&showiOS=false&showAndroid=false&showBringup=true&repo=flutter&branch=master <img width="571" height="634" alt="image" src="proxy.php?url=https://github.com/user-attachments/assets/3795023d-02a7-4e6e-8b19-57994e1e5e0b" /> Follows on: #181075 Towards: #62597 Ref PR: #168941 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Gustl22
force-pushed
the
164431-arm-binaries-stable
branch
from
6e2c304 to
3aa1611
Compare
|
Thank you for reviewing the other PRs @jtmcdole. 
I don't think this has to do with cherry-picking to the RCs as it should shown on master in any case? |
3 participants
According to #113931 and #123288 and flutter/engine#40546 (comment) the beta and stable builds were removed because Dart didn't support arm64 on stable, too. This is now obsolete as the Dart SDK for arm64 is available on stable (since Dart 3.3.0): https://dart.dev/get-dart/archive
Basically reverts flutter/engine#40546
Contributes to #164431, #62597, #137161
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.