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

Create a package-able incremental compiler #12681

Merged
jonahwilliams merged 13 commits into from
Oct 3, 2019
Merged

Create a package-able incremental compiler #12681

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
517ab4b
allow providing composite transformer to incremental compiler
Sep 29, 2019
11510fe
fix transformer type
Sep 29, 2019
8300b76
make build_integration, vm packagable
Sep 30, 2019
7855df1
build to directory with sky packages
Sep 30, 2019
5ab9de1
remove special composite transform type
Sep 30, 2019
b91fcb9
formatting
Sep 30, 2019
64aa2b1
package frontend server
Sep 30, 2019
b3129a7
include kernel and front_end
Sep 30, 2019
ee3730f
Update BUILD.gn
Sep 30, 2019
bb64ee1
replace build rules with python
Oct 2, 2019
5b4a902
delete dart file
Oct 2, 2019
2d0b97b
cleanup python
Oct 2, 2019
543105e
+x
Oct 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions frontend_server/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ if (is_fuchsia_host || is_fuchsia) {
main_dart = "bin/starter.dart"
deps = [
"$flutter_root/lib/snapshot:kernel_platform_files",
":flutter_package_vm",
":flutter_package_build_integration",
":flutter_package_frontend_server",
]
dot_packages = rebase_path(".packages")
flutter_patched_sdk = rebase_path("$root_out_dir/flutter_patched_sdk")
Expand Down Expand Up @@ -71,4 +74,115 @@ if (is_fuchsia_host || is_fuchsia) {

inputs = frontend_server_files
}

# Creates a packageable vm.
prebuilt_dart_action("flutter_package_vm") {

@zanderso zanderso Sep 30, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As discussed offline, add comments about these artifacts pointing at the github issue and add a reviewer from the engine team.

@chinmaygarde chinmaygarde Sep 30, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I understand the desire to use Dart in publish.dart but this template invocation now uses the Python to list the files, Dart to publish the same, and compiles the host VM (~2000 TUs) to boot. We do have a prebuilt VM you can use for complicated Dart scripts but I'd rather you just replace these steps with a single script action. That will be faster in cases where this copy step is the only step necessary (since you wont have to build the VM) and can be parallelized by the build system when other targets that need the VM are also being built.

@jonahwilliams jonahwilliams Sep 30, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Understood, I'll update the script to use python.

@zanderso zanderso Sep 30, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FYI prebuilt_dart_action() should be using the prebuilt Dart VM. If it isn't then that is a bug.

@chinmaygarde chinmaygarde Sep 30, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You're right. I was thinking of dart_action. I suppose I am now significantly less concerned about this but I still think it'd be cleaner and less verbose to just have a single action target that achieves this instead of a target definition that mixes both python and dart with a custom template.

@jonahwilliams jonahwilliams Oct 1, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What would be the resolution here - to write this as a single target in python? dart?

@zanderso zanderso Oct 1, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@chinmaygarde Would a single python script wrapped in a GN template scoped to this file address your concerns?

@chinmaygarde chinmaygarde Oct 1, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes. A single python script would be great. Thanks.

@jonahwilliams jonahwilliams Oct 2, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated to use a single/action python script.

@jonahwilliams jonahwilliams Oct 2, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated to use a single python script

packages = "//third_party/dart/.packages"

script = "$flutter_root/frontend_server/publish.dart"

inputs = exec_script("//third_party/dart/tools/list_dart_files.py",
[
"absolute",
rebase_path("../../third_party/dart/pkg/vm"),
],
"list lines")

outputs = ["$root_build_dir/dist/packages/vm/pubspec.yaml"]

args = [
rebase_path("//third_party/dart/pkg/vm"),
rebase_path("$root_gen_dir/dart-pkg/vm"),
"vm",
]
}

# Creates a packageable build_integration.
prebuilt_dart_action("flutter_package_build_integration") {
packages = "//third_party/dart/.packages"

script = "$flutter_root/frontend_server/publish.dart"

inputs = exec_script("//third_party/dart/tools/list_dart_files.py",
[
"absolute",
rebase_path("../../third_party/dart/pkg/build_integration"),
],
"list lines")

outputs = ["$root_build_dir/dist/packages/build_integration/pubspec.yaml"]

args = [
rebase_path("//third_party/dart/pkg/build_integration"),
rebase_path("$root_gen_dir/dart-pkg/build_integration"),
"build_integration",
]

}

# Creates a packageable frontend_server.
prebuilt_dart_action("flutter_package_frontend_server") {
packages = "//third_party/dart/.packages"

script = "$flutter_root/frontend_server/publish.dart"

inputs = exec_script("//third_party/dart/tools/list_dart_files.py",
[
"absolute",
rebase_path("lib"),
],
"list lines")

outputs = ["$root_build_dir/dist/packages/frontend_server/pubspec.yaml"]

args = [
rebase_path("lib"),
rebase_path("$root_gen_dir/dart-pkg/frontend_server"),
"frontend_server",
]
}

# Creates a packageable front_end.
prebuilt_dart_action("flutter_package_front_end") {
packages = "//third_party/dart/.packages"

script = "$flutter_root/frontend_server/publish.dart"

inputs = exec_script("//third_party/dart/tools/list_dart_files.py",
[
"absolute",
rebase_path("lib"),
],
"list lines")

outputs = ["$root_build_dir/dist/packages/front_end/pubspec.yaml"]

args = [
rebase_path("lib"),
rebase_path("$root_gen_dir/dart-pkg/front_end"),
"front_end",
]
}

# Creates a packageable kernel.
prebuilt_dart_action("flutter_package_kernel") {
packages = "//third_party/dart/.packages"

script = "$flutter_root/frontend_server/publish.dart"

inputs = exec_script("//third_party/dart/tools/list_dart_files.py",
[
"absolute",
rebase_path("lib"),
],
"list lines")

outputs = ["$root_build_dir/dist/packages/kernel/pubspec.yaml"]

args = [
rebase_path("lib"),
rebase_path("$root_gen_dir/dart-pkg/kernel"),
"kernel",
]
}
}
42 changes: 27 additions & 15 deletions frontend_server/lib/server.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,30 @@ import 'package:args/args.dart';
import 'package:path/path.dart' as path;

import 'package:vm/incremental_compiler.dart';
import 'package:vm/frontend_server.dart' as frontend show FrontendCompiler,
CompilerInterface, listenAndCompile, argParser, usage;
import 'package:vm/frontend_server.dart' as frontend
show
FrontendCompiler,
CompilerInterface,
listenAndCompile,
argParser,
usage,
ProgramTransformer;

/// Wrapper around [FrontendCompiler] that adds [widgetCreatorTracker] kernel
/// transformation to the compilation.
class _FlutterFrontendCompiler implements frontend.CompilerInterface{
class _FlutterFrontendCompiler implements frontend.CompilerInterface {
final frontend.CompilerInterface _compiler;

_FlutterFrontendCompiler(StringSink output,
{bool unsafePackageSerialization}) :
_compiler = frontend.FrontendCompiler(output,
unsafePackageSerialization: unsafePackageSerialization);
{bool unsafePackageSerialization,
frontend.ProgramTransformer transformer})
: _compiler = frontend.FrontendCompiler(output,
transformer: transformer,
unsafePackageSerialization: unsafePackageSerialization);

@override
Future<bool> compile(String filename, ArgResults options, {IncrementalCompiler generator}) async {
Future<bool> compile(String filename, ArgResults options,
{IncrementalCompiler generator}) async {
return _compiler.compile(filename, options, generator: generator);
}

Expand Down Expand Up @@ -57,8 +66,8 @@ class _FlutterFrontendCompiler implements frontend.CompilerInterface{
String libraryUri,
String klass,
bool isStatic) {
return _compiler.compileExpression(expression, definitions, typeDefinitions,
libraryUri, klass, isStatic);
return _compiler.compileExpression(
expression, definitions, typeDefinitions, libraryUri, klass, isStatic);
}

@override
Expand All @@ -77,11 +86,12 @@ class _FlutterFrontendCompiler implements frontend.CompilerInterface{
/// `compiler` is an optional parameter so it can be replaced with mocked
/// version for testing.
Future<int> starter(
List<String> args, {
frontend.CompilerInterface compiler,
Stream<List<int>> input,
StringSink output,
}) async {
List<String> args, {
frontend.CompilerInterface compiler,
Stream<List<int>> input,
StringSink output,
frontend.ProgramTransformer transformer,
}) async {
ArgResults options;
try {
options = frontend.argParser.parse(args);
Expand All @@ -98,7 +108,8 @@ Future<int> starter(

final String input = options.rest[0];
final String sdkRoot = options['sdk-root'];
final Directory temp = Directory.systemTemp.createTempSync('train_frontend_server');
final Directory temp =
Directory.systemTemp.createTempSync('train_frontend_server');
try {
final String outputTrainingDill = path.join(temp.path, 'app.dill');
options = frontend.argParser.parse(<String>[
Expand Down Expand Up @@ -126,6 +137,7 @@ Future<int> starter(
}

compiler ??= _FlutterFrontendCompiler(output,
transformer: transformer,
unsafePackageSerialization: options['unsafe-package-serialization']);

if (options.rest.isNotEmpty) {
Expand Down
92 changes: 92 additions & 0 deletions frontend_server/publish.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:io';
import 'package:path/path.dart' as path;

const String vmPubspec = r'''
name: vm
version: 0.0.1
environment:
sdk: ">=2.2.2 <3.0.0"

dependencies:
front_end: any
kernel: any
meta: any
build_integratioon: any
''';

const String buildIntegrationPubspec = r'''
name: build_integration
version: 0.0.1
environment:
sdk: ">=2.2.2 <3.0.0"

dependencies:
front_end: any
meta: any
''';

const String frontendServerPubspec = r'''

@aam aam Sep 30, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the reason to have pubspec.yaml contents hardcoded in this script?

@jonahwilliams jonahwilliams Sep 30, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The pubspec of vm/build_integration are not valid, while the pubspec of front_end and kernel say not to rely on the versions. To ensure I don't have version solving issues, I've replace the pubspecs here with any constraints. This will be used by a generated pubspec in the flutter_tool that uses the dependency_overrides section to specify path versions.

@aam aam Sep 30, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you know why those packages have invalid or reuse-discouraging pubspec.yamls? Is that to prevent deploying and distributing those packages for some reason? If we intend to have those packages to be distributed and available, wouldn't be cleaner to fix their pubspec.yamls instead of providing our replacements for those?

@jonahwilliams jonahwilliams Sep 30, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not really distributing or publishing them though, I'm just allowing them to be vendored by the flutter SDK. The versions/constraints are not really meaningful in that content, as they won't be user visible.

@aam aam Sep 30, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry what does "vendored by the flutter SDK" mean? Does this effectively gives flutter tools (and only flutter tools, because nobody else downloads flutter engine artifacts) access to those packages, while currently those packages(well, frontend server only) is only available as executable snapshot?

@jonahwilliams jonahwilliams Sep 30, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the would be in the cache artifacts where flutter_tools can see the source, and use them to regenerate the snapshot to include other kernel transforms per flutter/flutter#36738

name: frontend_server
version: 0.0.1
environment:
sdk: ">=2.2.2 <3.0.0"

dependencies:
args: any
path: any
vm: any
''';

const String kernelPubspec = r'''
name: kernel
version: 0.0.1
environment:
sdk: '>=2.2.2 <3.0.0'

dependencies:
args: any
meta: any
''';

const String frontendPubspec = r'''
name: front_end
version: 0.0.1
environment:
sdk: '>=2.2.2 <3.0.0'
dependencies:
kernel: any
package_config: any
meta: any
''';

const Map<String, String> packages = <String, String>{
'vm': vmPubspec,
'build_integration': buildIntegrationPubspec,
'frontend_server': frontendServerPubspec,
'kernel': kernelPubspec,
'front_end': frontendPubspec,
};

// A script for creating a packagable version of several SDK libraries.
void main(List<String> arguments) {
final Directory packageSource = Directory(arguments[0]);
final String destination = arguments[1];
final String packageName = arguments[2];
for (File file in packageSource.listSync(recursive: true).whereType<File>()) {
if (!file.path.contains('lib')) {
// Skip test/bin directories and pubspec.
continue;
}
final File destinationFile = File(path.join(destination, path.relative(file.path, from: packageSource.path)));
destinationFile.parent.createSync(recursive: true);
file.copySync(destinationFile.path);
}
if (packages.containsKey(packageName)) {
File(path.join(destination, 'pubspec.yaml'))
.writeAsStringSync(packages[packageName]);
}
}
37 changes: 4 additions & 33 deletions frontend_server/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ description: Communication pipe to Dart Frontend
homepage: http://flutter.io
author: Flutter Authors <flutter-dev@googlegroups.com>

environment:
# The pub client defaults to an <2.0.0 sdk constraint which we need to explicitly overwrite.
sdk: ">=2.2.2 <3.0.0"

dependencies:
args: any
async: any
Expand All @@ -22,36 +26,3 @@ dependencies:
typed_data: any
usage: any
vm: any

dev_dependencies:

@zanderso zanderso Sep 30, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are these not needed anymore?

@jonahwilliams jonahwilliams Sep 30, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No more test

analyzer: any
boolean_selector: any
cli_util: any
csslib: any
glob: any
html: any
http: any
http_multi_server: any
http_parser: any
matcher: any
mime: any
mockito: any
package_resolver: any
plugin: any
pool: any
pub_semver: any
shelf: any
shelf_packages_handler: any
shelf_static: any
shelf_web_socket: any
source_map_stack_trace: any
source_maps: any
stack_trace: any
stream_channel: any
string_scanner: any
test: any
utf: any
watcher: any
web_socket_channel: any
when: any
yaml: any
19 changes: 0 additions & 19 deletions frontend_server/test/server_test.dart
View file
Open in desktop

This file was deleted.