diff --git a/lib/src/pubspec.dart b/lib/src/pubspec.dart index 2186cdef17e236eccc1ade8cf444dce4273eaf95..d85ed7fe53aa03e79caacd081d96d8bab70569a6 100644 --- a/lib/src/pubspec.dart +++ b/lib/src/pubspec.dart @@ -427,7 +427,13 @@ class Pubspec { /// [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 42f1cd65e72b1a471b05610eba273b5815463a11..fb1a10425e153e3dd43303acad795b81d570f21c 100644 --- a/lib/src/solver/backtracking_solver.dart +++ b/lib/src/solver/backtracking_solver.dart @@ -468,6 +468,10 @@ class BacktrackingSolver { 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 b246f36a8f5834c8507a4f3b14a57a47f2255514..0ff40a99e0f931a742abc236e116a24e8c6a47ca 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 @@ class BoundGitSource extends CachedSource { "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 @@ class BoundGitSource extends CachedSource { } /// 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 @@ class BoundGitSource extends CachedSource { 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 @@ class BoundGitSource extends CachedSource { /// [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 5c2b76b6f8ac985e7774d2eeec613c577bb58905..92e4b489bb6d697b424107339d1d8617b5abf5c0 100644 --- a/lib/src/source/hosted.dart +++ b/lib/src/source/hosted.dart @@ -9,6 +9,7 @@ import "dart:convert"; 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 @@ class BoundHostedSource extends CachedSource { 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 @@ class BoundHostedSource extends CachedSource { return new Pair(successes, failures); } - /// Gets all of the packages that have been downloaded into the system cache - /// from the default server. - List<Package> getCachedPackages() => - _getCachedPackagesInDirectory(_urlToDirectory(source.defaultUrl)); + /// 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 - /// into [dir]. - List<Package> _getCachedPackagesInDirectory(String dir) { - var cacheDir = p.join(systemCacheRoot, dir); + /// from the default server. + 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 diff --git a/test/cache/repair/git.dart b/test/cache/repair/git.dart new file mode 100644 index 0000000000000000000000000000000000000000..41527ae69c18c29cde73ea1af5fbedd701a6f94e --- /dev/null +++ b/test/cache/repair/git.dart @@ -0,0 +1,134 @@ +// Copyright (c) 2014, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:path/path.dart' as path; +import 'package:pub/src/exit_codes.dart' as exit_codes; +import 'package:pub/src/io.dart'; +import 'package:scheduled_test/scheduled_test.dart'; + +import '../../descriptor.dart' as d; +import '../../test_pub.dart'; + +main() { + setUp(() { + // Create two cached revisions of foo. + d.git('foo.git', [ + d.libDir('foo'), + d.libPubspec('foo', '1.0.0') + ]).create(); + + d.appDir({"foo": {"git": "../foo.git"}}).create(); + pubGet(); + + d.git('foo.git', [ + d.libDir('foo'), + d.libPubspec('foo', '1.0.1') + ]).commit(); + + pubUpgrade(); + }); + + integration('reinstalls previously cached git packages', () { + // Break them. + List fooDirs; + schedule(() { + // Find the cached foo packages for each revision. + var gitCacheDir = path.join(sandboxDir, cachePath, "git"); + fooDirs = listDir(gitCacheDir) + .where((dir) => path.basename(dir).startsWith("foo-")).toList(); + + // Delete "foo.dart" from them. + for (var dir in fooDirs) { + deleteEntry(path.join(dir, "lib", "foo.dart")); + } + }); + + // Repair them. + schedulePub(args: ["cache", "repair"], + output: ''' + Resetting Git repository for foo 1.0.0... + Resetting Git repository for foo 1.0.1... + Reinstalled 2 packages.'''); + + // The missing libraries should have been replaced. + schedule(() { + var fooLibs = fooDirs.map((dir) { + var fooDirName = path.basename(dir); + return d.dir(fooDirName, [ + d.dir("lib", [d.file("foo.dart", 'main() => "foo";')]) + ]); + }).toList(); + + d.dir(cachePath, [ + d.dir("git", fooLibs) + ]).validate(); + }); + }); + + integration('deletes packages without pubspecs', () { + List<String> fooDirs; + schedule(() { + var gitCacheDir = path.join(sandboxDir, cachePath, "git"); + fooDirs = listDir(gitCacheDir) + .where((dir) => path.basename(dir).startsWith("foo-")).toList(); + + for (var dir in fooDirs) { + deleteEntry(path.join(dir, "pubspec.yaml")); + } + }); + + schedulePub(args: ["cache", "repair"], + error: allOf([ + contains('Failed to load package:'), + contains('Could not find a file named "pubspec.yaml" in '), + contains('foo-'), + ]), + output: allOf([ + startsWith('Failed to reinstall 2 packages:'), + contains('- foo 0.0.0 from git'), + contains('- foo 0.0.0 from git'), + ]), + exitCode: exit_codes.UNAVAILABLE); + + schedule(() { + d.dir(cachePath, [ + d.dir("git", + fooDirs.map((dir) => d.nothing(path.basename(dir)))) + ]).validate(); + }); + }); + + integration('deletes packages with invalid pubspecs', () { + List<String> fooDirs; + schedule(() { + var gitCacheDir = path.join(sandboxDir, cachePath, "git"); + fooDirs = listDir(gitCacheDir) + .where((dir) => path.basename(dir).startsWith("foo-")).toList(); + + for (var dir in fooDirs) { + writeTextFile(path.join(dir, "pubspec.yaml"), "{"); + } + }); + + schedulePub(args: ["cache", "repair"], + error: allOf([ + contains('Failed to load package:'), + contains('Error on line 1, column 2 of '), + contains('foo-'), + ]), + output: allOf([ + startsWith('Failed to reinstall 2 packages:'), + contains('- foo 0.0.0 from git'), + contains('- foo 0.0.0 from git'), + ]), + exitCode: exit_codes.UNAVAILABLE); + + schedule(() { + d.dir(cachePath, [ + d.dir("git", + fooDirs.map((dir) => d.nothing(path.basename(dir)))) + ]).validate(); + }); + }); +} diff --git a/test/cache/repair/hosted.dart b/test/cache/repair/hosted.dart new file mode 100644 index 0000000000000000000000000000000000000000..1dea0266602bc3f83ada1be89903563ed4d1a429 --- /dev/null +++ b/test/cache/repair/hosted.dart @@ -0,0 +1,128 @@ +// Copyright (c) 2014, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:pub/src/exit_codes.dart' as exit_codes; +import 'package:scheduled_test/scheduled_test.dart'; + +import '../../descriptor.dart' as d; +import '../../test_pub.dart'; + +main() { + setUp(() { + servePackages((builder) { + builder.serve("foo", "1.2.3"); + builder.serve("foo", "1.2.4"); + builder.serve("foo", "1.2.5"); + builder.serve("bar", "1.2.3"); + builder.serve("bar", "1.2.4"); + }); + }); + + integration('reinstalls previously cached hosted packages', () { + // Set up a cache with some broken packages. + d.dir(cachePath, [ + d.dir('hosted', [ + d.async(globalServer.port.then((p) => d.dir('localhost%58$p', [ + d.dir("foo-1.2.3", [ + d.libPubspec("foo", "1.2.3"), + d.file("broken.txt") + ]), + d.dir("foo-1.2.5", [ + d.libPubspec("foo", "1.2.5"), + d.file("broken.txt") + ]), + d.dir("bar-1.2.4", [ + d.libPubspec("bar", "1.2.4"), + d.file("broken.txt") + ]) + ]))) + ]) + ]).create(); + + // Repair them. + schedulePub(args: ["cache", "repair"], + output: ''' + Downloading bar 1.2.4... + Downloading foo 1.2.3... + Downloading foo 1.2.5... + Reinstalled 3 packages.'''); + + // The broken versions should have been replaced. + d.hostedCache([ + d.dir("bar-1.2.4", [d.nothing("broken.txt")]), + d.dir("foo-1.2.3", [d.nothing("broken.txt")]), + d.dir("foo-1.2.5", [d.nothing("broken.txt")]) + ]).validate(); + }); + + integration('deletes packages without pubspecs', () { + // Set up a cache with some broken packages. + d.dir(cachePath, [ + d.dir('hosted', [ + d.async(globalServer.port.then((p) => d.dir('localhost%58$p', [ + d.dir("bar-1.2.4", [d.file("broken.txt")]), + d.dir("foo-1.2.3", [d.file("broken.txt")]), + d.dir("foo-1.2.5", [d.file("broken.txt")]), + ]))) + ]) + ]).create(); + + schedulePub(args: ["cache", "repair"], + error: allOf([ + contains('Failed to load package:'), + contains('Could not find a file named "pubspec.yaml" in '), + contains('bar-1.2.4'), + contains('foo-1.2.3'), + contains('foo-1.2.5'), + ]), + output: allOf([ + startsWith('Failed to reinstall 3 packages:'), + contains('- bar 1.2.4'), + contains('- foo 1.2.3'), + contains('- foo 1.2.5'), + ]), + exitCode: exit_codes.UNAVAILABLE); + + d.hostedCache([ + d.nothing("bar-1.2.4"), + d.nothing("foo-1.2.3"), + d.nothing("foo-1.2.5"), + ]).validate(); + }); + + integration('deletes packages with invalid pubspecs', () { + // Set up a cache with some broken packages. + d.dir(cachePath, [ + d.dir('hosted', [ + d.async(globalServer.port.then((p) => d.dir('localhost%58$p', [ + d.dir("bar-1.2.4", [d.file("pubspec.yaml", "{")]), + d.dir("foo-1.2.3", [d.file("pubspec.yaml", "{")]), + d.dir("foo-1.2.5", [d.file("pubspec.yaml", "{")]), + ]))) + ]) + ]).create(); + + schedulePub(args: ["cache", "repair"], + error: allOf([ + contains('Failed to load package:'), + contains('Error on line 1, column 2 of '), + contains('bar-1.2.4'), + contains('foo-1.2.3'), + contains('foo-1.2.5'), + ]), + output: allOf([ + startsWith('Failed to reinstall 3 packages:'), + contains('- bar 1.2.4'), + contains('- foo 1.2.3'), + contains('- foo 1.2.5'), + ]), + exitCode: exit_codes.UNAVAILABLE); + + d.hostedCache([ + d.nothing("bar-1.2.4"), + d.nothing("foo-1.2.3"), + d.nothing("foo-1.2.5"), + ]).validate(); + }); +} diff --git a/test/cache/repair/reinstalls_git_packages_test.dart b/test/cache/repair/reinstalls_git_packages_test.dart deleted file mode 100644 index b6e8af8fa02b6bbea8bfdbed997ca1b1375112db..0000000000000000000000000000000000000000 --- a/test/cache/repair/reinstalls_git_packages_test.dart +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright (c) 2014, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -import 'package:path/path.dart' as path; -import 'package:pub/src/io.dart'; -import 'package:scheduled_test/scheduled_test.dart'; - -import '../../descriptor.dart' as d; -import '../../test_pub.dart'; - -main() { - integration('reinstalls previously cached git packages', () { - // Create two cached revisions of foo. - d.git('foo.git', [ - d.libDir('foo'), - d.libPubspec('foo', '1.0.0') - ]).create(); - - d.appDir({"foo": {"git": "../foo.git"}}).create(); - pubGet(); - - d.git('foo.git', [ - d.libDir('foo'), - d.libPubspec('foo', '1.0.1') - ]).commit(); - - pubUpgrade(); - - // Break them. - List fooDirs; - schedule(() { - // Find the cached foo packages for each revision. - var gitCacheDir = path.join(sandboxDir, cachePath, "git"); - fooDirs = listDir(gitCacheDir) - .where((dir) => path.basename(dir).startsWith("foo-")).toList(); - - // Delete "foo.dart" from them. - for (var dir in fooDirs) { - deleteEntry(path.join(dir, "lib", "foo.dart")); - } - }); - - // Repair them. - schedulePub(args: ["cache", "repair"], - output: ''' - Resetting Git repository for foo 1.0.0... - Resetting Git repository for foo 1.0.1... - Reinstalled 2 packages.'''); - - // The missing libraries should have been replaced. - schedule(() { - var fooLibs = fooDirs.map((dir) { - var fooDirName = path.basename(dir); - return d.dir(fooDirName, [ - d.dir("lib", [d.file("foo.dart", 'main() => "foo";')]) - ]); - }).toList(); - - d.dir(cachePath, [ - d.dir("git", fooLibs) - ]).validate(); - }); - }); -} diff --git a/test/cache/repair/reinstalls_hosted_packages_test.dart b/test/cache/repair/reinstalls_hosted_packages_test.dart deleted file mode 100644 index 07c866823a1548dd8400d14c8ee12abaa9352688..0000000000000000000000000000000000000000 --- a/test/cache/repair/reinstalls_hosted_packages_test.dart +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright (c) 2014, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -import '../../descriptor.dart' as d; -import '../../test_pub.dart'; - -main() { - integration('reinstalls previously cached hosted packages', () { - servePackages((builder) { - builder.serve("foo", "1.2.3"); - builder.serve("foo", "1.2.4"); - builder.serve("foo", "1.2.5"); - builder.serve("bar", "1.2.3"); - builder.serve("bar", "1.2.4"); - }); - - // Set up a cache with some broken packages. - d.dir(cachePath, [ - d.dir('hosted', [ - d.async(globalServer.port.then((p) => d.dir('localhost%58$p', [ - d.dir("foo-1.2.3", [ - d.libPubspec("foo", "1.2.3"), - d.file("broken.txt") - ]), - d.dir("foo-1.2.5", [ - d.libPubspec("foo", "1.2.5"), - d.file("broken.txt") - ]), - d.dir("bar-1.2.4", [ - d.libPubspec("bar", "1.2.4"), - d.file("broken.txt") - ]) - ]))) - ]) - ]).create(); - - // Repair them. - schedulePub(args: ["cache", "repair"], - output: ''' - Downloading bar 1.2.4... - Downloading foo 1.2.3... - Downloading foo 1.2.5... - Reinstalled 3 packages.'''); - - // The broken versions should have been replaced. - d.hostedCache([ - d.dir("bar-1.2.4", [d.nothing("broken.txt")]), - d.dir("foo-1.2.3", [d.nothing("broken.txt")]), - d.dir("foo-1.2.5", [d.nothing("broken.txt")]) - ]).validate(); - }); -} diff --git a/test/hosted/offline_test.dart b/test/hosted/offline_test.dart index 492045d32494a6c7ea75d1476379769c2554b376..6c64ee24e185fa4bbc308d919e60e61157c97a85 100644 --- a/test/hosted/offline_test.dart +++ b/test/hosted/offline_test.dart @@ -122,5 +122,45 @@ main() { d.appPackagesFile({"foo": "1.2.3"}).validate(); }); + + integration('skips invalid cached versions', () { + // Run the server so that we know what URL to use in the system cache. + serveErrors(); + + d.cacheDir({ + "foo": ["1.2.2", "1.2.3"] + }, includePubspecs: true).create(); + + d.hostedCache([ + d.dir("foo-1.2.3", [d.file("pubspec.yaml", "{")]) + ]).create(); + + d.appDir({"foo": "any"}).create(); + + pubCommand(command, args: ['--offline']); + + d.appPackagesFile({"foo": "1.2.2"}).validate(); + }); + + integration('skips invalid locked versions', () { + // Run the server so that we know what URL to use in the system cache. + serveErrors(); + + d.cacheDir({ + "foo": ["1.2.2", "1.2.3"] + }, includePubspecs: true).create(); + + d.hostedCache([ + d.dir("foo-1.2.3", [d.file("pubspec.yaml", "{")]) + ]).create(); + + d.appDir({"foo": "any"}).create(); + + createLockFile('myapp', hosted: {'foo': '1.2.3'}); + + pubCommand(command, args: ['--offline']); + + d.appPackagesFile({"foo": "1.2.2"}).validate(); + }); }); }