Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: add --env-file-if-exists flag #53060

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,8 @@ in the file, the value from the environment takes precedence.
You can pass multiple `--env-file` arguments. Subsequent files override
pre-existing variables defined in previous files.

An error is thrown if the file does not exist.

```bash
node --env-file=.env --env-file=.development.env index.js
```
Expand Down Expand Up @@ -867,6 +869,9 @@ Export keyword before a key is ignored:
export USERNAME="nodejs" # will result in `nodejs` as the value.
```

If you want to load environment variables from a file that may not exist, you
can use the [`--env-file-if-exists`][] flag instead.

### `-e`, `--eval "script"`

<!-- YAML
Expand Down Expand Up @@ -1761,6 +1766,15 @@ is being linked to Node.js. Sharing the OpenSSL configuration may have unwanted
implications and it is recommended to use a configuration section specific to
Node.js which is `nodejs_conf` and is default when this option is not used.

### `--env-file-if-exists=config`
anonrig marked this conversation as resolved.
Show resolved Hide resolved

<!-- YAML
added: REPLACEME
-->

Behavior is the same as [`--env-file`][], but an error is not thrown if the file
does not exist.

### `--pending-deprecation`

<!-- YAML
Expand Down Expand Up @@ -3548,6 +3562,8 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
[`--build-snapshot`]: #--build-snapshot
[`--cpu-prof-dir`]: #--cpu-prof-dir
[`--diagnostic-dir`]: #--diagnostic-dirdirectory
[`--env-file-if-exists`]: #--env-file-if-existsconfig
[`--env-file`]: #--env-fileconfig
[`--experimental-default-type=module`]: #--experimental-default-typetype
[`--experimental-sea-config`]: single-executable-applications.md#generating-single-executable-preparation-blobs
[`--experimental-strip-types`]: #--experimental-strip-types
Expand Down
18 changes: 12 additions & 6 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -854,20 +854,26 @@ static ExitCode InitializeNodeWithArgsInternal(
HandleEnvOptions(per_process::cli_options->per_isolate->per_env);

std::string node_options;
auto file_paths = node::Dotenv::GetPathFromArgs(*argv);
auto env_files = node::Dotenv::GetDataFromArgs(*argv);

if (!file_paths.empty()) {
if (!env_files.empty()) {
CHECK(!per_process::v8_initialized);

for (const auto& file_path : file_paths) {
switch (per_process::dotenv_file.ParsePath(file_path)) {
for (const auto& file_data : env_files) {
switch (per_process::dotenv_file.ParsePath(file_data.path)) {
case Dotenv::ParseResult::Valid:
break;
case Dotenv::ParseResult::InvalidContent:
errors->push_back(file_path + ": invalid format");
errors->push_back(file_data.path + ": invalid format");
break;
case Dotenv::ParseResult::FileError:
errors->push_back(file_path + ": not found");
if (file_data.is_optional) {
fprintf(stderr,
"%s not found. Continuing without it.\n",
file_data.path.c_str());
continue;
}
errors->push_back(file_data.path + ": not found");
break;
default:
UNREACHABLE();
Expand Down
48 changes: 32 additions & 16 deletions src/node_dotenv.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,36 +11,52 @@ using v8::NewStringType;
using v8::Object;
using v8::String;

std::vector<std::string> Dotenv::GetPathFromArgs(
std::vector<Dotenv::env_file_data> Dotenv::GetDataFromArgs(
const std::vector<std::string>& args) {
const std::string_view optional_env_file_flag = "--env-file-if-exists";

const auto find_match = [](const std::string& arg) {
return arg == "--" || arg == "--env-file" || arg.starts_with("--env-file=");
return arg == "--" || arg == "--env-file" ||
arg.starts_with("--env-file=") || arg == "--env-file-if-exists" ||
arg.starts_with("--env-file-if-exists=");
};
std::vector<std::string> paths;
auto path = std::find_if(args.begin(), args.end(), find_match);

while (path != args.end()) {
if (*path == "--") {
return paths;
std::vector<Dotenv::env_file_data> env_files;
// This will be an iterator, pointing to args.end() if no matches are found
auto matched_arg = std::find_if(args.begin(), args.end(), find_match);

while (matched_arg != args.end()) {
if (*matched_arg == "--") {
return env_files;
}
auto equal_char = path->find('=');

if (equal_char != std::string::npos) {
paths.push_back(path->substr(equal_char + 1));
auto equal_char_index = matched_arg->find('=');

if (equal_char_index != std::string::npos) {
// `--env-file=path`
auto flag = matched_arg->substr(0, equal_char_index);
auto file_path = matched_arg->substr(equal_char_index + 1);

struct env_file_data env_file_data = {
file_path, flag.starts_with(optional_env_file_flag)};
env_files.push_back(env_file_data);
} else {
auto next_path = std::next(path);
// `--env-file path`
auto file_path = std::next(matched_arg);

if (next_path == args.end()) {
return paths;
if (file_path == args.end()) {
return env_files;
}

paths.push_back(*next_path);
struct env_file_data env_file_data = {
*file_path, matched_arg->starts_with(optional_env_file_flag)};
env_files.push_back(env_file_data);
}

path = std::find_if(++path, args.end(), find_match);
matched_arg = std::find_if(++matched_arg, args.end(), find_match);
}

return paths;
return env_files;
}

void Dotenv::SetEnvironment(node::Environment* env) {
Expand Down
6 changes: 5 additions & 1 deletion src/node_dotenv.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ namespace node {
class Dotenv {
public:
enum ParseResult { Valid, FileError, InvalidContent };
struct env_file_data {
std::string path;
BoscoDomingo marked this conversation as resolved.
Show resolved Hide resolved
bool is_optional;
};

Dotenv() = default;
Dotenv(const Dotenv& d) = delete;
Expand All @@ -27,7 +31,7 @@ class Dotenv {
void SetEnvironment(Environment* env);
v8::Local<v8::Object> ToObject(Environment* env) const;

static std::vector<std::string> GetPathFromArgs(
static std::vector<env_file_data> GetDataFromArgs(
const std::vector<std::string>& args);

private:
Expand Down
4 changes: 4 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"set environment variables from supplied file",
&EnvironmentOptions::env_file);
Implies("--env-file", "[has_env_file_string]");
AddOption("--env-file-if-exists",
"set environment variables from supplied file",
BoscoDomingo marked this conversation as resolved.
Show resolved Hide resolved
&EnvironmentOptions::optional_env_file);
Implies("--env-file-if-exists", "[has_env_file_string]");
AddOption("--test",
"launch test runner on startup",
&EnvironmentOptions::test_runner);
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ class EnvironmentOptions : public Options {
std::string redirect_warnings;
std::string diagnostic_dir;
std::string env_file;
std::string optional_env_file;
bool has_env_file_string = false;
bool test_runner = false;
uint64_t test_runner_concurrency = 0;
Expand Down
54 changes: 45 additions & 9 deletions test/parallel/test-dotenv-edge-cases.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,30 +10,53 @@ const validEnvFilePath = '../fixtures/dotenv/valid.env';
const nodeOptionsEnvFilePath = '../fixtures/dotenv/node-options.env';

describe('.env supports edge cases', () => {

it('supports multiple declarations', async () => {
// process.env.BASIC is equal to `basic` because the second .env file overrides it.
it('supports multiple declarations, including optional ones', async () => {
BoscoDomingo marked this conversation as resolved.
Show resolved Hide resolved
const code = `
const assert = require('assert');
assert.strictEqual(process.env.BASIC, 'basic');
assert.strictEqual(process.env.NODE_NO_WARNINGS, '1');
`.trim();
const children = await Promise.all(Array.from({ length: 4 }, (_, i) =>
common.spawnPromisified(
process.execPath,
[
// Bitwise AND to create all 4 possible combinations:
// i & 0b01 is truthy when i has value 0bx1 (i.e. 0b01 (1) and 0b11 (3)), falsy otherwise.
// i & 0b10 is truthy when i has value 0b1x (i.e. 0b10 (2) and 0b11 (3)), falsy otherwise.
`${i & 0b01 ? '--env-file' : '--env-file-if-exists'}=${nodeOptionsEnvFilePath}`,
`${i & 0b10 ? '--env-file' : '--env-file-if-exists'}=${validEnvFilePath}`,
'--eval', code,
],
{ cwd: __dirname },
)));
assert.deepStrictEqual(children, Array.from({ length: 4 }, () => ({
code: 0,
signal: null,
stdout: '',
stderr: '',
})));
});

it('supports absolute paths', async () => {
const code = `
require('assert').strictEqual(process.env.BASIC, 'basic');
`.trim();
const child = await common.spawnPromisified(
process.execPath,
[ `--env-file=${nodeOptionsEnvFilePath}`, `--env-file=${validEnvFilePath}`, '--eval', code ],
{ cwd: __dirname },
[ `--env-file=${path.resolve(__dirname, validEnvFilePath)}`, '--eval', code ],
);
assert.strictEqual(child.stderr, '');
assert.strictEqual(child.code, 0);
});

it('supports absolute paths', async () => {
it('supports a space instead of \'=\' for the flag ', async () => {
const code = `
require('assert').strictEqual(process.env.BASIC, 'basic');
`.trim();
const child = await common.spawnPromisified(
process.execPath,
[ `--env-file=${path.resolve(__dirname, validEnvFilePath)}`, '--eval', code ],
[ '--env-file', validEnvFilePath, '--eval', code ],
{ cwd: __dirname },
);
assert.strictEqual(child.stderr, '');
assert.strictEqual(child.code, 0);
Expand All @@ -48,10 +71,23 @@ describe('.env supports edge cases', () => {
[ '--env-file=.env', '--eval', code ],
{ cwd: __dirname },
);
assert.notStrictEqual(child.stderr.toString(), '');
assert.notStrictEqual(child.stderr, '');
assert.strictEqual(child.code, 9);
});

it('should handle non-existent optional .env file', async () => {
const code = `
require('assert').strictEqual(1,1);
`.trim();
const child = await common.spawnPromisified(
process.execPath,
['--env-file-if-exists=.env', '--eval', code],
{ cwd: __dirname },
);
assert.notStrictEqual(child.stderr, '');
assert.strictEqual(child.code, 0);
});

it('should not override existing environment variables but introduce new vars', async () => {
const code = `
require('assert').strictEqual(process.env.BASIC, 'existing');
Expand Down Expand Up @@ -106,7 +142,7 @@ describe('.env supports edge cases', () => {
'--eval', `require('assert').strictEqual(process.env.BASIC, undefined);`,
'--', '--env-file', validEnvFilePath,
],
{ cwd: fixtures.path('dotenv') },
{ cwd: __dirname },
BoscoDomingo marked this conversation as resolved.
Show resolved Hide resolved
);
assert.strictEqual(child.stdout, '');
assert.strictEqual(child.stderr, '');
Expand Down
Loading