Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@matanlurey
Copy link
Contributor

@matanlurey matanlurey commented Oct 8, 2024

Closes flutter/flutter#148444 (code de-duplicated).
Closes flutter/flutter#150877 (--build-strategy=local).
Closes flutter/flutter#150884 (--build-strategy=remote).

Replaces duplicate code across ~3 commands (query, test, build) with a class called BuildPlan, which encapsulates (a) concurrency, (b) build configuration, (c) RBE, (d) LTO, and (e) build strategy. I also moved all of the validation of the build plan into build_plan_test, which gives us better coverage at a lower cost (less things to think about in that suite of tests).

I know the diff looks scary, but 1K of the 1.4K is tests.

/cc @gaaclarke @flar

Copy link
Contributor

@johnmccutchan johnmccutchan left a comment

Choose a reason for hiding this comment

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

This is a nice cleanup

import 'environment.dart';
import 'logger.dart';

const _flagConfig = 'config';
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we pulling flags out of flags.dart?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are no longer visible or relevant to the commands (they are implementation details).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is, the reason we had them in a common place no longer applies.

@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 8, 2024
@auto-submit auto-submit bot merged commit be2bc34 into flutter:main Oct 8, 2024
28 checks passed
@matanlurey matanlurey deleted the build-plan-refactor branch October 9, 2024 00:05
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 9, 2024
zanderso pushed a commit to flutter/flutter that referenced this pull request Oct 9, 2024
…156478)

flutter/engine@db0c0b7...4a97943

2024-10-09 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from
TlU-It6X_ZLrNqMjW... to xGr5ZkxX3CajAY1xu... (flutter/engine#55770)
2024-10-09 skia-flutter-autoroll@skia.org Roll Skia from a077e78e531f to
e0bb55353b27 (3 revisions) (flutter/engine#55768)
2024-10-09 chinmaygarde@google.com [Impeller] libImpeller: Allow
fetching OpenGL texture handle. (flutter/engine#55753)
2024-10-09 skia-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from
jCde9sMKJ3YAdG2DH... to _fkA2GjLQH4bc_n2p... (flutter/engine#55762)
2024-10-09 skia-flutter-autoroll@skia.org Roll Skia from 701b6e4b4bc4 to
a077e78e531f (3 revisions) (flutter/engine#55761)
2024-10-09 matanlurey@users.noreply.github.com
Release`onTrimMemoryListener` after `ImageReaderSurfaceProducer`
released. (flutter/engine#55760)
2024-10-09 bdero@google.com [Flutter GPU] Get the GLES backend/Windows
working. (flutter/engine#55694)
2024-10-08 1961493+harryterkelsen@users.noreply.github.com [canvaskit]
Fix incorrect clipping with Opacity scene layer (flutter/engine#55751)
2024-10-08 matanlurey@users.noreply.github.com Refactor multi-file build
parsing into a single `BuildPlan` class. (flutter/engine#55720)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from TlU-It6X_ZLr to xGr5ZkxX3Caj

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC jacksongardner@google.com,zra@google.com on the revert to
ensure that a human
is aware of the problem.

To file a bug in Flutter:
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
nick9822 pushed a commit to nick9822/flutter that referenced this pull request Dec 18, 2024
…lutter/engine#55720)

Closes flutter#148444 (code de-duplicated).
Closes flutter#150877 (`--build-strategy=local`).
Closes flutter#150884 (`--build-strategy=remote`).

Replaces duplicate code across ~3 commands (query, test, build) with a class called `BuildPlan`, which encapsulates (a) concurrency, (b) build configuration, (c) RBE, (d) LTO, and (e) build strategy. I also moved all of the validation of the build plan into `build_plan_test`, which gives us better coverage at a lower cost (less things to think about in that suite of tests).

I know the diff looks scary, but 1K of the 1.4K is tests.

/cc @gaaclarke @flar
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App

Projects

None yet

2 participants