diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart
index 897af2286b1e1a96a36f7bbf2ad657e2fb1400af..b52a139ed670156c1ad2ef4d8d2bef0ba840ab19 100644
--- a/lib/src/entrypoint.dart
+++ b/lib/src/entrypoint.dart
@@ -100,7 +100,7 @@ class Entrypoint {
/// directory, respecting the [LockFile] if present. Returns a [Future] that
/// completes when all dependencies are installed.
Future installDependencies() {
- return _loadLockFile()
+ return loadLockFile()
.chain((lockFile) => resolveVersions(cache.sources, root, lockFile))
.chain(_installDependencies);
}
@@ -117,7 +117,7 @@ class Entrypoint {
/// other dependencies as specified by the [LockFile] if possible. Returns a
/// [Future] that completes when all dependencies are installed.
Future updateDependencies(List<String> dependencies) {
- return _loadLockFile().chain((lockFile) {
+ return loadLockFile().chain((lockFile) {
var versionSolver = new VersionSolver(cache.sources, root, lockFile);
for (var dependency in dependencies) {
versionSolver.useLatestVersion(dependency);
@@ -141,10 +141,7 @@ class Entrypoint {
/// Loads the list of concrete package versions from the `pubspec.lock`, if it
/// exists. If it doesn't, this completes to an empty [LockFile].
- ///
- /// If there's an error reading the `pubspec.lock` file, this will print a
- /// warning message and act as though the file doesn't exist.
- Future<LockFile> _loadLockFile() {
+ Future<LockFile> loadLockFile() {
var lockFilePath = join(root.dir, 'pubspec.lock');
log.fine("Loading lockfile.");
diff --git a/lib/src/http.dart b/lib/src/http.dart
index abc3c435c3594329f44cfabce182264ba9d85d6c..0665c355cd26d6bfd203a163c379c10109d3b72e 100644
--- a/lib/src/http.dart
+++ b/lib/src/http.dart
@@ -22,10 +22,10 @@ final HTTP_TIMEOUT = 30 * 1000;
/// An HTTP client that transforms 40* errors and socket exceptions into more
/// user-friendly error messages.
class PubHttpClient extends http.BaseClient {
- final http.Client _inner;
+ http.Client inner;
PubHttpClient([http.Client inner])
- : _inner = inner == null ? new http.Client() : inner;
+ : this.inner = inner == null ? new http.Client() : inner;
Future<http.StreamedResponse> send(http.BaseRequest request) {
// TODO(rnystrom): Log request body when it's available and plaintext, but
@@ -41,7 +41,7 @@ class PubHttpClient extends http.BaseClient {
// TODO(nweiz): Ideally the timeout would extend to reading from the
// response input stream, but until issue 3657 is fixed that's not feasible.
- return timeout(_inner.send(request).chain((streamedResponse) {
+ return timeout(inner.send(request).chain((streamedResponse) {
log.fine("Got response ${streamedResponse.statusCode} "
"${streamedResponse.reasonPhrase}.");
diff --git a/lib/src/validator.dart b/lib/src/validator.dart
index 5c0c8507ce5e430629c677341f4737d2aedb681b..48f7768dd09c5e77fef39a325f08a98ef3b6b544 100644
--- a/lib/src/validator.dart
+++ b/lib/src/validator.dart
@@ -9,6 +9,7 @@ import 'log.dart' as log;
import 'io.dart';
import 'system_cache.dart';
import 'utils.dart';
+import 'validator/dependency.dart';
import 'validator/lib.dart';
import 'validator/license.dart';
import 'validator/name.dart';
@@ -44,7 +45,8 @@ abstract class Validator {
new LibValidator(entrypoint),
new LicenseValidator(entrypoint),
new NameValidator(entrypoint),
- new PubspecFieldValidator(entrypoint)
+ new PubspecFieldValidator(entrypoint),
+ new DependencyValidator(entrypoint)
];
// TODO(nweiz): The sleep 0 here forces us to go async. This works around
diff --git a/lib/src/validator/dependency.dart b/lib/src/validator/dependency.dart
new file mode 100644
index 0000000000000000000000000000000000000000..66d2ae22f53cddb804d42096f8d7c2cb46674fc3
--- /dev/null
+++ b/lib/src/validator/dependency.dart
@@ -0,0 +1,91 @@
+// 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 dependency_validator;
+
+import '../entrypoint.dart';
+import '../hosted_source.dart';
+import '../http.dart';
+import '../package.dart';
+import '../utils.dart';
+import '../validator.dart';
+import '../version.dart';
+
+/// A validator that validates a package's dependencies.
+class DependencyValidator extends Validator {
+ DependencyValidator(Entrypoint entrypoint)
+ : super(entrypoint);
+
+ Future validate() {
+ return Futures.forEach(entrypoint.root.pubspec.dependencies, (dependency) {
+ if (dependency.source is! HostedSource) {
+ return _warnAboutSource(dependency);
+ }
+
+ if (dependency.constraint.isAny &&
+ // TODO(nweiz): once we have development dependencies (issue 5358), we
+ // should warn about unittest. Until then, it's reasonable not to put
+ // a constraint on it.
+ dependency.name != 'unittest') {
+ return _warnAboutConstraint(dependency);
+ }
+
+ return new Future.immediate(null);
+ });
+ }
+
+ /// Warn that dependencies should use the hosted source.
+ Future _warnAboutSource(PackageRef ref) {
+ return entrypoint.cache.sources['hosted']
+ .getVersions(ref.name, ref.name)
+ .transformException((e) => <Version>[])
+ .transform((versions) {
+ var constraint;
+ var primary = Version.primary(versions);
+ if (primary != null) {
+ constraint = _constraintForVersion(primary);
+ } else {
+ constraint = ref.constraint.toString();
+ if (!ref.constraint.isAny && ref.constraint is! Version) {
+ constraint = '"$constraint"';
+ }
+ }
+
+ warnings.add('Don\'t depend on "${ref.name}" from the ${ref.source.name} '
+ 'source. Use the hosted source instead. For example:\n'
+ '\n'
+ 'dependencies:\n'
+ ' ${ref.name}: $constraint\n'
+ '\n'
+ 'Using the hosted source ensures that everyone can download your '
+ 'package\'s dependencies along with your package.');
+ });
+ }
+
+ /// Warn that dependencies should have version constraints.
+ Future _warnAboutConstraint(PackageRef ref) {
+ return entrypoint.loadLockFile().transform((lockFile) {
+ var message = 'Your dependency on "${ref.name}" should have a version '
+ 'constraint.';
+ var locked = lockFile.packages[ref.name];
+ if (locked != null) {
+ message = '$message For example:\n'
+ '\n'
+ 'dependencies:\n'
+ ' ${ref.name}: ${_constraintForVersion(locked.version)}\n';
+ }
+ warnings.add("$message\n"
+ "Without a constraint, you're promising to support all future "
+ "versions of ${ref.name}.");
+ });
+ }
+
+ /// Returns the suggested version constraint for a dependency that was tested
+ /// against [version].
+ String _constraintForVersion(Version version) {
+ if (version.major != 0) return '">=$version <${version.major + 1}.0.0"';
+ return '">=$version <${version.major}.${version.minor}.'
+ '${version.patch + 1}"';
+ }
+}
diff --git a/lib/src/version.dart b/lib/src/version.dart
index 567a7a7117434e6fbeca619036740dfab9e264fb..f60ad19cf6a77cdd50c5eaae55568ae7c6f427cf 100644
--- a/lib/src/version.dart
+++ b/lib/src/version.dart
@@ -70,6 +70,20 @@ class Version implements Comparable, VersionConstraint {
}
}
+ /// Returns the primary version out of a list of candidates. This is the
+ /// highest-numbered stable (non-prerelease) version. If there are no stable
+ /// versions, it's just the highest-numbered version.
+ static Version primary(List<Version> versions) {
+ var primary;
+ for (var version in versions) {
+ if (primary == null || (!version.isPreRelease && primary.isPreRelease) ||
+ (version.isPreRelease == primary.isPreRelease && version > primary)) {
+ primary = version;
+ }
+ }
+ return primary;
+ }
+
bool operator ==(other) {
if (other is! Version) return false;
return compareTo(other) == 0;
@@ -80,8 +94,12 @@ class Version implements Comparable, VersionConstraint {
bool operator <=(Version other) => compareTo(other) <= 0;
bool operator >=(Version other) => compareTo(other) >= 0;
+ bool get isAny => false;
bool get isEmpty => false;
+ /// Whether or not this is a pre-release version.
+ bool get isPreRelease => preRelease != null;
+
/// Tests if [other] matches this version exactly.
bool allows(Version other) => this == other;
@@ -234,6 +252,9 @@ abstract class VersionConstraint {
/// Returns `true` if this constraint allows no versions.
bool get isEmpty;
+ /// Returns `true` if this constraint allows all versions.
+ bool get isAny;
+
/// Returns `true` if this constraint allows [version].
bool allows(Version version);
@@ -297,6 +318,8 @@ class VersionRange implements VersionConstraint {
bool get isEmpty => false;
+ bool get isAny => !includeMin && !includeMax;
+
/// Tests if [other] matches falls within this version range.
bool allows(Version other) {
if (min != null && other < min) return false;
@@ -391,6 +414,7 @@ class _EmptyVersion implements VersionConstraint {
const _EmptyVersion();
bool get isEmpty => true;
+ bool get isAny => false;
bool allows(Version other) => false;
VersionConstraint intersect(VersionConstraint other) => this;
String toString() => '<empty>';
diff --git a/test/test_pub.dart b/test/test_pub.dart
index 44290b02e5c01cb0021a2386468ca0adb714c018..c9c8cc02c23807aa6bdc07d9265240cddcdcde7b 100644
--- a/test/test_pub.dart
+++ b/test/test_pub.dart
@@ -19,10 +19,12 @@ import 'dart:uri';
import '../../../pkg/oauth2/lib/oauth2.dart' as oauth2;
import '../../../pkg/path/lib/path.dart' as path;
import '../../../pkg/unittest/lib/unittest.dart';
+import '../../../pkg/http/lib/testing.dart';
import '../../lib/file_system.dart' as fs;
import '../../pub/entrypoint.dart';
import '../../pub/git_source.dart';
import '../../pub/hosted_source.dart';
+import '../../pub/http.dart';
import '../../pub/io.dart';
import '../../pub/sdk_source.dart';
import '../../pub/system_cache.dart';
@@ -712,6 +714,18 @@ void ensureGit() {
});
}
+/// Use [client] as the mock HTTP client for this test.
+///
+/// Note that this will only affect HTTP requests made via http.dart in the
+/// parent process.
+void useMockClient(MockClient client) {
+ var oldInnerClient = httpClient.inner;
+ httpClient.inner = client;
+ _scheduleCleanup((_) {
+ httpClient.inner = oldInnerClient;
+ });
+}
+
Future<Directory> _setUpSandbox() => createTempDir();
Future _runScheduled(Directory parentDir, List<_ScheduledEvent> scheduled) {
diff --git a/test/validator_test.dart b/test/validator_test.dart
index 40576ac34ca7ee23a42cca53364a9103441d1008..6f5764a95a4beabf134f6f640d83c019a743f784 100644
--- a/test/validator_test.dart
+++ b/test/validator_test.dart
@@ -9,9 +9,12 @@ import 'dart:json';
import 'test_pub.dart';
import '../../../pkg/unittest/lib/unittest.dart';
+import '../../../pkg/http/lib/http.dart' as http;
+import '../../../pkg/http/lib/testing.dart';
import '../../pub/entrypoint.dart';
import '../../pub/io.dart';
import '../../pub/validator.dart';
+import '../../pub/validator/dependency.dart';
import '../../pub/validator/lib.dart';
import '../../pub/validator/license.dart';
import '../../pub/validator/name.dart';
@@ -29,6 +32,9 @@ void expectValidationWarning(ValidatorCreator fn) {
expectLater(schedulePackageValidation(fn), pairOf(isEmpty, isNot(isEmpty)));
}
+Validator dependency(Entrypoint entrypoint) =>
+ new DependencyValidator(entrypoint);
+
Validator lib(Entrypoint entrypoint) => new LibValidator(entrypoint);
Validator license(Entrypoint entrypoint) => new LicenseValidator(entrypoint);
@@ -46,7 +52,10 @@ main() {
test('looks normal', () {
dir(appPath, [libPubspec("test_pkg", "1.0.0")]).scheduleCreate();
+ expectNoValidationError(dependency);
+ expectNoValidationError(lib);
expectNoValidationError(license);
+ expectNoValidationError(name);
expectNoValidationError(pubspecField);
run();
});
@@ -102,6 +111,16 @@ main() {
expectNoValidationError(lib);
run();
});
+
+ test('has an unconstrained dependency on "unittest"', () {
+ dir(appPath, [
+ libPubspec("test_pkg", "1.0.0", [
+ {'hosted': 'unittest'}
+ ])
+ ]).scheduleCreate();
+ expectNoValidationError(dependency);
+ run();
+ });
});
group('should consider a package invalid if it', () {
@@ -283,5 +302,234 @@ main() {
expectValidationError(lib);
run();
});
+
+ group('has a dependency with a non-hosted source', () {
+ group('where a hosted version of that dependency exists', () {
+ test("and should suggest the hosted package's primary version", () {
+ useMockClient(new MockClient((request) {
+ expect(request.method, equals("GET"));
+ expect(request.url.path, equals("/packages/foo.json"));
+
+ return new Future.immediate(new http.Response(JSON.stringify({
+ "name": "foo",
+ "uploaders": ["nweiz@google.com"],
+ "versions": ["3.0.0-pre", "2.0.0", "1.0.0"]
+ }), 200));
+ }));
+
+ dir(appPath, [
+ libPubspec("test_pkg", "1.0.0", [
+ {'git': 'git://github.com/dart-lang/foo'}
+ ])
+ ]).scheduleCreate();
+
+ expectLater(schedulePackageValidation(dependency),
+ pairOf(isEmpty, someElement(contains(
+ ' foo: ">=2.0.0 <3.0.0"'))));
+
+ run();
+ });
+
+ test("and should suggest the hosted package's prerelease version if "
+ "it's the only version available", () {
+ useMockClient(new MockClient((request) {
+ expect(request.method, equals("GET"));
+ expect(request.url.path, equals("/packages/foo.json"));
+
+ return new Future.immediate(new http.Response(JSON.stringify({
+ "name": "foo",
+ "uploaders": ["nweiz@google.com"],
+ "versions": ["3.0.0-pre", "2.0.0-pre"]
+ }), 200));
+ }));
+
+ dir(appPath, [
+ libPubspec("test_pkg", "1.0.0", [
+ {'git': 'git://github.com/dart-lang/foo'}
+ ])
+ ]).scheduleCreate();
+
+ expectLater(schedulePackageValidation(dependency),
+ pairOf(isEmpty, someElement(contains(
+ ' foo: ">=3.0.0-pre <4.0.0"'))));
+
+ run();
+ });
+
+ test("and should suggest a tighter constraint if the primary version "
+ "is pre-1.0.0", () {
+ useMockClient(new MockClient((request) {
+ expect(request.method, equals("GET"));
+ expect(request.url.path, equals("/packages/foo.json"));
+
+ return new Future.immediate(new http.Response(JSON.stringify({
+ "name": "foo",
+ "uploaders": ["nweiz@google.com"],
+ "versions": ["0.0.1", "0.0.2"]
+ }), 200));
+ }));
+
+ dir(appPath, [
+ libPubspec("test_pkg", "1.0.0", [
+ {'git': 'git://github.com/dart-lang/foo'}
+ ])
+ ]).scheduleCreate();
+
+ expectLater(schedulePackageValidation(dependency),
+ pairOf(isEmpty, someElement(contains(
+ ' foo: ">=0.0.2 <0.0.3"'))));
+
+ run();
+ });
+ });
+
+ group('where no hosted version of that dependency exists', () {
+ test("and should use the other source's version", () {
+ useMockClient(new MockClient((request) {
+ expect(request.method, equals("GET"));
+ expect(request.url.path, equals("/packages/foo.json"));
+
+ return new Future.immediate(new http.Response("not found", 404));
+ }));
+
+ dir(appPath, [
+ libPubspec("test_pkg", "1.0.0", [
+ {
+ 'git': {'url': 'git://github.com/dart-lang/foo'},
+ 'version': '>=1.0.0 <2.0.0'
+ }
+ ])
+ ]).scheduleCreate();
+
+ expectLater(schedulePackageValidation(dependency),
+ pairOf(isEmpty, someElement(contains(
+ ' foo: ">=1.0.0 <2.0.0"'))));
+
+ run();
+ });
+
+ test("and should use the other source's unquoted version if it's "
+ "concrete", () {
+ useMockClient(new MockClient((request) {
+ expect(request.method, equals("GET"));
+ expect(request.url.path, equals("/packages/foo.json"));
+
+ return new Future.immediate(new http.Response("not found", 404));
+ }));
+
+ dir(appPath, [
+ libPubspec("test_pkg", "1.0.0", [
+ {
+ 'git': {'url': 'git://github.com/dart-lang/foo'},
+ 'version': '0.2.3'
+ }
+ ])
+ ]).scheduleCreate();
+
+ expectLater(schedulePackageValidation(dependency),
+ pairOf(isEmpty, someElement(contains(' foo: 0.2.3'))));
+
+ run();
+ });
+ });
+ });
+
+ group('has an unconstrained dependency', () {
+ group('and it should not suggest a version', () {
+ test("if there's no lockfile", () {
+ dir(appPath, [
+ libPubspec("test_pkg", "1.0.0", [
+ {'hosted': 'foo'}
+ ])
+ ]).scheduleCreate();
+
+ expectLater(schedulePackageValidation(dependency),
+ pairOf(isEmpty, everyElement(isNot(contains("\n foo:")))));
+
+ run();
+ });
+
+ test("if the lockfile doesn't have an entry for the dependency", () {
+ dir(appPath, [
+ libPubspec("test_pkg", "1.0.0", [
+ {'hosted': 'foo'}
+ ]),
+ file("pubspec.lock", JSON.stringify({
+ 'packages': {
+ 'bar': {
+ 'version': '1.2.3',
+ 'source': 'hosted',
+ 'description': {
+ 'name': 'bar',
+ 'url': 'http://pub.dartlang.org'
+ }
+ }
+ }
+ }))
+ ]).scheduleCreate();
+
+ expectLater(schedulePackageValidation(dependency),
+ pairOf(isEmpty, everyElement(isNot(contains("\n foo:")))));
+
+ run();
+ });
+ });
+
+ group('with a lockfile', () {
+ test('and it should suggest a constraint based on the locked '
+ 'version', () {
+ dir(appPath, [
+ libPubspec("test_pkg", "1.0.0", [
+ {'hosted': 'foo'}
+ ]),
+ file("pubspec.lock", JSON.stringify({
+ 'packages': {
+ 'foo': {
+ 'version': '1.2.3',
+ 'source': 'hosted',
+ 'description': {
+ 'name': 'foo',
+ 'url': 'http://pub.dartlang.org'
+ }
+ }
+ }
+ }))
+ ]).scheduleCreate();
+
+ expectLater(schedulePackageValidation(dependency),
+ pairOf(isEmpty, someElement(contains(
+ ' foo: ">=1.2.3 <2.0.0"'))));
+
+ run();
+ });
+
+ test('and it should suggest a concrete constraint if the locked '
+ 'version is pre-1.0.0', () {
+ dir(appPath, [
+ libPubspec("test_pkg", "1.0.0", [
+ {'hosted': 'foo'}
+ ]),
+ file("pubspec.lock", JSON.stringify({
+ 'packages': {
+ 'foo': {
+ 'version': '0.1.2',
+ 'source': 'hosted',
+ 'description': {
+ 'name': 'foo',
+ 'url': 'http://pub.dartlang.org'
+ }
+ }
+ }
+ }))
+ ]).scheduleCreate();
+
+ expectLater(schedulePackageValidation(dependency),
+ pairOf(isEmpty, someElement(contains(
+ ' foo: ">=0.1.2 <0.1.3"'))));
+
+ run();
+ });
+ });
+ });
});
}