Skip to content

Commit

Permalink
src: allow to negate boolean CLI flags
Browse files Browse the repository at this point in the history
This change allows all boolean flags to be negated using the `--no-`
prefix.
Flags that are `true` by default (for example `--deprecation`) are
still documented as negations.
With this change, it becomes possible to easily flip the default
value of a boolean flag and to override the value of a flag passed
in the NODE_OPTIONS environment variable.

Co-authored-by: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
targos and addaleax committed Jun 13, 2021
1 parent 889ad35 commit 785cbb2
Show file tree
Hide file tree
Showing 10 changed files with 75 additions and 31 deletions.
2 changes: 1 addition & 1 deletion lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ function setupWarningHandler() {
const {
onWarning
} = require('internal/process/warning');
if (!getOptionValue('--no-warnings') &&
if (getOptionValue('--warnings') &&
process.env.NODE_NO_WARNINGS !== '1') {
process.on('warning', onWarning);
}
Expand Down
10 changes: 8 additions & 2 deletions lib/internal/main/print_help.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const {
MathMax,
ObjectKeys,
RegExp,
StringPrototypeSlice,
StringPrototypeTrimLeft,
StringPrototypeRepeat,
StringPrototypeReplace,
Expand Down Expand Up @@ -111,11 +112,16 @@ function format(
let maxFirstColumnUsed = 0;

for (const {
0: name, 1: { helpText, type, value }
0: name, 1: { helpText, type, value, defaultIsTrue }
} of ArrayPrototypeSort([...options.entries()])) {
if (!helpText) continue;

let displayName = name;

if (defaultIsTrue) {
displayName = `--no-${StringPrototypeSlice(displayName, 2)}`;
}

const argDescription = getArgDescription(type);
if (argDescription)
displayName += `=${argDescription}`;
Expand All @@ -138,7 +144,7 @@ function format(
}

let displayHelpText = helpText;
if (value === true) {
if (value === !defaultIsTrue) {
// Mark boolean options we currently have enabled.
// In particular, it indicates whether --use-openssl-ca
// or --use-bundled-ca is the (current) default.
Expand Down
8 changes: 6 additions & 2 deletions lib/internal/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@ const { options, aliases } = getOptions();

let warnOnAllowUnauthorized = true;

function getOptionValue(option) {
return options.get(option)?.value;
function getOptionValue(optionName) {
if (optionName.startsWith('--no-')) {
const option = options.get('--' + optionName.slice(5));
return option && !option.value;
}
return options.get(optionName)?.value;
}

function getAllowUnauthorized() {
Expand Down
2 changes: 1 addition & 1 deletion src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ void Environment::InitializeMainContext(Local<Context> context,
CreateProperties();
}

if (options_->no_force_async_hooks_checks) {
if (!options_->force_async_hooks_checks) {
async_hooks_.no_force_checks();
}

Expand Down
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ constexpr size_t kFsStatsBufferLength =
V(crypto_rsa_pss_string, "rsa-pss") \
V(cwd_string, "cwd") \
V(data_string, "data") \
V(default_is_true_string, "defaultIsTrue") \
V(deserialize_info_string, "deserializeInfo") \
V(dest_string, "dest") \
V(destroyed_string, "destroyed") \
Expand Down
6 changes: 3 additions & 3 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1132,9 +1132,9 @@ int Start(int argc, char** argv) {
Isolate::CreateParams params;
const std::vector<size_t>* indices = nullptr;
const EnvSerializeInfo* env_info = nullptr;
bool force_no_snapshot =
per_process::cli_options->per_isolate->no_node_snapshot;
if (!force_no_snapshot) {
bool use_node_snapshot =
per_process::cli_options->per_isolate->node_snapshot;
if (use_node_snapshot) {
v8::StartupData* blob = NodeMainInstance::GetEmbeddedSnapshotBlob();
if (blob != nullptr) {
params.snapshot_blob = blob;
Expand Down
31 changes: 27 additions & 4 deletions src/node_options-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@ template <typename Options>
void OptionsParser<Options>::AddOption(const char* name,
const char* help_text,
bool Options::* field,
OptionEnvvarSettings env_setting) {
OptionEnvvarSettings env_setting,
bool default_is_true) {
options_.emplace(name,
OptionInfo{kBoolean,
std::make_shared<SimpleOptionField<bool>>(field),
env_setting,
help_text});
help_text,
default_is_true});
}

template <typename Options>
Expand Down Expand Up @@ -225,6 +227,10 @@ inline std::string RequiresArgumentErr(const std::string& arg) {
return arg + " requires an argument";
}

inline std::string NegationImpliesBooleanError(const std::string& arg) {
return arg + " is an invalid negation because it is not a boolean option";
}

// We store some of the basic information around a single Parse call inside
// this struct, to separate storage of command line arguments and their
// handling. In particular, this makes it easier to introduce 'synthetic'
Expand Down Expand Up @@ -325,6 +331,13 @@ void OptionsParser<Options>::Parse(
name[i] = '-';
}

// Convert --no-foo to --foo and keep in mind that we're negating.
bool is_negation = false;
if (name.find("--no-") == 0) {
name.erase(2, 3); // remove no-
is_negation = true;
}

{
auto it = aliases_.end();
// Expand aliases:
Expand Down Expand Up @@ -367,7 +380,12 @@ void OptionsParser<Options>::Parse(
}

{
auto implications = implications_.equal_range(name);
std::string implied_name = name;
if (is_negation) {
// Implications for negated options are defined with "--no-".
implied_name.insert(2, "no-");
}
auto implications = implications_.equal_range(implied_name);
for (auto it = implications.first; it != implications.second; ++it) {
if (it->second.type == kV8Option) {
v8_args->push_back(it->second.name);
Expand Down Expand Up @@ -410,9 +428,14 @@ void OptionsParser<Options>::Parse(
}
}

// Some V8 options can be negated and they are validated by V8 later.
if (is_negation && info.type != kBoolean && info.type != kV8Option) {
errors->push_back(NegationImpliesBooleanError(arg));
}

switch (info.type) {
case kBoolean:
*Lookup<bool>(info.field, options) = true;
*Lookup<bool>(info.field, options) = !is_negation;
break;
case kInteger:
*Lookup<int64_t>(info.field, options) = std::atoll(value.c_str());
Expand Down
29 changes: 18 additions & 11 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -391,18 +391,21 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
kAllowedInEnvironment);
AddAlias("--es-module-specifier-resolution",
"--experimental-specifier-resolution");
AddOption("--no-deprecation",
AddOption("--deprecation",
"silence deprecation warnings",
&EnvironmentOptions::no_deprecation,
kAllowedInEnvironment);
AddOption("--no-force-async-hooks-checks",
&EnvironmentOptions::deprecation,
kAllowedInEnvironment,
true);
AddOption("--force-async-hooks-checks",
"disable checks for async_hooks",
&EnvironmentOptions::no_force_async_hooks_checks,
kAllowedInEnvironment);
AddOption("--no-warnings",
&EnvironmentOptions::force_async_hooks_checks,
kAllowedInEnvironment,
true);
AddOption("--warnings",
"silence all process warnings",
&EnvironmentOptions::no_warnings,
kAllowedInEnvironment);
&EnvironmentOptions::warnings,
kAllowedInEnvironment,
true);
AddOption("--force-context-aware",
"disable loading non-context-aware addons",
&EnvironmentOptions::force_context_aware,
Expand Down Expand Up @@ -594,9 +597,9 @@ PerIsolateOptionsParser::PerIsolateOptionsParser(
"track heap object allocations for heap snapshots",
&PerIsolateOptions::track_heap_objects,
kAllowedInEnvironment);
AddOption("--no-node-snapshot",
AddOption("--node-snapshot",
"", // It's a debug-only option.
&PerIsolateOptions::no_node_snapshot,
&PerIsolateOptions::node_snapshot,
kAllowedInEnvironment);

// Explicitly add some V8 flags to mark them as allowed in NODE_OPTIONS.
Expand Down Expand Up @@ -1014,6 +1017,10 @@ void GetOptions(const FunctionCallbackInfo<Value>& args) {
env->type_string(),
Integer::New(isolate, static_cast<int>(option_info.type)))
.FromMaybe(false) ||
!info->Set(context,
env->default_is_true_string(),
Boolean::New(isolate, option_info.default_is_true))
.FromMaybe(false) ||
info->Set(context, env->value_string(), value).IsNothing() ||
options->Set(context, name, info).IsEmpty()) {
return;
Expand Down
12 changes: 7 additions & 5 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,9 @@ class EnvironmentOptions : public Options {
int64_t heap_snapshot_near_heap_limit = 0;
std::string heap_snapshot_signal;
uint64_t max_http_header_size = 16 * 1024;
bool no_deprecation = false;
bool no_force_async_hooks_checks = false;
bool no_warnings = false;
bool deprecation = true;
bool force_async_hooks_checks = true;
bool warnings = true;
bool force_context_aware = false;
bool pending_deprecation = false;
bool preserve_symlinks = false;
Expand Down Expand Up @@ -193,7 +193,7 @@ class PerIsolateOptions : public Options {
public:
std::shared_ptr<EnvironmentOptions> per_env { new EnvironmentOptions() };
bool track_heap_objects = false;
bool no_node_snapshot = false;
bool node_snapshot = true;
bool report_uncaught_exception = false;
bool report_on_signal = false;
bool experimental_top_level_await = true;
Expand Down Expand Up @@ -301,7 +301,8 @@ class OptionsParser {
void AddOption(const char* name,
const char* help_text,
bool Options::* field,
OptionEnvvarSettings env_setting = kDisallowedInEnvironment);
OptionEnvvarSettings env_setting = kDisallowedInEnvironment,
bool default_is_true = false);
void AddOption(const char* name,
const char* help_text,
uint64_t Options::* field,
Expand Down Expand Up @@ -424,6 +425,7 @@ class OptionsParser {
std::shared_ptr<BaseOptionField> field;
OptionEnvvarSettings env_setting;
std::string help_text;
bool default_is_true = false;
};

// An implied option is composed of the information on where to store a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ assert.deepStrictEqual(v8OptionsLines, [...v8OptionsLines].sort());
const documented = new Set();
for (const line of [...nodeOptionsLines, ...v8OptionsLines]) {
for (const match of line.matchAll(/`(-[^`]+)`/g)) {
const option = match[1];
// Remove negation from the option's name.
const option = match[1].replace('--no-', '--');
assert(!documented.has(option),
`Option '${option}' was documented more than once as an ` +
`allowed option for NODE_OPTIONS in ${cliMd}.`);
Expand Down Expand Up @@ -89,7 +90,7 @@ assert(undocumented.delete('--debug-arraybuffer-allocations'));
assert(undocumented.delete('--es-module-specifier-resolution'));
assert(undocumented.delete('--experimental-report'));
assert(undocumented.delete('--experimental-worker'));
assert(undocumented.delete('--no-node-snapshot'));
assert(undocumented.delete('--node-snapshot'));
assert(undocumented.delete('--loader'));
assert(undocumented.delete('--verify-base-objects'));

Expand Down

0 comments on commit 785cbb2

Please sign in to comment.