Add pre-stable support for create on Windows #51895

stuartmorgan-g · 2020-03-03T22:44:56Z

Description

Adds initial support for flutter create of apps and plugins. This is derived from the current FDE example app and sample plugin, adding template values where relevant.

Since the APIs/tooling/template aren't stable yet, the app template includes a version marker, which will be updated each time there's a breaking change. The build now checks that the template version matches the version known by that version of the tool, and gives a specific error message when there's a mismatch, which improves over the current breaking change experience of hitting whatever build failure the breaking change causes and having to figure out that the problem is that the runner is out of date. It also adds a warning to the create output about the fact that it won't be stable.

Plugins don't currently have a version marker since in practice this is not a significant problem for plugins yet the way it is for runners; we can add it later if that changes.

Related Issues

#30704

Tests

I added the following tests:

Checks that the app and plugin templates include windows when requested, and don't when not requested.
Error message tests for mismatches of template version.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

No, no existing tests failed, so this is not a breaking change.
Yes, this is a breaking change.

stuartmorgan-g · 2020-03-03T22:46:09Z

I still need to do the manual tests on this one, but it should be good to review as-is.

jonahwilliams · 2020-03-04T08:52:14Z

packages/flutter_tools/templates/app/windows.tmpl/.gitignore

+
+# Since there are multiple workflows, uncomment next line to ignore bower_components
+# (https://github.com/github/gitignore/pull/1529#issuecomment-104372622)
+#bower_components/


blast from the past

I am torn on whether we should use this giant stock ignore file, or instead do just what we actually need (which is probably not much, but I may get wrong initially and have to adjust). Thoughts?

For the most part, we've followed the later policy. This works great until we need to add something, and users end up without. For a pre-stable template I think it won't matter as much. It is harder to explain "why is X here" when pointing at the catch-all

Though, maybe someone writes an app and just happens to call a folder bower_components 🤷‍♂

I'll redo this when I'm at my Windows machine, doing just what shows up for me in an edit+build cycle.

I slimmed it way down. I left a few of the very basic user-file patterns in case they are triggered by things I just didn't do.

I suspect we'll find a few things over time that I should have left in but didn't, but we can always re-add things as we find them.

packages/flutter_tools/templates/app/windows.tmpl/Runner.vcxproj.tmpl

jonahwilliams · 2020-03-04T08:54:45Z

The non UTF files need an exception added in dev/bots/test.dart . This is mostly to prevent us from accidentally adding large binaries or images, should be okay for templates

stuartmorgan-g · 2020-03-04T09:26:10Z

The non UTF files need an exception added in dev/bots/test.dart .

Added an exception for the .ico. It looks like the rc file can be UTF-8 in VS 2019, so I've pushed a version with that. I need to double-check when I'm at my Windows machine though since it sounds like the exact behavior changed during the development of VS 2019 so I'm not 100% sure it will preserve UTF-8 on edit+save, and if it doesn't I'll switch it back to UTF-16.

stuartmorgan-g · 2020-03-11T17:33:05Z

I'm not 100% sure it will preserve UTF-8 on edit+save

Tested adding a resource via VS 2019 and it still saved out in UTF-8, so it looks like that's fine to change!

stuartmorgan-g · 2020-03-11T18:01:00Z

Added an exception for the .ico.

Turns out I was looking at the wrong check.

@Hixie, the code I would actually need to change to allow adding the icon for this new template has multiple comments saying not to change it, but don't point to any suggestions about what to do instead and only say to talk to you.

What is the right way to have an icon in a new template now?
Can we have a more scalable solution in the documentation than "talk to one specific person who might not be in the office when you encounter this problem"?

jonahwilliams

LGTM

stuartmorgan-g · 2020-03-16T23:56:26Z

@jonahwilliams PTAL; this now includes the new code to use flutter_template_images. Requires flutter/packages#118 to land and be published before it'll work, but it can be reviewed in parallel.

Unit test coverage of template.dart is currently poor, but I did add a new unit test for the new functionality. I also de-global.fs'd it in support of that test.

stuartmorgan-g · 2020-03-17T21:20:20Z

flutter_template_images has been published; this now works end to end.

Hixie · 2020-03-17T21:36:35Z

Can you do me a favour and add a comment to the place that verifies that there's no binaries saying that if anyone wants to add or change images in the templates, they should do it via your package and pointing to the logic for how to do it?

Thanks for setting this logic up, this is great. I'm glad we have a real solution for avoiding images in the templates now.

stuartmorgan-g · 2020-03-17T23:20:09Z

Can you do me a favour and add a comment to the place that verifies that there's no binaries saying that if anyone wants to add or change images in the templates, they should do it via your package and pointing to the logic for how to do it?

Done. I meant to do that once I had something working, thanks for the reminder.

stuartmorgan-g · 2020-03-17T23:20:51Z

For reasons I don't understand adding a new dependency made me run update-packages --force-upgrade, which made a ton of changes, but none of them related to the package that I added. Is that normal?

jonahwilliams · 2020-03-18T15:46:23Z

packages/flutter_tools/lib/src/template.dart

+// Returns the directory containing the 'name' template directory in
+// flutter_template_images, to resolve image placeholder against.
+Directory templateImageDirectory(String name, FileSystem fileSystem) {
+  final String packageFilePath = fileSystem.path.join(


There might be cases where this .packages file doesn't exist. I think the zip uploaded SDKs do not include the packages since they would contains paths of the zipping machine they would be incorrect for clients.

Less commonly, the other way this could go wrong is that the user could update or delete their pub cache. Since the tool is snapshotted, it is still able to start even if the .packages is pointing to junk.

First you need to check for the existence of the .packages and the imagePackageLibDir. If either is missing you'll need to run a pub get in the flutter_tools directory. It might be easier to hoist this work out of template creation into a separate service class and drill that down instead

Done, thanks for flagging that as I'd never have hit it in my testing.

I haven't done a full sweep of tests locally so I may need to adjust the environment in some other tests the way I've done in project_tests.

jonahwilliams

This should handle missing .packages/cache as noted in my comment above, otherwise looks good!

…a filesystem

jonahwilliams

LGTM

Recreates testbed/windows/ from the new template added in flutter/flutter#51895 Diffs that don't ignore whitespace show most files have changed completely due to differences in how the two repos handle line endings on checkout; actual diffs are very small.

Hixie · 2020-03-23T21:17:47Z

For reasons I don't understand adding a new dependency made me run update-packages --force-upgrade, which made a ton of changes, but none of them related to the package that I added. Is that normal?

This is normal. When you add a dependency, we have to recalculate the full dependency chart so that we can make sure all the packages in the repo use the same versions of each package.

Hixie · 2020-03-23T21:18:14Z

(ideally we'd have a bot that does this daily so it would never be a lot of changes at once)

stuartmorgan-g added 5 commits March 3, 2020 23:12

Windows template and tests

f3a9ac3

Add create switch, and tests for create

6de7ab2

Minor fixes

58188dc

copypasta

af56bd8

Minor fixes

28f5a65

stuartmorgan-g requested a review from jonahwilliams March 3, 2020 22:44

fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Mar 3, 2020

googlebot added the cla: yes label Mar 3, 2020

Template typo fixes

f8f5d96

jonahwilliams reviewed Mar 4, 2020

View reviewed changes

packages/flutter_tools/templates/app/windows.tmpl/Runner.vcxproj.tmpl Show resolved Hide resolved

stuartmorgan-g added 2 commits March 4, 2020 10:21

Whitelist ico files

3d1d573

Use UTF-8 for Runner.rc; needs manual testing

83bf1be

Merge branch 'master' into windows-create-pre-stable

03d090c

Trim down .gitignore

401ac33

jonahwilliams approved these changes Mar 11, 2020

View reviewed changes

Switch to out-of-repo image storage

48f9fe5

stuartmorgan-g requested a review from jonahwilliams March 16, 2020 23:56

Merge branch 'master' into windows-create-pre-stable

24934d2

Add a note about template images in the binary check

e10f5f3

stuartmorgan-g added 3 commits March 17, 2020 19:06

Fix analyzer warning

ec03a74

Merge branch 'master' into windows-create-pre-stable

3d376a6

flutter update-packages --force-upgrade

e0ea3a9

stuartmorgan-g force-pushed the windows-create-pre-stable branch from 75120eb to e0ea3a9 Compare March 18, 2020 02:09

Handle missing .packages file to minimize impact to tests

af0d43c

jonahwilliams reviewed Mar 18, 2020

View reviewed changes

jonahwilliams suggested changes Mar 18, 2020

View reviewed changes

stuartmorgan-g added 5 commits March 18, 2020 13:46

Handle missing .packages and/or package

455706e

Test the new pub get logic. Includes converting PackageMap to taking …

b1f2dcf

…a filesystem

Merge branch 'master' into windows-create-pre-stable

5433277

Add another edge case test

ab053e8

Fix create_usage_test

caf4079

jonahwilliams approved these changes Mar 19, 2020

View reviewed changes

stuartmorgan-g mentioned this pull request Mar 19, 2020

Regenerate Windows testbed from template google/flutter-desktop-embedding#699

Merged

stuartmorgan-g merged commit 685e9d1 into flutter:master Mar 23, 2020

stuartmorgan-g deleted the windows-create-pre-stable branch March 23, 2020 17:42

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

Add pre-stable support for create on Windows #51895

Add pre-stable support for create on Windows #51895

Uh oh!

Conversation

stuartmorgan-g commented Mar 3, 2020

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

stuartmorgan-g commented Mar 3, 2020

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!

Uh oh!

jonahwilliams commented Mar 4, 2020

Uh oh!

stuartmorgan-g commented Mar 4, 2020

Uh oh!

stuartmorgan-g commented Mar 11, 2020

Uh oh!

stuartmorgan-g commented Mar 11, 2020

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g commented Mar 16, 2020

Uh oh!

stuartmorgan-g commented Mar 17, 2020

Uh oh!

Hixie commented Mar 17, 2020

Uh oh!

stuartmorgan-g commented Mar 17, 2020

Uh oh!

stuartmorgan-g commented Mar 17, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

Hixie commented Mar 23, 2020

Uh oh!

Hixie commented Mar 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants