Add package implementation.

Add package implementation.

See dart-lang/test#327

R=rnystrom@google.com

Committed: https://github.com/dart-lang/package_resolver/commit/0b63bf3d6c62adec83638565fcbc71ae7406c8d9

[nweiz, 2016-07-07 23:04:39]

Description was changed from

==========
Add package implementation.

R=rnystrom@google.com
==========

to

==========
Add package implementation.

See dart-lang/test#327

R=rnystrom@google.com
==========

[Bob Nystrom, 2016-07-21 18:00:33]

Aside from a few questions/suggestions, LGTM.

https://codereview.chromium.org/2132443003/diff/1/lib/src/current_isolate_res...
File lib/src/current_isolate_resolver.dart (right):

https://codereview.chromium.org/2132443003/diff/1/lib/src/current_isolate_res...
lib/src/current_isolate_resolver.dart:22: if (url == null) return null;
When this happens, it will retry again every time the getter is called. Is that
OK?

https://codereview.chromium.org/2132443003/diff/1/lib/src/package_resolver.dart
File lib/src/package_resolver.dart (right):

https://codereview.chromium.org/2132443003/diff/1/lib/src/package_resolver.da...
lib/src/package_resolver.dart:64: /// Fetches the package reslusion for [this]
and returns an object that
"resolution".

https://codereview.chromium.org/2132443003/diff/1/lib/src/package_resolver.da...
lib/src/package_resolver.dart:65: /// provides synchronuos access.
"synchronous".

https://codereview.chromium.org/2132443003/diff/1/lib/src/package_resolver.da...
lib/src/package_resolver.dart:74: /// convention supported by the Dart VM and
dart2js.
"conventions".

https://codereview.chromium.org/2132443003/diff/1/lib/src/package_resolver.da...
lib/src/package_resolver.dart:95: /// request is needed, [client] is used to
make that request.
It might be good to clarify that you don't need to pass a client even if you're
giving it an HTTP URI.

https://codereview.chromium.org/2132443003/diff/1/lib/src/package_root_resolv...
File lib/src/package_root_resolver.dart (right):

https://codereview.chromium.org/2132443003/diff/1/lib/src/package_root_resolv...
lib/src/package_root_resolver.dart:31: pathSegments:
packageRoot.pathSegments.toList()..add(""));
This logic pretty much exists elsewhere too. Make a utility function?

https://codereview.chromium.org/2132443003/diff/1/test/current_isolate_info_t...
File test/current_isolate_info_test.dart (right):

https://codereview.chromium.org/2132443003/diff/1/test/current_isolate_info_t...
test/current_isolate_info_test.dart:39: // If the isolate running this test
isn't using package config, create one
Long line.

https://codereview.chromium.org/2132443003/diff/1/test/current_isolate_info_t...
test/current_isolate_info_test.dart:45: map[p.basename(link.path)] =
p.toUri(await link.resolveSymbolicLinks());
Ditto.

https://codereview.chromium.org/2132443003/diff/1/test/no_package_info_test.dart
File test/no_package_info_test.dart (right):

https://codereview.chromium.org/2132443003/diff/1/test/no_package_info_test.d...
test/no_package_info_test.dart:33: expect(() =>
resolver.resolveUri("file:///zip/zap"), throwsFormatException);
Long line.

[nweiz, 2016-07-21 19:42:34]

Code review changes

[nweiz, 2016-07-21 19:42:44]

https://codereview.chromium.org/2132443003/diff/1/lib/src/current_isolate_res...
File lib/src/current_isolate_resolver.dart (right):

https://codereview.chromium.org/2132443003/diff/1/lib/src/current_isolate_res...
lib/src/current_isolate_resolver.dart:22: if (url == null) return null;
On 2016/07/21 18:00:31, Bob Nystrom wrote:
> When this happens, it will retry again every time the getter is called. Is
that
> OK?

I think so. Isolate.packageConfig is itself cached, so we don't lose anything
other than an additional microtask tick.

https://codereview.chromium.org/2132443003/diff/1/lib/src/package_resolver.dart
File lib/src/package_resolver.dart (right):

https://codereview.chromium.org/2132443003/diff/1/lib/src/package_resolver.da...
lib/src/package_resolver.dart:64: /// Fetches the package reslusion for [this]
and returns an object that
On 2016/07/21 18:00:31, Bob Nystrom wrote:
> "resolution".

Done.

https://codereview.chromium.org/2132443003/diff/1/lib/src/package_resolver.da...
lib/src/package_resolver.dart:65: /// provides synchronuos access.
On 2016/07/21 18:00:32, Bob Nystrom wrote:
> "synchronous".

Done.

https://codereview.chromium.org/2132443003/diff/1/lib/src/package_resolver.da...
lib/src/package_resolver.dart:74: /// convention supported by the Dart VM and
dart2js.
On 2016/07/21 18:00:31, Bob Nystrom wrote:
> "conventions".

Done.

https://codereview.chromium.org/2132443003/diff/1/lib/src/package_resolver.da...
lib/src/package_resolver.dart:95: /// request is needed, [client] is used to
make that request.
On 2016/07/21 18:00:31, Bob Nystrom wrote:
> It might be good to clarify that you don't need to pass a client even if
you're
> giving it an HTTP URI.

Done.

https://codereview.chromium.org/2132443003/diff/1/lib/src/package_root_resolv...
File lib/src/package_root_resolver.dart (right):

https://codereview.chromium.org/2132443003/diff/1/lib/src/package_root_resolv...
lib/src/package_root_resolver.dart:31: pathSegments:
packageRoot.pathSegments.toList()..add(""));
On 2016/07/21 18:00:32, Bob Nystrom wrote:
> This logic pretty much exists elsewhere too. Make a utility function?

Done.

https://codereview.chromium.org/2132443003/diff/1/test/current_isolate_info_t...
File test/current_isolate_info_test.dart (right):

https://codereview.chromium.org/2132443003/diff/1/test/current_isolate_info_t...
test/current_isolate_info_test.dart:39: // If the isolate running this test
isn't using package config, create one
On 2016/07/21 18:00:32, Bob Nystrom wrote:
> Long line.

Done.

https://codereview.chromium.org/2132443003/diff/1/test/current_isolate_info_t...
test/current_isolate_info_test.dart:45: map[p.basename(link.path)] =
p.toUri(await link.resolveSymbolicLinks());
On 2016/07/21 18:00:32, Bob Nystrom wrote:
> Ditto.

Done.

https://codereview.chromium.org/2132443003/diff/1/test/no_package_info_test.dart
File test/no_package_info_test.dart (right):

https://codereview.chromium.org/2132443003/diff/1/test/no_package_info_test.d...
test/no_package_info_test.dart:33: expect(() =>
resolver.resolveUri("file:///zip/zap"), throwsFormatException);
On 2016/07/21 18:00:32, Bob Nystrom wrote:
> Long line.

Done.

[nweiz, 2016-07-21 19:43:01]

Description was changed from

==========
Add package implementation.

See dart-lang/test#327

R=rnystrom@google.com
==========

to

==========
Add package implementation.

See dart-lang/test#327

R=rnystrom@google.com

Committed:
https://github.com/dart-lang/package_resolver/commit/0b63bf3d6c62adec83638565...
==========

[nweiz, 2016-07-21 19:43:04]

Committed patchset #2 (id:20001) manually as
0b63bf3d6c62adec83638565fcbc71ae7406c8d9 (presubmit successful).