Skip to content

Reland: implement build bundle with assemble - #40470

Merged
jonahwilliams merged 42 commits into
flutter:masterfrom
jonahwilliams:bundle_assemble
Sep 19, 2019
Merged

jonahwilliams merged 42 commits into
flutter:masterfrom
jonahwilliams:bundle_assemble

Conversation

@jonahwilliams

Copy link
Copy Markdown
Contributor

Description

This is a reland of #37508. The depfile's were not correctly escaped for windows, which caused failures when rebuilding.

Additionally fixes a bug in the file cache cleanup, and ensures we use a package uri when building a snapshot in assemble.

@fluttergithubbot fluttergithubbot added c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Sep 14, 2019
@jonahwilliams jonahwilliams changed the title Reland: implement build bundle with assemble [WIP] Reland: implement build bundle with assemble Sep 14, 2019
@codecov

codecov Bot commented Sep 14, 2019 •

Copy link
Copy Markdown

Codecov Report

Merging #40470 into master will decrease coverage by 0.46%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #40470      +/-   ##
==========================================
- Coverage   59.75%   59.29%   -0.47%     
==========================================
  Files         193      193              
  Lines       18705    18795      +90     
==========================================
- Hits        11177    11144      -33     
- Misses       7528     7651     +123
Flag Coverage Δ
#flutter_tool 59.29% <33.33%> (-0.47%) ⬇️
Impacted Files Coverage Δ
...er_tools/lib/src/build_system/file_hash_store.dart 91.8% <ø> (+1.48%) ⬆️
...kages/flutter_tools/lib/src/commands/assemble.dart 78.68% <ø> (-0.35%) ⬇️
...s/flutter_tools/lib/src/commands/build_bundle.dart 98.11% <ø> (ø) ⬆️
...ter_tools/lib/src/build_system/targets/assets.dart 64.86% <0%> (-22.41%) ⬇️
...utter_tools/lib/src/build_system/build_system.dart 86.87% <100%> (-0.28%) ⬇️
packages/flutter_tools/lib/src/artifacts.dart 69.2% <100%> (+0.1%) ⬆️
...utter_tools/lib/src/build_system/targets/dart.dart 47.05% <2.5%> (-29.51%) ⬇️
packages/flutter_tools/lib/src/bundle.dart 75.25% <74.19%> (-0.5%) ⬇️
...ges/flutter_tools/lib/src/commands/ide_config.dart 30.09% <0%> (-53.4%) ⬇️
...ckages/flutter_tools/lib/src/commands/upgrade.dart 4.34% <0%> (-39.14%) ⬇️
... and 14 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 4c724b9...4d026bb. Read the comment docs.

@jonahwilliams jonahwilliams changed the title [WIP] Reland: implement build bundle with assemble Reland: implement build bundle with assemble Sep 18, 2019

@jmagman jmagman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Last 3 commits since the master merge LGTM

@zanderso zanderso left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm w/ question and nit.

final PoolResource resource = await pool.request();
try {
final File file = fs.file(fs.path.join(environment.outputDir.path, entry.key));
file.parent.createSync(recursive: true);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What happens when one of these file operations fails?

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.

We should throw, will update

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.

actually, we're already propagating the future, just ensuring that the resource is released first


void _writeFilesToBuffer(List<File> files, StringBuffer buffer) {
for (File outputFile in files) {
if (platform.isWindows) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: 2 space indent

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.

Fixed

@jonahwilliams
jonahwilliams merged commit ea7876a into flutter:master Sep 19, 2019
@jonahwilliams

Copy link
Copy Markdown
Contributor Author

I can't see if the devicelab failure is real or not, so I'll rollback for now

jonahwilliams pushed a commit that referenced this pull request Sep 19, 2019
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants