Be more resilient to parse failures. (#1488)
In particular, "pub get --offline" and "pub cache repair" shouldn't
choke when there are parse failures in the version cache.
Closes #1479
diff --git a/lib/src/pubspec.dart b/lib/src/pubspec.dart
index 2186cde..d85ed7f 100644
--- a/lib/src/pubspec.dart
+++ b/lib/src/pubspec.dart
@@ -427,7 +427,13 @@
/// [Version.none].
factory Pubspec.parse(String contents, SourceRegistry sources,
{String expectedName, Uri location}) {
- var pubspecNode = loadYamlNode(contents, sourceUrl: location);
+ YamlNode pubspecNode;
+ try {
+ pubspecNode = loadYamlNode(contents, sourceUrl: location);
+ } on YamlException catch (error) {
+ throw new PubspecException(error.message, error.span);
+ }
+
Map pubspecMap;
if (pubspecNode is YamlScalar && pubspecNode.value == null) {
pubspecMap = new YamlMap(sourceUrl: location);
diff --git a/lib/src/solver/backtracking_solver.dart b/lib/src/solver/backtracking_solver.dart
index 42f1cd6..fb1a104 100644
--- a/lib/src/solver/backtracking_solver.dart
+++ b/lib/src/solver/backtracking_solver.dart
@@ -468,6 +468,10 @@
var pubspec;
try {
pubspec = await _getPubspec(id);
+ } on PubspecException catch (error) {
+ // The lockfile for the pubspec couldn't be parsed,
+ log.fine("Failed to parse pubspec for $id:\n$error");
+ throw new NoVersionException(id.name, null, id.version, []);
} on PackageNotFoundException {
// We can only get here if the lockfile refers to a specific package
// version that doesn't exist (probably because it was yanked).
diff --git a/lib/src/source/git.dart b/lib/src/source/git.dart
index b246f36..0ff40a9 100644
--- a/lib/src/source/git.dart
+++ b/lib/src/source/git.dart
@@ -4,7 +4,7 @@
import 'dart:async';
-import 'package:path/path.dart' as path;
+import 'package:path/path.dart' as p;
import 'package:pub_semver/pub_semver.dart';
import '../git.dart' as git;
@@ -221,7 +221,7 @@
"Please ensure Git is correctly installed.");
}
- ensureDir(path.join(systemCacheRoot, 'cache'));
+ ensureDir(p.join(systemCacheRoot, 'cache'));
await _ensureRevision(ref, id.description['resolved-ref']);
var revisionCachePath = getDirectory(id);
@@ -234,7 +234,7 @@
}
/// Returns the path to the revision-specific cache of [id].
- String getDirectory(PackageId id) => path.join(
+ String getDirectory(PackageId id) => p.join(
systemCacheRoot, "${id.name}-${id.description['resolved-ref']}");
List<Package> getCachedPackages() {
@@ -252,9 +252,19 @@
var failures = <PackageId>[];
var packages = listDir(systemCacheRoot)
- .where((entry) => dirExists(path.join(entry, ".git")))
- .map((packageDir) => new Package.load(
- null, packageDir, systemCache.sources))
+ .where((entry) => dirExists(p.join(entry, ".git")))
+ .map((packageDir) {
+ try {
+ return new Package.load(null, packageDir, systemCache.sources);
+ } catch (error, stackTrace) {
+ log.error("Failed to load package", error, stackTrace);
+ var name = p.basename(packageDir).split('-').first;
+ failures.add(new PackageId(name, source, Version.none, '???'));
+ tryDeleteEntry(packageDir);
+ return null;
+ }
+ })
+ .where((package) => package != null)
.toList();
// Note that there may be multiple packages with the same name and version
@@ -385,6 +395,6 @@
/// [id] (the one in `<system cache>/git/cache`).
String _repoCachePath(PackageRef ref) {
var repoCacheName = '${ref.name}-${sha1(ref.description['url'])}';
- return path.join(systemCacheRoot, 'cache', repoCacheName);
+ return p.join(systemCacheRoot, 'cache', repoCacheName);
}
}
diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart
index 5c2b76b..92e4b48 100644
--- a/lib/src/source/hosted.dart
+++ b/lib/src/source/hosted.dart
@@ -9,6 +9,7 @@
import 'package:http/http.dart' as http;
import 'package:path/path.dart' as p;
import 'package:pub_semver/pub_semver.dart';
+import 'package:stack_trace/stack_trace.dart';
import '../exceptions.dart';
import '../http.dart';
@@ -217,7 +218,18 @@
for (var serverDir in listDir(systemCacheRoot)) {
var url = _directoryToUrl(p.basename(serverDir));
- var packages = _getCachedPackagesInDirectory(p.basename(serverDir));
+
+ var packages = [];
+ for (var entry in listDir(serverDir)) {
+ try {
+ packages.add(new Package.load(null, entry, systemCache.sources));
+ } catch (error, stackTrace) {
+ log.error("Failed to load package", error, stackTrace);
+ failures.add(_idForBasename(p.basename(entry)));
+ tryDeleteEntry(entry);
+ }
+ }
+
packages.sort(Package.orderByNameAndVersion);
for (var package in packages) {
@@ -242,20 +254,37 @@
return new Pair(successes, failures);
}
+ /// Returns the best-guess package ID for [basename], which should be a
+ /// subdirectory in a hosted cache.
+ PackageId _idForBasename(String basename) {
+ var components = split1(basename, '-');
+ var version = Version.none;
+ if (components.length > 1) {
+ try {
+ version = new Version.parse(components.last);
+ } catch (_) {
+ // Default to Version.none.
+ }
+ }
+ return new PackageId(components.first, source, version, components.first);
+ }
+
/// Gets all of the packages that have been downloaded into the system cache
/// from the default server.
- List<Package> getCachedPackages() =>
- _getCachedPackagesInDirectory(_urlToDirectory(source.defaultUrl));
-
- /// Gets all of the packages that have been downloaded into the system cache
- /// into [dir].
- List<Package> _getCachedPackagesInDirectory(String dir) {
- var cacheDir = p.join(systemCacheRoot, dir);
+ List<Package> getCachedPackages() {
+ var cacheDir = p.join(systemCacheRoot, _urlToDirectory(source.defaultUrl));
if (!dirExists(cacheDir)) return [];
- return listDir(cacheDir)
- .map((entry) => new Package.load(null, entry, systemCache.sources))
- .toList();
+ return listDir(cacheDir).map((entry) {
+ try {
+ return new Package.load(null, entry, systemCache.sources);
+ } catch (error, stackTrace) {
+ log.fine(
+ "Failed to load package from $entry:\n"
+ "$error\n"
+ "${new Chain.forTrace(stackTrace)}");
+ }
+ }).toList();
}
/// Downloads package [package] at [version] from [server], and unpacks it