From 9cb2334b6c0c1a2be2ca0558b5aceb1f0a27216e Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Thu, 9 Jul 2015 12:46:09 -0700 Subject: [PATCH] Fix several package spec bugs. * The package spec's reference to the entrypoint package was pointing to the hosted cache rather than the package's directory. This was caused by the package graph's instance of the entrypoint package incorrectly pointing to the cache; the graph now re-uses `entrypoint.root`. * The package spec didn't contain relative paths for relative path dependencies. This was being missed by our tests because the package spec parser was automatically converting all paths to absolute at parse-time. Closes #1294 R=rnystrom@google.com Review URL: https://codereview.chromium.org//1228093003 . --- lib/src/command.dart | 2 +- lib/src/entrypoint.dart | 2 ++ test/descriptor/packages.dart | 21 +++++++++++---------- test/packages_file_test.dart | 31 ++++++++++++++++++++++--------- 4 files changed, 36 insertions(+), 20 deletions(-) diff --git a/lib/src/command.dart b/lib/src/command.dart index 3a1647154..582a42958 100644 --- a/lib/src/command.dart +++ b/lib/src/command.dart @@ -42,7 +42,7 @@ abstract class PubCommand extends Command { Entrypoint get entrypoint { // Lazy load it. if (_entrypoint == null) { - _entrypoint = new Entrypoint(path.current, cache, + _entrypoint = new Entrypoint('.', cache, packageSymlinks: globalResults['package-symlinks']); } return _entrypoint; diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index 2dc3d136e..ed0d087ca 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -470,6 +470,8 @@ class Entrypoint { var graph = await log.progress("Loading package graph", () async { if (result != null) { var packages = await Future.wait(result.packages.map((id) async { + if (id.name == root.name) return root; + var dir = await cache.sources[id.source].getDirectory(id); return new Package(result.pubspecs[id.name], dir); })); diff --git a/test/descriptor/packages.dart b/test/descriptor/packages.dart index 9d857c067..a70b483a1 100644 --- a/test/descriptor/packages.dart +++ b/test/descriptor/packages.dart @@ -44,8 +44,8 @@ class PackagesFileDescriptor extends Descriptor { // If it's a semver, it's a cache reference. packagePath = p.join(cachePath, "$package-$version"); } else { - // Otherwise it's a path relative to the .pubspec file, - // which is also the relative path wrt. the .packages file. + // Otherwise it's a path relative to the pubspec file, + // which is also relative to the .packages file. packagePath = p.fromUri(version); } mapping[package] = p.toUri(p.join(packagePath, "lib", "")); @@ -74,8 +74,11 @@ class PackagesFileDescriptor extends Descriptor { /// A function that throws an error if [binaryContents] doesn't match the /// expected contents of the descriptor. void _validateNow(List binaryContents, String fullPath) { - var fileUri = p.toUri(fullPath); - var map = packages_file.parse(binaryContents, fileUri); + // Resolve against a dummy URL so that we can test whether the URLs in + // the package file are themselves relative. We can't resolve against just + // "." due to sdk#23809. + var base = "/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p"; + var map = packages_file.parse(binaryContents, Uri.parse(base)); for (var package in _dependencies.keys) { if (!map.containsKey(package)) { @@ -89,14 +92,12 @@ class PackagesFileDescriptor extends Descriptor { "Expected $description, found location: ${map[package]}."); } } else { - var expected = p.normalize(p.join( - p.dirname(fullPath), p.fromUri(description), 'lib')); - expected = new File(expected).resolveSymbolicLinksSync(); - var actual = new File(p.normalize(p.absolute(p.fromUri(map[package])))) - .resolveSymbolicLinksSync(); + var expected = p.normalize(p.join(p.fromUri(description), 'lib')); + var actual = p.normalize(p.fromUri( + p.url.relative(map[package].toString(), from: p.dirname(base)))); if (expected != actual) { - fail("Relative path: Expected $description, found ${map[package]}"); + fail("Relative path: Expected $expected, found $actual"); } } } diff --git a/test/packages_file_test.dart b/test/packages_file_test.dart index 533b09326..ef810f08e 100644 --- a/test/packages_file_test.dart +++ b/test/packages_file_test.dart @@ -16,12 +16,15 @@ main() { deps: {"bar": "3.2.1"}, contents: [d.dir("lib", [])]); }); - d.appDir({"foo": "1.2.3"}).create(); + d.dir(appPath, [ + d.appPubspec({"foo": "1.2.3"}), + d.dir('lib') + ]).create(); pubCommand(command); d.dir(appPath, [d.packagesFile({ - "foo": "1.2.3", "bar": "3.2.1", "baz": "2.2.2", "myapp": "0.0.0"})]) + "foo": "1.2.3", "bar": "3.2.1", "baz": "2.2.2", "myapp": "."})]) .validate(); }); @@ -34,7 +37,10 @@ main() { deps: {"bar": "3.2.1"}, contents: [d.dir("lib", [])]); }); - d.appDir({"foo": "1.2.3"}).create(); + d.dir(appPath, [ + d.appPubspec({"foo": "1.2.3"}), + d.dir('lib') + ]).create(); var oldFile = d.dir(appPath, [d.packagesFile({"notFoo": "9.9.9"})]); oldFile.create(); @@ -43,12 +49,15 @@ main() { pubCommand(command); d.dir(appPath, [d.packagesFile({ - "foo": "1.2.3", "bar": "3.2.1", "baz": "2.2.2", "myapp": "0.0.0"})]) + "foo": "1.2.3", "bar": "3.2.1", "baz": "2.2.2", "myapp": "."})]) .validate(); }); integration('.packages file is not created if pub command fails', () { - d.appDir({"foo": "1.2.3"}).create(); + d.dir(appPath, [ + d.appPubspec({"foo": "1.2.3"}), + d.dir('lib') + ]).create(); pubCommand(command, args: ['--offline'], error: "Could not find package foo in cache.\n"); @@ -76,14 +85,18 @@ main() { "dependency_overrides": { "baz": {"path": "../local_baz"}, } - }) + }), + d.dir('lib') ]).create(); pubCommand(command); - d.dir(appPath, [d.packagesFile({"myapp": "0.0.0", - "baz": "../local_baz"})]).validate(); + d.dir(appPath, [ + d.packagesFile({ + "myapp": ".", + "baz": "../local_baz" + }) + ]).validate(); }); - }); }