Refactor URI processing of native extensions directive #import("dart-ext:foo").

runtime/bin/builtin.dart

Index: runtime/bin/builtin.dart
diff --git a/runtime/bin/builtin.dart b/runtime/bin/builtin.dart
index b61314748d2e6c80b82e200c9952a9a3884a3b07..4b458b6d1a989f565292735804b1306b87619361 100644
--- a/runtime/bin/builtin.dart
+++ b/runtime/bin/builtin.dart
@@ -67,37 +67,35 @@ String _resolveScriptUri(String cwd, String scriptName, bool windows) {
 String _resolveUri(String base, String userString) {
   var baseUri = new Uri.fromString(base);
   _logResolution("# Resolving: $userString from $base");
-  var resolved = baseUri.resolve(userString);
-  _logResolution("# Resolved to: $resolved");
-  return resolved.toString();
-}
 
-String _resolveExtensionUri(String base, String userString) {
+  // Relative URIs with scheme dart-ext should be resolved as if with no scheme.
   var uri = new Uri.fromString(userString);
-  if ("dart-ext" != uri.scheme) {
-    throw "Not a Dart extension uri: $uri";
+  var resolved;
+  if ('dart-ext' == uri.scheme) {
+    resolved = baseUri.resolve(uri.path);
+    resolved = new Uri(scheme: "dart-ext", path: resolved.path);
+    // resolved.scheme = 'foo';  // Causes a segfault.  Verify this is expected.

[Ivan Posva, 2012/04/17 18:50:06]

Since the bug is filed now, you can remove this commented out code.

[Bill Hesse, 2012/04/18 09:56:39]

This line will be removed.  A separate bug and reproduction
(http://code.google.com/p/dart/issues/detail?id=2621) has been filed.

+  } else {
+    resolved = baseUri.resolve(userString);
   }
-  var schemelessUri = new Uri(path: uri.path);
-  var baseUri = new Uri.fromString(base);
-  _logResolution("# Resolving: $userString from $base");
-  var resolved = baseUri.resolveUri(schemelessUri);
-  resolved = new Uri(scheme: 'dart-ext', path: resolved.path);
   _logResolution("# Resolved to: $resolved");
   return resolved.toString();
 }
 
+
 String _filePathFromUri(String userUri) {
   var uri = new Uri.fromString(userUri);
   _logResolution("# Getting file path from: $uri");
 
   var path;
   switch (uri.scheme) {
-  case 'file':    path = _filePathFromFileUri(uri); break;
+  case 'file':
+  case 'dart-ext': path = _filePathFromFileUri(uri); break;

[Ivan Posva, 2012/04/17 18:50:06]

Since dart-ext is not a file URI I am expecting this code to break once we
introduce an assert inside _filePathFromFileUri.

How about a remapping step before getting into this code converting the dart-ext
URI into the switch, such as this?

if (uri.scheme == "dart-ext") {
  uri = new Uri(scheme: "file", path: uri.path);
}

I can imagine that we could add other file-like URIs in the future.

[Bill Hesse, 2012/04/18 09:56:39]

Fixed by calling _filePathFromOtherUri.

I wrote code that did such a remapping, but replaced it because "Get a file path
from a URI, ignoring the scheme" is a clear, consistent functionality that is
needed, and implementing it by faking a file: scheme just so an added check can
pass seems like the wrong solution.

Adding the check would catch places where we accidentally call it with a
non-file: scheme, but we know that we are intentionally calling it with a
non-file: scheme in places.

As a temporary fix, I cloned the function _filePathFromFileUri, and called it
_filePathFromOtherUri.
I think a good refactoring in the future would be that the common code could
actually go into _filePathFromUri, and only package: would call an external
helper.

On 2012/04/17 18:50:06, Ivan Posva wrote:

   case 'package': path = _filePathFromPackageUri(uri); break;
 
   default:
-    // Only handling file and package URIs in standalone binary.
-    _logResolution("# Not a file or package URI.");
+    // Only handling file, dart-ext and package URIs in standalone binary.
+    _logResolution("# Not a file, dart-ext, or package URI.");
     throw "Not a known scheme: $uri";
   }