[modules] Do path resolution relative to each module file in d8

src/d8.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <errno.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/stat.h>
 
 #include <algorithm>
@@ skipping 493 matching lines @@
       }
     } else {
       v8::String::Utf8Value str(Stringify(isolate, result));
       fwrite(*str, sizeof(**str), str.length(), stdout);
       printf("\n");
     }
   }
   return true;
 }
 
+namespace {
+
+bool IsAbsolutePath(const char* path) {
+#if defined(_WIN32) || defined(_WIN64)
+  // TODO(adamk): Do something more robust.
+  const char* colon = strchr(path, ':');
+  const char* backslash = strchr(path, '\\');
+  const char* slash = strchr(path, '/');
+  return colon != nullptr && (backslash == nullptr || colon < backslash) &&
+         (slash == nullptr || colon < slash);
+#else

[neis, 2016/09/22 01:07:39]

I don't understand why you make it so complicated if it's clearly not correct in
general.  For now, can't you just test for a colon and that's it?

[adamk, 2016/09/22 19:06:57]

Just looking for a colon now.

+  return path[0] == '/';
+#endif
+}
+
+std::string GetWorkingDirectory() {
+#if defined(_WIN32) || defined(_WIN64)
+  char system_buffer[MAX_PATH];
+  // TODO(adamk): Support Unicode paths.
+  DWORD len = GetCurrentDirectoryA(MAX_PATH, system_buffer);
+  CHECK(len > 0);
+  return system_buffer;
+#else
+  char curdir[PATH_MAX];
+  CHECK_NOT_NULL(getcwd(curdir, PATH_MAX));
+  return curdir;
+#endif
+}
+
+MaybeLocal<Module> ResolveModuleCallback(Local<Context> context,
+                                         Local<String> specifier,
+                                         Local<Module> referrer,
+                                         Local<Value> data) {
+  Isolate* isolate = context->GetIsolate();
+  auto module_map = static_cast<std::map<std::string, Global<Module>>*>(
+      External::Cast(*data)->Value());
+  String::Utf8Value specifier_utf8(specifier);
+  CHECK(!IsAbsolutePath(*specifier_utf8));
+  Local<String> dir_name = Local<String>::Cast(referrer->GetEmbedderData());
+  std::string absolute_path = *String::Utf8Value(dir_name);
+  absolute_path.append(*specifier_utf8);
+  auto it = module_map->find(absolute_path);
+  if (it != module_map->end()) {
+    return it->second.Get(isolate);
+  }
+  return MaybeLocal<Module>();
+}
+
+}  // anonymous namespace
+
 MaybeLocal<Module> Shell::FetchModuleTree(
     Isolate* isolate, const std::string& file_name,
     std::map<std::string, Global<Module>>* module_map) {

[neis, 2016/09/22 01:07:39]

Add a DCHECK(IsAbsolute) here?

[adamk, 2016/09/22 19:06:56]

Done.

   TryCatch try_catch(isolate);
   try_catch.SetVerbose(true);
   Local<String> source_text = ReadFile(isolate, file_name.c_str());
   if (source_text.IsEmpty()) {
     printf("Error reading '%s'\n", file_name.c_str());
     Shell::Exit(1);
   }
   ScriptOrigin origin(
       String::NewFromUtf8(isolate, file_name.c_str(), NewStringType::kNormal)
           .ToLocalChecked());
   ScriptCompiler::Source source(source_text, origin);
   Local<Module> module;
   if (!ScriptCompiler::CompileModule(isolate, &source).ToLocal(&module)) {
     ReportException(isolate, &try_catch);
     return MaybeLocal<Module>();
   }
   module_map->insert(
       std::make_pair(file_name, Global<Module>(isolate, module)));
+
+  size_t last_slash = file_name.find_last_of("/\\");
+  CHECK(last_slash != std::string::npos);
+  std::string dir_name = file_name.substr(0, last_slash + 1);

[neis, 2016/09/22 01:07:39]

Please move this into a separate function.

[adamk, 2016/09/22 19:06:57]

Done.

+  module->SetEmbedderData(
+      String::NewFromUtf8(isolate, dir_name.c_str(), NewStringType::kNormal)
+          .ToLocalChecked());
+
   for (int i = 0, length = module->GetModuleRequestsLength(); i < length; ++i) {
     Local<String> name = module->GetModuleRequest(i);
-    // TODO(adamk): Resolve the imported module to a full path.
-    std::string str = *String::Utf8Value(name);
-    if (!module_map->count(str)) {
-      if (FetchModuleTree(isolate, str, module_map).IsEmpty()) {
+    String::Utf8Value utf8_value(name);
+    CHECK(!IsAbsolutePath(*utf8_value));

[neis, 2016/09/22 01:07:39]

Can you add a comment about this restriction?

[adamk, 2016/09/22 19:06:56]

Removed the restriction instead.

+    std::string absolute_path = dir_name;
+    absolute_path.append(*utf8_value);
+    if (!module_map->count(absolute_path)) {
+      if (FetchModuleTree(isolate, absolute_path, module_map).IsEmpty()) {
         return MaybeLocal<Module>();
       }
     }
   }
 
   return module;
 }
 
-namespace {
-
-MaybeLocal<Module> ResolveModuleCallback(Local<Context> context,
-                                         Local<String> specifier,
-                                         Local<Module> referrer,
-                                         Local<Value> data) {
-  Isolate* isolate = context->GetIsolate();
-  auto module_map = static_cast<std::map<std::string, Global<Module>>*>(
-      External::Cast(*data)->Value());
-  std::string str_specifier = *String::Utf8Value(specifier);
-  // TODO(adamk): Resolve the specifier using the referrer
-  auto it = module_map->find(str_specifier);
-  if (it != module_map->end()) {
-    return it->second.Get(isolate);
-  }
-  return MaybeLocal<Module>();
-}
-
-}  // anonymous namespace
-
 bool Shell::ExecuteModule(Isolate* isolate, const char* file_name) {
   HandleScope handle_scope(isolate);
 
   Local<Module> root_module;
   std::map<std::string, Global<Module>> module_map;
-  if (!FetchModuleTree(isolate, file_name, &module_map).ToLocal(&root_module)) {
+
+  std::string absolute_path;
+  if (IsAbsolutePath(file_name)) {
+    absolute_path = file_name;
+  } else {
+    absolute_path = GetWorkingDirectory();
+    absolute_path.push_back('/');
+    absolute_path.append(file_name);
+  }

[neis, 2016/09/22 01:07:39]

Can you move this stuff up a little?

[adamk, 2016/09/22 19:06:56]

Done.

+  if (!FetchModuleTree(isolate, absolute_path, &module_map)
+           .ToLocal(&root_module)) {
     return false;
   }
 
   TryCatch try_catch(isolate);
   try_catch.SetVerbose(true);
 
   MaybeLocal<Value> maybe_result;
   {
     PerIsolateData* data = PerIsolateData::Get(isolate);
     Local<Context> realm = data->realms_[data->realm_current_].Get(isolate);
@@ skipping 1310 matching lines @@
   thread_ = NULL;
   delete[] script_;
   script_ = NULL;
   in_queue_.Clear();
   out_queue_.Clear();
 }
 
 
 void Worker::StartExecuteInThread(const char* script) {
   running_ = true;
+#undef StrDup

[neis, 2016/09/22 01:07:39]

??

[adamk, 2016/09/22 19:06:56]

This was from a previous attempt to call the Win32 PathIsRelative API. Removed.

   script_ = i::StrDup(script);
   thread_ = new WorkerThread(this);
   thread_->Start();
 }
 
 
 void Worker::PostMessage(SerializationData* data) {
   in_queue_.Enqueue(data);
   in_semaphore_.Signal();
 }
@@ skipping 780 matching lines @@
 }
 
 }  // namespace v8
 
 
 #ifndef GOOGLE3
 int main(int argc, char* argv[]) {
   return v8::Shell::Main(argc, argv);
 }
 #endif