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

src: allow to negate boolean CLI flags · nodejs/node@3c7a7d9 · GitHub

/ node Public

Commit 3c7a7d9

Browse files
targos
authored and
danielleadams
committed
src: allow to negate boolean CLI flags
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. `process.allowedNodeEnvironmentFlags` contains both the negated and non-negated versions of boolean flags. Co-authored-by: Anna Henningsen <anna@addaleax.net> PR-URL: #39023 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent fed6411 commit 3c7a7d9

12 files changed

Lines changed: 150 additions & 33 deletions

‎lib/internal/bootstrap/pre_execution.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ function setupWarningHandler() {
140140
const {
141141
onWarning
142142
} = require('internal/process/warning');
143-
if (!getOptionValue('--no-warnings') &&
143+
if (getOptionValue('--warnings') &&
144144
process.env.NODE_NO_WARNINGS !== '1') {
145145
process.on('warning', onWarning);
146146
}

‎lib/internal/main/print_help.js‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ const {
88
MathMax,
99
ObjectKeys,
1010
RegExp,
11+
StringPrototypeLocaleCompare,
12+
StringPrototypeSlice,
1113
StringPrototypeTrimLeft,
1214
StringPrototypeRepeat,
1315
StringPrototypeReplace,
@@ -110,12 +112,30 @@ function format(
110112
let text = '';
111113
let maxFirstColumnUsed = 0;
112114

115+
const sortedOptions = ArrayPrototypeSort(
116+
[...options.entries()],
117+
({ 0: name1, 1: option1 }, { 0: name2, 1: option2 }) => {
118+
if (option1.defaultIsTrue) {
119+
name1 = `--no-${StringPrototypeSlice(name1, 2)}`;
120+
}
121+
if (option2.defaultIsTrue) {
122+
name2 = `--no-${StringPrototypeSlice(name2, 2)}`;
123+
}
124+
return StringPrototypeLocaleCompare(name1, name2);
125+
},
126+
);
127+
113128
for (const {
114-
0: name, 1: { helpText, type, value }
115-
} of ArrayPrototypeSort([...options.entries()])) {
129+
0: name, 1: { helpText, type, value, defaultIsTrue }
130+
} of sortedOptions) {
116131
if (!helpText) continue;
117132

118133
let displayName = name;
134+
135+
if (defaultIsTrue) {
136+
displayName = `--no-${StringPrototypeSlice(displayName, 2)}`;
137+
}
138+
119139
const argDescription = getArgDescription(type);
120140
if (argDescription)
121141
displayName += `=${argDescription}`;
@@ -138,7 +158,7 @@ function format(
138158
}
139159

140160
let displayHelpText = helpText;
141-
if (value === true) {
161+
if (value === !defaultIsTrue) {
142162
// Mark boolean options we currently have enabled.
143163
// In particular, it indicates whether --use-openssl-ca
144164
// or --use-bundled-ca is the (current) default.

‎lib/internal/options.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,13 @@ function getAliasesFromBinding() {
2525
return aliasesMap;
2626
}
2727

28-
function getOptionValue(option) {
29-
return getOptionsFromBinding().get(option)?.value;
28+
function getOptionValue(optionName) {
29+
const options = getOptionsFromBinding();
30+
if (optionName.startsWith('--no-')) {
31+
const option = options.get('--' + optionName.slice(5));
32+
return option && !option.value;
33+
}
34+
return options.get(optionName)?.value;
3035
}
3136

3237
function getAllowUnauthorized() {

‎lib/internal/process/per_thread.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,14 +259,19 @@ const trailingValuesRegex = /=.*$/;
259259
// from data in the config binding.
260260
function buildAllowedFlags() {
261261
const {
262-
envSettings: { kAllowedInEnvironment }
262+
envSettings: { kAllowedInEnvironment },
263+
types: { kBoolean },
263264
} = internalBinding('options');
264265
const { options, aliases } = require('internal/options');
265266

266267
const allowedNodeEnvironmentFlags = [];
267268
for (const { 0: name, 1: info } of options) {
268269
if (info.envVarSettings === kAllowedInEnvironment) {
269270
ArrayPrototypePush(allowedNodeEnvironmentFlags, name);
271+
if (info.type === kBoolean) {
272+
const negatedName = `--no-${name.slice(2)}`;
273+
ArrayPrototypePush(allowedNodeEnvironmentFlags, negatedName);
274+
}
270275
}
271276
}
272277

‎src/env.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,7 @@ void Environment::InitializeMainContext(Local<Context> context,
447447
CreateProperties();
448448
}
449449

450-
if (options_->no_force_async_hooks_checks) {
450+
if (!options_->force_async_hooks_checks) {
451451
async_hooks_.no_force_checks();
452452
}
453453

‎src/env.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ constexpr size_t kFsStatsBufferLength =
211211
V(crypto_rsa_pss_string, "rsa-pss") \
212212
V(cwd_string, "cwd") \
213213
V(data_string, "data") \
214+
V(default_is_true_string, "defaultIsTrue") \
214215
V(deserialize_info_string, "deserializeInfo") \
215216
V(dest_string, "dest") \
216217
V(destroyed_string, "destroyed") \

‎src/node.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1132,9 +1132,9 @@ int Start(int argc, char** argv) {
11321132
Isolate::CreateParams params;
11331133
const std::vector<size_t>* indices = nullptr;
11341134
const EnvSerializeInfo* env_info = nullptr;
1135-
bool force_no_snapshot =
1136-
per_process::cli_options->per_isolate->no_node_snapshot;
1137-
if (!force_no_snapshot) {
1135+
bool use_node_snapshot =
1136+
per_process::cli_options->per_isolate->node_snapshot;
1137+
if (use_node_snapshot) {
11381138
v8::StartupData* blob = NodeMainInstance::GetEmbeddedSnapshotBlob();
11391139
if (blob != nullptr) {
11401140
params.snapshot_blob = blob;

‎src/node_options-inl.h‎

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,14 @@ template <typename Options>
2323
void OptionsParser<Options>::AddOption(const char* name,
2424
const char* help_text,
2525
bool Options::* field,
26-
OptionEnvvarSettings env_setting) {
26+
OptionEnvvarSettings env_setting,
27+
bool default_is_true) {
2728
options_.emplace(name,
2829
OptionInfo{kBoolean,
2930
std::make_shared<SimpleOptionField<bool>>(field),
3031
env_setting,
31-
help_text});
32+
help_text,
33+
default_is_true});
3234
}
3335

3436
template <typename Options>
@@ -186,7 +188,8 @@ auto OptionsParser<Options>::Convert(
186188
return OptionInfo{original.type,
187189
Convert(original.field, get_child),
188190
original.env_setting,
189-
original.help_text};
191+
original.help_text,
192+
original.default_is_true};
190193
}
191194

192195
template <typename Options>
@@ -225,6 +228,10 @@ inline std::string RequiresArgumentErr(const std::string& arg) {
225228
return arg + " requires an argument";
226229
}
227230

231+
inline std::string NegationImpliesBooleanError(const std::string& arg) {
232+
return arg + " is an invalid negation because it is not a boolean option";
233+
}
234+
228235
// We store some of the basic information around a single Parse call inside
229236
// this struct, to separate storage of command line arguments and their
230237
// handling. In particular, this makes it easier to introduce 'synthetic'
@@ -325,6 +332,13 @@ void OptionsParser<Options>::Parse(
325332
name[i] = '-';
326333
}
327334

335+
// Convert --no-foo to --foo and keep in mind that we're negating.
336+
bool is_negation = false;
337+
if (name.find("--no-") == 0) {
338+
name.erase(2, 3); // remove no-
339+
is_negation = true;
340+
}
341+
328342
{
329343
auto it = aliases_.end();
330344
// Expand aliases:
@@ -367,7 +381,12 @@ void OptionsParser<Options>::Parse(
367381
}
368382

369383
{
370-
auto implications = implications_.equal_range(name);
384+
std::string implied_name = name;
385+
if (is_negation) {
386+
// Implications for negated options are defined with "--no-".
387+
implied_name.insert(2, "no-");
388+
}
389+
auto implications = implications_.equal_range(implied_name);
371390
for (auto it = implications.first; it != implications.second; ++it) {
372391
if (it->second.type == kV8Option) {
373392
v8_args->push_back(it->second.name);
@@ -384,6 +403,13 @@ void OptionsParser<Options>::Parse(
384403
}
385404

386405
const OptionInfo& info = it->second;
406+
407+
// Some V8 options can be negated and they are validated by V8 later.
408+
if (is_negation && info.type != kBoolean && info.type != kV8Option) {
409+
errors->push_back(NegationImpliesBooleanError(arg));
410+
break;
411+
}
412+
387413
std::string value;
388414
if (info.type != kBoolean && info.type != kNoOp && info.type != kV8Option) {
389415
if (equals_index != std::string::npos) {
@@ -412,7 +438,7 @@ void OptionsParser<Options>::Parse(
412438

413439
switch (info.type) {
414440
case kBoolean:
415-
*Lookup<bool>(info.field, options) = true;
441+
*Lookup<bool>(info.field, options) = !is_negation;
416442
break;
417443
case kInteger:
418444
*Lookup<int64_t>(info.field, options) = std::atoll(value.c_str());

‎src/node_options.cc‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -391,18 +391,21 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
391391
kAllowedInEnvironment);
392392
AddAlias("--es-module-specifier-resolution",
393393
"--experimental-specifier-resolution");
394-
AddOption("--no-deprecation",
394+
AddOption("--deprecation",
395395
"silence deprecation warnings",
396-
&EnvironmentOptions::no_deprecation,
397-
kAllowedInEnvironment);
398-
AddOption("--no-force-async-hooks-checks",
396+
&EnvironmentOptions::deprecation,
397+
kAllowedInEnvironment,
398+
true);
399+
AddOption("--force-async-hooks-checks",
399400
"disable checks for async_hooks",
400-
&EnvironmentOptions::no_force_async_hooks_checks,
401-
kAllowedInEnvironment);
402-
AddOption("--no-warnings",
401+
&EnvironmentOptions::force_async_hooks_checks,
402+
kAllowedInEnvironment,
403+
true);
404+
AddOption("--warnings",
403405
"silence all process warnings",
404-
&EnvironmentOptions::no_warnings,
405-
kAllowedInEnvironment);
406+
&EnvironmentOptions::warnings,
407+
kAllowedInEnvironment,
408+
true);
406409
AddOption("--force-context-aware",
407410
"disable loading non-context-aware addons",
408411
&EnvironmentOptions::force_context_aware,
@@ -594,9 +597,9 @@ PerIsolateOptionsParser::PerIsolateOptionsParser(
594597
"track heap object allocations for heap snapshots",
595598
&PerIsolateOptions::track_heap_objects,
596599
kAllowedInEnvironment);
597-
AddOption("--no-node-snapshot",
600+
AddOption("--node-snapshot",
598601
"", // It's a debug-only option.
599-
&PerIsolateOptions::no_node_snapshot,
602+
&PerIsolateOptions::node_snapshot,
600603
kAllowedInEnvironment);
601604

602605
// Explicitly add some V8 flags to mark them as allowed in NODE_OPTIONS.
@@ -1014,6 +1017,10 @@ void GetOptions(const FunctionCallbackInfo<Value>& args) {
10141017
env->type_string(),
10151018
Integer::New(isolate, static_cast<int>(option_info.type)))
10161019
.FromMaybe(false) ||
1020+
!info->Set(context,
1021+
env->default_is_true_string(),
1022+
Boolean::New(isolate, option_info.default_is_true))
1023+
.FromMaybe(false) ||
10171024
info->Set(context, env->value_string(), value).IsNothing() ||
10181025
options->Set(context, name, info).IsEmpty()) {
10191026
return;

‎src/node_options.h‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,9 @@ class EnvironmentOptions : public Options {
119119
int64_t heap_snapshot_near_heap_limit = 0;
120120
std::string heap_snapshot_signal;
121121
uint64_t max_http_header_size = 16 * 1024;
122-
bool no_deprecation = false;
123-
bool no_force_async_hooks_checks = false;
124-
bool no_warnings = false;
122+
bool deprecation = true;
123+
bool force_async_hooks_checks = true;
124+
bool warnings = true;
125125
bool force_context_aware = false;
126126
bool pending_deprecation = false;
127127
bool preserve_symlinks = false;
@@ -193,7 +193,7 @@ class PerIsolateOptions : public Options {
193193
public:
194194
std::shared_ptr<EnvironmentOptions> per_env { new EnvironmentOptions() };
195195
bool track_heap_objects = false;
196-
bool no_node_snapshot = false;
196+
bool node_snapshot = true;
197197
bool report_uncaught_exception = false;
198198
bool report_on_signal = false;
199199
bool experimental_top_level_await = true;
@@ -301,7 +301,8 @@ class OptionsParser {
301301
void AddOption(const char* name,
302302
const char* help_text,
303303
bool Options::* field,
304-
OptionEnvvarSettings env_setting = kDisallowedInEnvironment);
304+
OptionEnvvarSettings env_setting = kDisallowedInEnvironment,
305+
bool default_is_true = false);
305306
void AddOption(const char* name,
306307
const char* help_text,
307308
uint64_t Options::* field,
@@ -424,6 +425,7 @@ class OptionsParser {
424425
std::shared_ptr<BaseOptionField> field;
425426
OptionEnvvarSettings env_setting;
426427
std::string help_text;
428+
bool default_is_true = false;
427429
};
428430

429431
// An implied option is composed of the information on where to store a

0 commit comments

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