Lazily download artifacts: The Phantom Menace #27374
Lazily download artifacts: The Phantom Menace #27374jonahwilliams merged 35 commits intoflutter:masterfrom
Conversation
| BuildMode getBuildMode() { | ||
| final List<bool> modeFlags = <bool>[argResults['debug'], argResults['profile'], argResults['release']]; | ||
| if (modeFlags.where((bool flag) => flag).length > 1) | ||
| bool debug; |
There was a problem hiding this comment.
I've updated getBuildMode to not throw if used in a command that does not add the debug, profile, or release arguments.
| if (release) { | ||
| return dynamicFlag ? BuildMode.dynamicRelease : BuildMode.release; | ||
|
|
||
| if (_defaultBuildMode == BuildMode.debug && dynamicFlag) |
There was a problem hiding this comment.
These were unnecessary since with my change since I moved the check to the top
| } | ||
|
|
||
| String extraFrontEndOptions = | ||
| List<String> extraFrontEndOptions = |
There was a problem hiding this comment.
This and below was reported to me as a type error.
Fix flutter import
|
An additional benefit: we normally spend around 10 minutes downloading artifacts on CI, this PR is spending around 4-5 |
jonahwilliams
changed the title
goderbauer
added
the
tool
|
Cache issues were due to the engine binary issue |
This reverts commit 98971f3.
This reverts commit 98971f3.
| const UpdateResult({this.isUpToDate, this.clobber}); | ||
|
|
||
| final bool isUpToDate; | ||
| final bool clobber; |
There was a problem hiding this comment.
Can you document these? It's not clear what clobber represents.
There was a problem hiding this comment.
Addressed in linked PR
| }) { | ||
| if (!location.existsSync()) { | ||
| return const UpdateResult(isUpToDate: false, clobber: false); | ||
| } if (version != cache.getStampFor(name)) { |
There was a problem hiding this comment.
Formatting is weird here. Put this on a newline, or put an else in here?
There was a problem hiding this comment.
Addressed in linked PR
|
|
||
| /// Hook method for extra checks for being up-to-date. | ||
| bool isUpToDateInner() => true; | ||
| bool isUpToDateInner({BuildMode buildMode, TargetPlatform targetPlatform, bool skipUnknown}) => true; |
There was a problem hiding this comment.
Rather than having isUpToDateInner(), why not just have subclasses override isUpToDate() and say:
UpdateResult result = super.isUpToDate(...);
if (!result.isUpToDate)
return result;There was a problem hiding this comment.
Addressed in linked PR
| } | ||
|
|
||
| class UpdateResult { | ||
| const UpdateResult({this.isUpToDate, this.clobber}); |
There was a problem hiding this comment.
Default clobber to false?
There was a problem hiding this comment.
Addressed in linked PR
| return false; | ||
| return isUpToDateInner(); | ||
| UpdateResult isUpToDate({ | ||
| BuildMode buildMode, |
There was a problem hiding this comment.
buildMode and targetPlatform should be properties of the CachedArtifact itself, rather than exist on the update checks, no?
There was a problem hiding this comment.
These are passed through so that we can filter down the set of artifacts based on the current known configurations.
jonahwilliams
changed the title
5 participants
Fixes #6491
Only download the minimum*** set of artifacts needed for a particular host, target, and build mode.
*** I still include gradle and fonts in the minimum set.
Adds additional arguments to several Cache methods:
-skipUnknown, which allows downloading artifacts when targetPlatform and hostPlatform are unknown.
flutter precachewill still download the full set of artifacts for a given host platform.flutter precache -f/--forcewill re download all artifacts.Re-organizes engine artifacts into a class from lists of lists of strings.
Flutter commands now check the artifacts for the given or default build mode, and any connected devices. (note: android-arm-profile is downloaded unconditionally because we currently need it for a Flutter doctor check).
This is accomplished via a new method on FlutterCommand called
updateCache. This is only called ifshouldUpdateCacheis true. The default implementation is:Run, for example, looks at the current set of attached devices and ensures the binaries are downloaded for each kind.
Example doctor command: