From 8bcf638eb8934dda0a5ac8150a357c0bad7b0fe7 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 18 Jul 2024 09:18:09 -0700 Subject: [PATCH] Add a "pub workspace" to the root of the engine repository (#53539) ... and use it in `engine_tool` to prove everything is working, i.e. on CI and elsewhere. Partial work towards https://github.com/flutter/flutter/issues/147883. Supersedes and closes https://github.com/flutter/engine/pull/51782 (which was a prototype). See also: - https://flutter.dev/go/pub-workspace, the design doc on the feature. - https://github.com/dart-lang/pub-dev/pull/7762, an example PR. I'll probably end up moving the inline docs in `pubspec.yaml` to a `docs/*.md` once that's a thing. --- ci/licenses_golden/excluded_files | 3 + lib/ui/channel_buffers.dart | 2 + lib/ui/compositing.dart | 2 + lib/ui/experiments/scene.dart | 25 +++ lib/ui/key.dart | 4 + lib/ui/painting.dart | 2 + lib/ui/platform_dispatcher.dart | 2 + lib/ui/semantics.dart | 8 + pubspec.yaml | 173 ++++++++++++++++++ .../test/header_filter_regex_test.dart | 1 + tools/engine_tool/pubspec.yaml | 65 +------ tools/pub_get_offline.py | 20 +- 12 files changed, 248 insertions(+), 59 deletions(-) create mode 100644 pubspec.yaml diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index e288363567876..a8e366a4750d8 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -12,6 +12,7 @@ ../../../flutter/.ci.yaml ../../../flutter/.clang-format ../../../flutter/.clang-tidy +../../../flutter/.dart_tool ../../../flutter/.git ../../../flutter/.gitattributes ../../../flutter/.github @@ -254,6 +255,8 @@ ../../../flutter/lib/web_ui/pubspec.yaml ../../../flutter/lib/web_ui/test ../../../flutter/prebuilts +../../../flutter/pubspec.lock +../../../flutter/pubspec.yaml ../../../flutter/runtime/dart_isolate_unittests.cc ../../../flutter/runtime/dart_lifecycle_unittests.cc ../../../flutter/runtime/dart_plugin_registrant_unittests.cc diff --git a/lib/ui/channel_buffers.dart b/lib/ui/channel_buffers.dart index 4c0d16a6c5ffd..2ff1758e82e4c 100644 --- a/lib/ui/channel_buffers.dart +++ b/lib/ui/channel_buffers.dart @@ -400,6 +400,8 @@ class ChannelBuffers { @Native(symbol: 'PlatformConfigurationNativeApi::SendChannelUpdate') external static void _sendChannelUpdate(String name, bool listening); + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs void sendChannelUpdate(String name, {required bool listening}) => _sendChannelUpdate(name, listening); /// Deprecated. Migrate to [setListener] instead. diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 3e887cf53039e..da23f53f06043 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -246,6 +246,8 @@ class ShaderMaskEngineLayer extends _EngineLayerWrapper { /// This does not apply when using the `dart:ui` API directly, without using the /// Flutter framework bindings, `flutter_test` framework, et al. abstract class SceneBuilder { + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs factory SceneBuilder() = _NativeSceneBuilder; /// Pushes a transform operation onto the operation stack. diff --git a/lib/ui/experiments/scene.dart b/lib/ui/experiments/scene.dart index 34fbe6358560e..cdab6557fc98b 100644 --- a/lib/ui/experiments/scene.dart +++ b/lib/ui/experiments/scene.dart @@ -50,24 +50,34 @@ base class SceneNode extends NativeFieldWrapperClass1 { return result; } + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs static SceneNodeValue fromTransform(Float64List matrix4) { final SceneNode sceneNode = SceneNode._create(); sceneNode._initFromTransform(matrix4); return SceneNodeValue.fromValue(sceneNode); } + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs void addChild(SceneNode sceneNode) { _addChild(sceneNode); } + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs void setTransform(Float64List matrix4) { _setTransform(matrix4); } + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs void setAnimationState(String animationName, bool playing, bool loop, double weight, double timeScale) { _setAnimationState(animationName, playing, loop, weight, timeScale); } + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs void seekAnimation(String animationName, double time) { _seekAnimation(animationName, time); } @@ -126,17 +136,24 @@ base class SceneNode extends NativeFieldWrapperClass1 { /// Returns a fresh instance of [SceneShader]. SceneShader sceneShader() => SceneShader._(this, debugName: _debugName); + } +// TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. +// ignore: public_member_api_docs class SceneNodeValue { SceneNodeValue._(this._future, this._value) { _future?.then((SceneNode result) => _value = result); } + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs static SceneNodeValue fromFuture(Future future) { return SceneNodeValue._(future, null); } + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs static SceneNodeValue fromValue(SceneNode value) { return SceneNodeValue._(null, value); } @@ -144,14 +161,20 @@ class SceneNodeValue { final Future? _future; SceneNode? _value; + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs bool get isComplete { return _value != null; } + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs Future? get future { return _future; } + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs SceneNode? get value { return _value; } @@ -186,6 +209,8 @@ base class SceneShader extends Shader { // ignore: unused_field final String? _debugName; + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs void setCameraTransform(Float64List matrix4) { _setCameraTransform(matrix4); } diff --git a/lib/ui/key.dart b/lib/ui/key.dart index 25eab4ac94754..1ea37200849a2 100644 --- a/lib/ui/key.dart +++ b/lib/ui/key.dart @@ -16,6 +16,8 @@ enum KeyEventType { /// The key is held, causing a repeated key input. repeat; + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs String get label { return switch (this) { down => 'Key Down', @@ -46,6 +48,8 @@ enum KeyEventDeviceType { /// The device is a device connected to an HDMI bus. hdmi; + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs String get label { return switch (this) { keyboard => 'Keyboard', diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index dc3050f4f5ca4..cba1b0a33b86f 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -2598,6 +2598,8 @@ base class _NativeEngineLayer extends NativeFieldWrapperClass1 implements Engine /// Paths can be drawn on canvases using [Canvas.drawPath], and can /// used to create clip regions using [Canvas.clipPath]. abstract class Path { + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs factory Path() = _NativePath; /// Creates a copy of another [Path]. diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 4b07bdb711aec..3b9bfaeecdc61 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -2195,6 +2195,8 @@ class ViewConstraints { /// [DisplayFeatureState.postureHalfOpened]. For [DisplayFeatureType.cutout], /// the state is not used and has the [DisplayFeatureState.unknown] value. class DisplayFeature { + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs const DisplayFeature({ required this.bounds, required this.type, diff --git a/lib/ui/semantics.dart b/lib/ui/semantics.dart index 6da1f4d2d0556..d408625df7ef7 100644 --- a/lib/ui/semantics.dart +++ b/lib/ui/semantics.dart @@ -284,8 +284,12 @@ class SemanticsAction { _kFocusIndex: focus, }; + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs static List get values => _kActionById.values.toList(growable: false); + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs static SemanticsAction? fromIndex(int index) => _kActionById[index]; @override @@ -638,8 +642,12 @@ class SemanticsFlag { _kIsExpandedIndex: isExpanded, }; + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs static List get values => _kFlagById.values.toList(growable: false); + // TODO(matanlurey): have original authors document; see https://github.com/flutter/flutter/issues/151917. + // ignore: public_member_api_docs static SemanticsFlag? fromIndex(int index) => _kFlagById[index]; @override diff --git a/pubspec.yaml b/pubspec.yaml new file mode 100644 index 0000000000000..23ae972fa6e4f --- /dev/null +++ b/pubspec.yaml @@ -0,0 +1,173 @@ +# This file represents a "workspace" that applies to the whole repository. +# +# See for details. +# +# The `flutter/engine` repository is a quasi-monorepo, with multiple Dart tools +# and packages that are all interdependent. Third party dependencies are managed +# by the `DEPS` file in the root of the repository, and are synced to either the +# `third_party` (i.e. `//flutter/third_party`) or `../third_party` (i.e. +# `//third_party`) directories by `gclient sync`. +# +# Every dependency declared here are dependencies used by _one or more_ of the +# packages in the repository (though there is no enforcement of this). This file +# then generates a `.dart_tool/package_config.json` file that is used by the +# rest of the repository to resolve dependencies. +# +# ============================================================================== +# WORKFLOWS +# ============================================================================== +# +# ------------------------------------------------------------------------------ +# (1) ADDING A NEW DEPENDENCY +# ------------------------------------------------------------------------------ +# Dependencies need to be added either via the DEPS file, or by checking if they +# are available in the vendored Dart SDK (see notes below on library locations). +# If dependencies are available, see (4) for how to add dependencies to a package in the workspace. +# +# ------------------------------------------------------------------------------ +# (2) CREATING A NEW PACKAGE +# ------------------------------------------------------------------------------ +# If creating a package, say in ./tools or ./tools/pkg, ensure the following +# header in its respective `pubspec.yaml`: +# ``` +# # We don't publish packages to pub.dev from the engine repository. +# publish_to: none +# +# # Required for workspace support. +# environment: +# sdk: ^3.5.0-294.0.dev +# +# # This package is managed as part of the engine workspace. +# resolution: workspace +# ``` +# +# See (4) for how to add dependencies to a package in the workspace. +# +# ------------------------------------------------------------------------------ +# (3) MIGRATING A NON-WORKSPACE PACKAGE TO USING THE WORKSPACE +# ------------------------------------------------------------------------------ +# Many packages in this repo are still using a pre-workspace style pubspec.yaml, +# either with manually declared `dependency_overrides` (much of ./tools) or by +# using pub (./web_sdk, ./lib/web_ui). To migrate a package to the workspace: +# +# A. Add the `resolution: workspace` field to the pubspec.yaml. +# B. Update the minimum SDK version to at least `^3.5.0-294.0.dev`. +# C. Add the package to the `workspace` field in this file. +# D. Ensure every dependency in the package is added to the `dependencies` field +# in this file, following instructions in (4). +# +# Once `dart pub get` is run on the workspace, the package will be resolved as +# part of the workspace, and the `dependency_overrides` in this file will be +# applied to the package. +# +# ------------------------------------------------------------------------------ +# (4) ADDING DEPENDENCIES TO A PACKAGE IN THIS WORKSPACE +# ------------------------------------------------------------------------------ +# When adding a dependency to a package in the workspace, add the dependency to +# the `dependencies` field in this file. If the dependency is located within +# the repository, use the `path` field to point to the package. +# +# If the dependency is a third party package, add it to the +# `dependency_overrides` field in this file. The `any` version constraint is +# used to indicate that the version of the package is not important, as it is +# managed by the `DEPS` file. + +name: _engine_workspace + +# Required for workspace support. +environment: + sdk: ^3.5.0-294.0.dev + +# Declare all packages that are part of the workspace. +workspace: + - tools/engine_tool + +# Declare all dependencies that are used by one or more packages. +# +# A few notes: +# 1. There is no distinction between "dependencies" and "dev_dependencies"; +# those notions are for *publishing* packages, not for managing a workspace. +# Specific packages in the workspace itself will declare whether they are +# dependencies or dev_dependencies, but here it is a union of both. +# +# 2. The `any` version constraint is used to indicate that the version of the +# package is not important, as it is managed by the `DEPS` file. In other +# words, "if the test pass, ship it". +# +# While not enforced by tooling, try to keep this list in alphabetical order. +# TODO(matanlurey): https://dart.dev/tools/linter-rules/sort_pub_dependencies. +dependencies: + args: any + async_helper: any + expect: any + file: any + logging: any + meta: any + path: any + platform: any + process_runner: any + smith: any + +# Instructs pub on how to resolve the dependencies that are part of "DEPS". +# +# For historic reasons, there are ~3 or so places packages might be located: +# +# - `./third_party/pkg/{name}`: for packages vended directly as part of "DEPS". +# Usually these are Flutter engine specific packages, i.e. they did not exist +# in the Dart vended SDK (the other options below). Typically these originate +# from pub (https://pub.dev) and are mirrored into a Google Git repository: +# . +# +# - `./third_party/dart/pkg/{name}`: for packages that lives *in* the Dart SDK, +# which is in turn vendored into the Flutter engine repository. You can see +# a full list of available packages here: +# . +# +# - `./third_party/dart/third_party/pkg/{name}`: for packages that are vendored +# into the Dart SDK from pub.dev. These are often first-party packages from +# the Dart team, but not part of the Dart SDK itself. You can see a full list +# of available packages here: +# . +dependency_overrides: + args: + path: ./third_party/dart/third_party/pkg/args + async: + path: ./third_party/dart/third_party/pkg/async + async_helper: + path: ./third_party/dart/pkg/async_helper + collection: + path: ./third_party/dart/third_party/pkg/collection + engine_build_configs: + path: ./tools/pkg/engine_build_configs + engine_repo_tools: + path: ./tools/pkg/engine_repo_tools + expect: + path: ./third_party/dart/pkg/expect + file: + path: ./third_party/dart/third_party/pkg/file/packages/file + litetest: + path: ./testing/litetest + logging: + path: ./third_party/dart/third_party/pkg/logging + meta: + path: ./third_party/dart/pkg/meta + path: + path: ./third_party/dart/third_party/pkg/path + platform: + path: ./third_party/pkg/platform + process: + path: ./third_party/pkg/process + process_fakes: + path: ./tools/pkg/process_fakes + process_runner: + path: ./third_party/pkg/process_runner + smith: + path: ./third_party/dart/pkg/smith + source_span: + path: ./third_party/dart/third_party/pkg/source_span + string_scanner: + path: ./third_party/dart/third_party/pkg/string_scanner + term_glyph: + path: ./third_party/dart/third_party/pkg/term_glyph + yaml: + path: ./third_party/dart/third_party/pkg/yaml diff --git a/tools/clang_tidy/test/header_filter_regex_test.dart b/tools/clang_tidy/test/header_filter_regex_test.dart index 21db03162d6ec..a84fc2284dc6b 100644 --- a/tools/clang_tidy/test/header_filter_regex_test.dart +++ b/tools/clang_tidy/test/header_filter_regex_test.dart @@ -36,6 +36,7 @@ void main() { test('contains every root directory in the regex', () { // These are a list of directories that should _not_ be included. const Set intentionallyOmitted = { + '.dart_tool', '.git', '.github', 'build_overrides', diff --git a/tools/engine_tool/pubspec.yaml b/tools/engine_tool/pubspec.yaml index 0e0e14e159c96..f752846118a4f 100644 --- a/tools/engine_tool/pubspec.yaml +++ b/tools/engine_tool/pubspec.yaml @@ -4,25 +4,18 @@ name: engine_tool publish_to: none -environment: - # Required for extension types. - sdk: ^3.3.0 -# Do not add any dependencies that require more than what is provided in -# //third_party/pkg, //third_party/dart/pkg, or -# //third_party/dart/third_party/pkg. In particular, package:test is not usable -# here. +# Required for workspace support. +environment: + sdk: ^3.5.0-294.0.dev -# If you do add packages here, make sure you can run `pub get --offline`, and -# check the .packages and .package_config to make sure all the paths are -# relative to this directory into //third_party/dart +# This package is managed as part of the engine workspace. +resolution: workspace dependencies: args: any - engine_build_configs: - path: ../pkg/engine_build_configs - engine_repo_tools: - path: ../pkg/engine_repo_tools + engine_build_configs: any + engine_repo_tools: any file: any logging: any meta: any @@ -31,47 +24,5 @@ dependencies: process_runner: any dev_dependencies: - async_helper: any - expect: any litetest: any - process_fakes: - path: ../pkg/process_fakes - smith: any - -dependency_overrides: - args: - path: ../../third_party/dart/third_party/pkg/args - async: - path: ../../third_party/dart/third_party/pkg/async - async_helper: - path: ../../third_party/dart/pkg/async_helper - collection: - path: ../../third_party/dart/third_party/pkg/collection - expect: - path: ../../third_party/dart/pkg/expect - file: - path: ../../third_party/dart/third_party/pkg/file/packages/file - litetest: - path: ../../testing/litetest - logging: - path: ../../third_party/dart/third_party/pkg/logging - meta: - path: ../../third_party/dart/pkg/meta - path: - path: ../../third_party/dart/third_party/pkg/path - platform: - path: ../../third_party/pkg/platform - process: - path: ../../third_party/pkg/process - process_runner: - path: ../../third_party/pkg/process_runner - smith: - path: ../../third_party/dart/pkg/smith - source_span: - path: ../../third_party/dart/third_party/pkg/source_span - string_scanner: - path: ../../third_party/dart/third_party/pkg/string_scanner - term_glyph: - path: ../../third_party/dart/third_party/pkg/term_glyph - yaml: - path: ../../third_party/dart/third_party/pkg/yaml + process_fakes: any diff --git a/tools/pub_get_offline.py b/tools/pub_get_offline.py index 9452e5449ed71..9da5137d72104 100644 --- a/tools/pub_get_offline.py +++ b/tools/pub_get_offline.py @@ -14,10 +14,15 @@ import subprocess import sys +THIS_DIR = os.path.abspath(os.path.dirname(__file__)) +sys.path += [os.path.join(THIS_DIR, '..', 'third_party', 'pyyaml', 'lib3')] +import yaml # pylint: disable=import-error, wrong-import-position + SRC_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) ENGINE_DIR = os.path.join(SRC_ROOT, 'flutter') ALL_PACKAGES = [ + os.path.join(ENGINE_DIR), os.path.join(ENGINE_DIR, 'ci'), os.path.join(ENGINE_DIR, 'flutter_frontend_server'), os.path.join(ENGINE_DIR, 'impeller', 'tessellator', 'dart'), @@ -64,7 +69,14 @@ def fetch_package(pub, package): return 0 -def check_package(package): +def package_uses_workspace_resolution(package): + pubspec = os.path.join(package, 'pubspec.yaml') + + with open(pubspec) as pubspec_file: + return yaml.safe_load(pubspec_file).get('resolution') == 'workspace' + + +def check_package_config(package): package_config = os.path.join(package, '.dart_tool', 'package_config.json') pub_count = 0 with open(package_config) as config_file: @@ -119,6 +131,9 @@ def find_unlisted_packages(): def main(): + # Intentionally use the Dart SDK prebuilt instead of the Flutter prebuilt + # (i.e. prebuilts/{platform}/dart-sdk/bin/dart) because the script has to run + # in a monorepo build *before* the newer Dart SDK has been built from source. dart_sdk_bin = os.path.join( SRC_ROOT, 'flutter', 'third_party', 'dart', 'tools', 'sdks', 'dart-sdk', 'bin' ) @@ -139,7 +154,8 @@ def main(): for package in ALL_PACKAGES: if fetch_package(pubcmd, package) != 0: return 1 - pub_count = pub_count + check_package(package) + if not package_uses_workspace_resolution(package): + pub_count = pub_count + check_package_config(package) if pub_count > 0: return 1