src: support multiple --env-file declarations

PR-URL: nodejs#49542 Refs: nodejs#49148 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
rluvaton · Sep 11, 2023 · 022f1b7 · 022f1b7
1 parent 154b1c2
commit 022f1b7
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 36 deletions.
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -1012,6 +1012,13 @@ variables which configure Node.js][environment_variables], such as `NODE_OPTIONS
 are parsed and applied. If the same variable is defined in the environment and
 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.
+
+```bash
+node --env-file=.env --env-file=.development.env index.js
+```
+
 The format of the file should be one line per key-value pair of environment
 variable name and value separated by `=`:
 

diff --git a/src/node.cc b/src/node.cc
@@ -841,13 +841,17 @@ static ExitCode InitializeNodeWithArgsInternal(
   HandleEnvOptions(per_process::cli_options->per_isolate->per_env);
 
   std::string node_options;
-  auto file_path = node::Dotenv::GetPathFromArgs(*argv);
+  auto file_paths = node::Dotenv::GetPathFromArgs(*argv);
 
-  if (file_path.has_value()) {
-    auto cwd = Environment::GetCwd(Environment::GetExecPath(*argv));
-    std::string path = cwd + kPathSeparator + file_path.value();
+  if (!file_paths.empty()) {
     CHECK(!per_process::v8_initialized);
-    per_process::dotenv_file.ParsePath(path);
+    auto cwd = Environment::GetCwd(Environment::GetExecPath(*argv));
+
+    for (const auto& file_path : file_paths) {
+      std::string path = cwd + kPathSeparator + file_path;
+      per_process::dotenv_file.ParsePath(path);
+    }
+
     per_process::dotenv_file.AssignNodeOptionsIfAvailable(&node_options);
   }
 

diff --git a/src/node_dotenv.cc b/src/node_dotenv.cc
@@ -8,34 +8,34 @@ namespace node {
 using v8::NewStringType;
 using v8::String;
 
-std::optional<std::string> Dotenv::GetPathFromArgs(
+std::vector<std::string> Dotenv::GetPathFromArgs(
     const std::vector<std::string>& args) {
-  std::string_view flag = "--env-file";
-  // Match the last `--env-file`
-  // This is required to imitate the default behavior of Node.js CLI argument
-  // matching.
-  auto path =
-      std::find_if(args.rbegin(), args.rend(), [&flag](const std::string& arg) {
-        return strncmp(arg.c_str(), flag.data(), flag.size()) == 0;
-      });
-
-  if (path == args.rend()) {
-    return std::nullopt;
-  }
-
-  auto equal_char = path->find('=');
-
-  if (equal_char != std::string::npos) {
-    return path->substr(equal_char + 1);
-  }
-
-  auto next_arg = std::prev(path);
+  const auto find_match = [](const std::string& arg) {
+    const std::string_view flag = "--env-file";
+    return strncmp(arg.c_str(), flag.data(), flag.size()) == 0;
+  };
+  std::vector<std::string> paths;
+  auto path = std::find_if(args.begin(), args.end(), find_match);
+
+  while (path != args.end()) {
+    auto equal_char = path->find('=');
+
+    if (equal_char != std::string::npos) {
+      paths.push_back(path->substr(equal_char + 1));
+    } else {
+      auto next_path = std::next(path);
+
+      if (next_path == args.end()) {
+        return paths;
+      }
+
+      paths.push_back(*next_path);
+    }
 
-  if (next_arg == args.rend()) {
-    return std::nullopt;
+    path = std::find_if(++path, args.end(), find_match);
   }
 
-  return *next_arg;
+  return paths;
 }
 
 void Dotenv::SetEnvironment(node::Environment* env) {
@@ -163,7 +163,7 @@ void Dotenv::ParseLine(const std::string_view line) {
       value.erase(value.size() - 1);
   }
 
-  store_.emplace(key, value);
+  store_.insert_or_assign(std::string(key), value);
 }
 
 }  // namespace node
diff --git a/src/node_dotenv.h b/src/node_dotenv.h
@@ -6,7 +6,6 @@
 #include "util-inl.h"
 
 #include <map>
-#include <optional>
 
 namespace node {
 
@@ -23,7 +22,7 @@ class Dotenv {
   void AssignNodeOptionsIfAvailable(std::string* node_options);
   void SetEnvironment(Environment* env);
 
-  static std::optional<std::string> GetPathFromArgs(
+  static std::vector<std::string> GetPathFromArgs(
       const std::vector<std::string>& args);
 
  private:

diff --git a/test/fixtures/dotenv/node-options.env b/test/fixtures/dotenv/node-options.env
@@ -3,3 +3,4 @@ NODE_NO_WARNINGS=1
 NODE_OPTIONS="--experimental-permission --allow-fs-read=*"
 TZ=Pacific/Honolulu
 UV_THREADPOOL_SIZE=5
+BASIC=overridden
diff --git a/test/parallel/test-dotenv-edge-cases.js b/test/parallel/test-dotenv-edge-cases.js
@@ -5,17 +5,20 @@ const assert = require('node:assert');
 const { describe, it } = require('node:test');
 
 const validEnvFilePath = '../fixtures/dotenv/valid.env';
-const relativePath = '../fixtures/dotenv/node-options.env';
+const nodeOptionsEnvFilePath = '../fixtures/dotenv/node-options.env';
 
 describe('.env supports edge cases', () => {
 
-  it('should use the last --env-file declaration', async () => {
+  it('supports multiple declarations', async () => {
+    // process.env.BASIC is equal to `basic` because the second .env file overrides it.
     const code = `
-      require('assert').strictEqual(process.env.BASIC, 'basic');
+      const assert = require('assert');
+      assert.strictEqual(process.env.BASIC, 'basic');
+      assert.strictEqual(process.env.NODE_NO_WARNINGS, '1');
     `.trim();
     const child = await common.spawnPromisified(
       process.execPath,
-      [ `--env-file=${relativePath}`, `--env-file=${validEnvFilePath}`, '--eval', code ],
+      [ `--env-file=${nodeOptionsEnvFilePath}`, `--env-file=${validEnvFilePath}`, '--eval', code ],
       { cwd: __dirname },
     );
     assert.strictEqual(child.stderr, '');