Make sure pub build/serve get at the right time:

Make sure pub build/serve get at the right time:

1. Take overrides into account.
2. Do get if a cache package is not installed.

BUG=http://dartbug.com/14590, http://dartbug.com/15180
R=nweiz@google.com

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

[nweiz, 2013-11-26 21:43:11]

https://codereview.chromium.org/83173011/diff/30001/sdk/lib/_internal/pub/lib...
File sdk/lib/_internal/pub/lib/src/entrypoint.dart (right):

https://codereview.chromium.org/83173011/diff/30001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/entrypoint.dart:201: Future<bool>
_ensurePackagesAvailable(LockFile lockFile) {
This name is inaccurate, since the method doesn't actually install the packages
if they're unavailable. I suggest [_packagesAreAvailable] or something similar.

https://codereview.chromium.org/83173011/diff/30001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/entrypoint.dart:204: assert(source != null);
This makes me uncomfortable. If we add a source in the future and a lockfile
with that source is seen by an older version of pub, this assert will fail and
the user will be confused. At the very least throw an error with a reasonable
message here.

https://codereview.chromium.org/83173011/diff/30001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/entrypoint.dart:254: if (!upToDate) return
getDependencies().then((_) {
Don't hang a multiline expression off a bracketless if statement. I'd write this
as

    if (upToDate) return null;
    return getDependencies().then(...);

https://codereview.chromium.org/83173011/diff/30001/sdk/lib/_internal/pub/lib...
File sdk/lib/_internal/pub/lib/src/package.dart (right):

https://codereview.chromium.org/83173011/diff/30001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/package.dart:46: List<PackageDep> get
immediateDependencies {
Since the dependency list has no explicit order, should this be a Set?

[Bob Nystrom, 2013-12-03 19:00:48]

Thanks!

https://codereview.chromium.org/83173011/diff/30001/sdk/lib/_internal/pub/lib...
File sdk/lib/_internal/pub/lib/src/entrypoint.dart (right):

https://codereview.chromium.org/83173011/diff/30001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/entrypoint.dart:201: Future<bool>
_ensurePackagesAvailable(LockFile lockFile) {
On 2013/11/26 21:43:11, nweiz wrote:
> This name is inaccurate, since the method doesn't actually install the
packages
> if they're unavailable. I suggest [_packagesAreAvailable] or something
similar.

Done: _arePackagesAvailable.

https://codereview.chromium.org/83173011/diff/30001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/entrypoint.dart:204: assert(source != null);
On 2013/11/26 21:43:11, nweiz wrote:
> This makes me uncomfortable. If we add a source in the future and a lockfile
> with that source is seen by an older version of pub, this assert will fail and
> the user will be confused. At the very least throw an error with a reasonable
> message here.

Added a comment explaining the assert. We should only get here if the sources
are already known to be valid.

https://codereview.chromium.org/83173011/diff/30001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/entrypoint.dart:254: if (!upToDate) return
getDependencies().then((_) {
On 2013/11/26 21:43:11, nweiz wrote:
> Don't hang a multiline expression off a bracketless if statement. I'd write
this
> as
> 
>     if (upToDate) return null;
>     return getDependencies().then(...);

Done.

https://codereview.chromium.org/83173011/diff/30001/sdk/lib/_internal/pub/lib...
File sdk/lib/_internal/pub/lib/src/package.dart (right):

https://codereview.chromium.org/83173011/diff/30001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/package.dart:46: List<PackageDep> get
immediateDependencies {
On 2013/11/26 21:43:11, nweiz wrote:
> Since the dependency list has no explicit order, should this be a Set?

Done. I did a List to be consistent with the others, but a Set makes more sense.

[nweiz, 2013-12-03 20:56:48]

lgtm

[Bob Nystrom, 2013-12-03 23:14:13]

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