Don't download via the cache in `pub unpack` (#4785)
diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart
index b149d68..d13db32 100644
--- a/lib/src/source/hosted.dart
+++ b/lib/src/source/hosted.dart
@@ -1221,7 +1221,7 @@
'Try again without --offline.',
);
}
- contentHash = await _download(id, packageDir, cache);
+ contentHash = await _downloadAtomically(id, packageDir, cache);
}
return DownloadPackageResult(
PackageId(
@@ -1355,7 +1355,7 @@
);
try {
deleteEntry(package.dir);
- await _download(id, package.dir, cache);
+ await _downloadAtomically(id, package.dir, cache);
return RepairResult(id.name, id.version, this, success: true);
} catch (error, stackTrace) {
var message =
@@ -1444,18 +1444,53 @@
.toList();
}
- Future<void> downloadInto(PackageId id, String destPath, SystemCache cache) =>
- _download(id, destPath, cache);
+ Future<void> downloadInto(
+ PackageId id,
+ String destPath,
+ SystemCache cache,
+ ) async {
+ try {
+ // For the functionality of `unpack` it is important that the unpack
+ // doesn't go via the cache, as that would not allow unpacking into a
+ // directory on a different device than the cache. So we don't use
+ // `_downloadAtomically` here.
+ await _downloadAndExtract(id, destPath, cache);
+ } catch (e) {
+ tryDeleteEntry(destPath);
+ rethrow;
+ }
+ }
/// Downloads package [id] from the archive_url and unpacks it into
- /// [destPath].
+ /// [destPath]. The unpack is done to a temporary directory and then moved
+ /// into [destPath] atomically.
///
/// If there is no archive_url, try to fetch it from
/// `$server/packages/$package/versions/$version.tar.gz` where server comes
/// from `id.description`.
///
/// Returns the content-hash of the downloaded archive.
- Future<Uint8List> _download(
+ Future<Uint8List> _downloadAtomically(
+ PackageId id,
+ String destPath,
+ SystemCache cache,
+ ) async {
+ final tempDir = cache.createTempDir();
+ try {
+ final contentHash = await _downloadAndExtract(id, tempDir, cache);
+ ensureDir(p.dirname(destPath));
+ tryRenameDir(tempDir, destPath);
+ return contentHash;
+ } catch (e) {
+ deleteEntry(tempDir);
+ rethrow;
+ }
+ }
+
+ /// Downloads the archive for [id] and extracts it to [destPath].
+ ///
+ /// Returns the content-hash of the downloaded archive.
+ Future<Uint8List> _downloadAndExtract(
PackageId id,
String destPath,
SystemCache cache,
@@ -1568,25 +1603,11 @@
_throwFriendlyError(error, stackTrace, id.name, description.url);
}
- final tempDir = cache.createTempDir();
try {
- try {
- await extractTarGz(readBinaryFileAsStream(archivePath), tempDir);
- } on FormatException catch (e) {
- dataError('Failed to extract `$archivePath`: ${e.message}.');
- }
- ensureDir(p.dirname(destPath));
- } catch (e) {
- deleteEntry(tempDir);
- rethrow;
+ await extractTarGz(readBinaryFileAsStream(archivePath), destPath);
+ } on FormatException catch (e) {
+ dataError('Failed to extract `$archivePath`: ${e.message}.');
}
- // Now that the get has succeeded, move it to the real location in the
- // cache.
- //
- // If this fails with a "directory not empty" exception we assume that
- // another pub process has installed the same package version while we
- // downloaded.
- tryRenameDir(tempDir, destPath);
return contentHash;
});
}
diff --git a/test/testdata/goldens/embedding/embedding_test/logfile is written with --verbose and on unexpected exceptions.txt b/test/testdata/goldens/embedding/embedding_test/logfile is written with --verbose and on unexpected exceptions.txt
index cf58928..29480b5 100644
--- a/test/testdata/goldens/embedding/embedding_test/logfile is written with --verbose and on unexpected exceptions.txt
+++ b/test/testdata/goldens/embedding/embedding_test/logfile is written with --verbose and on unexpected exceptions.txt
@@ -32,6 +32,7 @@
[E] SLVR: Version solving took: $TIME
[E] | Tried 1 solutions.
[E] FINE: Resolving dependencies finished ($TIME)
+[E] IO : Created temp directory $DIR
[E] IO : Get package from http://localhost:$PORT/packages/foo/versions/1.0.0.tar.gz.
[E] FINE: Downloading foo 1.0.0...
[E] IO : Created temp directory $DIR
@@ -53,7 +54,6 @@
[E] FINE: Contents:
[E] | c35f6caa60870d3b6c5c6aebe5a5d4b56ca4733a52a80ded3445c0c5fa4b62df
[E] FINE: Created $FILE from stream
-[E] IO : Created temp directory $DIR
[E] IO : Reading binary file $FILE.
[E] FINE: Extracting .tar.gz stream to $DIR
[E] IO : Creating $FILE from stream
@@ -61,8 +61,8 @@
[E] IO : Creating $FILE from stream
[E] FINE: Created $FILE from stream
[E] FINE: Extracted .tar.gz to $DIR
-[E] IO : Renaming directory $A to $B
[E] IO : Deleting directory $DIR
+[E] IO : Renaming directory $A to $B
[E] IO : Writing $N characters to text file $SANDBOX/cache/README.md.
[E] FINE: Contents:
[E] | Pub Package Cache
@@ -209,6 +209,7 @@
| Tried 1 solutions.
FINE: Resolving dependencies finished ($TIME)
MSG : Downloading packages...
+IO : Created temp directory $DIR
IO : Get package from http://localhost:$PORT/packages/foo/versions/1.0.0.tar.gz.
FINE: Downloading foo 1.0.0...
IO : Created temp directory $DIR
@@ -230,7 +231,6 @@
FINE: Contents:
| c35f6caa60870d3b6c5c6aebe5a5d4b56ca4733a52a80ded3445c0c5fa4b62df
FINE: Created $FILE from stream
-IO : Created temp directory $DIR
IO : Reading binary file $FILE.
FINE: Extracting .tar.gz stream to $DIR
IO : Creating $FILE from stream
@@ -238,8 +238,8 @@
IO : Creating $FILE from stream
FINE: Created $FILE from stream
FINE: Extracted .tar.gz to $DIR
-IO : Renaming directory $A to $B
IO : Deleting directory $DIR
+IO : Renaming directory $A to $B
IO : Writing $N characters to text file $SANDBOX/cache/README.md.
FINE: Contents:
| Pub Package Cache
