Use URIs rather than paths in front end API.

Use URIs rather than paths in front end API.

This carries a number of benefits:

- It allows the front end to trivially support schemes other than
  "file:" (e.g. "http:") by allowing the client to supply a FileSystem
  implementation that handles them.

- It is more consistent with the functionality of the ".packages" file
  (which allows packages to map to any kind of URI).

- It allows the "bazel root" feature to be rewritten to use a magic
  scheme rather than a magic path.  (This eliminates concerns about
  the magic path overlapping with a user's use case).  Note that this
  feature has been renamed to "multi root" since it is sufficiently
  generic to be applicable to build systems other than Bazel.

- It reduces the risk of forgetting to use the front end's FileSystem
  abstraction to access the file system, since the native file system
  interfaces do not accept URIs.

R=danrubel@google.com

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

[Paul Berry, 2017-01-06 23:03:43]

Description was changed from

==========
Use URIs rather than paths in front end API.

This carries a number of benefits:

- It allows the front end to trivially support schemes other than
  "file:" (e.g. "http:") by allowing the client to supply a FileSystem
  implementation that handles them.

- It is more consistent with the functionality of the ".packages" file
  (which allows packages to map to any kind of URI).

- It allows the "bazel root" feature to be rewritten to use a magic
  scheme rather than a magic path.  (This eliminates concerns about
  the magic path overlapping with a user's use case).

- It reduces the risk of forgetting to use the front end's FileSystem
  abstraction to access the file system, since the native file system
  interfaces do not accept URIs.
==========

to

==========
Use URIs rather than paths in front end API.

This carries a number of benefits:

- It allows the front end to trivially support schemes other than
  "file:" (e.g. "http:") by allowing the client to supply a FileSystem
  implementation that handles them.

- It is more consistent with the functionality of the ".packages" file
  (which allows packages to map to any kind of URI).

- It allows the "bazel root" feature to be rewritten to use a magic
  scheme rather than a magic path.  (This eliminates concerns about
  the magic path overlapping with a user's use case).  Note that this
  feature has been renamed to "multi root" since it is sufficiently
  generic to be applicable to build systems other than Bazel.

- It reduces the risk of forgetting to use the front end's FileSystem
  abstraction to access the file system, since the native file system
  interfaces do not accept URIs.
==========

[Paul Berry, 2017-01-06 23:04:06]

paulberry@google.com changed reviewers:
+ danrubel@google.com, sigmund@google.com

[danrubel, 2017-01-08 17:07:38]

LGTM

There are several places where locals are named "path" but hold a Uri. IMO this
makes the code harder to read and maintain and I recommend renaming them in the
near future.

https://codereview.chromium.org/2614063007/diff/20001/pkg/front_end/lib/compi...
File pkg/front_end/lib/compiler_options.dart (right):

https://codereview.chromium.org/2614063007/diff/20001/pkg/front_end/lib/compi...
pkg/front_end/lib/compiler_options.dart:27: Uri sdkPath;
Since this is no longer a path, perhaps rename sdkPath to sdkRoot? sdkUri?

https://codereview.chromium.org/2614063007/diff/20001/pkg/front_end/lib/compi...
pkg/front_end/lib/compiler_options.dart:42: Uri packagesFilePath;
Same as above... packagesFileUri?

https://codereview.chromium.org/2614063007/diff/20001/pkg/front_end/lib/src/d...
File pkg/front_end/lib/src/dependency_grapher_impl.dart (right):

https://codereview.chromium.org/2614063007/diff/20001/pkg/front_end/lib/src/d...
pkg/front_end/lib/src/dependency_grapher_impl.dart:108: var contents = await
walker.fileSystem.entityForUri(path).readAsString();
rename "path" to "resolvedUri" ? "fileUri"?

[Paul Berry, 2017-01-09 17:24:42]

https://codereview.chromium.org/2614063007/diff/20001/pkg/front_end/lib/compi...
File pkg/front_end/lib/compiler_options.dart (right):

https://codereview.chromium.org/2614063007/diff/20001/pkg/front_end/lib/compi...
pkg/front_end/lib/compiler_options.dart:27: Uri sdkPath;
On 2017/01/08 17:07:38, danrubel wrote:
> Since this is no longer a path, perhaps rename sdkPath to sdkRoot? sdkUri?

Done.

https://codereview.chromium.org/2614063007/diff/20001/pkg/front_end/lib/compi...
pkg/front_end/lib/compiler_options.dart:42: Uri packagesFilePath;
On 2017/01/08 17:07:38, danrubel wrote:
> Same as above... packagesFileUri?

Done.

[danrubel, 2017-01-09 18:33:13]

LGTM

[Paul Berry, 2017-01-09 19:19:43]

Description was changed from

==========
Use URIs rather than paths in front end API.

This carries a number of benefits:

- It allows the front end to trivially support schemes other than
  "file:" (e.g. "http:") by allowing the client to supply a FileSystem
  implementation that handles them.

- It is more consistent with the functionality of the ".packages" file
  (which allows packages to map to any kind of URI).

- It allows the "bazel root" feature to be rewritten to use a magic
  scheme rather than a magic path.  (This eliminates concerns about
  the magic path overlapping with a user's use case).  Note that this
  feature has been renamed to "multi root" since it is sufficiently
  generic to be applicable to build systems other than Bazel.

- It reduces the risk of forgetting to use the front end's FileSystem
  abstraction to access the file system, since the native file system
  interfaces do not accept URIs.
==========

to

==========
Use URIs rather than paths in front end API.

This carries a number of benefits:

- It allows the front end to trivially support schemes other than
  "file:" (e.g. "http:") by allowing the client to supply a FileSystem
  implementation that handles them.

- It is more consistent with the functionality of the ".packages" file
  (which allows packages to map to any kind of URI).

- It allows the "bazel root" feature to be rewritten to use a magic
  scheme rather than a magic path.  (This eliminates concerns about
  the magic path overlapping with a user's use case).  Note that this
  feature has been renamed to "multi root" since it is sufficiently
  generic to be applicable to build systems other than Bazel.

- It reduces the risk of forgetting to use the front end's FileSystem
  abstraction to access the file system, since the native file system
  interfaces do not accept URIs.

R=danrubel@google.com

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

[Paul Berry, 2017-01-09 19:19:44]

Committed patchset #4 (id:60001) manually as
ba241c0c8d23b22717eb0de99dcbf1b53591a42d (presubmit successful).

[Siggi Cherem (dart-lang), 2017-01-11 22:59:53]

lgtm

https://codereview.chromium.org/2614063007/diff/60001/pkg/front_end/lib/physi...
File pkg/front_end/lib/physical_file_system.dart (right):

https://codereview.chromium.org/2614063007/diff/60001/pkg/front_end/lib/physi...
pkg/front_end/lib/physical_file_system.dart:57: Future<List<int>> readAsBytes()
=> new io.File(_path).readAsBytes();
would `new io.File.fromUri` work as well (and no need to convert to a path
altogether)?

https://codereview.chromium.org/2614063007/diff/60001/pkg/front_end/tool/perf...
File pkg/front_end/tool/perf.dart (right):

https://codereview.chromium.org/2614063007/diff/60001/pkg/front_end/tool/perf...
pkg/front_end/tool/perf.dart:118: ..packagesFileUri = new Uri.file('.packages')
probably doesn't matter, but another option here would be to make it absolute
right away by doing `Uri.base.resolve('relative-path')`.

[Paul Berry, 2017-01-12 23:10:04]

https://codereview.chromium.org/2614063007/diff/60001/pkg/front_end/lib/physi...
File pkg/front_end/lib/physical_file_system.dart (right):

https://codereview.chromium.org/2614063007/diff/60001/pkg/front_end/lib/physi...
pkg/front_end/lib/physical_file_system.dart:57: Future<List<int>> readAsBytes()
=> new io.File(_path).readAsBytes();
On 2017/01/11 22:59:53, Siggi Cherem (dart-lang) wrote:
> would `new io.File.fromUri` work as well (and no need to convert to a path
> altogether)?

Yes, that simplifies things.  Thanks.

https://codereview.chromium.org/2614063007/diff/60001/pkg/front_end/tool/perf...
File pkg/front_end/tool/perf.dart (right):

https://codereview.chromium.org/2614063007/diff/60001/pkg/front_end/tool/perf...
pkg/front_end/tool/perf.dart:118: ..packagesFileUri = new Uri.file('.packages')
On 2017/01/11 22:59:53, Siggi Cherem (dart-lang) wrote:
> probably doesn't matter, but another option here would be to make it absolute
> right away by doing `Uri.base.resolve('relative-path')`.

One of the goals of the FileSystem abstraction is to make it unnecessary for
client code to deal with stuff like converting paths to absolute.  So I'd prefer
to leave it as is.