Add a validator for dependency version constraints.

Add a validator for dependency version constraints.

BUG=7045
Committed: https://code.google.com/p/dart/source/detail?r=16415

[Bob Nystrom, 2012-12-21 00:11:46]

Couple of nits/suggestions. LGTM.

https://codereview.chromium.org/11635060/diff/1/utils/pub/validator/dependenc...
File utils/pub/validator/dependency.dart (right):

https://codereview.chromium.org/11635060/diff/1/utils/pub/validator/dependenc...
utils/pub/validator/dependency.dart:29: // on it.
Mention bug #5358 here.

https://codereview.chromium.org/11635060/diff/1/utils/pub/validator/dependenc...
utils/pub/validator/dependency.dart:55: warnings.add('Don\'t depend on
"${ref.name}" from the ${ref.source.name} '
I believe we do have package authors uploading packages with git dependencies.
We've never told them not to do that. If pub is going to start suggesting
otherwise, we should have a public discussion about it too.

https://codereview.chromium.org/11635060/diff/1/utils/pub/validator/dependenc...
utils/pub/validator/dependency.dart:85: /// against [version].
Move this into version.dart? Seems generally useful.

https://codereview.chromium.org/11635060/diff/1/utils/pub/validator/dependenc...
utils/pub/validator/dependency.dart:87: if (version.major == 0) return
version.toString();
For 0.x.x packages, web_ui has been using (I think) a constraint that *does*
allow patch updates. I think something like ">=0.x.y <0.x.(y+1)".

https://codereview.chromium.org/11635060/diff/1/utils/tests/pub/validator_tes...
File utils/tests/pub/validator_test.dart (right):

https://codereview.chromium.org/11635060/diff/1/utils/tests/pub/validator_tes...
utils/tests/pub/validator_test.dart:115: test('has an unconstrainted dependency
on "unittest"', () {
"unconstrainted" -> "unconstrained"

https://codereview.chromium.org/11635060/diff/1/utils/tests/pub/validator_tes...
utils/tests/pub/validator_test.dart:333: test("and should suggest the hosted
package's prerelease version if it's "
Long line.

[nweiz, 2012-12-21 01:01:50]

https://codereview.chromium.org/11635060/diff/1/utils/pub/validator/dependenc...
File utils/pub/validator/dependency.dart (right):

https://codereview.chromium.org/11635060/diff/1/utils/pub/validator/dependenc...
utils/pub/validator/dependency.dart:29: // on it.
On 2012/12/21 00:11:46, Bob Nystrom wrote:
> Mention bug #5358 here.

Done.

https://codereview.chromium.org/11635060/diff/1/utils/pub/validator/dependenc...
utils/pub/validator/dependency.dart:55: warnings.add('Don\'t depend on
"${ref.name}" from the ${ref.source.name} '
On 2012/12/21 00:11:46, Bob Nystrom wrote:
> I believe we do have package authors uploading packages with git dependencies.
> We've never told them not to do that. If pub is going to start suggesting
> otherwise, we should have a public discussion about it too.

I think it's very reasonable to discourage Git dependencies, especially now that
those packages can be easily uploaded to pub.dartlang.org. Not only does Git not
play well with version constraints, but a package having a Git dependency means
that no one can use it (even transitively) without having Git installed.

Since this is just a warning anyway, I think it's fine to put in. I'm fine with
mentioning it on the mailing list as well if you want.

Empirically, only two packages on pub.dartlang.org have ever had Git
dependencies, and of those only one (buckshot) still does. I suspect this is
accidental, as the Git packages it depends on have their most recent versions on
pub.dartlang.org, and previous versions of buckshot used hosted dependencies.

https://codereview.chromium.org/11635060/diff/1/utils/pub/validator/dependenc...
utils/pub/validator/dependency.dart:85: /// against [version].
On 2012/12/21 00:11:46, Bob Nystrom wrote:
> Move this into version.dart? Seems generally useful.

Most of what this method is actually doing is formatting the version to put into
the YAML snippet, which I don't think is generally useful.

https://codereview.chromium.org/11635060/diff/1/utils/pub/validator/dependenc...
utils/pub/validator/dependency.dart:87: if (version.major == 0) return
version.toString();
On 2012/12/21 00:11:46, Bob Nystrom wrote:
> For 0.x.x packages, web_ui has been using (I think) a constraint that *does*
> allow patch updates. I think something like ">=0.x.y <0.x.(y+1)".

Done.

https://codereview.chromium.org/11635060/diff/1/utils/tests/pub/validator_tes...
File utils/tests/pub/validator_test.dart (right):

https://codereview.chromium.org/11635060/diff/1/utils/tests/pub/validator_tes...
utils/tests/pub/validator_test.dart:115: test('has an unconstrainted dependency
on "unittest"', () {
On 2012/12/21 00:11:46, Bob Nystrom wrote:
> "unconstrainted" -> "unconstrained"

Done.

https://codereview.chromium.org/11635060/diff/1/utils/tests/pub/validator_tes...
utils/tests/pub/validator_test.dart:333: test("and should suggest the hosted
package's prerelease version if it's "
On 2012/12/21 00:11:46, Bob Nystrom wrote:
> Long line.

Done.

[Bob Nystrom, 2012-12-21 01:48:06]

https://codereview.chromium.org/11635060/diff/1/utils/pub/validator/dependenc...
File utils/pub/validator/dependency.dart (right):

https://codereview.chromium.org/11635060/diff/1/utils/pub/validator/dependenc...
utils/pub/validator/dependency.dart:55: warnings.add('Don\'t depend on
"${ref.name}" from the ${ref.source.name} '
On 2012/12/21 01:01:50, nweiz wrote:
> On 2012/12/21 00:11:46, Bob Nystrom wrote:
> > I believe we do have package authors uploading packages with git
dependencies.
> > We've never told them not to do that. If pub is going to start suggesting
> > otherwise, we should have a public discussion about it too.
> 
> I think it's very reasonable to discourage Git dependencies, especially now
that
> those packages can be easily uploaded to http://pub.dartlang.org. Not only
does Git not
> play well with version constraints, but a package having a Git dependency
means
> that no one can use it (even transitively) without having Git installed.
> 
> Since this is just a warning anyway, I think it's fine to put in. I'm fine
with
> mentioning it on the mailing list as well if you want.
> 
> Empirically, only two packages on http://pub.dartlang.org have ever had Git
> dependencies, and of those only one (buckshot) still does. I suspect this is
> accidental, as the Git packages it depends on have their most recent versions
on
> http://pub.dartlang.org, and previous versions of buckshot used hosted
dependencies.

That sounds reasonable to me. We should probably document this somewhere, too.

[nweiz, 2012-12-21 01:54:57]

https://codereview.chromium.org/11635060/diff/1/utils/pub/validator/dependenc...
File utils/pub/validator/dependency.dart (right):

https://codereview.chromium.org/11635060/diff/1/utils/pub/validator/dependenc...
utils/pub/validator/dependency.dart:55: warnings.add('Don\'t depend on
"${ref.name}" from the ${ref.source.name} '
On 2012/12/21 01:48:06, Bob Nystrom wrote:
> On 2012/12/21 01:01:50, nweiz wrote:
> > On 2012/12/21 00:11:46, Bob Nystrom wrote:
> > > I believe we do have package authors uploading packages with git
> dependencies.
> > > We've never told them not to do that. If pub is going to start suggesting
> > > otherwise, we should have a public discussion about it too.
> > 
> > I think it's very reasonable to discourage Git dependencies, especially now
> that
> > those packages can be easily uploaded to http://pub.dartlang.org. Not only
> does Git not
> > play well with version constraints, but a package having a Git dependency
> means
> > that no one can use it (even transitively) without having Git installed.
> > 
> > Since this is just a warning anyway, I think it's fine to put in. I'm fine
> with
> > mentioning it on the mailing list as well if you want.
> > 
> > Empirically, only two packages on http://pub.dartlang.org have ever had Git
> > dependencies, and of those only one (buckshot) still does. I suspect this is
> > accidental, as the Git packages it depends on have their most recent
versions
> on
> > http://pub.dartlang.org, and previous versions of buckshot used hosted
> dependencies.
> 
> That sounds reasonable to me. We should probably document this somewhere, too.

Filed 7553 to track that.