Flutter assemble for macos take 2! #36987

jonahwilliams · 2019-07-26T03:50:39Z

Description

Updates the BuildResult to track input and output files. Due to some issues with other parts of the tool re-writing these files constantly, some have been factored out of the list.
Adds an "optional" source and a pod rule to use it. Until I add a configuration/planning phase I need to add the pod step to every build, but only run it when a podfile exists.

Implementation of flutter assemble for the macOS build. This updates the entrypoint script to move all artifact copying/generation into build rules. This also generates an input/output xcfilelist that can be used by xcode to skip the entire script phase.

This requires the project changes in google/flutter-desktop-embedding#459

#32921

codecov · 2019-07-26T04:29:16Z

Codecov Report

Merging #36987 into master will increase coverage by 1.41%.
The diff coverage is 81.59%.

@@            Coverage Diff             @@
##           master   #36987      +/-   ##
==========================================
+ Coverage   54.19%    55.6%   +1.41%     
==========================================
  Files         193      193              
  Lines       17892    17982      +90     
==========================================
+ Hits         9696     9999     +303     
+ Misses       8196     7983     -213

Flag	Coverage Δ
#flutter_tool	`55.6% <81.59%> (+1.41%)`	⬆️

Impacted Files	Coverage Δ
...ackages/flutter_tools/lib/src/commands/attach.dart	`74.19% <ø> (-0.14%)`	⬇️
...ackages/flutter_tools/lib/src/commands/daemon.dart	`34.34% <ø> (ø)`	⬆️
...es/flutter_tools/lib/src/commands/build_macos.dart	`84.21% <ø> (ø)`	⬆️
packages/flutter_tools/lib/src/commands/run.dart	`60.57% <ø> (-0.19%)`	⬇️
packages/flutter_tools/lib/src/project.dart	`82.84% <100%> (+2.06%)`	⬆️
...utter_tools/lib/src/build_system/targets/dart.dart	`73.77% <100%> (+1.35%)`	⬆️
...ackages/flutter_tools/lib/src/resident_runner.dart	`53.4% <100%> (+0.18%)`	⬆️
packages/flutter_tools/lib/src/run_hot.dart	`66.81% <100%> (+0.07%)`	⬆️
...utter_tools/lib/src/build_system/build_system.dart	`93.01% <100%> (+0.88%)`	⬆️
packages/flutter_tools/lib/src/ios/xcodeproj.dart	`86.66% <100%> (ø)`	⬆️
... and 24 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 bd47a31...c98c32d. Read the comment docs.

jonahwilliams · 2019-07-26T20:13:15Z

packages/flutter_tools/bin/macos_build_flutter_assets.sh

-    ${local_engine_flag}
+    ${local_engine_flag}                                                    \
+    assemble                                                                \
+    -dTargetFile="${target_path}"                                           \


TBD: flavor?

Maybe make this a TODO that points to the flavors issue.

zanderso · 2019-07-29T16:48:31Z

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

+    // Remove "inputs" that were outputs of previous builds.
+    buildInstance.inputFiles.removeWhere((String path, File file) {
+      // Leaking implementation detail: remove pubpsec, flutter-plugins, generated.
+      return buildInstance.outputFiles.containsKey(path) ||


How do these make it into the input list? Is there a sanity check that can be done here instead ahead-of-time?

Moved this check into the input list construction

zanderso · 2019-07-29T16:50:14Z

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

@@ -472,10 +480,28 @@ class BuildSystem {
      // Always persist the file cache to disk.
      fileCache.persist();
    }
+    // Remove "inputs" that were outputs of previous builds.
+    buildInstance.inputFiles.removeWhere((String path, File file) {
+      // Leaking implementation detail: remove pubpsec, flutter-plugins, generated.


This comment should explain why these are being blacklisted and/or the criteria for adding items to this list.

Updated with a longer TODO that explains why this is here for now, and what needs to be done to remove it.

zanderso · 2019-07-29T16:53:05Z

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

@@ -105,6 +105,9 @@ class SourceVisitor {
        }
      }
    } else {


Flip the sense to if (!hasWildcard) and return early under that if to unindent the big chunk.

zanderso · 2019-07-29T16:56:45Z

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

+
+  @override
+  List<Source> get outputs => const <Source>[
+    // No outputs because Cocoapods is fully responsible for tracking. plus there


Can this produce something like a .stamp file that dependent rules can use to track?

We'd need to right it such that the file contents changed as well. Just a stamp wont work since we're using file hashes, at that point it is probably close to the same as .flutter-plugins.

I've updated this with an explicit todo to add a planning phase so I can conditionally insert the pod rule. Then I can use the regular outputs.

…le_impl_2

jonahwilliams · 2019-07-30T17:58:35Z

@zanderso PTAL

zanderso

lgtm w/ nits

zanderso · 2019-07-31T04:37:40Z

packages/flutter_tools/bin/macos_build_flutter_assets.sh

-    ${local_engine_flag}
+    ${local_engine_flag}                                                    \
+    assemble                                                                \
+    -dTargetFile="${target_path}"                                           \


Maybe make this a TODO that points to the flavors issue.

zanderso · 2019-07-31T04:41:29Z

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

@@ -39,7 +39,7 @@ class SourceVisitor {
  /// Visit a [Source] which contains a file uri.
  ///
  /// The uri may that may include constants defined in an [Environment].
-  void visitPattern(String pattern) {
+  void visitPattern(String pattern, bool optional) {


Describe 'optional' parameter in the doc comment.

zanderso · 2019-07-31T04:46:34Z

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

+}
+
+// TODO(jonahwilliams): real AOT implementation.
+class ReleaseMacOSApplication extends DebugMacOSApplication {


Should these have a build method that throws an UnimplementedError?

The release mode is a bit of a lie today, but it does exist

zanderso · 2019-07-31T04:48:24Z

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

@@ -106,10 +112,34 @@ class AssembleCommand extends FlutterCommand {
        printError('Target ${data.key} failed: ${data.value.exception}');
        printError('${data.value.exception}');
      }
-      throwToolExit('build failed');
+      throwToolExit('build failed.');
    } else {


else not needed.

zanderso · 2019-07-31T04:49:42Z

packages/flutter_tools/lib/src/macos/build_macos.dart

-  if (!flutterBuildDir.existsSync()) {
-    flutterBuildDir.createSync(recursive: true);
-  }
+  // Create the environment used to process the build. This need sto mtach what


this need sto -> this needs to

zanderso · 2019-07-31T04:53:03Z

packages/flutter_tools/lib/src/macos/build_macos.dart

  // Set debug or release mode.
  String config = 'Debug';
-  if (buildInfo.isRelease) {
+  if (buildInfo.isRelease == true) {


Is this because isRelease could be null? In that case, how about buildInfo.isRelease ?? false

zanderso · 2019-07-31T04:56:06Z

packages/flutter_tools/lib/src/macos/macos_device.dart

@@ -81,26 +81,28 @@ class MacOSDevice extends Device {
    bool ipv6 = false,
  }) async {
    // Stop any running applications with the same executable.
+    PrebuiltMacOSApp prebuiltMacOSApp;
    if (!prebuiltApplication) {


Think positive (unless there's a good reason not to):

if (prebuiltApplication) { ... } else { ... }

Also, should the Cache lock be released early in both cases, or just when prebuiltAppllication is false.

stuartmorgan · 2019-08-06T03:37:18Z

packages/flutter_tools/bin/macos_build_flutter_assets.sh

-track_widget_creation_flag=""
-if [[ -n "$TRACK_WIDGET_CREATION" ]]; then
-  track_widget_creation_flag="--track-widget-creation"
-fi


Removing this regressed the bug where hot reload crashes in IDEs; I'm not sure why it was taken out.

I'll send a follow-up PR shortly.

jonahwilliams added 2 commits July 25, 2019 12:55

re implementation of macOS assemble

3f8c1c1

make it go vroom vroom

9c28d16

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

googlebot added the cla: yes label Jul 26, 2019

jonahwilliams mentioned this pull request Jul 26, 2019

[WIP] Updates to macOS build to support flutter assemble #36164

Closed

add more test cases

0ed4b40

jonahwilliams added the platform-mac Building on or for macOS specifically label Jul 26, 2019

jonahwilliams marked this pull request as ready for review July 26, 2019 20:03

Update macos_build_flutter_assets.sh

0a17c6c

jonahwilliams commented Jul 26, 2019

View reviewed changes

jonahwilliams requested a review from zanderso July 26, 2019 20:18

zanderso reviewed Jul 29, 2019

View reviewed changes

jonahwilliams added 4 commits July 29, 2019 10:51

address comments

8cf7669

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

87b6b87

…le_impl_2

Update source.dart

839b185

add comment and fix typo

cab5e1b

jonahwilliams changed the title ~~Fluter assemble for macos take 2!~~ Flutter assemble for macos take 2! Jul 29, 2019

Merge branch 'master' into macos_assemble_impl_2

3f732f0

fix from merge

a12aa30

zanderso approved these changes Jul 31, 2019

View reviewed changes

jonahwilliams added 3 commits July 31, 2019 10:35

address comments

43fad62

Update source.dart

99a0469

Update build_macos.dart

c98c32d

jonahwilliams merged commit 2ab4699 into flutter:master Jul 31, 2019

jonahwilliams deleted the macos_assemble_impl_2 branch July 31, 2019 23:19

stuartmorgan reviewed Aug 6, 2019

View reviewed changes

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

Flutter assemble for macos take 2! #36987

Flutter assemble for macos take 2! #36987

jonahwilliams commented Jul 26, 2019 •

edited

Loading

codecov bot commented Jul 26, 2019 •

edited

Loading

jonahwilliams Jul 26, 2019

zanderso Jul 31, 2019

jonahwilliams Jul 31, 2019

zanderso Jul 29, 2019

jonahwilliams Jul 29, 2019

zanderso Jul 29, 2019

jonahwilliams Jul 29, 2019

zanderso Jul 29, 2019

jonahwilliams Jul 29, 2019

zanderso Jul 29, 2019

jonahwilliams Jul 29, 2019

jonahwilliams commented Jul 30, 2019

zanderso left a comment

zanderso Jul 31, 2019

zanderso Jul 31, 2019

jonahwilliams Jul 31, 2019

zanderso Jul 31, 2019

jonahwilliams Jul 31, 2019

zanderso Jul 31, 2019

jonahwilliams Jul 31, 2019

zanderso Jul 31, 2019

jonahwilliams Jul 31, 2019

zanderso Jul 31, 2019

jonahwilliams Jul 31, 2019

zanderso Jul 31, 2019

jonahwilliams Jul 31, 2019

stuartmorgan Aug 6, 2019

Flutter assemble for macos take 2! #36987

Flutter assemble for macos take 2! #36987

Conversation

jonahwilliams commented Jul 26, 2019 • edited Loading

Description

codecov bot commented Jul 26, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonahwilliams commented Jul 30, 2019

zanderso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonahwilliams commented Jul 26, 2019 •

edited

Loading

codecov bot commented Jul 26, 2019 •

edited

Loading