src: add --run-from runtime flag

[mertcanaltin]

add --run-from runtime flag

Fixes #57489

[nodejs-github-bot]

Review requested:

• @nodejs/config

[aduh95]

The commit message doesn't adhere to our guidelines, it should start with an imperative verb so e.g. src: add `--run-in` runtime flag

[anonrig]

cc @flakey5

[anonrig]

We need to also change the pull-request title and description to reflect the changes.

[github-actions]

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @anonrig.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

[anonrig]

LGTM if tests pass (and the new requested test is added)

[GeoffreyBooth]

so e.g. src: add `--run-in` runtime flag

I find the name --run-dir confusing; you're not running a directory, you're specifying the folder to look in to find the package.json file. Why not just ask for the path to the package.json file directly? Or allow either a path to the file or the path to the containing folder. Something like:

node --run-path=./app/package.json --run test:backend
# Or
node --run-path=./app --run test:backend

[mertcanaltin]

so e.g. src: add `--run-in` runtime flag

I find the name --run-dir confusing; you're not running a directory, you're specifying the folder to look in to find the package.json file. Why not just ask for the path to the package.json file directly? Or allow either a path to the file or the path to the containing folder. Something like:

node --run-path=./app/package.json --run test:backend
# Or
node --run-path=./app --run test:backend

I understand your concern, I will wait for a few more ideas to make a collective decision, thank you very much for your suggestion 🙏

[mertcanaltin]

thanks a lot, now I am merging all commits and fixing my first commit message 🚀

[GeoffreyBooth]

the ask is for a way of setting the working directory of the --run command, not the process as a whole.

What does it mean to apply only to --run? Does that just tell it where to find package.json or are there other effects?

[mertcanaltin]

the ask is for a way of setting the working directory of the --run command, not the process as a whole.

What does it mean to apply only to --run? Does that just tell it where to find package.json or are there other effects?

only determines the directory where the --run command finds the package.json file; it does not change the process-wide working directory.

[GeoffreyBooth]

only determines the directory where the --run command finds the package.json file; it does not change the process-wide working directory.

Okay, can it accept as input either the path to the containing folder or the path directly to the package.json file?

(As in, if this isn't the current behavior is it possible to update the PR so that this is possible.)

[targos]

I'm not sure this feature will work as expected if it doesn't set the process's cwd. Most scripts I write wouldn't work FWIW

[mertcanaltin]

only determines the directory where the --run command finds the package.json file; it does not change the process-wide working directory.

Okay, can it accept as input either the path to the containing folder or the path directly to the package.json file?

(As in, if this isn't the current behavior is it possible to update the PR so that this is possible.)

yes, I'm trying now

[mertcanaltin]

hello i sent an improvement @GeoffreyBooth @ovflowd @targos

I tried to implement that if given a package.json directly, this directory is now selected as the parent directory

[GeoffreyBooth]

I'm not sure this feature will work as expected if it doesn't set the process's cwd. Most scripts I write wouldn't work FWIW

I'm wondering how this can work in practice too. Does it solve #57489? Are the scripts in that package.json runnable from a different CWD?

[anonrig]

This will not work since this doesn't add the current node_modules to path environment variable.

[anonrig]

only determines the directory where the --run command finds the package.json file; it does not change the process-wide working directory.

@mertcanaltin this is wrong. you're also changing the cwd of the process that we are spawning.

To answer other reviewers: I think this solution has lots of flaws. It doesn't properly propagate path variable (according to the given --run-from), and makes lots of unnecessary system calls.

[mertcanaltin]

only determines the directory where the --run command finds the package.json file; it does not change the process-wide working directory.

@mertcanaltin this is wrong. you're also changing the cwd of the process that we are spawning.

To answer other reviewers: I think this solution has lots of flaws. It doesn't properly propagate path variable (according to the given --run-from), and makes lots of unnecessary system calls.

now only the cwd of the spawned process (options_.cwd) is set, without changing the process-wide cwd. Also, if the file path is given for --run-from, the parent directory is used. I hope this addresses your concerns.

[mertcanaltin]

@anonrig I tried to process the path given with --run-from and add the node_modules/.bin directory under the cwd to the PATH Without changing the cwd of the parent process, only the cwd of the spawned process will be set, is this a good solution ?