Skip to content

Commit 88ef361

Browse files
Mr-Pepebkonyi
andauthored
Add progress indicator to artifact downloads (#181808)
<!-- Thanks for filing a pull request! Reviewers are typically assigned within a week of filing a request. To learn more about code review, see our documentation on Tree Hygiene: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md --> This is a first step towards resolving #14268. This change adds a simple progress indicator telling the user how many artifact (sets) need downloading. The output also makes the structure of artifact sets clearer by indenting the artifacts inside a set. Here is the output of downloading all artifacts: <details><summary>Details</summary> <p> ``` ❯ flutter precache -f -a Building flutter tool... Resolving dependencies... Downloading packages... Got dependencies. Downloading Flutter dependencies... [1/18] Material Fonts 278ms [2/18] Gradle Wrapper 28ms [3/18] Android SDK ├─ [1/21] android-arm-profile/darwin-x64 303ms ├─ [2/21] android-arm-release/darwin-x64 243ms ├─ [3/21] android-arm64-profile/darwin-x64 787ms ├─ [4/21] android-arm64-release/darwin-x64 235ms ├─ [5/21] android-x64-profile/darwin-x64 365ms ├─ [6/21] android-x64-release/darwin-x64 249ms ├─ [7/21] android-arm-profile/linux-x64 189ms ├─ [8/21] android-arm-release/linux-x64 181ms ├─ [9/21] android-arm64-profile/linux-x64 184ms ├─ [10/21] android-arm64-release/linux-x64 1,009ms ├─ [11/21] android-x64-profile/linux-x64 275ms ├─ [12/21] android-x64-release/linux-x64 179ms ├─ [13/21] android-arm-profile/windows-x64 164ms ├─ [14/21] android-arm-release/windows-x64 178ms ├─ [15/21] android-arm64-profile/windows-x64 187ms ├─ [16/21] android-arm64-release/windows-x64 183ms ├─ [17/21] android-x64-profile/windows-x64 996ms ├─ [18/21] android-x64-release/windows-x64 193ms ├─ [19/21] dart-sdk-darwin-x64 6.6s ├─ [20/21] dart-sdk-linux-x64 5.5s └─ [21/21] dart-sdk-windows-x64 5.3s [4/18] Android Internal Build Artifacts ├─ [1/10] android-x86 4.8s ├─ [2/10] android-x64 4.8s ├─ [3/10] android-arm 4.9s ├─ [4/10] android-arm-profile 2,507ms ├─ [5/10] android-arm-release 1,626ms ├─ [6/10] android-arm64 5.0s ├─ [7/10] android-arm64-profile 2,204ms ├─ [8/10] android-arm64-release 2,161ms ├─ [9/10] android-x64-profile 2,374ms └─ [10/10] android-x64-release 2,179ms [5/18] iOS SDK ├─ [1/3] ios 3.1s ├─ [2/3] ios-profile 2,606ms └─ [3/3] ios-release 4.0s [6/18] Web SDK 2,075ms [7/18] Engine Information 161ms [8/18] Flutter SDK ├─ [1/7] sky_engine 144ms ├─ [2/7] flutter_gpu 358ms ├─ [3/7] flutter_patched_sdk 202ms ├─ [4/7] flutter_patched_sdk_product 179ms ├─ [5/7] windows-x64 2,010ms ├─ [6/7] linux-x64 1,652ms └─ [7/7] darwin-x64 1,856ms [9/18] Windows SDK ├─ [1/4] windows-x64-debug/windows-x64-flutter 3.1s ├─ [2/4] windows-x64/flutter-cpp-client-wrapper 147ms ├─ [3/4] windows-x64-profile/windows-x64-flutter 3.6s └─ [4/4] windows-x64-release/windows-x64-flutter 3.6s [10/18] MacOS SDK ├─ [1/8] darwin-x64/framework 2,017ms ├─ [2/8] darwin-x64/gen_snapshot 1,204ms ├─ [3/8] darwin-x64-profile/framework 1,329ms ├─ [4/8] darwin-x64-profile 978ms ├─ [5/8] darwin-x64-profile/gen_snapshot 680ms ├─ [6/8] darwin-x64-release/framework 3.9s ├─ [7/8] darwin-x64-release 1,006ms └─ [8/8] darwin-x64-release/gen_snapshot 1,271ms [11/18] Linux SDK ├─ [1/3] linux-x64-debug/linux-x64-flutter-gtk 2,293ms ├─ [2/3] linux-x64-profile/linux-x64-flutter-gtk 1,485ms └─ [3/3] linux-x64-release/linux-x64-flutter-gtk 1,462ms [12/18] iOS USB Artifact: libimobiledevice 43ms [13/18] iOS USB Artifact: libusbmuxd 34ms [14/18] iOS USB Artifact: libplist 25ms [15/18] iOS USB Artifact: openssl 55ms [16/18] iOS USB Artifact: libimobiledeviceglue 26ms [17/18] iOS USB Artifact: ios-deploy 26ms [18/18] Font Subsets ├─ [1/3] darwin-x64/font-subset 956ms ├─ [2/3] linux-x64/font-subset 172ms └─ [3/3] windows-x64/font-subset 168ms ``` </p> </details> Each artifact set now has a pretty display name. It would look nicer if the iOS USB artifacts were bundled in one artifact set but maybe there is a reason why they are separate? ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md Co-authored-by: Ben Konyi <[email protected]>
1 parent 4f5478c commit 88ef361

3 files changed

Lines changed: 301 additions & 56 deletions

File tree

‎packages/flutter_tools/lib/src/cache.dart‎

Lines changed: 109 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -744,11 +744,12 @@ class Cache {
744744
return true;
745745
}
746746

747-
/// Update the cache to contain all `requiredArtifacts`.
748-
Future<void> updateAll(Set<DevelopmentArtifact> requiredArtifacts, {bool offline = false}) async {
749-
if (!_lockEnabled) {
750-
return;
751-
}
747+
/// Returns the list of artifacts that need updating from [requiredArtifacts].
748+
Future<List<ArtifactSet>> _collectArtifactsToUpdate(
749+
Set<DevelopmentArtifact> requiredArtifacts,
750+
) async {
751+
final artifactsToUpdate = <ArtifactSet>[];
752+
752753
for (final ArtifactSet artifact in _artifacts) {
753754
if (!requiredArtifacts.contains(artifact.developmentArtifact)) {
754755
_logger.printTrace('Artifact $artifact is not required, skipping update.');
@@ -757,6 +758,41 @@ class Cache {
757758
if (await artifact.isUpToDate(_fileSystem)) {
758759
continue;
759760
}
761+
artifactsToUpdate.add(artifact);
762+
}
763+
return artifactsToUpdate;
764+
}
765+
766+
/// Update the cache to contain all `requiredArtifacts`.
767+
Future<void> updateAll(Set<DevelopmentArtifact> requiredArtifacts, {bool offline = false}) async {
768+
if (!_lockEnabled) {
769+
return;
770+
}
771+
772+
final List<ArtifactSet> artifactsToUpdate = await _collectArtifactsToUpdate(requiredArtifacts);
773+
774+
if (artifactsToUpdate.isEmpty) {
775+
return;
776+
}
777+
778+
// Download artifacts and display progress
779+
final int total = artifactsToUpdate.length;
780+
for (var i = 0; i < artifactsToUpdate.length; i++) {
781+
final ArtifactSet artifact = artifactsToUpdate[i];
782+
final int current = i + 1;
783+
784+
// Set progress context for the artifact updater
785+
_artifactUpdater.setProgressContext(
786+
artifactIndex: current,
787+
artifactTotal: total,
788+
downloadTotal: artifact.downloadCount,
789+
);
790+
791+
// For artifacts containing multiple downloads, print the artifact name
792+
if (artifact.downloadCount > 1) {
793+
_logger.printStatus('[$current/$total] ${artifact.displayName}');
794+
}
795+
760796
try {
761797
await artifact.update(_artifactUpdater, _logger, _fileSystem, _osUtils, offline: offline);
762798
} on SocketException catch (e) {
@@ -771,6 +807,7 @@ class Cache {
771807
rethrow;
772808
}
773809
}
810+
_artifactUpdater.resetProgressContext();
774811
}
775812

776813
Future<bool> areRemoteArtifactsAvailable({
@@ -828,10 +865,20 @@ abstract class ArtifactSet {
828865
/// The canonical name of the artifact.
829866
String get name;
830867

868+
/// A prettier display name.
869+
///
870+
/// Defaults to the canonical name.
871+
String get displayName => name;
872+
831873
/// The name of the stamp file.
832874
///
833875
/// Defaults to the same as the artifact name.
834876
String get stampName => name;
877+
878+
/// The number of individual downloads this artifact will perform.
879+
///
880+
/// Defaults to 1.
881+
int get downloadCount => 1;
835882
}
836883

837884
/// An artifact set managed by the cache.
@@ -930,6 +977,9 @@ abstract class EngineCachedArtifact extends CachedArtifact {
930977
@override
931978
String? get version => cache.engineRevision;
932979

980+
@override
981+
int get downloadCount => getPackageDirs().length + getBinaryDirs().length;
982+
933983
/// Return a list of (directory path, download URL path) tuples.
934984
List<List<String>> getBinaryDirs();
935985

@@ -975,25 +1025,16 @@ abstract class EngineCachedArtifact extends CachedArtifact {
9751025

9761026
final Directory pkgDir = cache.getCacheDir('pkg');
9771027
for (final String pkgName in getPackageDirs()) {
978-
await artifactUpdater.downloadZipArchive(
979-
'Downloading package $pkgName...',
980-
Uri.parse('$url$pkgName.zip'),
981-
pkgDir,
982-
);
1028+
await artifactUpdater.downloadZipArchive(pkgName, Uri.parse('$url$pkgName.zip'), pkgDir);
9831029
}
9841030

9851031
for (final List<String> toolsDir in getBinaryDirs()) {
9861032
final String cacheDir = toolsDir[0];
9871033
final String urlPath = toolsDir[1];
9881034
final Directory dir = fileSystem.directory(fileSystem.path.join(location.path, cacheDir));
9891035

990-
// Avoid printing things like 'Downloading linux-x64 tools...' multiple times.
9911036
final String friendlyName = urlPath.replaceAll('/artifacts.zip', '').replaceAll('.zip', '');
992-
await artifactUpdater.downloadZipArchive(
993-
'Downloading $friendlyName tools...',
994-
Uri.parse(url + urlPath),
995-
dir,
996-
);
1037+
await artifactUpdater.downloadZipArchive(friendlyName, Uri.parse(url + urlPath), dir);
9971038

9981039
_makeFilesExecutable(dir, operatingSystemUtils);
9991040
}
@@ -1094,6 +1135,36 @@ class ArtifactUpdater {
10941135
@visibleForTesting
10951136
final downloadedFiles = <File>[];
10961137

1138+
// Progress tracking state for download output formatting.
1139+
int _artifactIndex = 0;
1140+
int _artifactTotal = 0;
1141+
int _downloadIndex = 0;
1142+
int _downloadTotal = 0;
1143+
1144+
/// Sets the progress context for artifact downloads.
1145+
///
1146+
/// This is called before each artifact update to enable progress output.
1147+
/// The [downloadIndex] can be used to set the current download index
1148+
/// within an artifact (1-based).
1149+
void setProgressContext({
1150+
required int artifactIndex,
1151+
required int artifactTotal,
1152+
required int downloadTotal,
1153+
int downloadIndex = 0,
1154+
}) {
1155+
_artifactIndex = artifactIndex;
1156+
_artifactTotal = artifactTotal;
1157+
_downloadIndex = downloadIndex;
1158+
_downloadTotal = downloadTotal;
1159+
}
1160+
1161+
void resetProgressContext() {
1162+
_artifactIndex = 0;
1163+
_artifactTotal = 0;
1164+
_downloadIndex = 0;
1165+
_downloadTotal = 0;
1166+
}
1167+
10971168
/// These filenames, should they exist after extracting an archive, should be deleted.
10981169
static const _denylistedBasenames = <String>{
10991170
'entitlements.txt',
@@ -1112,25 +1183,37 @@ class ArtifactUpdater {
11121183
}
11131184

11141185
/// Download a zip archive from the given [url] and unzip it to [location].
1115-
Future<void> downloadZipArchive(String message, Uri url, Directory location) {
1116-
return _downloadArchive(message, url, location, _operatingSystemUtils.unzip);
1186+
Future<void> downloadZipArchive(String artifactName, Uri url, Directory location) {
1187+
return _downloadArchive(artifactName, url, location, _operatingSystemUtils.unzip);
11171188
}
11181189

11191190
/// Download a gzipped tarball from the given [url] and unpack it to [location].
1120-
Future<void> downloadZippedTarball(String message, Uri url, Directory location) {
1121-
return _downloadArchive(message, url, location, _operatingSystemUtils.unpack);
1191+
Future<void> downloadZippedTarball(String artifactName, Uri url, Directory location) {
1192+
return _downloadArchive(artifactName, url, location, _operatingSystemUtils.unpack);
11221193
}
11231194

11241195
/// Download a file from the given [url] and copy it to [location].
1125-
Future<void> downloadFile(String message, Uri url, Directory location) {
1126-
return _downloadArchive(message, url, location, (File file, Directory dir) {
1196+
Future<void> downloadFile(String artifactName, Uri url, Directory location) {
1197+
return _downloadArchive(artifactName, url, location, (File file, Directory dir) {
11271198
file.copySync(dir.childFile(file.basename).path);
11281199
});
11291200
}
11301201

1202+
/// Formats a download message with progress context.
1203+
@visibleForTesting
1204+
String formatProgressMessage(String artifactName) {
1205+
final int displayIndex = _downloadIndex + 1;
1206+
if (_downloadTotal == 1) {
1207+
return '[$_artifactIndex/$_artifactTotal] $artifactName';
1208+
} else {
1209+
final prefix = displayIndex == _downloadTotal ? '└─' : '├─';
1210+
return ' $prefix [$displayIndex/$_downloadTotal] $artifactName';
1211+
}
1212+
}
1213+
11311214
/// Download an archive from the given [url] and unzip it to [location].
11321215
Future<void> _downloadArchive(
1133-
String message,
1216+
String artifactName,
11341217
Uri url,
11351218
Directory location,
11361219
void Function(File, Directory) extractor,
@@ -1139,9 +1222,11 @@ class ArtifactUpdater {
11391222
final File tempFile = _createDownloadFile(downloadPath);
11401223
Status status;
11411224
int retries = _kRetryCount;
1225+
final String formattedMessage = formatProgressMessage(artifactName);
1226+
_downloadIndex++;
11421227

11431228
while (retries > 0) {
1144-
status = _logger.startProgress(message);
1229+
status = _logger.startProgress(formattedMessage);
11451230
try {
11461231
_ensureExists(tempFile.parent);
11471232
if (tempFile.existsSync()) {

0 commit comments

Comments
 (0)