Select packages that match SDK constraints.

Select packages that match SDK constraints.

BUG=https://code.google.com/p/dart/issues/detail?id=9216

Committed: https://code.google.com/p/dart/source/detail?r=21659

[nweiz, 2013-04-16 23:52:32]

I'm worried about the following use case:

* The user learns about a new feature in some_library.
* They run "pub update some_library", pub completes successfully.
* They try and fail to use said feature.
* They run "pub update" to update everything.
* They try and fail again to use said feature.
* They cry themselves to sleep.

What I'm saying is that this makes it easy for users to get silently trapped in
an outdated world because their SDK is old. I'd like to find a way to provide a
little more feedback that can help to indicate that their packages are being
held back because of SDK conflicts.

https://codereview.chromium.org/14298006/diff/1/utils/pub/command_install.dart
File utils/pub/command_install.dart (left):

https://codereview.chromium.org/14298006/diff/1/utils/pub/command_install.dar...
utils/pub/command_install.dart:21: .then((_) =>
entrypoint.validateSdkConstraints());
Keep this in (here and in update) until we remove the old solver.

https://codereview.chromium.org/14298006/diff/1/utils/pub/solver/backtracking...
File utils/pub/solver/backtracking_solver.dart (right):

https://codereview.chromium.org/14298006/diff/1/utils/pub/solver/backtracking...
utils/pub/solver/backtracking_solver.dart:129: _attemptedSolutions++;
Why is this moving in here? Won't this just decrement the reported number of
solutions by 1? It doesn't look like you've adjusted the maxTries flags on the
tests accordingly.

https://codereview.chromium.org/14298006/diff/1/utils/pub/solver/version_solv...
File utils/pub/solver/version_solver.dart (right):

https://codereview.chromium.org/14298006/diff/1/utils/pub/solver/version_solv...
utils/pub/solver/version_solver.dart:52: void validateSdkConstraint(Pubspec
pubspec) {
It seems really weird that this is a top-level, public function in this library.
It's just used by the two solvers, right? Not by any clients of this library? It
seems like it belongs in a utils library.

https://codereview.chromium.org/14298006/diff/1/utils/tests/pub/sdk_constrain...
File utils/tests/pub/sdk_constraint_test.dart (left):

https://codereview.chromium.org/14298006/diff/1/utils/tests/pub/sdk_constrain...
utils/tests/pub/sdk_constraint_test.dart:1: // Copyright (c) 2012, the Dart
project authors.  Please see the AUTHORS file
Keep this in until we take out the old solver as well.

[Bob Nystrom, 2013-04-17 17:28:44]

https://codereview.chromium.org/14298006/diff/1/utils/pub/command_install.dart
File utils/pub/command_install.dart (left):

https://codereview.chromium.org/14298006/diff/1/utils/pub/command_install.dar...
utils/pub/command_install.dart:21: .then((_) =>
entrypoint.validateSdkConstraints());
On 2013/04/16 23:52:32, nweiz wrote:
> Keep this in (here and in update) until we remove the old solver.

Done.

https://codereview.chromium.org/14298006/diff/1/utils/pub/solver/backtracking...
File utils/pub/solver/backtracking_solver.dart (right):

https://codereview.chromium.org/14298006/diff/1/utils/pub/solver/backtracking...
utils/pub/solver/backtracking_solver.dart:129: _attemptedSolutions++;
On 2013/04/16 23:52:32, nweiz wrote:
> Why is this moving in here?

This handles a weird little corner case. If the root package's SDK constraint
does not match the current SDK, then VersionSolver will report an error before
ever calling _traverseSolution(). This meant that in that case, it would return
a SolveResult with 0 attempted solutions instead of 1.

So this change ensures the attempted solutions always start at 1.

> Won't this just decrement the reported number of
> solutions by 1?

It would except the initializer for _attemptedSolutions now starts it at 1.

> It doesn't look like you've adjusted the maxTries flags on the
> tests accordingly.

The end result is the same.

https://codereview.chromium.org/14298006/diff/1/utils/pub/solver/version_solv...
File utils/pub/solver/version_solver.dart (right):

https://codereview.chromium.org/14298006/diff/1/utils/pub/solver/version_solv...
utils/pub/solver/version_solver.dart:52: void validateSdkConstraint(Pubspec
pubspec) {
On 2013/04/16 23:52:32, nweiz wrote:
> It seems really weird that this is a top-level, public function in this
library.
> It's just used by the two solvers, right?

It was being used by both VersionSolver and BacktrackingSolver. But to keep the
old SDK constraint stuff still working, I moved it so that only
BacktrackingSolver uses it, so I just moved this into that library.

https://codereview.chromium.org/14298006/diff/1/utils/tests/pub/sdk_constrain...
File utils/tests/pub/sdk_constraint_test.dart (left):

https://codereview.chromium.org/14298006/diff/1/utils/tests/pub/sdk_constrain...
utils/tests/pub/sdk_constraint_test.dart:1: // Copyright (c) 2012, the Dart
project authors.  Please see the AUTHORS file
On 2013/04/16 23:52:32, nweiz wrote:
> Keep this in until we take out the old solver as well.

Done.

[Bob Nystrom, 2013-04-17 17:42:15]

On 2013/04/16 23:52:32, nweiz wrote:
> I'm worried about the following use case:
> 
> * The user learns about a new feature in some_library.
> * They run "pub update some_library", pub completes successfully.
> * They try and fail to use said feature.
> * They run "pub update" to update everything.
> * They try and fail again to use said feature.
> * They cry themselves to sleep.
> 
> What I'm saying is that this makes it easy for users to get silently trapped
in
> an outdated world because their SDK is old. I'd like to find a way to provide
a
> little more feedback that can help to indicate that their packages are being
> held back because of SDK conflicts.

I'm worried about this too. I think it will be pretty easy to add some messaging
here to help explain to users what's going on. I'd like to do that in a later a
patch both for simplicity and to put this in front of users and see what kinds
of questions they actually have so we can get a feel for what messaging will
work best.

- bob

[nweiz, 2013-04-17 18:40:54]

lgtm

[Bob Nystrom, 2013-04-17 23:14:51]

Committed patchset #3 manually as r21659 (presubmit successful).