diff --git a/doc/api/cli.md b/doc/api/cli.md index dda77612f215aa..4d6c18912bd8bb 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -1864,6 +1864,11 @@ 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 @@ -1871,9 +1876,13 @@ changes: 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. diff --git a/src/node_task_runner.cc b/src/node_task_runner.cc index 82dcfdc5c3cf70..f053321e31f0b4 100644 --- a/src/node_task_runner.cc +++ b/src/node_task_runner.cc @@ -1,35 +1,27 @@ #include "node_task_runner.h" #include "util.h" -#include #include // 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 result, - std::string_view package_json_path, + const std::filesystem::path& package_json_path, std::string_view script_name, std::string_view command, + std::string_view path_env_var, const PositionalArgs& positional_args) { 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 + package_json_path_ = std::filesystem::path(package_json_path); + script_name_ = std::string(script_name); + path_env_var_ = path_env_var; // Inherit stdin, stdout, and stderr from the parent process. options_.stdio_count = 3; @@ -54,10 +46,7 @@ ProcessRunner::ProcessRunner(std::shared_ptr result, // 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); @@ -106,10 +95,7 @@ ProcessRunner::ProcessRunner(std::shared_ptr 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,8 +116,8 @@ 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); } @@ -139,19 +125,12 @@ void ProcessRunner::SetEnvironmentVariables(const std::string& current_bin_path, // 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(new char*[env_vars_.size() + 1]); options_.env = env.get(); @@ -240,19 +219,56 @@ void ProcessRunner::Run() { uv_run(loop_, UV_RUN_DEFAULT); } +std::optional> +FindPackageJson(const std::filesystem::path& cwd) { + auto package_json_path = cwd / "package.json"; + std::string raw_content; + std::string path_env_var; + + for (auto directory_path = cwd; directory_path != cwd.root_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"; + // No need to exclude BOM since simdjson will skip it. + USE(ReadFileSync(&raw_content, package_json_path.c_str())); + } + } + + // 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 std::tuple(package_json_path, raw_content, path_env_var); +} + void RunTask(std::shared_ptr result, std::string_view command_id, const std::vector& 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 to the package.json file. + auto path = std::get<0>(*package_json); + // Raw content of the package.json file. + auto raw_json = std::get<1>(*package_json); + // This represents the `PATH` environment variable. + // It always ends with ";" or ":" depending on the platform. + auto path_env_var = std::get<2>(*package_json); + simdjson::ondemand::parser json_parser; simdjson::ondemand::document document; simdjson::ondemand::object main_object; @@ -302,8 +318,8 @@ void RunTask(std::shared_ptr 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 +333,11 @@ PositionalArgs GetPositionalArgs(const std::vector& 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; } diff --git a/src/node_task_runner.h b/src/node_task_runner.h index ea430a6e32c747..740ba15f258d71 100644 --- a/src/node_task_runner.h +++ b/src/node_task_runner.h @@ -8,8 +8,10 @@ #include "spawn_sync.h" #include "uv.h" +#include #include #include +#include namespace node { namespace task_runner { @@ -23,9 +25,10 @@ using PositionalArgs = std::vector; class ProcessRunner { public: ProcessRunner(std::shared_ptr 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,18 +48,37 @@ 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> +FindPackageJson(const std::filesystem::path& cwd); + void RunTask(std::shared_ptr result, std::string_view command_id, const PositionalArgs& positional_args); diff --git a/test/fixtures/run-script/sub-directory/.gitkeep b/test/fixtures/run-script/sub-directory/.gitkeep new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/test/fixtures/run-script/sub-directory/node_modules/.bin/.gitkeep b/test/fixtures/run-script/sub-directory/node_modules/.bin/.gitkeep new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/test/parallel/test-node-run.js b/test/parallel/test-node-run.js index 89c014bddd2f1b..caefe73ad4847c 100644 --- a/test/parallel/test-node-run.js +++ b/test/parallel/test-node-run.js @@ -70,13 +70,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 +102,16 @@ describe('node run [command]', () => { assert.strictEqual(child.stderr, ''); assert.strictEqual(child.code, 0); }); + + it('will search subdirectories 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); + }); });