Skip to content

Conversation

@matanlurey
Copy link
Contributor

Towards #161406.

The goal is to, assuming this is WAI, move the bringup: true build (the newly created one) to presubmit, and then in a follow-up PR, remove the tests: [...] (and test dependencies) from linux_host_engine.json, meaning that it would be a build/archive only builder, and this new builder would be a build/test one.

We talked about it being hypothetically better to not build the engine multiple times, and decided for now to depend on RBE and not try to create stages or download artifacts created from another builder, but if @zanderso has another suggestion here is the place to chime in :)

(Btw if we like this approach, I'll create a task list of every builder that needs to be migrated)

@github-actions github-actions bot added the engine flutter/engine related. See also e: labels. label Jan 13, 2025
Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

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

approach sgtm.

},
"gn": [
"--target-dir",
"ci/host_debug",
Copy link
Member

Choose a reason for hiding this comment

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

This has to match the ninja "config" field. That is, this should be ci/host_debug_unit_tests. Here and below as well.

Could you also please double check for me that run_tests.py doesn't have logic expecting debug, profile, release to be at the end of the config name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

re: run_tests.py, the only reference I could see that might be troublesome I changed from looking for a hardcoded not host_profile and not host_release to having the word debug in the build. It's not great, but it's similar tot other checks we already make in run_tests.py.

PTAL.

@matanlurey matanlurey requested a review from zanderso January 13, 2025 19:26
return len(matches) > 0

if ('engine' in types or 'font-subset' in types) and not should_skip(args.variant):
if ('engine' in types or 'font-subset' in types) and 'debug' in build_dir:
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a half-finished migration from using engine to run these tests to using font-subset to run them. Instead of using the filters on the config name, we should just use the font-subset type. Doesn't have to be in this PR though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed #161538 to complete this.

Copy link
Member

@jtmcdole jtmcdole left a comment

Choose a reason for hiding this comment

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

lgtm

@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 13, 2025
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM

@christopherfujino
Copy link
Contributor

For posterity, the high level strategy is to:

  1. add new release=false ci.yaml targets that both compile targets and run tests (but do NOT archive). these will be run in parallel with framework tests, AFTER we've compiled and uploaded engine targets. They will rely on RBE to speed up compilation of targets we've already compiled previously.
  2. remove the tests from the original ci.yaml targets that are marked release=true.

@auto-submit auto-submit bot added this pull request to the merge queue Jan 13, 2025
Merged via the queue into flutter:master with commit 1d38029 Jan 14, 2025
183 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jan 14, 2025
Roll Flutter from 72db8f6 to 40c2b86 (33 revisions)

flutter/flutter@72db8f6...40c2b86

2025-01-14 [email protected] update changelog for 3.27.2 release (flutter/flutter#161569)
2025-01-14 [email protected] Fix `showLicensePage` does not inherit ambient `Theme` (flutter/flutter#161599)
2025-01-14 [email protected] Added special case for fat width arcs (flutter/flutter#161255)
2025-01-14 [email protected] Replace `fetch `with `gclient sync`. (flutter/flutter#161565)
2025-01-14 [email protected] Roll Packages from 3c3bc68 to d1fd623 (4 revisions) (flutter/flutter#161597)
2025-01-14 [email protected] Remove unused method (flutter/flutter#161572)
2025-01-14 [email protected] Fix crash when closing a window with `Alt+F4` in multi-win Flutter on Windows (flutter/flutter#161375)
2025-01-14 [email protected] Update InputDecoration.border documentation (flutter/flutter#161415)
2025-01-14 [email protected] [Web] Allow specifying the strategy on when to use <img> element to display images (flutter/flutter#159917)
2025-01-14 [email protected] Roll Dart to  Version 3.7.0-323.0.dev (flutter/flutter#161567)
2025-01-14 [email protected] Use wildcards (flutter/flutter#161548)
2025-01-14 [email protected] Autocomplete Options Width (flutter/flutter#143249)
2025-01-13 [email protected] Move the analyzer_benchmark to Mac arm64 devicelab bots (flutter/flutter#161405)
2025-01-13 [email protected] Remove references to `cirrus`, mostly in doc comments. (flutter/flutter#161529)
2025-01-13 [email protected] Fix paths when running clang-tidy on git diffs (flutter/flutter#161496)
2025-01-13 [email protected] [web:a11y] treat empty tappables as buttons (flutter/flutter#161360)
2025-01-13 [email protected] Add route settings to CupertinoSheetRoute (flutter/flutter#161528)
2025-01-13 [email protected] Copy `linux_host_engine` as `linux_host_engine_test`, removing `archives: [...]`. (flutter/flutter#161532)
2025-01-13 [email protected] Remove last two references to Cirrus CI. (flutter/flutter#161530)
2025-01-13 [email protected] Mark `Mac_mokey microbenchmarks` as flakey (flutter/flutter#161550)
2025-01-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (#160241)" (flutter/flutter#161555)
2025-01-13 [email protected] Add validator execution times to `flutter doctor --verbose` (flutter/flutter#158124)
2025-01-13 [email protected] Explain more specifically how to use `flutter drive`/what it does (flutter/flutter#161450)
2025-01-13 [email protected] Fixed repeated strings for incompatible Gradle or AGP version in `create` command (flutter/flutter#161223)
2025-01-13 [email protected] Remove `WEB_SHARD_COUNT`, which no longer exists post-Cirrus. (flutter/flutter#161527)
2025-01-13 [email protected] [Impeller] Update guidance on prebuilt artifacts. (flutter/flutter#161251)
2025-01-13 [email protected] Migrate DisplayList unit tests to DL/Impeller geometry classes (flutter/flutter#161453)
2025-01-13 [email protected] Context menu button callback docs clarification (flutter/flutter#161451)
2025-01-13 [email protected] Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (flutter/flutter#160241)
2025-01-13 [email protected] Udpate documentation on the third_party directories (flutter/flutter#161407)
2025-01-13 [email protected] Propagate environment variables when `flutter drive` is invoked. (flutter/flutter#161452)
2025-01-13 [email protected] Convert base application name handling to kotlin source (start of FGP kt conversion) (flutter/flutter#155963)
2025-01-13 [email protected] [Impeller] remove API 30 restriction for SurfaceControl testing. (flutter/flutter#161438)

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] 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:
...
maheshj01 pushed a commit to maheshj01/flutter that referenced this pull request Jan 15, 2025
…ves: [...]`. (flutter#161532)

Towards flutter#161406.

The goal is to, assuming this is WAI, move the `bringup: true` build
(the newly created one) to presubmit, and then in a follow-up PR, remove
the `tests: [...]` (and test dependencies) from
`linux_host_engine.json`, meaning that it would be a build/archive only
builder, and this _new_ builder would be a build/test one.

We talked about it being hypothetically better to _not_ build the engine
multiple times, and decided for now to depend on RBE and not try to
create stages or download artifacts created from another builder, but if
@zanderso has another suggestion here is the place to chime in :)

(Btw if we like this approach, I'll create a task list of every builder
that needs to be migrated)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025
androidseb pushed a commit to androidseb/packages that referenced this pull request Jun 8, 2025
Roll Flutter from 72db8f6 to 40c2b86 (33 revisions)

flutter/flutter@72db8f6...40c2b86

2025-01-14 [email protected] update changelog for 3.27.2 release (flutter/flutter#161569)
2025-01-14 [email protected] Fix `showLicensePage` does not inherit ambient `Theme` (flutter/flutter#161599)
2025-01-14 [email protected] Added special case for fat width arcs (flutter/flutter#161255)
2025-01-14 [email protected] Replace `fetch `with `gclient sync`. (flutter/flutter#161565)
2025-01-14 [email protected] Roll Packages from 3c3bc68 to d1fd623 (4 revisions) (flutter/flutter#161597)
2025-01-14 [email protected] Remove unused method (flutter/flutter#161572)
2025-01-14 [email protected] Fix crash when closing a window with `Alt+F4` in multi-win Flutter on Windows (flutter/flutter#161375)
2025-01-14 [email protected] Update InputDecoration.border documentation (flutter/flutter#161415)
2025-01-14 [email protected] [Web] Allow specifying the strategy on when to use <img> element to display images (flutter/flutter#159917)
2025-01-14 [email protected] Roll Dart to  Version 3.7.0-323.0.dev (flutter/flutter#161567)
2025-01-14 [email protected] Use wildcards (flutter/flutter#161548)
2025-01-14 [email protected] Autocomplete Options Width (flutter/flutter#143249)
2025-01-13 [email protected] Move the analyzer_benchmark to Mac arm64 devicelab bots (flutter/flutter#161405)
2025-01-13 [email protected] Remove references to `cirrus`, mostly in doc comments. (flutter/flutter#161529)
2025-01-13 [email protected] Fix paths when running clang-tidy on git diffs (flutter/flutter#161496)
2025-01-13 [email protected] [web:a11y] treat empty tappables as buttons (flutter/flutter#161360)
2025-01-13 [email protected] Add route settings to CupertinoSheetRoute (flutter/flutter#161528)
2025-01-13 [email protected] Copy `linux_host_engine` as `linux_host_engine_test`, removing `archives: [...]`. (flutter/flutter#161532)
2025-01-13 [email protected] Remove last two references to Cirrus CI. (flutter/flutter#161530)
2025-01-13 [email protected] Mark `Mac_mokey microbenchmarks` as flakey (flutter/flutter#161550)
2025-01-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (#160241)" (flutter/flutter#161555)
2025-01-13 [email protected] Add validator execution times to `flutter doctor --verbose` (flutter/flutter#158124)
2025-01-13 [email protected] Explain more specifically how to use `flutter drive`/what it does (flutter/flutter#161450)
2025-01-13 [email protected] Fixed repeated strings for incompatible Gradle or AGP version in `create` command (flutter/flutter#161223)
2025-01-13 [email protected] Remove `WEB_SHARD_COUNT`, which no longer exists post-Cirrus. (flutter/flutter#161527)
2025-01-13 [email protected] [Impeller] Update guidance on prebuilt artifacts. (flutter/flutter#161251)
2025-01-13 [email protected] Migrate DisplayList unit tests to DL/Impeller geometry classes (flutter/flutter#161453)
2025-01-13 [email protected] Context menu button callback docs clarification (flutter/flutter#161451)
2025-01-13 [email protected] Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (flutter/flutter#160241)
2025-01-13 [email protected] Udpate documentation on the third_party directories (flutter/flutter#161407)
2025-01-13 [email protected] Propagate environment variables when `flutter drive` is invoked. (flutter/flutter#161452)
2025-01-13 [email protected] Convert base application name handling to kotlin source (start of FGP kt conversion) (flutter/flutter#155963)
2025-01-13 [email protected] [Impeller] remove API 30 restriction for SurfaceControl testing. (flutter/flutter#161438)

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] 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:
...
FMorschel pushed a commit to FMorschel/packages that referenced this pull request Jun 9, 2025
Roll Flutter from 72db8f6 to 40c2b86 (33 revisions)

flutter/flutter@72db8f6...40c2b86

2025-01-14 [email protected] update changelog for 3.27.2 release (flutter/flutter#161569)
2025-01-14 [email protected] Fix `showLicensePage` does not inherit ambient `Theme` (flutter/flutter#161599)
2025-01-14 [email protected] Added special case for fat width arcs (flutter/flutter#161255)
2025-01-14 [email protected] Replace `fetch `with `gclient sync`. (flutter/flutter#161565)
2025-01-14 [email protected] Roll Packages from 3c3bc68 to d1fd623 (4 revisions) (flutter/flutter#161597)
2025-01-14 [email protected] Remove unused method (flutter/flutter#161572)
2025-01-14 [email protected] Fix crash when closing a window with `Alt+F4` in multi-win Flutter on Windows (flutter/flutter#161375)
2025-01-14 [email protected] Update InputDecoration.border documentation (flutter/flutter#161415)
2025-01-14 [email protected] [Web] Allow specifying the strategy on when to use <img> element to display images (flutter/flutter#159917)
2025-01-14 [email protected] Roll Dart to  Version 3.7.0-323.0.dev (flutter/flutter#161567)
2025-01-14 [email protected] Use wildcards (flutter/flutter#161548)
2025-01-14 [email protected] Autocomplete Options Width (flutter/flutter#143249)
2025-01-13 [email protected] Move the analyzer_benchmark to Mac arm64 devicelab bots (flutter/flutter#161405)
2025-01-13 [email protected] Remove references to `cirrus`, mostly in doc comments. (flutter/flutter#161529)
2025-01-13 [email protected] Fix paths when running clang-tidy on git diffs (flutter/flutter#161496)
2025-01-13 [email protected] [web:a11y] treat empty tappables as buttons (flutter/flutter#161360)
2025-01-13 [email protected] Add route settings to CupertinoSheetRoute (flutter/flutter#161528)
2025-01-13 [email protected] Copy `linux_host_engine` as `linux_host_engine_test`, removing `archives: [...]`. (flutter/flutter#161532)
2025-01-13 [email protected] Remove last two references to Cirrus CI. (flutter/flutter#161530)
2025-01-13 [email protected] Mark `Mac_mokey microbenchmarks` as flakey (flutter/flutter#161550)
2025-01-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (#160241)" (flutter/flutter#161555)
2025-01-13 [email protected] Add validator execution times to `flutter doctor --verbose` (flutter/flutter#158124)
2025-01-13 [email protected] Explain more specifically how to use `flutter drive`/what it does (flutter/flutter#161450)
2025-01-13 [email protected] Fixed repeated strings for incompatible Gradle or AGP version in `create` command (flutter/flutter#161223)
2025-01-13 [email protected] Remove `WEB_SHARD_COUNT`, which no longer exists post-Cirrus. (flutter/flutter#161527)
2025-01-13 [email protected] [Impeller] Update guidance on prebuilt artifacts. (flutter/flutter#161251)
2025-01-13 [email protected] Migrate DisplayList unit tests to DL/Impeller geometry classes (flutter/flutter#161453)
2025-01-13 [email protected] Context menu button callback docs clarification (flutter/flutter#161451)
2025-01-13 [email protected] Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (flutter/flutter#160241)
2025-01-13 [email protected] Udpate documentation on the third_party directories (flutter/flutter#161407)
2025-01-13 [email protected] Propagate environment variables when `flutter drive` is invoked. (flutter/flutter#161452)
2025-01-13 [email protected] Convert base application name handling to kotlin source (start of FGP kt conversion) (flutter/flutter#155963)
2025-01-13 [email protected] [Impeller] remove API 30 restriction for SurfaceControl testing. (flutter/flutter#161438)

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] 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:
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine flutter/engine related. See also e: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants