Per isolate package root.

runtime/bin/main.cc

Index: runtime/bin/main.cc
diff --git a/runtime/bin/main.cc b/runtime/bin/main.cc
index 941aafee5d8c1e899afee4a5c6a929fa0ec02559..e29488037edcebd68b3ae3ff9de05721f16a6dc7 100644
--- a/runtime/bin/main.cc
+++ b/runtime/bin/main.cc
@@ -54,7 +54,7 @@ static const int DEFAULT_DEBUG_PORT = 5858;
 // Value of the --package-root flag.
 // (This pointer points into an argv buffer and does not need to be
 // free'd.)
-static const char* package_root = NULL;
+static const char* commandline_package_root = NULL;
 
 
 // Global flag that is used to indicate that we want to compile all the
@@ -136,7 +136,7 @@ static bool ProcessPackageRootOption(const char* arg,
   if (*arg == '\0' || *arg == '-') {
     return false;
   }
-  package_root = arg;
+  commandline_package_root = arg;
   return true;
 }
 
@@ -587,14 +587,16 @@ static Dart_Isolate CreateIsolateAndSetupHelper(const char* script_uri,
       Builtin::LoadAndCheckLibrary(Builtin::kBuiltinLibrary);
   CHECK_RESULT(builtin_lib);
 
+  IsolateData* isolate_data = reinterpret_cast<IsolateData*>(data);

[Ivan Posva, 2014/09/08 05:46:35]

Unneeded.

[Anders Johnsen, 2014/09/09 06:45:43]

Done.

+  ASSERT(isolate_data != NULL);
+  ASSERT(isolate_data->script_url != NULL);
+  const char* package_root = isolate_data->package_root;
+
   // Prepare for script loading by setting up the 'print' and 'timer'
   // closures and setting up 'package root' for URI resolution.
   result = DartUtils::PrepareForScriptLoading(package_root, builtin_lib);
   CHECK_RESULT(result);
 
-  IsolateData* isolate_data = reinterpret_cast<IsolateData*>(data);
-  ASSERT(isolate_data != NULL);
-  ASSERT(isolate_data->script_url != NULL);
   result = DartUtils::LoadScript(isolate_data->script_url, builtin_lib);
   CHECK_RESULT(result);
 
@@ -636,21 +638,25 @@ static Dart_Isolate CreateIsolateAndSetupHelper(const char* script_uri,
 
 static Dart_Isolate CreateIsolateAndSetup(const char* script_uri,
                                           const char* main,
+                                          const char* package_root,
                                           void* data, char** error) {
+  IsolateData* parent_isolate_data = reinterpret_cast<IsolateData*>(data);
   bool is_compile_error = false;
   if (script_uri == NULL) {
     if (data == NULL) {
       *error = strdup("Invalid 'callback_data' - Unable to spawn new isolate");
       return NULL;
     }
-    IsolateData* parent_isolate_data = reinterpret_cast<IsolateData*>(data);
     script_uri = parent_isolate_data->script_url;
     if (script_uri == NULL) {
       *error = strdup("Invalid 'callback_data' - Unable to spawn new isolate");
       return NULL;
     }
   }
-  IsolateData* isolate_data = new IsolateData(script_uri);
+  if (package_root == NULL) {
+    package_root = parent_isolate_data->package_root;
+  }
+  IsolateData* isolate_data = new IsolateData(script_uri, package_root);
   return CreateIsolateAndSetupHelper(script_uri,
                                      main,
                                      isolate_data,

[turnidge, 2014/09/08 19:47:00]

You are treating script_uri and package_root assymetrically here.

I see that you are tucking away script_uri and package_root in the data for
later.  But you then explicilty pass "script_uri" to CreateIsolateAndSetupHelper
but rely on isolate_data to pass in "package_root".  Instead how about you pass
in package_root as an explicit parameter? 

[Anders Johnsen, 2014/09/09 06:45:43]

It would needed to be passed both places (like script_uri). Changing, but let me
know if this is not what you meant.

@@ -669,7 +675,7 @@ static Dart_Isolate CreateIsolateAndSetup(const char* script_uri,
 
 static Dart_Isolate CreateServiceIsolate(void* data, char** error) {
   const char* script_uri = DartUtils::kVMServiceLibURL;
-  IsolateData* isolate_data = new IsolateData(script_uri);
+  IsolateData* isolate_data = new IsolateData(script_uri, NULL);
   Dart_Isolate isolate =
       Dart_CreateIsolate(script_uri, "main", snapshot_buffer, isolate_data,
                          error);
@@ -693,9 +699,10 @@ static Dart_Isolate CreateServiceIsolate(void* data, char** error) {
   CHECK_RESULT(builtin_lib);
   // Prepare for script loading by setting up the 'print' and 'timer'
   // closures and setting up 'package root' for URI resolution.
-  result = DartUtils::PrepareForScriptLoading(package_root, builtin_lib);
+  result = DartUtils::PrepareForScriptLoading(commandline_package_root,
+                                              builtin_lib);

[Ivan Posva, 2014/09/08 05:46:35]

I would expect the service isolate to not expect a valid package root. The
command line package root is certainly not the right one to use.

[Anders Johnsen, 2014/09/09 06:45:43]

Will pass 'NULL' then.

   CHECK_RESULT(result);
-  Platform::SetPackageRoot(package_root);
+  Platform::SetPackageRoot(commandline_package_root);
   Dart_Handle io_lib_url = DartUtils::NewString(DartUtils::kIOLibURL);
   CHECK_RESULT(io_lib_url);
   Dart_Handle io_lib = Dart_LookupLibrary(io_lib_url);
@@ -1044,7 +1051,8 @@ void main(int argc, char** argv) {
   char* error = NULL;
   bool is_compile_error = false;
   char* isolate_name = BuildIsolateName(script_name, "main");
-  IsolateData* isolate_data = new IsolateData(script_name);
+  IsolateData* isolate_data = new IsolateData(script_name,
+                                              commandline_package_root);
   Dart_Isolate isolate = CreateIsolateAndSetupHelper(script_name,
                                                      "main",
                                                      isolate_data,