From 6a4d9b060bf23669f4215bb5e527c84ac120f417 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Sat, 25 May 2024 20:17:48 -0400 Subject: [PATCH] src: traverse parent folders while running `--run` --- doc/api/cli.md | 17 ++- src/node_task_runner.cc | 107 +++++++++++------- src/node_task_runner.h | 33 +++++- .../run-script/sub-directory/.gitkeep | 0 test/parallel/test-node-run.js | 16 ++- 5 files changed, 120 insertions(+), 53 deletions(-) create mode 100644 test/fixtures/run-script/sub-directory/.gitkeep diff --git a/doc/api/cli.md b/doc/api/cli.md index dda77612f215aa..cb038b721c266b 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: `--run` will traverse 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,12 @@ 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 subdirectory of +the current directory, to the `PATH` in order to execute the binaries from +dependencies where multiple `node_modules` directories are present. For example, the following command will run the `test` script of the `package.json` in the current folder: @@ -1898,9 +1906,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..ef60fbb9d94583 100644 --- a/src/node_task_runner.cc +++ b/src/node_task_runner.cc @@ -7,29 +7,22 @@ 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 +47,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 +96,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 +117,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 +126,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 +220,66 @@ void ProcessRunner::Run() { uv_run(loop_, UV_RUN_DEFAULT); } +std::optional> +FindPackageJson(const std::filesystem::path& cwd) { + // Two different paths are created only for improving readability. + // Otherwise, it is not needed. + auto directory_path = cwd; + auto package_json_path = directory_path / "package.json"; + std::string raw_content; + std::string path_env_var = + (directory_path / "node_modules" / ".bin").string() + env_var_separator; + + USE(ReadFileSync(&raw_content, package_json_path.c_str())); + + do { + directory_path = directory_path.parent_path(); + + // Always append "node_modules/.bin" to the env var. + // This is in par with existing package managers. + path_env_var += + (directory_path / "node_modules" / ".bin").string() + env_var_separator; + + // No need to exclude BOM since simdjson will skip it. + if (raw_content.empty()) { + package_json_path = directory_path / "package.json"; + USE(ReadFileSync(&raw_content, package_json_path.c_str())); + } + + // Traverse up to the root directory. + // Note: root directory parent path is the root directory itself. + // Hence, "/".parent_path() == "/". + } while (directory_path != directory_path.parent_path()); + + // 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 +329,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(); } diff --git a/src/node_task_runner.h b/src/node_task_runner.h index ea430a6e32c747..7b409c8b0841ef 100644 --- a/src/node_task_runner.h +++ b/src/node_task_runner.h @@ -10,6 +10,7 @@ #include #include +#include namespace node { namespace task_runner { @@ -23,9 +24,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 +47,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/parallel/test-node-run.js b/test/parallel/test-node-run.js index 89c014bddd2f1b..c5677aa7625eff 100644 --- a/test/parallel/test-node-run.js +++ b/test/parallel/test-node-run.js @@ -70,13 +70,15 @@ 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') }, ); assert.ok(child.stdout.includes(fixtures.path('run-script/node_modules/.bin'))); + // The following test ensures that we traverse up to the root directory. + assert.ok(child.stdout.includes(fixtures.path('node_modules/.bin'))); assert.strictEqual(child.stderr, ''); assert.strictEqual(child.code, 0); }); @@ -94,4 +96,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); + }); });