Skip to content

Commit 2071c44

Browse files
marco-ippolitomarco-ippolito
authored
src: allow empty --experimental-config-file
PR-URL: #61610 Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
1 parent 0dceddd commit 2071c44

18 files changed

+287
-184
lines changed

‎doc/api/cli.md‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,7 +1045,7 @@ added:
10451045
10461046
Enable experimental import support for `.node` addons.
10471047

1048-
### `--experimental-config-file=config`
1048+
### `--experimental-config-file=path`, `--experimental-config-file`
10491049

10501050
<!-- YAML
10511051
added:
@@ -1056,6 +1056,12 @@ added:
10561056
> Stability: 1.0 - Early development
10571057
10581058
If present, Node.js will look for a configuration file at the specified path.
1059+
If the path is not specified, Node.js will look for a `node.config.json` file
1060+
in the current working directory.
1061+
To specify a custom path, use the `--experimental-config-file=path` form.
1062+
The space-separated `--experimental-config-file path` form is not supported.
1063+
The alias `--experimental-default-config-file` is equivalent to
1064+
`--experimental-config-file` without an argument.
10591065
Node.js will read the configuration file and apply the settings. The
10601066
configuration file should be a JSON file with the following structure. `vX.Y.Z`
10611067
in the `$schema` must be replaced with the version of Node.js you are using.
@@ -1162,9 +1168,10 @@ added:
11621168

11631169
> Stability: 1.0 - Early development
11641170
1165-
If the `--experimental-default-config-file` flag is present, Node.js will look for a
1171+
This flag is an alias for `--experimental-config-file` without an argument.
1172+
If present, Node.js will look for a
11661173
`node.config.json` file in the current working directory and load it as a
1167-
as configuration file.
1174+
configuration file.
11681175

11691176
### `--experimental-eventsource`
11701177

‎doc/api/test.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4535,7 +4535,7 @@ test.describe('my suite', (suite) => {
45354535
[`suite()`]: #suitename-options-fn
45364536
[`test()`]: #testname-options-fn
45374537
[code coverage]: #collecting-code-coverage
4538-
[configuration files]: cli.md#--experimental-config-fileconfig
4538+
[configuration files]: cli.md#--experimental-config-filepath---experimental-config-file
45394539
[describe options]: #describename-options-fn
45404540
[it options]: #testname-options-fn
45414541
[module customization hooks]: module.md#customization-hooks

‎doc/node.1‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -623,8 +623,12 @@ It is possible to run code containing inline types unless the
623623
.It Fl -experimental-addon-modules
624624
Enable experimental import support for \fB.node\fR addons.
625625
.
626-
.It Fl -experimental-config-file Ns = Ns Ar config
626+
.It Fl -experimental-config-file= Ns Ar config , Fl -experimental-config-file , Fl -experimental-default-config-file
627627
If present, Node.js will look for a configuration file at the specified path.
628+
If the path is not specified, Node.js will look for a \fBnode.config.json\fR file
629+
in the current working directory.
630+
To specify a custom path, use the \fB--experimental-config-file=\fR\fBpath\fR form.
631+
The space-separated \fB--experimental-config-file path\fR form is not supported.
628632
Node.js will read the configuration file and apply the settings. The
629633
configuration file should be a JSON file with the following structure. \fBvX.Y.Z\fR
630634
in the \fB$schema\fR must be replaced with the version of Node.js you are using.
@@ -710,9 +714,10 @@ Node.js will not sanitize or perform validation on the user-provided configurati
710714
so \fBNEVER\fR use untrusted configuration files.
711715
.
712716
.It Fl -experimental-default-config-file
713-
If the \fB--experimental-default-config-file\fR flag is present, Node.js will look for a
717+
This flag is an alias for \fB--experimental-config-file\fR without an argument.
718+
If present, Node.js will look for a
714719
\fBnode.config.json\fR file in the current working directory and load it as a
715-
as configuration file.
720+
configuration file.
716721
.
717722
.It Fl -experimental-eventsource
718723
Enable exposition of EventSource Web API on the global scope.

‎lib/internal/main/watch_mode.js‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
'use strict';
22
const {
33
ArrayPrototypeForEach,
4-
ArrayPrototypeIncludes,
54
ArrayPrototypeJoin,
65
ArrayPrototypeMap,
76
ArrayPrototypePush,
@@ -67,11 +66,8 @@ for (let i = 0; i < process.execArgv.length; i++) {
6766
}
6867
continue;
6968
}
70-
if (StringPrototypeStartsWith(arg, '--experimental-config-file')) {
71-
if (!ArrayPrototypeIncludes(arg, '=')) {
72-
// Skip the flag and the next argument (the config file path)
73-
i++;
74-
}
69+
if (arg === '--experimental-config-file' ||
70+
StringPrototypeStartsWith(arg, '--experimental-config-file=')) {
7571
continue;
7672
}
7773
if (arg === '--experimental-default-config-file') {

‎lib/internal/process/pre_execution.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -397,8 +397,7 @@ function setupSQLite() {
397397
}
398398

399399
function initializeConfigFileSupport() {
400-
if (getOptionValue('--experimental-default-config-file') ||
401-
getOptionValue('--experimental-config-file')) {
400+
if (getOptionValue('--experimental-config-file')) {
402401
emitExperimentalWarning('--experimental-config-file');
403402
}
404403
}

‎lib/internal/test_runner/runner.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,11 @@ function createTestFileList(patterns, cwd) {
161161

162162
function filterExecArgv(arg, i, arr) {
163163
return !ArrayPrototypeIncludes(kFilterArgs, arg) &&
164-
!ArrayPrototypeSome(kFilterArgValues, (p) => arg === p || (i > 0 && arr[i - 1] === p) || StringPrototypeStartsWith(arg, `${p}=`));
164+
!ArrayPrototypeSome(kFilterArgValues, (p) => {
165+
return arg === p ||
166+
StringPrototypeStartsWith(arg, `${p}=`) ||
167+
(p !== '--experimental-config-file' && i > 0 && arr[i - 1] === p);
168+
});
165169
}
166170

167171
function getRunArgs(path, { forceExit,

‎src/node.cc‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -900,15 +900,21 @@ static ExitCode InitializeNodeWithArgsInternal(
900900
}
901901

902902
std::string node_options_from_config;
903-
if (auto path = per_process::config_reader.GetDataFromArgs(*argv)) {
904-
switch (per_process::config_reader.ParseConfig(*path)) {
903+
auto config_path = per_process::config_reader.GetDataFromArgs(argv);
904+
if (per_process::config_reader.HasInvalidDefaultConfigFileArgument()) {
905+
errors->push_back("--experimental-default-config-file does not take an "
906+
"argument");
907+
return ExitCode::kInvalidCommandLineArgument;
908+
}
909+
if (config_path) {
910+
switch (per_process::config_reader.ParseConfig(*config_path)) {
905911
case ParseResult::Valid:
906912
break;
907913
case ParseResult::InvalidContent:
908-
errors->push_back(std::string(*path) + ": invalid content");
914+
errors->push_back(std::string(*config_path) + ": invalid content");
909915
break;
910916
case ParseResult::FileError:
911-
errors->push_back(std::string(*path) + ": not found");
917+
errors->push_back(std::string(*config_path) + ": not found");
912918
break;
913919
default:
914920
UNREACHABLE();

‎src/node_config_file.cc‎

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,39 +2,53 @@
22
#include "debug_utils-inl.h"
33
#include "simdjson.h"
44

5-
#include <string>
6-
75
namespace node {
86

7+
constexpr std::string_view kConfigFileFlag = "--experimental-config-file";
8+
constexpr std::string_view kDefaultConfigFileFlag =
9+
"--experimental-default-config-file";
10+
constexpr std::string_view kDefaultConfigFileName = "node.config.json";
11+
12+
inline bool HasEqualsPrefix(std::string_view arg, std::string_view flag) {
13+
return arg.size() > flag.size() && arg.starts_with(flag) &&
14+
arg[flag.size()] == '=';
15+
}
16+
917
std::optional<std::string_view> ConfigReader::GetDataFromArgs(
10-
const std::vector<std::string>& args) {
11-
constexpr std::string_view flag_path = "--experimental-config-file";
12-
constexpr std::string_view default_file =
13-
"--experimental-default-config-file";
14-
15-
bool has_default_config_file = false;
16-
17-
for (auto it = args.begin(); it != args.end(); ++it) {
18-
if (*it == flag_path) {
19-
// Case: "--experimental-config-file foo"
20-
if (auto next = std::next(it); next != args.end()) {
21-
return *next;
18+
std::vector<std::string>* args) {
19+
std::optional<std::string_view> result;
20+
invalid_default_config_file_argument_ = false;
21+
22+
for (size_t i = 0; i < args->size(); ++i) {
23+
std::string& arg = (*args)[i];
24+
25+
if (arg == kConfigFileFlag) {
26+
// --experimental-config-file
27+
arg = std::string(kConfigFileFlag) + "=" +
28+
std::string(kDefaultConfigFileName);
29+
result = kDefaultConfigFileName;
30+
} else if (HasEqualsPrefix(arg, kConfigFileFlag)) {
31+
// --experimental-config-file=path
32+
std::string_view path =
33+
std::string_view(arg).substr(kConfigFileFlag.size() + 1);
34+
if (!path.empty()) {
35+
result = path;
2236
}
23-
} else if (it->starts_with(flag_path)) {
24-
// Case: "--experimental-config-file=foo"
25-
if (it->size() > flag_path.size() && (*it)[flag_path.size()] == '=') {
26-
return std::string_view(*it).substr(flag_path.size() + 1);
27-
}
28-
} else if (*it == default_file || it->starts_with(default_file)) {
29-
has_default_config_file = true;
37+
} else if (arg == kDefaultConfigFileFlag) {
38+
// --experimental-default-config-file
39+
arg = std::string(kConfigFileFlag) + "=" +
40+
std::string(kDefaultConfigFileName);
41+
result = kDefaultConfigFileName;
42+
} else if (HasEqualsPrefix(arg, kDefaultConfigFileFlag)) {
43+
invalid_default_config_file_argument_ = true;
3044
}
3145
}
3246

33-
if (has_default_config_file) {
34-
return "node.config.json";
35-
}
47+
return result;
48+
}
3649

37-
return std::nullopt;
50+
bool ConfigReader::HasInvalidDefaultConfigFileArgument() const {
51+
return invalid_default_config_file_argument_;
3852
}
3953

4054
ParseResult ConfigReader::ProcessOptionValue(

‎src/node_config_file.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ class ConfigReader {
3030
ParseResult ParseConfig(const std::string_view& config_path);
3131

3232
std::optional<std::string_view> GetDataFromArgs(
33-
const std::vector<std::string>& args);
33+
std::vector<std::string>* args);
34+
bool HasInvalidDefaultConfigFileArgument() const;
3435

3536
std::string GetNodeOptions();
3637
const std::vector<std::string>& GetNamespaceFlags() const;
@@ -53,6 +54,7 @@ class ConfigReader {
5354

5455
std::vector<std::string> node_options_;
5556
std::vector<std::string> namespace_options_;
57+
bool invalid_default_config_file_argument_ = false;
5658

5759
// Cache for fast lookup of environment options
5860
std::unordered_map<std::string, options_parser::OptionMappingDetails>

‎src/node_options.cc‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -900,11 +900,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
900900
&EnvironmentOptions::optional_env_file);
901901
Implies("--env-file-if-exists", "[has_env_file_string]");
902902
AddOption("--experimental-config-file",
903-
"set config file from supplied file",
904-
&EnvironmentOptions::experimental_config_file_path);
905-
AddOption("--experimental-default-config-file",
906-
"set config file from default config file",
907-
&EnvironmentOptions::experimental_default_config_file);
903+
"set config file path",
904+
&EnvironmentOptions::experimental_config_file_path,
905+
kDisallowedInEnvvar);
906+
AddAlias("--experimental-default-config-file", "--experimental-config-file");
908907
AddOption("--test",
909908
"launch test runner on startup",
910909
&EnvironmentOptions::test_runner,

0 commit comments

Comments
 (0)