Rearrange flutter assemble implementation #36240

jonahwilliams · 2019-07-16T06:02:08Z

Description

Updates to flutter assemble

Change target class to one meant to be extended. Cleans up the API
update build method to take a List of input files instead of a map. I'm still iterating on this API, I don't think the change type was actually useful.
Remove unused assemble APIs. These didn't end up being useful for iOS or macOS so I'll take them out for now.
fix interpolation bug with FLUTTER_ROOT and add test
update all targets to take the file with the rule as an input
Update computeChanges to not track Update/Modify/Deleted since these are no currently used by any APIs

Changes to testing

split build_system_test.dart
remove extra groups to reduce nesting.

codecov · 2019-07-16T06:38:45Z

Codecov Report

Merging #36240 into master will decrease coverage by 0.54%.
The diff coverage is 85.87%.

@@            Coverage Diff             @@
##           master   #36240      +/-   ##
==========================================
- Coverage    54.8%   54.25%   -0.55%     
==========================================
  Files         187      187              
  Lines       17246    17208      -38     
==========================================
- Hits         9451     9337     -114     
- Misses       7795     7871      +76

Flag	Coverage Δ
#flutter_tool	`54.25% <85.87%> (-0.55%)`	⬇️

Impacted Files	Coverage Δ
packages/flutter_tools/lib/src/context_runner.dart	`70% <0%> (ø)`	⬆️
...er_tools/lib/src/build_system/targets/windows.dart	`100% <100%> (ø)`	⬆️
...utter_tools/lib/src/build_system/build_system.dart	`86.44% <100%> (-9.66%)`	⬇️
...tter_tools/lib/src/build_system/targets/linux.dart	`100% <100%> (ø)`	⬆️
...ges/flutter_tools/lib/src/build_system/source.dart	`88.57% <100%> (+2.85%)`	⬆️
...ter_tools/lib/src/build_system/targets/assets.dart	`100% <100%> (ø)`	⬆️
...lutter_tools/lib/src/build_system/targets/ios.dart	`71.73% <71.73%> (-28.27%)`	⬇️
...tter_tools/lib/src/build_system/targets/macos.dart	`80% <80%> (+3.52%)`	⬆️
...utter_tools/lib/src/build_system/targets/dart.dart	`86.79% <85.36%> (+2.17%)`	⬆️
...kages/flutter_tools/lib/src/commands/assemble.dart	`68.42% <87.5%> (+2.59%)`	⬆️
... and 17 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 35b07a1...67e80d2. Read the comment docs.

…rejigger_api

codecov-io · 2019-07-18T18:15:51Z

Codecov Report

Merging #36240 into master will decrease coverage by 1.24%.
The diff coverage is 89.01%.

@@            Coverage Diff             @@
##           master   #36240      +/-   ##
==========================================
- Coverage   55.91%   54.67%   -1.25%     
==========================================
  Files         190      190              
  Lines       17590    17545      -45     
==========================================
- Hits         9836     9592     -244     
- Misses       7754     7953     +199

Flag	Coverage Δ
#flutter_tool	`54.67% <89.01%> (-1.25%)`	⬇️

Impacted Files	Coverage Δ
packages/flutter_tools/lib/src/context_runner.dart	`66.66% <0%> (-1.97%)`	⬇️
...er_tools/lib/src/build_system/targets/windows.dart	`100% <100%> (ø)`	⬆️
...utter_tools/lib/src/build_system/build_system.dart	`95.03% <100%> (-1.07%)`	⬇️
...tter_tools/lib/src/build_system/targets/linux.dart	`100% <100%> (ø)`	⬆️
...ges/flutter_tools/lib/src/build_system/source.dart	`88.57% <100%> (+2.85%)`	⬆️
...ter_tools/lib/src/build_system/targets/assets.dart	`100% <100%> (ø)`	⬆️
...tter_tools/lib/src/build_system/targets/macos.dart	`80% <80%> (+3.52%)`	⬆️
...lutter_tools/lib/src/build_system/targets/ios.dart	`82.97% <82.97%> (-17.03%)`	⬇️
...utter_tools/lib/src/build_system/targets/dart.dart	`86.79% <85.36%> (-4.35%)`	⬇️
...kages/flutter_tools/lib/src/commands/assemble.dart	`69.23% <85.71%> (+3.44%)`	⬆️
... and 26 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 1b57006...8da0cb6. Read the comment docs.

jonahwilliams · 2019-07-22T23:28:41Z

cc @zanderso, did you want to sit down and discuss this PR a bit sometime this week?

zanderso · 2019-07-24T19:24:51Z

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

      results.add(file);
    }
    return results;
  }


In AssetBehavior.outputs the for loop doesn't touch entry.value, so I guess it could instead loop over assetBundle.entries.values instead. I notice that that's what inputs is doing, but inputs is also filtering on the type of the value. If outputs doesn't need to do that as well, then a comment there would be good.

zanderso · 2019-07-24T19:31:34Z

packages/flutter_tools/lib/src/commands/assemble.dart

 class AssembleCommand extends FlutterCommand {
  AssembleCommand() {
-    addSubcommand(AssembleRun());
-    addSubcommand(AssembleDescribe());


Were the describe, etc. subcommands not useful? Have you considered exposing them under flags to assemble for e.g. dependency debugging?

For the iOS case, I need to generate/reference a file for the xcodeproj to read so having this as a command doesn't help. Gradle might be able to use the commands, but probably not as JSON - something like flutter assemble --list-outputs foo which lists the outputfiles on stdout

jonahwilliams added 3 commits July 15, 2019 20:23

reorganize assemble implementation

1b680e8

test fixes and rule clean up

4be669e

fix typo

2144a62

fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 16, 2019

googlebot added the cla: yes label Jul 16, 2019

Add license header to source_test

3d13a5b

jonahwilliams marked this pull request as ready for review July 16, 2019 16:52

jonahwilliams added 2 commits July 16, 2019 11:00

fix asset test

9ae23be

Merge branch 'rejigger_api' of github.com:jonahwilliams/flutter into …

67e80d2

…rejigger_api

jonahwilliams added 2 commits July 23, 2019 20:57

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

7a90700

Update linux_test.dart

40cfc6d

zanderso reviewed Jul 24, 2019

View reviewed changes

add comment and remove extra keys

8da0cb6

zanderso approved these changes Jul 24, 2019

View reviewed changes

jonahwilliams merged commit 188093c into flutter:master Jul 25, 2019

jonahwilliams deleted the rejigger_api branch July 25, 2019 15:50

johnsonmh pushed a commit to johnsonmh/flutter that referenced this pull request Jul 30, 2019

Rearrange flutter assemble implementation (flutter#36240)

a57b580

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Rearrange flutter assemble implementation #36240

Rearrange flutter assemble implementation #36240

Uh oh!

jonahwilliams commented Jul 16, 2019 •

edited

Loading

Uh oh!

codecov bot commented Jul 16, 2019 •

edited

Loading

Uh oh!

codecov-io commented Jul 18, 2019 •

edited by codecov bot

Loading

Uh oh!

jonahwilliams commented Jul 22, 2019

Uh oh!

zanderso Jul 24, 2019

Uh oh!

zanderso Jul 24, 2019

Uh oh!

jonahwilliams Jul 24, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Rearrange flutter assemble implementation #36240

Rearrange flutter assemble implementation #36240

Uh oh!

Conversation

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

Uh oh!

Description

Updates to flutter assemble

Changes to testing

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

codecov-io commented Jul 18, 2019 • edited by codecov bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jonahwilliams commented Jul 22, 2019

Uh oh!

zanderso Jul 24, 2019

Choose a reason for hiding this comment

Uh oh!

zanderso Jul 24, 2019

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Jul 24, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jonahwilliams commented Jul 16, 2019 •

edited

Loading

codecov bot commented Jul 16, 2019 •

edited

Loading

codecov-io commented Jul 18, 2019 •

edited by codecov bot

Loading