build bundle with assemble #37508

jonahwilliams · 2019-08-02T19:17:14Z

Description

Update build bundle to delegate the call to build bundle to flutter assemble. Then copy the required assets out of the cache directory and into the build/* directory, or where told.

Problem: depfile generated by dart rule will have the wrong paths. I don't really think we should hack around this, and instead work on shrinking flutter gradle to a single assemble invocation. The tool can then output inputs/outputs in a depfile format

codecov · 2019-08-02T19:50:39Z

Codecov Report

Merging #37508 into master will increase coverage by 0.16%.
The diff coverage is 28.88%.

@@            Coverage Diff             @@
##           master   #37508      +/-   ##
==========================================
+ Coverage   58.92%   59.08%   +0.16%     
==========================================
  Files         192      192              
  Lines       18442    18527      +85     
==========================================
+ Hits        10867    10947      +80     
- Misses       7575     7580       +5

Flag	Coverage Δ
#flutter_tool	`59.08% <28.88%> (+0.16%)`	⬆️

Impacted Files	Coverage Δ
...kages/flutter_tools/lib/src/commands/assemble.dart	`78.68% <ø> (-0.35%)`	⬇️
...s/flutter_tools/lib/src/commands/build_bundle.dart	`98.11% <ø> (ø)`	⬆️
...utter_tools/lib/src/build_system/targets/dart.dart	`47.57% <0%> (-28.99%)`	⬇️
...ter_tools/lib/src/build_system/targets/assets.dart	`64.86% <0%> (-22.41%)`	⬇️
...utter_tools/lib/src/build_system/build_system.dart	`87.55% <100%> (+0.05%)`	⬆️
packages/flutter_tools/lib/src/artifacts.dart	`69.2% <100%> (+0.1%)`	⬆️
packages/flutter_tools/lib/src/bundle.dart	`76.08% <76.92%> (+0.32%)`	⬆️
packages/flutter_tools/lib/src/version.dart	`90.9% <0%> (-1.92%)`	⬇️
packages/flutter_tools/lib/src/cache.dart	`43.5% <0%> (-0.73%)`	⬇️
packages/flutter_tools/lib/src/vmservice.dart	`37.27% <0%> (-0.61%)`	⬇️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f877c97...631667a. Read the comment docs.

…to bundle_assemble

jonahwilliams · 2019-08-27T18:10:00Z

This should be less risky now, and gives us a path to incrementally adopting assemble.

jmagman · 2019-08-27T20:14:14Z

packages/flutter_tools/lib/src/build_system/targets/dart.dart

+
+  @override
+  List<Source> get outputs => const <Source>[
+    Source.pattern('{BUILD_DIR}/vm_snapshot_data'),


Are these going to get converted to OUTPUT_DIR in https://github.com/flutter/flutter/pull/39274/files?

Yeah, if I land the output dir change this is all much simpler

zanderso · 2019-08-27T20:22:53Z

packages/flutter_tools/lib/src/build_system/targets/dart.dart

      targetModel: TargetModel.flutter,
      targetProductVm: buildMode == BuildMode.release,
      outputFilePath: environment.buildDir.childFile('app.dill').path,
-      depFilePath: null,


Should/can you use the kernel compiler depfile as a sanity check?

zanderso · 2019-08-27T20:25:41Z

packages/flutter_tools/lib/src/bundle.dart

+
+  @override
+  List<Target> get dependencies => const <Target>[
+    CopyAssets(),


Library prefixes could help with namespacing targets, at least internally.

blasten · 2019-08-27T20:56:30Z

packages/flutter_tools/lib/src/bundle.dart

    applicationKernelFilePath ??= getDefaultApplicationKernelPath(trackWidgetCreation: trackWidgetCreation);
    final FlutterProject flutterProject = FlutterProject.current();

+    if (shouldBuildWithAssemble) {


do you plan to enable assemble for the remaining cases?

I think we could, but I'd like to be conservative here

blasten · 2019-08-27T21:02:31Z

packages/flutter_tools/lib/src/bundle.dart

+  const _BundleTarget();
+
+  @override
+  Future<void> build(List<File> inputFiles, Environment environment) async { }


Would it be common to have an empty build method if the target is used to combine other targets? If that is the case, maybe it needs a lower abstraction (e.g. without build)?

This what the planning phase is attempting to address, but until that lands it is necessary for now

blasten · 2019-08-27T21:09:52Z

packages/flutter_tools/test/general.shard/bundle_shim_test.dart

+      flutterProject: FlutterProject.current(),
+      mainPath: fs.path.join('lib', 'main.dart'),
+      outputDir: 'example',
+      targetPlatform: TargetPlatform.ios


nit: trailing comma

jonahwilliams · 2019-08-28T21:30:21Z

Updated to use the new OUTPUT_DIR pattern, and to produce a correct depfile for gradle (it was excluding all of the flutter assets).

@blasten, were the flutter assets being included as a dependency in some other way?

jonahwilliams · 2019-08-28T21:34:00Z

packages/flutter_tools/lib/src/bundle.dart

+  @required String mainPath,
+  @required String outputDir,
+}) async {
+  final Environment environment = Environment(


@jmagman This is an example of mapping a build * command to an assemble command. For example, in the iOS framework case this could include multiple calls if need be.

…to bundle_assemble

jonahwilliams · 2019-09-08T00:08:27Z

This change requires #39654 to land

zanderso · 2019-09-10T16:34:02Z

packages/flutter_tools/lib/src/build_system/targets/dart.dart

+        try {
+          final File file = fs.file(fs.path.join(environment.outputDir.path, entry.key));
+          file.parent.createSync(recursive: true);
+          await file.writeAsBytes(await entry.value.contentsAsBytes());


How about testing if entry.value is a DevFSFileContent and if so use File.copy() instead?

jonahwilliams · 2019-09-10T17:32:41Z

Landed the perquisite PR, will attempt to land if CI passes.

jonahwilliams · 2019-09-10T22:46:31Z

Per my discussion with @yjbanov, I'm going to ignore web failures on this (since it is unrelated). Will land once build is green

blasten · 2019-09-10T23:08:38Z

dev/devicelab/bin/tasks/gradle_plugin_light_apk_test.dart

-    } catch (e) {
+    } catch (e, st) {
+      print(e);
+      print(st);


nit: this print and the one above

jonahwilliams · 2019-09-11T02:28:27Z

We believe that the web tests are broken and have disabled them. I'm ignoring them for this PR.

This reverts commit a7aff56.

build bundle with assemble

3781ed2

jonahwilliams added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 2, 2019

googlebot added the cla: yes label Aug 2, 2019

Jonah Williams added 14 commits August 2, 2019 13:04

Update bundle_shim_test.dart

fa7d25a

Merge branch 'master' of github.com:flutter/flutter into bundle_assemble

74c1cfb

remove asserts from dev

636c64a

Merge branch 'bundle_assemble' of github.com:jonahwilliams/flutter in…

bb59559

…to bundle_assemble

always name kernel_blob.bin

0941509

copy precompiled runtime and remove bogus test changes

dac2d4c

Update bundle.dart

47a52f3

Update bundle.dart

6f238bd

Update bundle.dart

b2ee553

fix tests

d0d6f6c

plumb through platform

720d325

produce depfile

a34a1cc

remove testing of depfile

91d8cca

Update bundle.dart

2cea2a9

jonahwilliams requested review from blasten, jmagman and zanderso August 27, 2019 18:10

jmagman reviewed Aug 27, 2019

View reviewed changes

zanderso reviewed Aug 27, 2019

View reviewed changes

blasten reviewed Aug 27, 2019

View reviewed changes

Merge branch 'master' of github.com:flutter/flutter into bundle_assemble

a1883ed

jonahwilliams commented Aug 28, 2019

View reviewed changes

Jonah Williams added 7 commits August 28, 2019 19:10

Update apk_utils.dart

189c806

fix split and asset name error

74a3e00

Update dart.dart

56bc074

Merge branch 'master' of github.com:flutter/flutter into bundle_assemble

2c83990

Merge branch 'bundle_assemble' of github.com:jonahwilliams/flutter in…

af99bc6

…to bundle_assemble

mergE

3f8a2c3

switch to print trace

d87fb12

zanderso reviewed Sep 10, 2019

View reviewed changes

jonahwilliams added 2 commits September 10, 2019 10:08

Merge branch 'master' of github.com:flutter/flutter into bundle_assemble

4f2fdfd

improve file copy time

acbb269

zanderso approved these changes Sep 10, 2019

View reviewed changes

jonahwilliams added 2 commits September 10, 2019 12:17

make sure build directory exists

448e60a

Update bundle.dart

0e1d1ba

blasten reviewed Sep 10, 2019

View reviewed changes

Jonah Williams added 2 commits September 10, 2019 16:10

Update gradle_plugin_light_apk_test.dart

72349bc

Update gradle_plugin_light_apk_test.dart

631667a

jonahwilliams merged commit a7aff56 into flutter:master Sep 11, 2019

jonahwilliams deleted the bundle_assemble branch September 11, 2019 02:28

jonahwilliams restored the bundle_assemble branch September 11, 2019 03:03

jonahwilliams pushed a commit that referenced this pull request Sep 11, 2019

Revert "build bundle with assemble (#37508)"

b31206c

This reverts commit a7aff56.

jonahwilliams mentioned this pull request Sep 11, 2019

Revert "build bundle with assemble" #40204

Merged

jonahwilliams pushed a commit that referenced this pull request Sep 11, 2019

Revert "build bundle with assemble (#37508)" (#40204)

609a78f

jonahwilliams mentioned this pull request Sep 14, 2019

Reland: implement build bundle with assemble #40470

Merged

Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019

build bundle with assemble (flutter#37508)

858dce7

Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019

Revert "build bundle with assemble (flutter#37508)" (flutter#40204)

7bcbdf2

github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2021

build bundle with assemble #37508

build bundle with assemble #37508

Uh oh!

Conversation

jonahwilliams commented Aug 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

codecov bot commented Aug 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jonahwilliams commented Aug 27, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blasten Aug 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Aug 28, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Sep 8, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Sep 10, 2019

Uh oh!

jonahwilliams commented Sep 10, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Sep 11, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jonahwilliams commented Aug 2, 2019 •

edited

Loading

codecov bot commented Aug 2, 2019 •

edited

Loading

blasten Aug 27, 2019 •

edited

Loading