Refactor project to remove need for build.sh by jonahwilliams · Pull Request #336 · google/flutter-desktop-embedding

jonahwilliams · 2019-04-19T22:05:37Z

Removes indirection by moving the macOS build script to flutter_tools. This still uses the FDE asset syncing. This uses almost the same scheme/flag passing setup as the existing iOS build.

~~Currently the track-widget-creation flag is still broken though, it seems like some part of the process is using the regular .dill instead of the .track.dill - still investigating.~~

Fixed, need to make sure I referenced config file

See also: flutter/flutter#31329

stuartmorgan-g · 2019-04-20T20:01:12Z

example/macos/Runner.xcodeproj/project.pbxproj

 			runOnlyForDeploymentPostprocessing = 0;
 			shellPath = /bin/sh;
-			shellScript = "\"$FDE_ROOT\"/tools/build_flutter_assets \"$PROJECT_DIR\"/..\n";
+			shellScript = "\"$FLUTTER_ROOT\"/packages/flutter_tools/bin/macos_build_flutter_assets.sh";


This creates a new requirement that flutter build must be run before the Xcode project will work; that's not an issue for a long-term flutter created version of this, but would be potentially breaking for this project. I think we should check in a Generated.xcconfig with this change that defaults to ../../../flutter to make it work in a clean checkout that uses the suggested layout. (Not perfect, since it won't handle .flutter_location_config, but better than nothing.)

Putting it together this way it doesn't quite work when i run from Xcode - possibly due to the lack of the OBJROOT, SYMROOT, and derivedData overrides? I might have my setup configured wrong.

I'll take a look tomorrow and see what's broken. The build output will go somewhere else if you build from Xcode, but I wouldn't expect that to break anything (it's what happens right now with build.sh vs Xcode UI).

I realized while doing the Windows version that we shouldn't check in a copy of the generated settings, since it needs to be in the .gitignore, so I think we'll just live with needing to use 'fluter build' or 'flutter run' at least once. I can add a note about that to the docs.

example/macos/Runner.xcodeproj/project.pbxproj

stuartmorgan-g

testbed/ should get the same changes—except that we can't switch away from tools/build_fluter_assets until there's local engine support in macos_build_flutter_assets.sh—since the flutter_tools side stops using build.sh.

stuartmorgan-g · 2019-04-22T03:12:44Z

example/macos/Runner.xcodeproj/project.pbxproj

 			runOnlyForDeploymentPostprocessing = 0;
 			shellPath = /bin/sh;
-			shellScript = "\"$FDE_ROOT\"/tools/build_flutter_assets \"$PROJECT_DIR\"/..\n";
+			shellScript = "\"$FLUTTER_ROOT\"/packages/flutter_tools/bin/macos_build_flutter_assets.sh";


I'll take a look tomorrow and see what's broken. The build output will go somewhere else if you build from Xcode, but I wouldn't expect that to break anything (it's what happens right now with build.sh vs Xcode UI).

…into add_track_widget_creation_flag

stuartmorgan-g

Looks good modulo the comment below, removing Generated.xcconfig and adding it to .gitignore instead, and then duplicating most of this to testbed/

stuartmorgan-g · 2019-04-22T17:11:35Z

example/macos/Runner.xcodeproj/project.pbxproj

 			name = Frameworks;
 			sourceTree = "<group>";
 		};
+		D76E93BA226BCA8F00F7932B /* Recovered References */ = {


The configs should be referenced in the project, rather than being considered recovered files. Please rename this group in the file tree "Configs", move it up between Resources and Frameworks, and (for ease of inspection) add Generated.xcconfig to that group as well (make sure to uncheck the option to add it to any of the targets)

We don't reference the Generated.xcconfig in our iOS projects, should this be changed in the templates for those as well?

It doesn't affect the build in any way, it just means that if you have the project open you can easily inspect all the configs. I find it helpful in general, but if nobody has ever complained it's up to you whether it's worth changing iOS.

…into add_track_widget_creation_flag

…iams/flutter-desktop-embedding into add_track_widget_creation_flag

jonahwilliams · 2019-04-22T19:49:01Z

Looks like travis will be red until it uses the latest flutter

stuartmorgan-g · 2019-04-22T19:57:38Z

Looks like travis will be red until it uses the latest flutter

It clones from master; it's red because your other PR hasn't landed...

stuartmorgan-g · 2019-04-22T20:55:58Z

example/.gitignore

 pubspec.lock
+
+# Generated by flutter run and flutter build
+Generated.xcconfig


This should be in a .gitignore in example/macos (especially since the goal is to make those folders relocatable shortly)

stuartmorgan-g · 2019-04-22T20:56:12Z

testbed/.gitignore

 pubspec.lock
+
+# generated by flutter run/build
+Generated.xcconfig


stuartmorgan-g · 2019-04-22T20:58:44Z

testbed/macos/Runner.xcodeproj/project.pbxproj

 			runOnlyForDeploymentPostprocessing = 0;
 			shellPath = /bin/sh;
-			shellScript = "\"$FDE_ROOT\"/tools/build_flutter_assets \"$PROJECT_DIR\"/..\n";
+			shellScript = "\"$FLUTTER_ROOT\"/packages/flutter_tools/bin/macos_build_flutter_assets.sh";


Don't change this yet for testbed; that would break the documented (and heavily used by me) local engine workflow.

Later we can make the Flutter scripts support local engine and migrate the workflow and docs to that.

…iams/flutter-desktop-embedding into add_track_widget_creation_flag

stuartmorgan-g

testbed/macos/build.sh should be removed too; otherwise LGTM

jonahwilliams added 2 commits April 19, 2019 14:58

refactor project to remove need for build.sh

2188a01

remove generated file

1038466

jonahwilliams mentioned this pull request Apr 19, 2019

Add Xcode build script for macOS target flutter/flutter#31329

Merged

9 tasks

reference generated xcconfig

8591b39

jonahwilliams requested a review from stuartmorgan-g April 20, 2019 18:44

stuartmorgan-g suggested changes Apr 20, 2019

View reviewed changes

jonahwilliams added 2 commits April 20, 2019 16:31

update to use 2 different configurations that import generated

cbf9199

Merge branch 'master' into add_track_widget_creation_flag

56886b8

stuartmorgan-g reviewed Apr 22, 2019

View reviewed changes

Merge branch 'master' of github.com:google/flutter-desktop-embedding …

362573c

…into add_track_widget_creation_flag

stuartmorgan-g reviewed Apr 22, 2019

View reviewed changes

jonahwilliams added 5 commits April 22, 2019 10:37

attempt to re-organize xcode project

deb9fb4

Merge branch 'master' of github.com:google/flutter-desktop-embedding …

6c71aad

…into add_track_widget_creation_flag

Merge branch 'add_track_widget_creation_flag' of github.com:jonahwill…

dc3130a

…iams/flutter-desktop-embedding into add_track_widget_creation_flag

Update Debug.xcconfig

0033e36

Update Release.xcconfig

79fac09

Update .gitignore

0a954f6

stuartmorgan-g reviewed Apr 22, 2019

View reviewed changes

Jonah Williams added 3 commits April 22, 2019 14:03

Update project.pbxproj

b37ce1a

move gitignore to macOS sub folder

6da4f21

Merge branch 'add_track_widget_creation_flag' of github.com:jonahwill…

a894c06

…iams/flutter-desktop-embedding into add_track_widget_creation_flag

stuartmorgan-g approved these changes Apr 22, 2019

View reviewed changes

remove testbed build.sh

67aa79f

jonahwilliams merged commit 349cacf into google:master Apr 22, 2019

jonahwilliams deleted the add_track_widget_creation_flag branch April 22, 2019 22:04

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

Conversation

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

Uh oh!

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!

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g left a comment

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!

jonahwilliams commented Apr 22, 2019

Uh oh!

stuartmorgan-g commented Apr 22, 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!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jonahwilliams commented Apr 19, 2019 •

edited

Loading