From 8bf75818e5ae718553c01e31528c1e8b69652279 Mon Sep 17 00:00:00 2001
From: "rnystrom@google.com" <rnystrom@google.com>
Date: Mon, 18 Nov 2013 21:51:24 +0000
Subject: [PATCH] Hook up dependency overrides to the rest of pub.
- Parse them in pubspec.
- Pass them to solver.
- Warn when they are used.
- Error on publish.
BUG=https://code.google.com/p/dart/issues/detail?id=8566
R=nweiz@google.com
Review URL: https://codereview.chromium.org//74013007
git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge@30376 260f80e4-7a28-3924-810f-c04153c831b5
---
lib/src/entrypoint.dart | 12 +-
lib/src/package.dart | 3 +
lib/src/pubspec.dart | 15 ++-
lib/src/solver/backtracking_solver.dart | 12 +-
lib/src/solver/version_solver.dart | 12 +-
lib/src/validator.dart | 2 +
lib/src/validator/dependency_override.dart | 28 +++++
test/dependency_override_test.dart | 113 +++++++++++++++++++
test/pubspec_test.dart | 32 ++++++
test/validator/dependency_override_test.dart | 30 +++++
test/version_solver_test.dart | 36 +++---
11 files changed, 266 insertions(+), 29 deletions(-)
create mode 100644 lib/src/validator/dependency_override.dart
create mode 100644 test/dependency_override_test.dart
create mode 100644 test/validator/dependency_override_test.dart
diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart
index e9e513fa..b4ef32b8 100644
--- a/lib/src/entrypoint.dart
+++ b/lib/src/entrypoint.dart
@@ -143,6 +143,16 @@ class Entrypoint {
return new Future.sync(() {
if (!result.succeeded) throw result.error;
+ // Warn the user if any overrides were in effect.
+ if (result.overrides.isNotEmpty) {
+ var buffer = new StringBuffer();
+ buffer.write("Warning: You are overriding these dependencies:");
+ for (var override in result.overrides) {
+ buffer.write("\n- $override");
+ }
+ log.warning(buffer);
+ }
+
cleanDir(packagesDir);
return Future.wait(result.packages.map((id) {
if (id.isRoot) return new Future.value(id);
@@ -196,7 +206,7 @@ class Entrypoint {
/// pubspec.
Future ensureLockFileIsUpToDate() {
return new Future.sync(() {
- if (isLockFileUpToDate()) return;
+ if (isLockFileUpToDate()) return null;
if (lockFileExists) {
log.message(
diff --git a/lib/src/package.dart b/lib/src/package.dart
index e6aa20cd..6b31e566 100644
--- a/lib/src/package.dart
+++ b/lib/src/package.dart
@@ -37,6 +37,9 @@ class Package {
/// The immediate dev dependencies this package specifies in its pubspec.
List<PackageDep> get devDependencies => pubspec.devDependencies;
+ /// The dependency overrides this package specifies in its pubspec.
+ List<PackageDep> get dependencyOverrides => pubspec.dependencyOverrides;
+
/// Returns the path to the README file at the root of the entrypoint, or null
/// if no README file is found. If multiple READMEs are found, this uses the
/// same conventions as pub.dartlang.org for choosing the primary one: the
diff --git a/lib/src/pubspec.dart b/lib/src/pubspec.dart
index bcef655b..c5c5c1af 100644
--- a/lib/src/pubspec.dart
+++ b/lib/src/pubspec.dart
@@ -102,6 +102,18 @@ class Pubspec {
}
List<PackageDep> _devDependencies;
+ /// The dependency constraints that this package overrides when it is the
+ /// root package.
+ ///
+ /// Dependencies here will replace any dependency on a package with the same
+ /// name anywhere in the dependency graph.
+ List<PackageDep> get dependencyOverrides {
+ if (_dependencyOverrides != null) return _dependencyOverrides;
+ _dependencyOverrides = _parseDependencies('dependency_overrides');
+ return _dependencyOverrides;
+ }
+ List<PackageDep> _dependencyOverrides;
+
/// The ids of the transformers to use for this package.
List<Set<TransformerId>> get transformers {
if (_transformers != null) return _transformers;
@@ -220,7 +232,8 @@ class Pubspec {
}
Pubspec(this._name, this._version, this._dependencies, this._devDependencies,
- this._environment, this._transformers, [Map fields])
+ this._dependencyOverrides, this._environment, this._transformers,
+ [Map fields])
: this.fields = fields == null ? {} : fields,
_sources = null,
_location = null;
diff --git a/lib/src/solver/backtracking_solver.dart b/lib/src/solver/backtracking_solver.dart
index 6a0a5397..43009095 100644
--- a/lib/src/solver/backtracking_solver.dart
+++ b/lib/src/solver/backtracking_solver.dart
@@ -90,7 +90,7 @@ class BacktrackingSolver {
var _attemptedSolutions = 1;
BacktrackingSolver(SourceRegistry sources, this.root, this.lockFile,
- Iterable<PackageDep> overrides, List<String> useLatest)
+ List<String> useLatest)
: sources = sources,
cache = new PubspecCache(sources) {
for (var package in useLatest) {
@@ -98,7 +98,7 @@ class BacktrackingSolver {
lockFile.packages.remove(package);
}
- for (var override in overrides) {
+ for (var override in root.dependencyOverrides) {
_overrides[override.name] = override;
}
}
@@ -110,6 +110,10 @@ class BacktrackingSolver {
_logParameters();
+ // Sort the overrides by package name to make sure they're deterministic.
+ var overrides = _overrides.values.toList();
+ overrides.sort((a, b) => a.name.compareTo(b.name));
+
return newFuture(() {
stopwatch.start();
@@ -119,12 +123,12 @@ class BacktrackingSolver {
_validateSdkConstraint(root.pubspec);
return _traverseSolution();
}).then((packages) {
- return new SolveResult(packages, null, attemptedSolutions);
+ return new SolveResult(packages, overrides, null, attemptedSolutions);
}).catchError((error) {
if (error is! SolveFailure) throw error;
// Wrap a failure in a result so we can attach some other data.
- return new SolveResult(null, error, attemptedSolutions);
+ return new SolveResult(null, overrides, error, attemptedSolutions);
}).whenComplete(() {
// Gather some solving metrics.
var buffer = new StringBuffer();
diff --git a/lib/src/solver/version_solver.dart b/lib/src/solver/version_solver.dart
index 30bd2d09..54c5ab46 100644
--- a/lib/src/solver/version_solver.dart
+++ b/lib/src/solver/version_solver.dart
@@ -25,14 +25,12 @@ import 'backtracking_solver.dart';
/// packages will be used. This is for forcing an upgrade to one or more
/// packages.
Future<SolveResult> resolveVersions(SourceRegistry sources, Package root,
- {LockFile lockFile, List<PackageRef> overrides, List<String> useLatest}) {
+ {LockFile lockFile, List<String> useLatest}) {
if (lockFile == null) lockFile = new LockFile.empty();
- if (overrides == null) overrides = [];
if (useLatest == null) useLatest = [];
return log.progress('Resolving dependencies', () {
- return new BacktrackingSolver(sources, root, lockFile, overrides,
- useLatest).solve();
+ return new BacktrackingSolver(sources, root, lockFile, useLatest).solve();
});
}
@@ -45,6 +43,9 @@ class SolveResult {
/// reachable from the root, or `null` if the solver failed.
final List<PackageId> packages;
+ /// The dependency overrides that were used in the solution.
+ final List<PackageDep> overrides;
+
/// The error that prevented the solver from finding a solution or `null` if
/// it was successful.
final SolveFailure error;
@@ -55,7 +56,8 @@ class SolveResult {
/// solution.
final int attemptedSolutions;
- SolveResult(this.packages, this.error, this.attemptedSolutions);
+ SolveResult(this.packages, this.overrides, this.error,
+ this.attemptedSolutions);
String toString() {
if (!succeeded) {
diff --git a/lib/src/validator.dart b/lib/src/validator.dart
index 98ef43e6..640c54ff 100644
--- a/lib/src/validator.dart
+++ b/lib/src/validator.dart
@@ -11,6 +11,7 @@ import 'log.dart' as log;
import 'utils.dart';
import 'validator/compiled_dartdoc.dart';
import 'validator/dependency.dart';
+import 'validator/dependency_override.dart';
import 'validator/directory.dart';
import 'validator/lib.dart';
import 'validator/license.dart';
@@ -55,6 +56,7 @@ abstract class Validator {
new NameValidator(entrypoint),
new PubspecFieldValidator(entrypoint),
new DependencyValidator(entrypoint),
+ new DependencyOverrideValidator(entrypoint),
new DirectoryValidator(entrypoint),
new CompiledDartdocValidator(entrypoint),
new Utf8ReadmeValidator(entrypoint)
diff --git a/lib/src/validator/dependency_override.dart b/lib/src/validator/dependency_override.dart
new file mode 100644
index 00000000..a5ce2998
--- /dev/null
+++ b/lib/src/validator/dependency_override.dart
@@ -0,0 +1,28 @@
+// Copyright (c) 2012, 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.
+
+library pub.validator.dependency_override;
+
+import 'dart:async';
+
+import '../entrypoint.dart';
+import '../validator.dart';
+
+/// A validator that validates a package's dependencies overrides (or the
+/// absence thereof).
+class DependencyOverrideValidator extends Validator {
+ DependencyOverrideValidator(Entrypoint entrypoint)
+ : super(entrypoint);
+
+ Future validate() {
+ if (entrypoint.root.dependencyOverrides.isNotEmpty) {
+ errors.add(
+ 'Your pubspec.yaml must not have a "dependency_overrides" field.\n'
+ 'This ensures you test your package against the same versions of '
+ 'its dependencies\n'
+ 'that users will have when they use it.');
+ }
+ return new Future.value();
+ }
+}
diff --git a/test/dependency_override_test.dart b/test/dependency_override_test.dart
new file mode 100644
index 00000000..c930dd5b
--- /dev/null
+++ b/test/dependency_override_test.dart
@@ -0,0 +1,113 @@
+// Copyright (c) 2013, 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() {
+ initConfig();
+ forBothPubGetAndUpgrade((command) {
+ integration("chooses best version matching override constraint", () {
+ servePackages([
+ packageMap("foo", "1.0.0"),
+ packageMap("foo", "2.0.0"),
+ packageMap("foo", "3.0.0")
+ ]);
+
+ d.dir(appPath, [
+ d.pubspec({
+ "name": "myapp",
+ "dependencies": {
+ "foo": ">2.0.0"
+ },
+ "dependency_overrides": {
+ "foo": "<3.0.0"
+ }
+ })
+ ]).create();
+
+ pubCommand(command);
+
+ d.packagesDir({
+ "foo": "2.0.0"
+ }).validate();
+ });
+
+ integration("treats override as implicit dependency", () {
+ servePackages([
+ packageMap("foo", "1.0.0")
+ ]);
+
+ d.dir(appPath, [
+ d.pubspec({
+ "name": "myapp",
+ "dependency_overrides": {
+ "foo": "any"
+ }
+ })
+ ]).create();
+
+ pubCommand(command);
+
+ d.packagesDir({
+ "foo": "1.0.0"
+ }).validate();
+ });
+
+ integration("ignores other constraints on overridden package", () {
+ servePackages([
+ packageMap("foo", "1.0.0"),
+ packageMap("foo", "2.0.0"),
+ packageMap("foo", "3.0.0"),
+ packageMap("bar", "1.0.0", {"foo": "5.0.0-nonexistent"})
+ ]);
+
+ d.dir(appPath, [
+ d.pubspec({
+ "name": "myapp",
+ "dependencies": {
+ "bar": "any"
+ },
+ "dependency_overrides": {
+ "foo": "<3.0.0"
+ }
+ })
+ ]).create();
+
+ pubCommand(command);
+
+ d.packagesDir({
+ "foo": "2.0.0",
+ "bar": "1.0.0"
+ }).validate();
+ });
+
+ integration("warns about overridden dependencies", () {
+ servePackages([
+ packageMap("foo", "1.0.0"),
+ packageMap("bar", "1.0.0"),
+ packageMap("baz", "1.0.0")
+ ]);
+
+ d.dir(appPath, [
+ d.pubspec({
+ "name": "myapp",
+ "dependency_overrides": {
+ "foo": "any",
+ "bar": "any",
+ "baz": "any"
+ }
+ })
+ ]).create();
+
+ schedulePub(args: [command.name], output: command.success, error:
+ """
+ Warning: You are overriding these dependencies:
+ - bar any from hosted (bar)
+ - baz any from hosted (baz)
+ - foo any from hosted (foo)
+ """);
+ });
+ });
+}
\ No newline at end of file
diff --git a/test/pubspec_test.dart b/test/pubspec_test.dart
index afc04684..70eba930 100644
--- a/test/pubspec_test.dart
+++ b/test/pubspec_test.dart
@@ -100,6 +100,29 @@ dev_dependencies:
expect(pubspec.devDependencies, isEmpty);
});
+ test("allows a version constraint for dependency overrides", () {
+ var pubspec = new Pubspec.parse('''
+dependency_overrides:
+ foo:
+ mock: ok
+ version: ">=1.2.3 <3.4.5"
+''', sources);
+
+ var foo = pubspec.dependencyOverrides[0];
+ expect(foo.name, equals('foo'));
+ expect(foo.constraint.allows(new Version(1, 2, 3)), isTrue);
+ expect(foo.constraint.allows(new Version(1, 2, 5)), isTrue);
+ expect(foo.constraint.allows(new Version(3, 4, 5)), isFalse);
+ });
+
+ test("allows an empty dependency overrides map", () {
+ var pubspec = new Pubspec.parse('''
+dependency_overrides:
+''', sources);
+
+ expect(pubspec.dependencyOverrides, isEmpty);
+ });
+
test("allows an unknown source", () {
var pubspec = new Pubspec.parse('''
dependencies:
@@ -143,6 +166,15 @@ dev_dependencies:
''', (pubspec) => pubspec.devDependencies);
});
+ test("throws if it has an override on itself", () {
+ expectPubspecException('''
+name: myapp
+dependency_overrides:
+ myapp:
+ mock: ok
+''', (pubspec) => pubspec.dependencyOverrides);
+ });
+
test("throws if the description isn't valid", () {
expectPubspecException('''
dependencies:
diff --git a/test/validator/dependency_override_test.dart b/test/validator/dependency_override_test.dart
new file mode 100644
index 00000000..6f336a27
--- /dev/null
+++ b/test/validator/dependency_override_test.dart
@@ -0,0 +1,30 @@
+// Copyright (c) 2013, 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 '../../lib/src/entrypoint.dart';
+import '../../lib/src/validator.dart';
+import '../../lib/src/validator/dependency_override.dart';
+import '../descriptor.dart' as d;
+import '../test_pub.dart';
+import 'utils.dart';
+
+Validator dependencyOverride(Entrypoint entrypoint) =>
+ new DependencyOverrideValidator(entrypoint);
+
+main() {
+ initConfig();
+
+ integration('invalidates a package if it has dependency overrides', () {
+ d.dir(appPath, [
+ d.pubspec({
+ "name": "myapp",
+ "dependency_overrides": {
+ "foo": "<3.0.0"
+ }
+ })
+ ]).create();
+
+ expectValidationError(dependencyOverride);
+ });
+}
diff --git a/test/version_solver_test.dart b/test/version_solver_test.dart
index 00b3e1bb..83551b1e 100644
--- a/test/version_solver_test.dart
+++ b/test/version_solver_test.dart
@@ -1023,7 +1023,8 @@ _testResolve(void testFn(String description, Function body),
var root;
packages.forEach((description, dependencies) {
var id = parseSpec(description);
- var package = mockPackage(id, dependencies);
+ var package = mockPackage(id, dependencies,
+ id.name == 'myapp' ? overrides : null);
if (id.name == 'myapp') {
// Don't add the root package to the server, so we can verify that Pub
// doesn't try to look up information about the local package on the
@@ -1055,15 +1056,6 @@ _testResolve(void testFn(String description, Function body),
});
}
- // Parse the overrides.
- var realOverrides = [];
- if (overrides != null) {
- overrides.forEach((spec, constraint) {
- realOverrides.add(parseSpec(spec).withConstraint(
- new VersionConstraint.parse(constraint)));
- });
- }
-
// Make a version number like the continuous build's version.
var previousVersion = sdk.version;
if (useBleedingEdgeSdkVersion) {
@@ -1071,8 +1063,7 @@ _testResolve(void testFn(String description, Function body),
}
// Resolve the versions.
- var future = resolveVersions(cache.sources, root,
- lockFile: realLockFile, overrides: realOverrides);
+ var future = resolveVersions(cache.sources, root, lockFile: realLockFile);
var matcher;
if (result != null) {
@@ -1331,20 +1322,20 @@ class MockSource extends Source {
}
}
-Package mockPackage(PackageId id, Map dependencyStrings) {
+Package mockPackage(PackageId id, Map dependencyStrings, Map overrides) {
var sdkConstraint = null;
// Build the pubspec dependencies.
var dependencies = <PackageDep>[];
var devDependencies = <PackageDep>[];
- dependencyStrings.forEach((description, constraint) {
- var isDev = description.startsWith("(dev) ");
+ dependencyStrings.forEach((spec, constraint) {
+ var isDev = spec.startsWith("(dev) ");
if (isDev) {
- description = description.substring("(dev) ".length);
+ spec = spec.substring("(dev) ".length);
}
- var dep = parseSpec(description).withConstraint(
+ var dep = parseSpec(spec).withConstraint(
new VersionConstraint.parse(constraint));
if (dep.name == 'sdk') {
@@ -1359,8 +1350,17 @@ Package mockPackage(PackageId id, Map dependencyStrings) {
}
});
+ var dependencyOverrides = <PackageDep>[];
+ if (overrides != null) {
+ overrides.forEach((spec, constraint) {
+ dependencyOverrides.add(parseSpec(spec).withConstraint(
+ new VersionConstraint.parse(constraint)));
+ });
+ }
+
var pubspec = new Pubspec(id.name, id.version, dependencies,
- devDependencies, new PubspecEnvironment(sdkConstraint), []);
+ devDependencies, dependencyOverrides,
+ new PubspecEnvironment(sdkConstraint), []);
return new Package.inMemory(pubspec);
}
--
GitLab