Make artifact downloading more robust for flaky networks (#13984)
* First version
* Add a clarifying comment
* Add verify to ZipDecoder.decodeBytes call
* Review comments
* Need OS Error for missing files
* FileSystemException, not OSError.
diff --git a/packages/flutter_tools/lib/src/base/os.dart b/packages/flutter_tools/lib/src/base/os.dart
index 476cae1..a1c7d8a 100644
--- a/packages/flutter_tools/lib/src/base/os.dart
+++ b/packages/flutter_tools/lib/src/base/os.dart
@@ -47,8 +47,14 @@
void unzip(File file, Directory targetDirectory);
+ /// Returns true if the ZIP is not corrupt.
+ bool verifyZip(File file);
+
void unpack(File gzippedTarFile, Directory targetDirectory);
+ /// Returns true if the gzip is not corrupt (does not check tar).
+ bool verifyGzip(File gzippedFile);
+
/// Returns a pretty name string for the current operating system.
///
/// If available, the detailed version of the OS is included.
@@ -99,6 +105,9 @@
runSync(<String>['unzip', '-o', '-q', file.path, '-d', targetDirectory.path]);
}
+ @override
+ bool verifyZip(File zipFile) => exitsHappy(<String>['zip', '-T', zipFile.path]);
+
// tar -xzf tarball -C dest
@override
void unpack(File gzippedTarFile, Directory targetDirectory) {
@@ -106,6 +115,9 @@
}
@override
+ bool verifyGzip(File gzippedFile) => exitsHappy(<String>['gzip', '-t', gzippedFile.path]);
+
+ @override
File makePipe(String path) {
runSync(<String>['mkfifo', path]);
return fs.file(path);
@@ -179,6 +191,18 @@
}
@override
+ bool verifyZip(File zipFile) {
+ try {
+ new ZipDecoder().decodeBytes(zipFile.readAsBytesSync(), verify: true);
+ } on FileSystemException catch (_) {
+ return false;
+ } on ArchiveException catch (_) {
+ return false;
+ }
+ return true;
+ }
+
+ @override
void unpack(File gzippedTarFile, Directory targetDirectory) {
final Archive archive = new TarDecoder().decodeBytes(
new GZipDecoder().decodeBytes(gzippedTarFile.readAsBytesSync()),
@@ -186,6 +210,18 @@
_unpackArchive(archive, targetDirectory);
}
+ @override
+ bool verifyGzip(File gzipFile) {
+ try {
+ new GZipDecoder().decodeBytes(gzipFile.readAsBytesSync(), verify: true);
+ } on FileSystemException catch (_) {
+ return false;
+ } on ArchiveException catch (_) {
+ return false;
+ }
+ return true;
+ }
+
void _unpackArchive(Archive archive, Directory targetDirectory) {
for (ArchiveFile archiveFile in archive.files) {
// The archive package doesn't correctly set isFile.
diff --git a/packages/flutter_tools/lib/src/cache.dart b/packages/flutter_tools/lib/src/cache.dart
index fd9dcb9..503ffbd 100644
--- a/packages/flutter_tools/lib/src/cache.dart
+++ b/packages/flutter_tools/lib/src/cache.dart
@@ -162,6 +162,9 @@
return dir;
}
+ /// Return the top-level directory for artifact downloads.
+ Directory getDownloadDir() => getCacheDir('downloads');
+
/// Return the top-level mutable directory in the cache; this is `bin/cache/artifacts`.
Directory getCacheArtifacts() => getCacheDir('artifacts');
@@ -232,6 +235,12 @@
Directory get location => cache.getArtifactDirectory(name);
String get version => cache.getVersionFor(name);
+ /// Keep track of the files we've downloaded for this execution so we
+ /// can delete them after completion. We don't delete them right after
+ /// extraction in case [update] is interrupted, so we can restart without
+ /// starting from scratch.
+ final List<File> _downloadedFiles = <File>[];
+
bool isUpToDate() {
if (!location.existsSync())
return false;
@@ -246,6 +255,14 @@
location.createSync(recursive: true);
await updateInner();
cache.setStampFor(name, version);
+ _removeDownloadedFiles();
+ }
+
+ /// Clear any zip/gzip files downloaded.
+ void _removeDownloadedFiles() {
+ for (File f in _downloadedFiles) {
+ f.deleteSync();
+ }
}
/// Hook method for extra checks for being up-to-date.
@@ -263,6 +280,36 @@
}
Uri _toStorageUri(String path) => Uri.parse('$_storageBaseUrl/$path');
+
+ /// Download a zip archive from the given [url] and unzip it to [location].
+ Future<Null> _downloadZipArchive(Uri url, Directory location) {
+ return _withDownloadFile('${_flattenName(url)}.zip', (File tempFile) async {
+ if (!os.verifyZip(tempFile)) {
+ await _downloadFile(url, tempFile);
+ }
+ _ensureExists(location);
+ os.unzip(tempFile, location);
+ });
+ }
+
+ /// Download a gzipped tarball from the given [url] and unpack it to [location].
+ Future<Null> _downloadZippedTarball(Uri url, Directory location) {
+ return _withDownloadFile('${_flattenName(url)}.tgz', (File tempFile) async {
+ if (!os.verifyGzip(tempFile)) {
+ await _downloadFile(url, tempFile);
+ }
+ _ensureExists(location);
+ os.unpack(tempFile, location);
+ });
+ }
+
+ /// Create a temporary file and invoke [onTemporaryFile] with the file as
+ /// argument, then add the temporary file to the [_downloadedFiles].
+ Future<Null> _withDownloadFile(String name, Future<Null> onTemporaryFile(File file)) async {
+ final File tempFile = fs.file(fs.path.join(cache.getDownloadDir().path, name));
+ _downloadedFiles.add(tempFile);
+ await onTemporaryFile(tempFile);
+ }
}
bool _hasWarnedAboutStorageOverride = false;
@@ -446,6 +493,32 @@
}
}
+// Many characters are problematic in filenames, especially on Windows.
+final Map<int, List<int>> _flattenNameSubstitutions = <int, List<int>>{
+ r'@'.codeUnitAt(0): '@@'.codeUnits,
+ r'/'.codeUnitAt(0): '@s@'.codeUnits,
+ r'\'.codeUnitAt(0): '@bs@'.codeUnits,
+ r':'.codeUnitAt(0): '@c@'.codeUnits,
+ r'%'.codeUnitAt(0): '@per@'.codeUnits,
+ r'*'.codeUnitAt(0): '@ast@'.codeUnits,
+ r'<'.codeUnitAt(0): '@lt@'.codeUnits,
+ r'>'.codeUnitAt(0): '@gt@'.codeUnits,
+ r'"'.codeUnitAt(0): '@q@'.codeUnits,
+ r'|'.codeUnitAt(0): '@pip@'.codeUnits,
+ r'?'.codeUnitAt(0): '@ques@'.codeUnits
+};
+
+/// Given a name containing slashes, colons, and backslashes, expand it into
+/// something that doesn't.
+String _flattenName(Uri url) {
+ final String urlName = url.toString();
+ final List<int> replacedCodeUnits = <int>[];
+ for (int codeUnit in urlName.codeUnits) {
+ replacedCodeUnits.addAll(_flattenNameSubstitutions[codeUnit] ?? <int>[codeUnit]);
+ }
+ return new String.fromCharCodes(replacedCodeUnits);
+}
+
/// Download a file from the given [url] and write it to [location].
Future<Null> _downloadFile(Uri url, File location) async {
_ensureExists(location.parent);
@@ -453,35 +526,6 @@
location.writeAsBytesSync(fileBytes, flush: true);
}
-/// Download a zip archive from the given [url] and unzip it to [location].
-Future<Null> _downloadZipArchive(Uri url, Directory location) {
- return _withTemporaryFile('download.zip', (File tempFile) async {
- await _downloadFile(url, tempFile);
- _ensureExists(location);
- os.unzip(tempFile, location);
- });
-}
-
-/// Download a gzipped tarball from the given [url] and unpack it to [location].
-Future<Null> _downloadZippedTarball(Uri url, Directory location) {
- return _withTemporaryFile('download.tgz', (File tempFile) async {
- await _downloadFile(url, tempFile);
- _ensureExists(location);
- os.unpack(tempFile, location);
- });
-}
-
-/// Create a file with the given name in a new temporary directory, invoke
-/// [onTemporaryFile] with the file as argument, then delete the temporary
-/// directory.
-Future<Null> _withTemporaryFile(String name, Future<Null> onTemporaryFile(File file)) async {
- final Directory tempDir = fs.systemTempDirectory.createTempSync();
- final File tempFile = fs.file(fs.path.join(tempDir.path, name));
- await onTemporaryFile(tempFile).whenComplete(() {
- tempDir.delete(recursive: true);
- });
-}
-
/// Create the given [directory] and parents, as necessary.
void _ensureExists(Directory directory) {
if (!directory.existsSync())
