-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Description
Remaining work is now tracked by this label:
EDITED by @matanlurey so it's more visible to others. Original post is below:
Background on buildroot by @chinmaygarde
The Flutter Buildroot and the Engine used to live in a common repository. In 2016, in order to facilitate building the Flutter Engine in the Fuchsia as well Flutter buildroots, the buildroot used by Flutter was separated into its own repository with the engine DEPS file referencing the version of the buildroot to use. The Flutter Engine GN rules had to be written such that they worked when the engine was present in either buildroot.
Flutter no longer builds in the Fuchsia buildroot. Instead, Fuchsia artifacts are built from the Flutter buildroot using the Fuchsia SDK (which was not present at the time the split occurred) and vended to Flutter application authors. So, the original use case of the split is no longer present.
Even though there is no use-case served by the split, there are numerous downsides to this approach:
- Patches to GN rules (especially in the secondary buildroot) need to be split across multiple commits to the buildroot and the engine.
- Any buildroot patch necessitates a DEPS update in the engine.
- If the DEPS update fails for any reason, the same patch must be backed out in the buildroot ASAP. If someone else patches the buildroot in the meantime, they must hold till the failed buildroot update has been reverted lest their DEPS roll fail too.
- The buildroot has no presubmits. Authors who want to test buildroot updates must first update the buildroot in their fork and then point the patch that rolls DEPS in the engine to point to their fork of the buildroot so that presubmits can run. Before landing, they must back out this redirection after landing the patch in the buildroot.
- The separate PR queue for buildroot patches means the patches there are often missed by reviewers.
Personally, I dread instances where I have to update the buildroot. Not because it is hard, but because it is incredibly tedious. And quite unnecessarily so. The buildroot itself is a significant source of technical debt and having build rules split across two repos also makes incrementally tackling this debt hard. Merging the repos would get rid of one source of unnecessary complexity in the build process.
The buildroot has no presubmit or its own issue tracker, so I don't expect any infra related issues with this switch.
The original split occurred by preserving no history. The team has grown significantly since then and the Git history may be valuable. So maybe we should consider a subtree import instead of just copying over the files. But, I don't have a strong opinion either way.
The first PR is WIP, flutter/engine#46733, which moves third_party/glfw.
Roughly, the process is:
-
Update
DEPSforsrc/third_partydependencies:- 'src/third_party/glfw': + 'src/flutter/third_party/glfw':
-
Update source references, such as any
BUILD.gnfiles (but other files are also possible):deps = [ "//flutter/shell/platform/embedder:embedder", - "//third_party/glfw", + "//flutter/third_party/glfw", "//third_party/vulkan-deps/vulkan-headers/src:vulkan_headers", ]
-
Update the license checker (WIP, @cbracken is hacking on this now).
Here is a list of all dependencies that need to be migrated.
Some of these (i.e. skia) will be more difficult, and might require a few incremental PRs, or closer work with folks on the engine team. Others, like the Dart packages, or leaf-level C++ packages that aren't highly used (like glfw) are simpler, and can be done more simply (see flutter/engine#46733).
To volunteer, edit this table and replace None with <link-to-GH-issue>, and assign yourself to the GH issue.
You can follow this template:
As part of eliminating the Flutter buildroot (#67373), we are moving all third-party dependencies from
//third_partyto//flutter/third_party.Steps:
- Buildroot: Migrate
BUILD.gntobuild/secondary/flutter/third_party/BUILD.gnMove //third_party/glfw to //flutter/third_party/glfw buildroot#777- Engine: Update
DEPSand references to use new location/targets Move //third_party/glfw to //flutter/third_party/glfw engine#46733This is an abridged form of a longer, stepwise migration that we may use for more complex future migrations:
- Buildroot: Copy
BUILD.gntobuild/secondary/flutter/third_party/BUILD.gn- Engine: Update
DEPSand references to use new location- Buildroot: Eliminate old
BUILD.gnfrombuild/secondary/third_party/BUILD.gn- Engine: Eliminate
DEPSreferences to old locationPart of #67373
C++ or Utilities
| Done | Source | Destination | Issue |
|---|---|---|---|
| ✅ | src/third_party/abseil-cpp |
src/flutter/third_party/abseil-cpp |
#144201 |
| ✅ | src/third_party/angle |
src/flutter/third_party/angle |
#144786 |
| ✅ | src/third_party/benchmark |
src/flutter/third_party/benchmark |
flutter/engine#47652 |
| ✅ | src/third_party/boringssl |
src/flutter/third_party/boringssl |
flutter/engine#50601 |
| ✅ | src/third_party/expat |
src/flutter/third_party/expat |
flutter/engine#48193 |
| ✅ | src/third_party/flatbuffers |
src/flutter/third_party/flatbuffers |
flutter/engine#47387 |
| ✅ | src/third_party/freetype2 |
src/flutter/third_party/freetype2 |
flutter/engine#50830 |
| ✅ | src/third_party/glfw |
src/flutter/third_party/glfw |
#136284 |
| ✅ | src/third_party/googletest |
src/flutter/third_party/googletest |
flutter/engine#50830 |
| ✅ | src/third_party/gtest-parallel |
src/flutter/third_party/gtest-parallel |
flutter/engine#47393 |
| ✅ | src/third_party/harfbuzz |
src/flutter/third_party/harfbuzz |
flutter/engine#50830 |
| ✅ | src/third_party/icu |
src/flutter/third_party/icu |
flutter/engine#50924 |
| ✅ | src/third_party/imgui |
src/flutter/third_party/imgui |
flutter/engine#47031 |
| ✅ | src/third_party/inja |
src/flutter/third_party/inja |
flutter/engine#47408 |
| ✅ | src/third_party/json |
src/flutter/third_party/json |
None |
| ✅ | src/third_party/khronos |
src/flutter/third_party/khronos |
flutter/engine#47398 |
| ⬜ | src/third_party/libcxx |
src/flutter/third_party/libcxx |
None |
| ⬜ | src/third_party/libcxxabi |
src/flutter/third_party/libcxxabi |
None |
| ✅ | src/third_party/libjpeg-turbo |
src/flutter/third_party/libjpeg-turbo |
flutter/engine#48193 |
| ✅ | src/third_party/libpng |
src/flutter/third_party/libpng |
flutter/engine#50571 |
| ✅ | src/third_party/libtess2 |
src/flutter/third_party/libtess2 |
flutter/engine#47408 |
| ✅ | src/third_party/libwebp |
src/flutter/third_party/libwebp |
flutter/engine#48193 |
| ✅ | src/third_party/libxml |
src/flutter/third_party/libxml |
flutter/engine#48906 |
| ✅ | src/third_party/ocmock |
src/flutter/third_party/ocmock |
flutter/engine#48193 |
| ✅ | src/third_party/perfetto |
src/flutter/third_party/perfetto |
#144204 |
| ✅ | src/third_party/protobuf |
src/flutter/third_party/protobuf |
#144204 |
| ✅ | src/third_party/pyyaml |
src/flutter/third_party/pyyaml |
flutter/engine#48193 |
| ✅ | src/third_party/rapidjson |
src/flutter/third_party/rapidjson |
flutter/engine#47354 |
| ✅ | src/third_party/shaderc |
src/flutter/third_party/shaderc |
flutter/engine#47383 |
| ✅ | src/third_party/skia |
src/flutter/third_party/skia |
flutter/engine#47913 |
| ✅ | src/third_party/stb |
src/flutter/third_party/stb |
None |
| ✅ | src/third_party/sqlite |
src/flutter/third_party/sqlite |
flutter/engine#47408 |
| ✅ | src/third_party/tinygltf |
src/flutter/third_party/tinygltf |
flutter/engine#48852 |
| ✅ | src/third_party/swiftshader |
src/flutter/third_party/swiftshader |
None |
| ✅ | src/third_party/vulkan-deps |
src/flutter/third_party/vulkan-deps |
#144205 |
| ✅ | src/third_party/vulkan_memory_allocator |
src/flutter/third_party/vulkan_memory_allocator |
#144812 |
| ✅ | src/third_party/wuffs |
src/flutter/third_party/wuffs |
flutter/engine#48193 |
| ✅ | src/third_party/yapf |
src/flutter/third_party/yapf |
(flutter/engine#48847 |
Android and Java
TBD.
Dart Packages
| Done | Source | Destination | Issue |
|---|---|---|---|
| ✅ | src/third_party/pkg/archive |
src/flutter/third_party/pkg/archive |
flutter/engine#47654 |
| ✅ | src/third_party/pkg/equatable |
src/flutter/third_party/pkg/equatable |
flutter/engine#47654 |
| ✅ | src/third_party/pkg/flutter_packages |
src/flutter/third_party/pkg/flutter_packages |
flutter/engine#47654 |
| ✅ | src/third_party/pkg/gcloud |
src/flutter/third_party/pkg/gcloud |
flutter/engine#47654 |
| ✅ | src/third_party/pkg/googleapis |
src/flutter/third_party/pkg/googleapis |
flutter/engine#47654 |
| ✅ | src/third_party/pkg/platform |
src/flutter/third_party/pkg/platform |
flutter/engine#47654 |
| ✅ | src/third_party/pkg/process |
src/flutter/third_party/pkg/process |
flutter/engine#47654 |
| ✅ | src/third_party/pkg/process_runner |
src/flutter/third_party/pkg/process_runner |
flutter/engine#47654 |
| ✅ | src/third_party/pkg/vector_math |
src/flutter/third_party/pkg/vector_math |
flutter/engine#47654 |
Notably,
src/third_party/dartis missing above. That is intentional, we plan to work on that in a second phase.
General build infra
| Done | Source | Destination | Issue |
|---|---|---|---|
| ✅ | src/build_overrides |
src/flutter/build_overrides |
#144790 |