src: traverse parent folders while running --run

doc/api/cli.md

@@ -1864,16 +1864,25 @@ changes:
  - version: REPLACEME
  pr-url: https://github.com/nodejs/node/pull/53058
  description: NODE_RUN_PACKAGE_JSON_PATH environment variable is added.
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/53154
+ description: Traverses up to the root directory and finds
+ a `package.json` file to run the command from, and updates
+ `PATH` environment variable accordingly.
 -->
 
 > Stability: 1.1 - Active development
 
 This runs a specified command from a package.json's `"scripts"` object.
 If no `"command"` is provided, it will list the available scripts.
 
-`--run` prepends `./node_modules/.bin`, relative to the current
-working directory, to the `PATH` in order to execute the binaries from
-dependencies.
+`--run` will traverse up to the root directory and finds a `package.json`
+file to run the command from.
+
+`--run` prepends `./node_modules/.bin` for each ancestor of
+the current directory, to the `PATH` in order to execute the binaries from
+different folders where multiple `node_modules` directories are present, if
+`ancestor-folder/node_modules/.bin` is a directory.
 
 For example, the following command will run the `test` script of
 the `package.json` in the current folder:
@@ -1898,9 +1907,6 @@ cases.
 Some features of other `run` implementations that are intentionally excluded
 are:
 
-* Searching for `package.json` files outside the current folder.
-* Prepending the `.bin` or `node_modules/.bin` paths of folders outside the
- current folder.
 * Running `pre` or `post` scripts in addition to the specified script.
 * Defining package manager-specific environment variables.
 

src/node_task_runner.cc

@@ -1,36 +1,28 @@
 #include "node_task_runner.h"
 #include "util.h"
 
-#include <filesystem>
 #include <regex> // NOLINT(build/c++11)
 
 namespace node::task_runner {
 
 #ifdef _WIN32
-static constexpr const char* bin_path = "\\node_modules\\.bin";
+static constexpr const char* env_var_separator = ";";
 #else
-static constexpr const char* bin_path = "/node_modules/.bin";
+static constexpr const char* env_var_separator = ":";
 #endif // _WIN32
 
 ProcessRunner::ProcessRunner(std::shared_ptr<InitializationResultImpl> result,
- std::string_view package_json_path,
+const std::filesystem::path& package_json_path,
  std::string_view script_name,
  std::string_view command,
- const PositionalArgs& positional_args) {
+ std::string_view path_env_var,
+ const PositionalArgs& positional_args)
+ : init_result(std::move(result)),
+ package_json_path_(package_json_path),
+ script_name_(script_name),
+ path_env_var_(path_env_var) {
  memset(&options_, 0, sizeof(uv_process_options_t));
 
- // Get the current working directory.
- char cwd[PATH_MAX_BYTES];
- size_t cwd_size = PATH_MAX_BYTES;
- CHECK_EQ(uv_cwd(cwd, &cwd_size), 0);
- CHECK_GT(cwd_size, 0);
-
-#ifdef _WIN32
- std::string current_bin_path = cwd + std::string(bin_path) + ";";
-#else
- std::string current_bin_path = cwd + std::string(bin_path) + ":";
-#endif // _WIN32
-
  // Inherit stdin, stdout, and stderr from the parent process.
  options_.stdio_count = 3;
  child_stdio[0].flags = UV_INHERIT_FD;
@@ -46,18 +38,13 @@ ProcessRunner::ProcessRunner(std::shared_ptr<InitializationResultImpl> result,
  options_.flags |= UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS;
 #endif
 
- init_result = std::move(result);
-
  // Set the process handle data to this class instance.
  // This is used to access the class instance from the OnExit callback.
  // It is required because libuv doesn't allow passing lambda functions as a
  // callback.
  process_.data = this;
 
- SetEnvironmentVariables(current_bin_path,
- std::string_view(cwd, cwd_size),
- package_json_path,
- script_name);
+ SetEnvironmentVariables();
 
  std::string command_str(command);
 
@@ -73,12 +60,7 @@ ProcessRunner::ProcessRunner(std::shared_ptr<InitializationResultImpl> result,
  }
 
 #ifdef _WIN32
- // We check whether file_ ends with cmd.exe in a case-insensitive manner.
- // C++20 provides ends_with, but we roll our own for compatibility.
- const char* cmdexe = "cmd.exe";
- if (file_.size() >= strlen(cmdexe) &&
- StringEqualNoCase(cmdexe,
- file_.c_str() + file_.size() - strlen(cmdexe))) {
+ if (file_.ends_with("cmd.exe")) {
  // If the file is cmd.exe, use the following command line arguments:
  // "/c" Carries out the command and exit.
  // "/d" Disables execution of AutoRun commands.
@@ -106,10 +88,7 @@ ProcessRunner::ProcessRunner(std::shared_ptr<InitializationResultImpl> result,
  options_.args[argc] = nullptr;
 }
 
-void ProcessRunner::SetEnvironmentVariables(const std::string& current_bin_path,
- std::string_view cwd,
- std::string_view package_json_path,
- std::string_view script_name) {
+void ProcessRunner::SetEnvironmentVariables() {
  uv_env_item_t* env_items;
  int env_count;
  CHECK_EQ(0, uv_os_environ(&env_items, &env_count));
@@ -130,28 +109,21 @@ void ProcessRunner::SetEnvironmentVariables(const std::string& current_bin_path,
 #endif // _WIN32
 
  if (StringEqualNoCase(name.c_str(), "path")) {
- // Add bin_path to the beginning of the PATH
- value = current_bin_path + value;
+ // Add path env variable to the beginning of the PATH
+ value = path_env_var_ + value;
  }
  env_vars_.push_back(name + "=" + value);
  }
  uv_os_free_environ(env_items, env_count);
 
  // Add NODE_RUN_SCRIPT_NAME environment variable to the environment
  // to indicate which script is being run.
- env_vars_.push_back("NODE_RUN_SCRIPT_NAME=" + std::string(script_name));
+ env_vars_.push_back("NODE_RUN_SCRIPT_NAME=" + script_name_);
 
  // Add NODE_RUN_PACKAGE_JSON_PATH environment variable to the environment to
  // indicate which package.json is being processed.
- if (std::filesystem::path(package_json_path).is_absolute()) {
- // TODO(anonrig): Traverse up the directory tree until we find a
- // package.json
- env_vars_.push_back("NODE_RUN_PACKAGE_JSON_PATH=" +
- std::string(package_json_path));
- } else {
- auto path = std::filesystem::path(cwd) / std::string(package_json_path);
- env_vars_.push_back("NODE_RUN_PACKAGE_JSON_PATH=" + path.string());
- }
+ env_vars_.push_back("NODE_RUN_PACKAGE_JSON_PATH=" +
+ package_json_path_.string());
 
  env = std::unique_ptr<char*[]>(new char*[env_vars_.size() + 1]);
  options_.env = env.get();
@@ -240,19 +212,58 @@ void ProcessRunner::Run() {
  uv_run(loop_, UV_RUN_DEFAULT);
 }
 
+std::optional<std::tuple<std::filesystem::path, std::string, std::string>>
+FindPackageJson(const std::filesystem::path& cwd) {
+ auto package_json_path = cwd / "package.json";
+ std::string raw_content;
+ std::string path_env_var;
+ auto root_path = cwd.root_path();
+
+ for (auto directory_path = cwd;
+ !std::filesystem::equivalent(root_path, directory_path);
+ directory_path = directory_path.parent_path()) {
+ // Append "path/node_modules/.bin" to the env var, if it is a directory.
+ auto node_modules_bin = directory_path / "node_modules" / ".bin";
+ if (std::filesystem::is_directory(node_modules_bin)) {
+ path_env_var += node_modules_bin.string() + env_var_separator;
+ }
+
+ if (raw_content.empty()) {
+ package_json_path = directory_path / "package.json";
+ // This is required for Windows because std::filesystem::path::c_str()
+ // returns wchar_t* on Windows, and char* on other platforms.
+ std::string contents = package_json_path.string();
+ USE(ReadFileSync(&raw_content, contents.c_str()) > 0);
+ }
+ }
+
+ // This means that there is no package.json until the root directory.
+ // In this case, we just return nullopt, which will terminate the process..
+ if (raw_content.empty()) {
+ return std::nullopt;
+ }
+
+ return {{package_json_path, raw_content, path_env_var}};
+}
+
 void RunTask(std::shared_ptr<InitializationResultImpl> result,
  std::string_view command_id,
  const std::vector<std::string_view>& positional_args) {
-std::string_view path = "package.json";
-std::string raw_json;
+auto cwd = std::filesystem::current_path();
+auto package_json = FindPackageJson(cwd);
 
- // No need to exclude BOM since simdjson will skip it.
- if (ReadFileSync(&raw_json, path.data()) < 0) {
+ if (!package_json.has_value()) {
  fprintf(stderr, "Can't read package.json\n");
  result->exit_code_ = ExitCode::kGenericUserError;
  return;
  }
 
+ // - path: Path to the package.json file.
+ // - raw_json: Raw content of the package.json file.
+ // - path_env_var: This represents the `PATH` environment variable.
+ // It always ends with ";" or ":" depending on the platform.
+ auto [path, raw_json, path_env_var] = *package_json;
+
  simdjson::ondemand::parser json_parser;
  simdjson::ondemand::document document;
  simdjson::ondemand::object main_object;
@@ -302,8 +313,8 @@ void RunTask(std::shared_ptr<InitializationResultImpl> result,
  return;
  }
 
- auto runner =
-ProcessRunner(result, path, command_id, command, positional_args);
+ auto runner = ProcessRunner(
+ result, path, command_id, command, path_env_var, positional_args);
  runner.Run();
 }
 
@@ -317,10 +328,11 @@ PositionalArgs GetPositionalArgs(const std::vector<std::string>& args) {
  if (auto dash_dash = std::find(args.begin(), args.end(), "--");
  dash_dash != args.end()) {
  PositionalArgs positional_args{};
+ positional_args.reserve(args.size() - (dash_dash - args.begin()));
  for (auto it = dash_dash + 1; it != args.end(); ++it) {
  // SAFETY: The following code is safe because the lifetime of the
  // arguments is guaranteed to be valid until the end of the task runner.
- positional_args.push_back(std::string_view(it->c_str(), it->size()));
+ positional_args.emplace_back(it->c_str(), it->size());
  }
  return positional_args;
  }

src/node_task_runner.h

@@ -8,11 +8,12 @@
 #include "spawn_sync.h"
 #include "uv.h"
 
+#include <filesystem>
 #include <optional>
 #include <string_view>
+#include <tuple>
 
-namespace node {
-namespace task_runner {
+namespace node::task_runner {
 
 using PositionalArgs = std::vector<std::string_view>;
 
@@ -23,9 +24,10 @@ using PositionalArgs = std::vector<std::string_view>;
 class ProcessRunner {
  public:
  ProcessRunner(std::shared_ptr<InitializationResultImpl> result,
- std::string_view package_json_path,
+const std::filesystem::path& package_json_path,
  std::string_view script_name,
- std::string_view command_id,
+ std::string_view command,
+ std::string_view path_env_var,
  const PositionalArgs& positional_args);
  void Run();
  static void ExitCallback(uv_process_t* req,
@@ -45,26 +47,44 @@ class ProcessRunner {
 
  // OnExit is the callback function that is called when the process exits.
  void OnExit(int64_t exit_status, int term_signal);
- void SetEnvironmentVariables(const std::string& bin_path,
- std::string_view cwd,
- std::string_view package_json_path,
- std::string_view script_name);
+ void SetEnvironmentVariables();
 
 #ifdef _WIN32
  std::string file_ = "cmd.exe";
 #else
  std::string file_ = "/bin/sh";
 #endif // _WIN32
+
+ // Represents the absolute path to the package.json file.
+ std::filesystem::path package_json_path_;
+ // Represents the name of the script that is being run.
+ std::string script_name_;
+ // Represents PATH environment variable that contains
+ // all subdirectory paths appended with node_modules/.bin suffix.
+ std::string path_env_var_;
 };
 
+// This function traverses up to the root directory.
+// While traversing up, if it finds a package.json file, it reads its content.
+// If it cannot find a package.json file, it returns std::nullopt.
+// Otherwise, it returns a tuple of:
+// - the path to the package.json file
+// - package.json file content
+// - `path_env_var` variable
+//
+// For example, on POSIX, it returns the following for `path_env_var`,
+// if the current directory is `/anonrig`:
+// `/anonrig/node_modules/.bin:/node_modules/.bin`
+std::optional<std::tuple<std::filesystem::path, std::string, std::string>>
+FindPackageJson(const std::filesystem::path& cwd);
+
 void RunTask(std::shared_ptr<InitializationResultImpl> result,
  std::string_view command_id,
  const PositionalArgs& positional_args);
 PositionalArgs GetPositionalArgs(const std::vector<std::string>& args);
-std::string EscapeShell(const std::string_view command);
+std::string EscapeShell(std::string_view command);
 
-} // namespace task_runner
-} // namespace node
+} // namespace node::task_runner
 
 #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 

test/parallel/test-node-run.js

@@ -1,4 +1,3 @@
-// Flags: --expose-internals
 'use strict';
 
 const common = require('../common');
@@ -8,7 +7,7 @@ const assert = require('node:assert');
 const fixtures = require('../common/fixtures');
 const envSuffix = common.isWindows ? '-windows' : '';
 
-describe('node run [command]', () => {
+describe('node --run [command]', () => {
  it('should emit experimental warning', async () => {
  const child = await common.spawnPromisified(
  process.execPath,
@@ -70,13 +69,21 @@ describe('node run [command]', () => {
  assert.strictEqual(child.code, 0);
  });
 
- it('should set PATH environment variable to node_modules/.bin', async () => {
+ it('should set PATH environment variable with paths appended with node_modules/.bin', async () => {
  const child = await common.spawnPromisified(
  process.execPath,
  [ '--no-warnings', '--run', `path-env${envSuffix}`],
- { cwd: fixtures.path('run-script') },
+ { cwd: fixtures.path('run-script/sub-directory') },
  );
  assert.ok(child.stdout.includes(fixtures.path('run-script/node_modules/.bin')));
+
+ // The following test ensures that we do not add paths that does not contain
+ // "node_modules/.bin"
+ assert.ok(!child.stdout.includes(fixtures.path('node_modules/.bin')));
+
+ // The following test ensures that we add paths that contains "node_modules/.bin"
+ assert.ok(child.stdout.includes(fixtures.path('run-script/sub-directory/node_modules/.bin')));
+
  assert.strictEqual(child.stderr, '');
  assert.strictEqual(child.code, 0);
  });
@@ -94,4 +101,16 @@ describe('node run [command]', () => {
  assert.strictEqual(child.stderr, '');
  assert.strictEqual(child.code, 0);
  });
+
+ it('will search parent directories for a package.json file', async () => {
+ const packageJsonPath = fixtures.path('run-script/package.json');
+ const child = await common.spawnPromisified(
+ process.execPath,
+ [ '--no-warnings', '--run', `special-env-variables${envSuffix}`],
+ { cwd: fixtures.path('run-script/sub-directory') },
+ );
+ assert.ok(child.stdout.includes(packageJsonPath));
+ assert.strictEqual(child.stderr, '');
+ assert.strictEqual(child.code, 0);
+ });
 });