- Implement .packages specification.

- Implement .packages specification.

Limitation: Currently only works for file: based .package files. If a script is loaded from http: then the VM currently assumes a co-located packages/ directory.

R=asiva@google.com

Committed: https://github.com/dart-lang/sdk/commit/b07a9ddbcac8425a4468014a694a81f403e1798d

[Ivan Posva, 2015-07-28 17:38:14]

iposva@google.com changed reviewers:
+ asiva@google.com

[siva, 2015-07-28 23:50:09]

lgtm

https://codereview.chromium.org/1232593003/diff/120001/runtime/bin/builtin.dart
File runtime/bin/builtin.dart (right):

https://codereview.chromium.org/1232593003/diff/120001/runtime/bin/builtin.da...
runtime/bin/builtin.dart:107: 
Can you move the _log(msg) function down here, it seems to intrude into the
comments and the variable declarations up there.

https://codereview.chromium.org/1232593003/diff/120001/runtime/bin/main.cc
File runtime/bin/main.cc (right):

https://codereview.chromium.org/1232593003/diff/120001/runtime/bin/main.cc#ne...
runtime/bin/main.cc:390: { "--packages=", ProcessPackagesOption },
PrintUsage needs to be updated for this option.

https://codereview.chromium.org/1232593003/diff/120001/runtime/bin/vmservice/...
File runtime/bin/vmservice/loader.dart (right):

https://codereview.chromium.org/1232593003/diff/120001/runtime/bin/vmservice/...
runtime/bin/vmservice/loader.dart:226: var packageName = new
String.fromCharCodes(data, start, separator);
packageName duplicate check (needs to be a warning or a hard error according to
the spec). If it is a warning it needs to be a hard error when the package name
is used.

https://codereview.chromium.org/1232593003/diff/120001/runtime/bin/vmservice/...
runtime/bin/vmservice/loader.dart:334: }
This chunk of code for finiding ".packages" file could be factored into a helper
function as it is repeated twice.

https://codereview.chromium.org/1232593003/diff/120001/runtime/bin/vmservice/...
runtime/bin/vmservice/loader.dart:351: if (id == -1) {
-1 and -2 seem like magic numbers maybe we can have a common import file for the
two files with these defined as named constants.

[Ivan Posva, 2015-07-29 18:22:18]

Thanks.

-Ivan

https://codereview.chromium.org/1232593003/diff/120001/runtime/bin/builtin.dart
File runtime/bin/builtin.dart (right):

https://codereview.chromium.org/1232593003/diff/120001/runtime/bin/builtin.da...
runtime/bin/builtin.dart:107: 
On 2015/07/28 23:50:09, siva wrote:
> Can you move the _log(msg) function down here, it seems to intrude into the
> comments and the variable declarations up there.

Done.

https://codereview.chromium.org/1232593003/diff/120001/runtime/bin/main.cc
File runtime/bin/main.cc (right):

https://codereview.chromium.org/1232593003/diff/120001/runtime/bin/main.cc#ne...
runtime/bin/main.cc:390: { "--packages=", ProcessPackagesOption },
On 2015/07/28 23:50:09, siva wrote:
> PrintUsage needs to be updated for this option.

Done.

https://codereview.chromium.org/1232593003/diff/120001/runtime/bin/vmservice/...
File runtime/bin/vmservice/loader.dart (right):

https://codereview.chromium.org/1232593003/diff/120001/runtime/bin/vmservice/...
runtime/bin/vmservice/loader.dart:226: var packageName = new
String.fromCharCodes(data, start, separator);
On 2015/07/28 23:50:09, siva wrote:
> packageName duplicate check (needs to be a warning or a hard error according
to
> the spec). If it is a warning it needs to be a hard error when the package
name
> is used.

Added TODO in builtin.dart where we are adding the entries into the map.
Separate CL.

https://codereview.chromium.org/1232593003/diff/120001/runtime/bin/vmservice/...
runtime/bin/vmservice/loader.dart:334: }
On 2015/07/28 23:50:09, siva wrote:
> This chunk of code for finiding ".packages" file could be factored into a
helper
> function as it is repeated twice.

Refactored into a single loop. It probably now makes it easier to understand.

[Ivan Posva, 2015-07-29 18:30:38]

Committed patchset #8 (id:140001) manually as
b07a9ddbcac8425a4468014a694a81f403e1798d (presubmit successful).