FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: refactor options parsing · nodejs/node@8fd55ff · GitHub

/ node Public

Commit 8fd55ff

Browse files
addaleax
authored and
targos
committed
src: refactor options parsing
This is a major refactor of our Node’s parser. See `node_options.cc` for how it is used, and `node_options-inl.h` for the bulk of its implementation. Unfortunately, the implementation has come to have some complexity, in order to meet the following goals: - Make it easy to *use* for defining or changing options. - Keep it (mostly) backwards-compatible. - No tests were harmed as part of this commit. - Be as consistent as possible. - In particular, options can now generally accept arguments through both `--foo=bar` notation and `--foo bar` notation. We were previously very inconsistent on this point. - Separate into different levels of scope, namely per-process (global), per-Isolate and per-Environment (+ debug options). - Allow programmatic accessibility in the future. - This includes a possible expansion for `--help` output. This commit also leaves a number of `TODO` comments, mostly for improving consistency even more (possibly with having to modify tests), improving embedder support, as well as removing pieces of exposed configuration variables that should never have become part of the public API but unfortunately are at this point. PR-URL: #22392 Backport-PR-URL: #22558 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
1 parent 5af6a89 commit 8fd55ff

24 files changed

Lines changed: 1356 additions & 893 deletions

‎node.gyp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,14 +342,14 @@
342342
'src/node_config.cc',
343343
'src/node_constants.cc',
344344
'src/node_contextify.cc',
345-
'src/node_debug_options.cc',
346345
'src/node_domain.cc',
347346
'src/node_encoding.cc',
348347
'src/node_errors.h',
349348
'src/node_file.cc',
350349
'src/node_http2.cc',
351350
'src/node_http_parser.cc',
352351
'src/node_messaging.cc',
352+
'src/node_options.cc',
353353
'src/node_os.cc',
354354
'src/node_platform.cc',
355355
'src/node_perf.cc',
@@ -406,14 +406,15 @@
406406
'src/node_code_cache.h',
407407
'src/node_constants.h',
408408
'src/node_contextify.h',
409-
'src/node_debug_options.h',
410409
'src/node_file.h',
411410
'src/node_http2.h',
412411
'src/node_http2_state.h',
413412
'src/node_internals.h',
414413
'src/node_javascript.h',
415414
'src/node_messaging.h',
416415
'src/node_mutex.h',
416+
'src/node_options.h',
417+
'src/node_options-inl.h',
417418
'src/node_perf.h',
418419
'src/node_perf_common.h',
419420
'src/node_persistent.h',

‎src/env-inl.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,14 @@ Environment::file_handle_read_wrap_freelist() {
559559
return file_handle_read_wrap_freelist_;
560560
}
561561

562+
inline std::shared_ptr<EnvironmentOptions> Environment::options() {
563+
return options_;
564+
}
565+
566+
inline std::shared_ptr<PerIsolateOptions> IsolateData::options() {
567+
return options_;
568+
}
569+
562570
void Environment::CreateImmediate(native_immediate_callback cb,
563571
void* data,
564572
v8::Local<v8::Object> obj,

‎src/env.cc‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ IsolateData::IsolateData(Isolate* isolate,
4444
if (platform_ != nullptr)
4545
platform_->RegisterIsolate(this, event_loop);
4646

47+
options_.reset(new PerIsolateOptions(*per_process_opts->per_isolate));
48+
4749
// Create string and private symbol properties as internalized one byte
4850
// strings after the platform is properly initialized.
4951
//
@@ -116,9 +118,6 @@ Environment::Environment(IsolateData* isolate_data,
116118
emit_env_nonstring_warning_(true),
117119
makecallback_cntr_(0),
118120
should_abort_on_uncaught_toggle_(isolate_, 1),
119-
#if HAVE_INSPECTOR
120-
inspector_agent_(new inspector::Agent(this)),
121-
#endif
122121
http_parser_buffer_(nullptr),
123122
fs_stats_field_array_(isolate_, kFsStatsFieldsLength * 2),
124123
fs_stats_field_bigint_array_(isolate_, kFsStatsFieldsLength * 2),
@@ -128,6 +127,19 @@ Environment::Environment(IsolateData* isolate_data,
128127
v8::Context::Scope context_scope(context);
129128
set_as_external(v8::External::New(isolate(), this));
130129

130+
// We create new copies of the per-Environment option sets, so that it is
131+
// easier to modify them after Environment creation. The defaults are
132+
// part of the per-Isolate option set, for which in turn the defaults are
133+
// part of the per-process option set.
134+
options_.reset(new EnvironmentOptions(*isolate_data->options()->per_env));
135+
options_->debug_options.reset(new DebugOptions(*options_->debug_options));
136+
137+
#if HAVE_INSPECTOR
138+
// We can only create the inspector agent after having cloned the options.
139+
inspector_agent_ =
140+
std::unique_ptr<inspector::Agent>(new inspector::Agent(this));
141+
#endif
142+
131143
AssignToContext(context, ContextInfo(""));
132144

133145
destroy_async_id_list_.reserve(512);
@@ -176,10 +188,8 @@ Environment::~Environment() {
176188
delete[] http_parser_buffer_;
177189
}
178190

179-
void Environment::Start(int argc,
180-
const char* const* argv,
181-
int exec_argc,
182-
const char* const* exec_argv,
191+
void Environment::Start(const std::vector<std::string>& args,
192+
const std::vector<std::string>& exec_args,
183193
bool start_profiler_idle_notifier) {
184194
HandleScope handle_scope(isolate());
185195
Context::Scope context_scope(context());
@@ -222,7 +232,7 @@ void Environment::Start(int argc,
222232
process_template->GetFunction()->NewInstance(context()).ToLocalChecked();
223233
set_process_object(process_object);
224234

225-
SetupProcessObject(this, argc, argv, exec_argc, exec_argv);
235+
SetupProcessObject(this, args, exec_args);
226236

227237
static uv_once_t init_once = UV_ONCE_INIT;
228238
uv_once(&init_once, InitThreadLocalOnce);

‎src/env.h‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "uv.h"
3535
#include "v8.h"
3636
#include "node.h"
37+
#include "node_options.h"
3738
#include "node_http2_state.h"
3839

3940
#include <list>
@@ -364,6 +365,7 @@ class IsolateData {
364365
inline uv_loop_t* event_loop() const;
365366
inline uint32_t* zero_fill_field() const;
366367
inline MultiIsolatePlatform* platform() const;
368+
inline std::shared_ptr<PerIsolateOptions> options();
367369

368370
#define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
369371
#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
@@ -397,6 +399,7 @@ class IsolateData {
397399
uv_loop_t* const event_loop_;
398400
uint32_t* const zero_fill_field_;
399401
MultiIsolatePlatform* platform_;
402+
std::shared_ptr<PerIsolateOptions> options_;
400403

401404
DISALLOW_COPY_AND_ASSIGN(IsolateData);
402405
};
@@ -584,10 +587,8 @@ class Environment {
584587
tracing::AgentWriterHandle* tracing_agent_writer);
585588
~Environment();
586589

587-
void Start(int argc,
588-
const char* const* argv,
589-
int exec_argc,
590-
const char* const* exec_argv,
590+
void Start(const std::vector<std::string>& args,
591+
const std::vector<std::string>& exec_args,
591592
bool start_profiler_idle_notifier);
592593

593594
typedef void (*HandleCleanupCb)(Environment* env,
@@ -858,6 +859,8 @@ class Environment {
858859
v8::EmbedderGraph* graph,
859860
void* data);
860861

862+
inline std::shared_ptr<EnvironmentOptions> options();
863+
861864
private:
862865
inline void CreateImmediate(native_immediate_callback cb,
863866
void* data,
@@ -887,6 +890,8 @@ class Environment {
887890
size_t makecallback_cntr_;
888891
std::vector<double> destroy_async_id_list_;
889892

893+
std::shared_ptr<EnvironmentOptions> options_;
894+
890895
AliasedBuffer<uint32_t, v8::Uint32Array> should_abort_on_uncaught_toggle_;
891896
int should_not_abort_scope_counter_ = 0;
892897

‎src/inspector_agent.cc‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -608,11 +608,14 @@ class NodeInspectorClient : public V8InspectorClient {
608608
std::unique_ptr<MainThreadInterface> interface_;
609609
};
610610

611-
Agent::Agent(Environment* env) : parent_env_(env) {}
611+
Agent::Agent(Environment* env)
612+
: parent_env_(env),
613+
debug_options_(env->options()->debug_options) {}
612614

613615
Agent::~Agent() = default;
614616

615-
bool Agent::Start(const std::string& path, const DebugOptions& options) {
617+
bool Agent::Start(const std::string& path,
618+
std::shared_ptr<DebugOptions> options) {
616619
path_ = path;
617620
debug_options_ = options;
618621
client_ = std::make_shared<NodeInspectorClient>(parent_env_);
@@ -626,8 +629,8 @@ bool Agent::Start(const std::string& path, const DebugOptions& options) {
626629
StartDebugSignalHandler();
627630
}
628631

629-
bool wait_for_connect = options.wait_for_connect();
630-
if (!options.inspector_enabled() || !StartIoThread()) {
632+
bool wait_for_connect = options->wait_for_connect();
633+
if (!options->inspector_enabled || !StartIoThread()) {
631634
return false;
632635
}
633636
if (wait_for_connect) {
@@ -789,7 +792,7 @@ void Agent::ContextCreated(Local<Context> context, const ContextInfo& info) {
789792
}
790793

791794
bool Agent::WillWaitForConnect() {
792-
return debug_options_.wait_for_connect();
795+
return debug_options_->wait_for_connect();
793796
}
794797

795798
bool Agent::IsActive() {

‎src/inspector_agent.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#error("This header can only be used when inspector is enabled")
1010
#endif
1111

12-
#include "node_debug_options.h"
12+
#include "node_options.h"
1313
#include "node_persistent.h"
1414
#include "v8.h"
1515

@@ -45,7 +45,7 @@ class Agent {
4545
~Agent();
4646

4747
// Create client_, may create io_ if option enabled
48-
bool Start(const std::string& path, const DebugOptions& options);
48+
bool Start(const std::string& path, std::shared_ptr<DebugOptions> options);
4949
// Stop and destroy io_
5050
void Stop();
5151

@@ -96,7 +96,7 @@ class Agent {
9696
// Calls StartIoThread() from off the main thread.
9797
void RequestIoThreadStart();
9898

99-
DebugOptions& options() { return debug_options_; }
99+
std::shared_ptr<DebugOptions> options() { return debug_options_; }
100100
void ContextCreated(v8::Local<v8::Context> context, const ContextInfo& info);
101101

102102
private:
@@ -109,7 +109,7 @@ class Agent {
109109
// Interface for transports, e.g. WebSocket server
110110
std::unique_ptr<InspectorIo> io_;
111111
std::string path_;
112-
DebugOptions debug_options_;
112+
std::shared_ptr<DebugOptions> debug_options_;
113113

114114
bool pending_enable_async_hook_ = false;
115115
bool pending_disable_async_hook_ = false;

‎src/inspector_io.cc‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ class InspectorIoDelegate: public node::inspector::SocketServerDelegate {
242242
std::unique_ptr<InspectorIo> InspectorIo::Start(
243243
std::shared_ptr<MainThreadHandle> main_thread,
244244
const std::string& path,
245-
const DebugOptions& options) {
245+
std::shared_ptr<DebugOptions> options) {
246246
auto io = std::unique_ptr<InspectorIo>(
247247
new InspectorIo(main_thread, path, options));
248248
if (io->request_queue_->Expired()) { // Thread is not running
@@ -253,7 +253,7 @@ std::unique_ptr<InspectorIo> InspectorIo::Start(
253253

254254
InspectorIo::InspectorIo(std::shared_ptr<MainThreadHandle> main_thread,
255255
const std::string& path,
256-
const DebugOptions& options)
256+
std::shared_ptr<DebugOptions> options)
257257
: main_thread_(main_thread), options_(options),
258258
thread_(), script_name_(path), id_(GenerateID()) {
259259
Mutex::ScopedLock scoped_lock(thread_start_lock_);
@@ -288,7 +288,8 @@ void InspectorIo::ThreadMain() {
288288
new InspectorIoDelegate(queue, main_thread_, id_,
289289
script_path, script_name_));
290290
InspectorSocketServer server(std::move(delegate), &loop,
291-
options_.host_name(), options_.port());
291+
options_->host().c_str(),
292+
options_->port());
292293
request_queue_ = queue->handle();
293294
// Its lifetime is now that of the server delegate
294295
queue.reset();

‎src/inspector_io.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
#define SRC_INSPECTOR_IO_H_
33

44
#include "inspector_socket_server.h"
5-
#include "node_debug_options.h"
65
#include "node_mutex.h"
76
#include "uv.h"
87

@@ -46,19 +45,20 @@ class InspectorIo {
4645
// Returns empty pointer if thread was not started
4746
static std::unique_ptr<InspectorIo> Start(
4847
std::shared_ptr<MainThreadHandle> main_thread, const std::string& path,
49-
const DebugOptions& options);
48+
std::shared_ptr<DebugOptions> options);
5049

5150
// Will block till the transport thread shuts down
5251
~InspectorIo();
5352

5453
void StopAcceptingNewConnections();
55-
std::string host() const { return options_.host_name(); }
54+
const std::string& host() const { return options_->host(); }
5655
int port() const { return port_; }
5756
std::vector<std::string> GetTargetIds() const;
5857

5958
private:
6059
InspectorIo(std::shared_ptr<MainThreadHandle> handle,
61-
const std::string& path, const DebugOptions& options);
60+
const std::string& path,
61+
std::shared_ptr<DebugOptions> options);
6262

6363
// Wrapper for agent->ThreadMain()
6464
static void ThreadMain(void* agent);
@@ -72,7 +72,7 @@ class InspectorIo {
7272
// Used to post on a frontend interface thread, lives while the server is
7373
// running
7474
std::shared_ptr<RequestQueue> request_queue_;
75-
const DebugOptions options_;
75+
std::shared_ptr<DebugOptions> options_;
7676

7777
// The IO thread runs its own uv_loop to implement the TCP server off
7878
// the main thread.

‎src/inspector_js_api.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,12 +242,12 @@ void Open(const FunctionCallbackInfo<Value>& args) {
242242

243243
if (args.Length() > 0 && args[0]->IsUint32()) {
244244
uint32_t port = args[0]->Uint32Value();
245-
agent->options().set_port(static_cast<int>(port));
245+
agent->options()->host_port.port = port;
246246
}
247247

248248
if (args.Length() > 1 && args[1]->IsString()) {
249249
Utf8Value host(env->isolate(), args[1].As<String>());
250-
agent->options().set_host_name(*host);
250+
agent->options()->host_port.host_name = *host;
251251
}
252252

253253
if (args.Length() > 2 && args[2]->IsBoolean()) {

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL