From 33d8a03545ebacd821aa9cd688156e792c9c6fff Mon Sep 17 00:00:00 2001 From: jcollins-g Date: Tue, 9 Jan 2018 12:15:58 -0800 Subject: [PATCH] Revert "Make artifact downloading more robust for flaky networks" (#13995) * Revert "Change github readme links to be served by github (#13929)" This reverts commit fd516d9538e23dcfebf48ddc0efd7041fe8068cd. * Revert " Rename SemanticsFlags to SemanticsFlag (#13994)" This reverts commit 24e3f7053671713e05ee6be29c5ba88ed8fce543. * Revert "Make artifact downloading more robust for flaky networks (#13984)" This reverts commit 4d37e03e1d93eb88ed47a43fe5b08a25dca86171. --- packages/flutter_tools/lib/src/base/os.dart | 36 ------- packages/flutter_tools/lib/src/cache.dart | 102 ++++++-------------- 2 files changed, 29 insertions(+), 109 deletions(-) diff --git a/packages/flutter_tools/lib/src/base/os.dart b/packages/flutter_tools/lib/src/base/os.dart index a1c7d8abc9d..476cae1242a 100644 --- a/packages/flutter_tools/lib/src/base/os.dart +++ b/packages/flutter_tools/lib/src/base/os.dart @@ -47,14 +47,8 @@ abstract class OperatingSystemUtils { 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. @@ -105,18 +99,12 @@ class _PosixUtils extends OperatingSystemUtils { runSync(['unzip', '-o', '-q', file.path, '-d', targetDirectory.path]); } - @override - bool verifyZip(File zipFile) => exitsHappy(['zip', '-T', zipFile.path]); - // tar -xzf tarball -C dest @override void unpack(File gzippedTarFile, Directory targetDirectory) { runSync(['tar', '-xzf', gzippedTarFile.path, '-C', targetDirectory.path]); } - @override - bool verifyGzip(File gzippedFile) => exitsHappy(['gzip', '-t', gzippedFile.path]); - @override File makePipe(String path) { runSync(['mkfifo', path]); @@ -190,18 +178,6 @@ class _WindowsUtils extends OperatingSystemUtils { _unpackArchive(archive, targetDirectory); } - @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( @@ -210,18 +186,6 @@ class _WindowsUtils extends OperatingSystemUtils { _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 503ffbd8c90..fd9dcb9f4ab 100644 --- a/packages/flutter_tools/lib/src/cache.dart +++ b/packages/flutter_tools/lib/src/cache.dart @@ -162,9 +162,6 @@ class Cache { 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'); @@ -235,12 +232,6 @@ abstract class CachedArtifact { 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 _downloadedFiles = []; - bool isUpToDate() { if (!location.existsSync()) return false; @@ -255,14 +246,6 @@ abstract class CachedArtifact { 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. @@ -280,36 +263,6 @@ abstract class CachedArtifact { } Uri _toStorageUri(String path) => Uri.parse('$_storageBaseUrl/$path'); - - /// Download a zip archive from the given [url] and unzip it to [location]. - Future _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 _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 _withDownloadFile(String name, Future 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; @@ -493,32 +446,6 @@ class GradleWrapper extends CachedArtifact { } } -// Many characters are problematic in filenames, especially on Windows. -final Map> _flattenNameSubstitutions = >{ - 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 replacedCodeUnits = []; - for (int codeUnit in urlName.codeUnits) { - replacedCodeUnits.addAll(_flattenNameSubstitutions[codeUnit] ?? [codeUnit]); - } - return new String.fromCharCodes(replacedCodeUnits); -} - /// Download a file from the given [url] and write it to [location]. Future _downloadFile(Uri url, File location) async { _ensureExists(location.parent); @@ -526,6 +453,35 @@ Future _downloadFile(Uri url, File location) async { location.writeAsBytesSync(fileBytes, flush: true); } +/// Download a zip archive from the given [url] and unzip it to [location]. +Future _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 _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 _withTemporaryFile(String name, Future 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())