Merge lp:~marcustomlinson/v8-cpp/lp-1474858 into lp:v8-cpp

Proposed by Marcus Tomlinson on 2015-07-16
Status: Merged
Merged at revision: 22
Proposed branch: lp:~marcustomlinson/v8-cpp/lp-1474858
Merge into: lp:v8-cpp
Diff against target: 350 lines (+109/-57)
10 files modified
example/module.cpp (+1/-1)
src/internal/class.h (+2/-2)
src/internal/class_info.h (+3/-3)
src/internal/function.h (+2/-2)
src/internal/require.h (+69/-40)
src/module.h (+2/-2)
src/run.h (+8/-5)
tests/errors/test.cpp (+20/-0)
tests/run/scripts/test.js (+1/-1)
tests/run/scripts/test2.js (+1/-1)
To merge this branch: bzr merge lp:~marcustomlinson/v8-cpp/lp-1474858
Reviewer Review Type Date Requested Status
Marcus Tomlinson Pending
Review via email: mp+264999@code.launchpad.net

Commit message

Fixed require() to load modules relative to script path

To post a comment you must log in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'example/module.cpp'
2--- example/module.cpp 2015-07-08 06:20:26 +0000
3+++ example/module.cpp 2015-07-16 14:32:42 +0000
4@@ -136,5 +136,5 @@
5 exports->SetPrototype(module.create_prototype());
6 }
7
8-NODE_MODULE(addon, InitAll)
9+///!NODE_MODULE(addon, InitAll)
10 V8CPP_MODULE(addon, InitAll)
11
12=== modified file 'src/internal/class.h'
13--- src/internal/class.h 2015-06-23 08:40:22 +0000
14+++ src/internal/class.h 2015-07-16 14:32:42 +0000
15@@ -192,10 +192,10 @@
16 {
17 return args.GetReturnValue().Set(instance(isolate).export_object(args));
18 }
19- catch (std::exception const& ex)
20+ catch (std::exception const& e)
21 {
22 auto exception =
23- isolate->ThrowException(v8::Exception::Error(v8::String::NewFromUtf8(isolate, ex.what())));
24+ isolate->ThrowException(v8::Exception::Error(v8::String::NewFromUtf8(isolate, e.what())));
25 args.GetReturnValue().Set(exception);
26 }
27 });
28
29=== modified file 'src/internal/class_info.h'
30--- src/internal/class_info.h 2015-07-02 09:52:54 +0000
31+++ src/internal/class_info.h 2015-07-16 14:32:42 +0000
32@@ -42,7 +42,7 @@
33
34 using CastFunction = void* (*)(void*);
35
36- void add_base_class(ClassInfo* info, CastFunction cast)
37+ inline void add_base_class(ClassInfo* info, CastFunction cast)
38 {
39 auto it = std::find_if(base_classes_.begin(), base_classes_.end(), [info](BaseClassInfo const& base_class)
40 {
41@@ -55,7 +55,7 @@
42 }
43 }
44
45- bool cast(void*& ptr, size_t type) const
46+ inline bool cast(void*& ptr, size_t type) const
47 {
48 // Type already matches
49 if (type == type_)
50@@ -111,7 +111,7 @@
51 }
52
53 protected:
54- static size_t register_class()
55+ inline static size_t register_class()
56 {
57 static size_t next_index = 0;
58 return next_index++;
59
60=== modified file 'src/internal/function.h'
61--- src/internal/function.h 2015-07-16 13:04:34 +0000
62+++ src/internal/function.h 2015-07-16 14:32:42 +0000
63@@ -102,9 +102,9 @@
64 {
65 forward_invoke<F>(args);
66 }
67- catch (std::exception const& ex)
68+ catch (std::exception const& e)
69 {
70- auto exception = isolate->ThrowException(v8::Exception::Error(v8::String::NewFromUtf8(isolate, ex.what())));
71+ auto exception = isolate->ThrowException(v8::Exception::Error(v8::String::NewFromUtf8(isolate, e.what())));
72 args.GetReturnValue().Set(exception);
73 }
74 }
75
76=== modified file 'src/internal/require.h'
77--- src/internal/require.h 2015-07-16 08:05:48 +0000
78+++ src/internal/require.h 2015-07-16 14:32:42 +0000
79@@ -29,7 +29,6 @@
80
81 using ModuleInitFunc = void(v8::Handle<v8::Object> exports);
82 ///!ModuleInitFunc* node_init_func_;
83-///!std::shared_ptr<std::string> v8cpp_script_path_;
84
85 struct NodeModule
86 {
87@@ -62,27 +61,29 @@
88 public:
89 inline static void log(v8::FunctionCallbackInfo<v8::Value> const& args)
90 {
91+ v8::Isolate* isolate = args.GetIsolate();
92+
93 for (int i = 0; i < args.Length(); ++i)
94 {
95 if (args[i]->IsUint32())
96 {
97- std::cout << v8cpp::from_v8<unsigned int>(v8::Isolate::GetCurrent(), args[i]);
98+ std::cout << v8cpp::from_v8<unsigned int>(isolate, args[i]);
99 }
100 else if (args[i]->IsInt32())
101 {
102- std::cout << v8cpp::from_v8<int>(v8::Isolate::GetCurrent(), args[i]);
103+ std::cout << v8cpp::from_v8<int>(isolate, args[i]);
104 }
105 else if (args[i]->IsNumber())
106 {
107- std::cout << v8cpp::from_v8<float>(v8::Isolate::GetCurrent(), args[i]);
108+ std::cout << v8cpp::from_v8<float>(isolate, args[i]);
109 }
110 else if (args[i]->IsBoolean())
111 {
112- std::cout << v8cpp::from_v8<bool>(v8::Isolate::GetCurrent(), args[i]);
113+ std::cout << v8cpp::from_v8<bool>(isolate, args[i]);
114 }
115 else if (args[i]->IsString())
116 {
117- std::cout << v8cpp::from_v8<std::string>(v8::Isolate::GetCurrent(), args[i]);
118+ std::cout << v8cpp::from_v8<std::string>(isolate, args[i]);
119 }
120 }
121 std::cout << std::endl;
122@@ -90,50 +91,78 @@
123 }
124 };
125
126-inline v8::Local<v8::Object> require(std::string const& module_path)
127+class Require
128 {
129- ///!node_init_func_ = nullptr;
130- std::string script_path; ///! = v8cpp_script_path_ ? *v8cpp_script_path_ : "";
131+public:
132+ explicit Require(std::string const& script_path)
133+ : script_path_(script_path) {}
134
135- // Try append ".node" to module_path
136- std::string suffixed_module_path = script_path + module_path + ".node";
137- auto module = dlopen(suffixed_module_path.c_str(), RTLD_LAZY);
138- if (!module)
139+ inline static void require(v8::FunctionCallbackInfo<v8::Value> const& args)
140 {
141- // Didn't work, now try append ".so" to module_path
142- suffixed_module_path = script_path + module_path + ".so";
143- module = dlopen(suffixed_module_path.c_str(), RTLD_LAZY);
144- if (!module)
145+ v8::Isolate* isolate = args.GetIsolate();
146+ v8::EscapableHandleScope scope(isolate);
147+
148+ try
149 {
150- // Still didn't work, just try module_path as is
151- suffixed_module_path = script_path + module_path;
152- module = dlopen(suffixed_module_path.c_str(), RTLD_LAZY);
153+ Require* this_ = internal::import_value<Require*>(args.Data());
154+
155+ std::string module_path = from_v8<std::string>(isolate, args[0], "");
156+ if (module_path.empty())
157+ {
158+ throw std::runtime_error("require() call missing string argument");
159+ }
160+
161+ ///!node_init_func_ = nullptr;
162+
163+ // Try append ".node" to module_path
164+ std::string suffixed_module_path = this_->script_path_ + module_path + ".node";
165+ auto module = dlopen(suffixed_module_path.c_str(), RTLD_LAZY);
166 if (!module)
167 {
168- throw std::runtime_error("dlopen failed: " + std::string(dlerror()));
169+ // Didn't work, now try append ".so" to module_path
170+ suffixed_module_path = this_->script_path_ + module_path + ".so";
171+ module = dlopen(suffixed_module_path.c_str(), RTLD_LAZY);
172+ if (!module)
173+ {
174+ // Still didn't work, just try module_path as is
175+ suffixed_module_path = this_->script_path_ + module_path;
176+ module = dlopen(suffixed_module_path.c_str(), RTLD_LAZY);
177+ if (!module)
178+ {
179+ throw std::runtime_error("dlopen failed: " + std::string(dlerror()));
180+ }
181+ }
182 }
183+
184+ v8::Local<v8::Object> exports = v8::Object::New(isolate);
185+
186+ ///!if (node_init_func_)
187+ ///!{
188+ ///! node_init_func_(exports);
189+ ///!}
190+ ///!else
191+ ///!{
192+ auto v8cpp_init_func = (ModuleInitFunc*)dlsym(module, "init_module");
193+ if (!v8cpp_init_func)
194+ {
195+ throw std::runtime_error("dlsym failed: " + std::string(dlerror()));
196+ }
197+
198+ v8cpp_init_func(exports);
199+ ///!}
200+
201+ args.GetReturnValue().Set(scope.Escape(exports));
202+ }
203+ catch (std::exception const& e)
204+ {
205+ auto exception = isolate->ThrowException(v8::Exception::Error(v8::String::NewFromUtf8(isolate, e.what())));
206+ args.GetReturnValue().Set(scope.Escape(exception));
207 }
208 }
209
210- v8::Local<v8::Object> exports = v8::Object::New(v8::Isolate::GetCurrent());
211-
212-///! if (node_init_func_)
213-///! {
214-///! node_init_func_(exports);
215-///! }
216-///! else
217-///! {
218- auto v8cpp_init_func = (ModuleInitFunc*)dlsym(module, "init_module");
219- if (!v8cpp_init_func)
220- {
221- throw std::runtime_error("dlsym failed: " + std::string(dlerror()));
222- }
223-
224- v8cpp_init_func(exports);
225-///! }
226-
227- return exports;
228-}
229+private:
230+ std::string script_path_;
231+};
232
233 } // namespace internal
234 } // namespace v8cpp
235
236=== modified file 'src/module.h'
237--- src/module.h 2015-06-18 12:32:56 +0000
238+++ src/module.h 2015-07-16 14:32:42 +0000
239@@ -53,12 +53,12 @@
240 }
241
242 // Create an instance of this module in V8
243- v8::Local<v8::Object> create_prototype()
244+ inline v8::Local<v8::Object> create_prototype()
245 {
246 return object_template_->NewInstance();
247 }
248
249- v8::Handle<v8::ObjectTemplate> object_template()
250+ inline v8::Handle<v8::ObjectTemplate> object_template()
251 {
252 return object_template_;
253 }
254
255=== modified file 'src/run.h'
256--- src/run.h 2015-07-15 13:18:16 +0000
257+++ src/run.h 2015-07-16 14:32:42 +0000
258@@ -45,8 +45,6 @@
259 delete instances;
260 isolate->SetData(0, nullptr);
261 isolate->Dispose();
262-
263- ///!internal::v8cpp_script_path_.reset();
264 });
265
266 // Create an isolate scope.
267@@ -67,13 +65,17 @@
268 v8::Context::Scope context_scope(v8::Context::New(isolate.get()));
269
270 // Store the script filename for use in require() later
271+ std::string script_path;
272 std::size_t found = filename.find_last_of("/");
273 if (found != std::string::npos)
274 {
275- ///!internal::v8cpp_script_path_ = std::make_shared<std::string>(filename.substr(0, found) + "/");
276+ script_path = filename.substr(0, found) + "/";
277 }
278
279- module.add_function("require", &internal::require);
280+ v8::Handle<v8::Value> require = internal::export_value(isolate.get(), new internal::Require(script_path));
281+ module.object_template()->Set(isolate.get(), "require",
282+ v8::FunctionTemplate::New(isolate.get(), internal::Require::require, require));
283+
284 module.add_class("console", console);
285 }
286 v8::Local<v8::Context> context = v8::Context::New(isolate.get(), nullptr, module.object_template());
287@@ -82,7 +84,8 @@
288 v8::Context::Scope context_scope(context);
289
290 // Compile the script.
291- v8::Local<v8::Script> script = v8::Script::Compile(v8cpp::to_v8(isolate.get(), source), v8cpp::to_v8(isolate.get(), filename));
292+ v8::Local<v8::Script> script = v8::Script::Compile(v8cpp::to_v8(isolate.get(), source),
293+ v8cpp::to_v8(isolate.get(), filename));
294
295 // Run the script.
296 if (script.IsEmpty())
297
298=== modified file 'tests/errors/test.cpp'
299--- tests/errors/test.cpp 2015-07-08 09:29:21 +0000
300+++ tests/errors/test.cpp 2015-07-16 14:32:42 +0000
301@@ -22,6 +22,26 @@
302
303 #include <gtest/gtest.h>
304
305+TEST(Test, empty_module_path)
306+{
307+ try
308+ {
309+ v8cpp::run_script("require()");
310+ }
311+ catch (std::exception const& e)
312+ {
313+ EXPECT_STREQ(e.what(), "Uncaught Error: require() call missing string argument");
314+ }
315+ try
316+ {
317+ v8cpp::run_script("require('')");
318+ }
319+ catch (std::exception const& e)
320+ {
321+ EXPECT_STREQ(e.what(), "Uncaught Error: require() call missing string argument");
322+ }
323+}
324+
325 TEST(Test, non_existent_module)
326 {
327 try
328
329=== modified file 'tests/run/scripts/test.js'
330--- tests/run/scripts/test.js 2015-07-15 13:18:16 +0000
331+++ tests/run/scripts/test.js 2015-07-16 14:32:42 +0000
332@@ -16,6 +16,6 @@
333 * Authored by: Marcus Tomlinson <marcus.tomlinson@canonical.com>
334 */
335
336-var module = require("./test-run-module"); ///! ../
337+var module = require("../test-run-module");
338 var test_object = new module.TestClass(1, 2);
339 test_object;
340
341=== modified file 'tests/run/scripts/test2.js'
342--- tests/run/scripts/test2.js 2015-07-15 13:18:16 +0000
343+++ tests/run/scripts/test2.js 2015-07-16 14:32:42 +0000
344@@ -21,5 +21,5 @@
345 console.log(true, " > ", false);
346 console.log("hello", " ", "there");
347
348-var module = require("./test-run-module"); ///! ../
349+var module = require("../test-run-module");
350 new module.TestClass(3, 4);

Subscribers

People subscribed via source and target branches

to all changes: