Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 048a1ab

Browse filesBrowse files
anonrigtargos
anonrig
authored and
targos
committed
cli: ensure --run has proper pwd
PR-URL: #54949 Refs: #53600 Reviewed-By: Matthew Aitken <maitken033380023@gmail.com> Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
1 parent 90cce6e commit 048a1ab
Copy full SHA for 048a1ab

File tree

Expand file treeCollapse file tree

9 files changed

+180
-36
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

9 files changed

+180
-36
lines changed
Open diff view settings
Collapse file

‎doc/api/cli.md‎

Copy file name to clipboardExpand all lines: doc/api/cli.md
+2Lines changed: 2 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -2110,6 +2110,8 @@ the current directory, to the `PATH` in order to execute the binaries from
21102110
different folders where multiple `node_modules` directories are present, if
21112111
`ancestor-folder/node_modules/.bin` is a directory.
21122112

2113+
`--run` executes the command in the directory containing the related `package.json`.
2114+
21132115
For example, the following command will run the `test` script of
21142116
the `package.json` in the current folder:
21152117

Collapse file

‎src/node_task_runner.cc‎

Copy file name to clipboardExpand all lines: src/node_task_runner.cc
+55-29Lines changed: 55 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ std::string EscapeShell(const std::string_view input) {
149149
#endif
150150
}
151151

152-
static const std::string_view forbidden_characters =
152+
static constexpr std::string_view forbidden_characters =
153153
"[\t\n\r \"#$&'()*;<>?\\\\`|~]";
154154

155155
// Check if input contains any forbidden characters
@@ -191,7 +191,7 @@ std::string EscapeShell(const std::string_view input) {
191191
void ProcessRunner::ExitCallback(uv_process_t* handle,
192192
int64_t exit_status,
193193
int term_signal) {
194-
auto self = reinterpret_cast<ProcessRunner*>(handle->data);
194+
const auto self = static_cast<ProcessRunner*>(handle->data);
195195
uv_close(reinterpret_cast<uv_handle_t*>(handle), nullptr);
196196
self->OnExit(exit_status, term_signal);
197197
}
@@ -205,6 +205,9 @@ void ProcessRunner::OnExit(int64_t exit_status, int term_signal) {
205205
}
206206

207207
void ProcessRunner::Run() {
208+
// keeps the string alive until destructor
209+
cwd = package_json_path_.parent_path().string();
210+
options_.cwd = cwd.c_str();
208211
if (int r = uv_spawn(loop_, &process_, &options_)) {
209212
fprintf(stderr, "Error: %s\n", uv_strerror(r));
210213
}
@@ -246,14 +249,16 @@ FindPackageJson(const std::filesystem::path& cwd) {
246249
return {{package_json_path, raw_content, path_env_var}};
247250
}
248251

249-
void RunTask(std::shared_ptr<InitializationResultImpl> result,
252+
void RunTask(const std::shared_ptr<InitializationResultImpl>& result,
250253
std::string_view command_id,
251254
const std::vector<std::string_view>& positional_args) {
252255
auto cwd = std::filesystem::current_path();
253256
auto package_json = FindPackageJson(cwd);
254257

255258
if (!package_json.has_value()) {
256-
fprintf(stderr, "Can't read package.json\n");
259+
fprintf(stderr,
260+
"Can't find package.json for directory %s\n",
261+
cwd.string().c_str());
257262
result->exit_code_ = ExitCode::kGenericUserError;
258263
return;
259264
}
@@ -267,46 +272,67 @@ void RunTask(std::shared_ptr<InitializationResultImpl> result,
267272
simdjson::ondemand::parser json_parser;
268273
simdjson::ondemand::document document;
269274
simdjson::ondemand::object main_object;
270-
simdjson::error_code error = json_parser.iterate(raw_json).get(document);
271275

276+
if (json_parser.iterate(raw_json).get(document)) {
277+
fprintf(stderr, "Can't parse %s\n", path.string().c_str());
278+
result->exit_code_ = ExitCode::kGenericUserError;
279+
return;
280+
}
272281
// If document is not an object, throw an error.
273-
if (error || document.get_object().get(main_object)) {
274-
fprintf(stderr, "Can't parse package.json\n");
282+
if (auto root_error = document.get_object().get(main_object)) {
283+
if (root_error == simdjson::error_code::INCORRECT_TYPE) {
284+
fprintf(stderr,
285+
"Root value unexpected not an object for %s\n\n",
286+
path.string().c_str());
287+
} else {
288+
fprintf(stderr, "Can't parse %s\n", path.string().c_str());
289+
}
275290
result->exit_code_ = ExitCode::kGenericUserError;
276291
return;
277292
}
278293

279294
// If package_json object doesn't have "scripts" field, throw an error.
280295
simdjson::ondemand::object scripts_object;
281296
if (main_object["scripts"].get_object().get(scripts_object)) {
282-
fprintf(stderr, "Can't find \"scripts\" field in package.json\n");
297+
fprintf(
298+
stderr, "Can't find \"scripts\" field in %s\n", path.string().c_str());
283299
result->exit_code_ = ExitCode::kGenericUserError;
284300
return;
285301
}
286302

287303
// If the command_id is not found in the scripts object, throw an error.
288304
std::string_view command;
289-
if (scripts_object[command_id].get_string().get(command)) {
290-
fprintf(stderr,
291-
"Missing script: \"%.*s\"\n\n",
292-
static_cast<int>(command_id.size()),
293-
command_id.data());
294-
fprintf(stderr, "Available scripts are:\n");
295-
296-
// Reset the object to iterate over it again
297-
scripts_object.reset();
298-
simdjson::ondemand::value value;
299-
for (auto field : scripts_object) {
300-
std::string_view key_str;
301-
std::string_view value_str;
302-
if (!field.unescaped_key().get(key_str) && !field.value().get(value) &&
303-
!value.get_string().get(value_str)) {
304-
fprintf(stderr,
305-
" %.*s: %.*s\n",
306-
static_cast<int>(key_str.size()),
307-
key_str.data(),
308-
static_cast<int>(value_str.size()),
309-
value_str.data());
305+
if (auto command_error =
306+
scripts_object[command_id].get_string().get(command)) {
307+
if (command_error == simdjson::error_code::INCORRECT_TYPE) {
308+
fprintf(stderr,
309+
"Script \"%.*s\" is unexpectedly not a string for %s\n\n",
310+
static_cast<int>(command_id.size()),
311+
command_id.data(),
312+
path.string().c_str());
313+
} else {
314+
fprintf(stderr,
315+
"Missing script: \"%.*s\" for %s\n\n",
316+
static_cast<int>(command_id.size()),
317+
command_id.data(),
318+
path.string().c_str());
319+
fprintf(stderr, "Available scripts are:\n");
320+
321+
// Reset the object to iterate over it again
322+
scripts_object.reset();
323+
simdjson::ondemand::value value;
324+
for (auto field : scripts_object) {
325+
std::string_view key_str;
326+
std::string_view value_str;
327+
if (!field.unescaped_key().get(key_str) && !field.value().get(value) &&
328+
!value.get_string().get(value_str)) {
329+
fprintf(stderr,
330+
" %.*s: %.*s\n",
331+
static_cast<int>(key_str.size()),
332+
key_str.data(),
333+
static_cast<int>(value_str.size()),
334+
value_str.data());
335+
}
310336
}
311337
}
312338
result->exit_code_ = ExitCode::kGenericUserError;
Collapse file

‎src/node_task_runner.h‎

Copy file name to clipboardExpand all lines: src/node_task_runner.h
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ class ProcessRunner {
4444
std::vector<std::string> env_vars_{};
4545
std::unique_ptr<char* []> env {}; // memory for options_.env
4646
std::unique_ptr<char* []> arg {}; // memory for options_.args
47+
std::string cwd;
4748

4849
// OnExit is the callback function that is called when the process exits.
4950
void OnExit(int64_t exit_status, int term_signal);
@@ -78,7 +79,7 @@ class ProcessRunner {
7879
std::optional<std::tuple<std::filesystem::path, std::string, std::string>>
7980
FindPackageJson(const std::filesystem::path& cwd);
8081

81-
void RunTask(std::shared_ptr<InitializationResultImpl> result,
82+
void RunTask(const std::shared_ptr<InitializationResultImpl>& result,
8283
std::string_view command_id,
8384
const PositionalArgs& positional_args);
8485
PositionalArgs GetPositionalArgs(const std::vector<std::string>& args);
Collapse file

‎test/fixtures/run-script/invalid-json/package.json‎

Copy file name to clipboard
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"scripts": {},
3+
}
Collapse file

‎test/fixtures/run-script/invalid-schema/package.json‎

Copy file name to clipboard
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"scripts": {
3+
"array": [],
4+
"boolean": true,
5+
"null": null,
6+
"number": 1.0,
7+
"object": {}
8+
}
9+
}
Collapse file

‎test/fixtures/run-script/missing-scripts/package.json‎

Copy file name to clipboard
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{}
Collapse file

‎test/fixtures/run-script/package.json‎

Copy file name to clipboardExpand all lines: test/fixtures/run-script/package.json
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
"path-env": "path-env",
1111
"path-env-windows": "path-env.bat",
1212
"special-env-variables": "special-env-variables",
13-
"special-env-variables-windows": "special-env-variables.bat"
13+
"special-env-variables-windows": "special-env-variables.bat",
14+
"pwd": "pwd",
15+
"pwd-windows": "cd"
1416
}
1517
}
Collapse file

‎test/message/node_run_non_existent.out‎

Copy file name to clipboardExpand all lines: test/message/node_run_non_existent.out
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
Missing script: "non-existent-command"
1+
Missing script: "non-existent-command" for *
22

33
Available scripts are:
44
test: echo "Error: no test specified" && exit 1
@@ -12,3 +12,5 @@ Available scripts are:
1212
path-env-windows: path-env.bat
1313
special-env-variables: special-env-variables
1414
special-env-variables-windows: special-env-variables.bat
15+
pwd: pwd
16+
pwd-windows: cd
Collapse file

‎test/parallel/test-node-run.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-node-run.js
+102-4Lines changed: 102 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
'use strict';
22

33
const common = require('../common');
4+
common.requireNoPackageJSONAbove();
5+
46
const { it, describe } = require('node:test');
57
const assert = require('node:assert');
68

@@ -15,7 +17,6 @@ describe('node --run [command]', () => {
1517
{ cwd: __dirname },
1618
);
1719
assert.match(child.stderr, /ExperimentalWarning: Task runner is an experimental feature and might change at any time/);
18-
assert.match(child.stderr, /Can't read package\.json/);
1920
assert.strictEqual(child.stdout, '');
2021
assert.strictEqual(child.code, 1);
2122
});
@@ -26,7 +27,9 @@ describe('node --run [command]', () => {
2627
[ '--no-warnings', '--run', 'test'],
2728
{ cwd: __dirname },
2829
);
29-
assert.match(child.stderr, /Can't read package\.json/);
30+
assert.match(child.stderr, /Can't find package\.json[\s\S]*/);
31+
// Ensure we show the path that starting path for the search
32+
assert(child.stderr.includes(__dirname));
3033
assert.strictEqual(child.stdout, '');
3134
assert.strictEqual(child.code, 1);
3235
});
@@ -53,6 +56,101 @@ describe('node --run [command]', () => {
5356
assert.strictEqual(child.code, 0);
5457
});
5558

59+
it('chdirs into package directory', async () => {
60+
const child = await common.spawnPromisified(
61+
process.execPath,
62+
[ '--no-warnings', '--run', `pwd${envSuffix}`],
63+
{ cwd: fixtures.path('run-script/sub-directory') },
64+
);
65+
assert.strictEqual(child.stdout.trim(), fixtures.path('run-script'));
66+
assert.strictEqual(child.stderr, '');
67+
assert.strictEqual(child.code, 0);
68+
});
69+
70+
it('includes actionable info when possible', async () => {
71+
{
72+
const child = await common.spawnPromisified(
73+
process.execPath,
74+
[ '--no-warnings', '--run', 'missing'],
75+
{ cwd: fixtures.path('run-script') },
76+
);
77+
assert.strictEqual(child.stdout, '');
78+
assert(child.stderr.includes(fixtures.path('run-script/package.json')));
79+
assert(child.stderr.includes('no test specified'));
80+
assert.strictEqual(child.code, 1);
81+
}
82+
{
83+
const child = await common.spawnPromisified(
84+
process.execPath,
85+
[ '--no-warnings', '--run', 'test'],
86+
{ cwd: fixtures.path('run-script/missing-scripts') },
87+
);
88+
assert.strictEqual(child.stdout, '');
89+
assert(child.stderr.includes(fixtures.path('run-script/missing-scripts/package.json')));
90+
assert.strictEqual(child.code, 1);
91+
}
92+
{
93+
const child = await common.spawnPromisified(
94+
process.execPath,
95+
[ '--no-warnings', '--run', 'test'],
96+
{ cwd: fixtures.path('run-script/invalid-json') },
97+
);
98+
assert.strictEqual(child.stdout, '');
99+
assert(child.stderr.includes(fixtures.path('run-script/invalid-json/package.json')));
100+
assert.strictEqual(child.code, 1);
101+
}
102+
{
103+
const child = await common.spawnPromisified(
104+
process.execPath,
105+
[ '--no-warnings', '--run', 'array'],
106+
{ cwd: fixtures.path('run-script/invalid-schema') },
107+
);
108+
assert.strictEqual(child.stdout, '');
109+
assert(child.stderr.includes(fixtures.path('run-script/invalid-schema/package.json')));
110+
assert.strictEqual(child.code, 1);
111+
}
112+
{
113+
const child = await common.spawnPromisified(
114+
process.execPath,
115+
[ '--no-warnings', '--run', 'boolean'],
116+
{ cwd: fixtures.path('run-script/invalid-schema') },
117+
);
118+
assert.strictEqual(child.stdout, '');
119+
assert(child.stderr.includes(fixtures.path('run-script/invalid-schema/package.json')));
120+
assert.strictEqual(child.code, 1);
121+
}
122+
{
123+
const child = await common.spawnPromisified(
124+
process.execPath,
125+
[ '--no-warnings', '--run', 'null'],
126+
{ cwd: fixtures.path('run-script/invalid-schema') },
127+
);
128+
assert.strictEqual(child.stdout, '');
129+
assert(child.stderr.includes(fixtures.path('run-script/invalid-schema/package.json')));
130+
assert.strictEqual(child.code, 1);
131+
}
132+
{
133+
const child = await common.spawnPromisified(
134+
process.execPath,
135+
[ '--no-warnings', '--run', 'number'],
136+
{ cwd: fixtures.path('run-script/invalid-schema') },
137+
);
138+
assert.strictEqual(child.stdout, '');
139+
assert(child.stderr.includes(fixtures.path('run-script/invalid-schema/package.json')));
140+
assert.strictEqual(child.code, 1);
141+
}
142+
{
143+
const child = await common.spawnPromisified(
144+
process.execPath,
145+
[ '--no-warnings', '--run', 'object'],
146+
{ cwd: fixtures.path('run-script/invalid-schema') },
147+
);
148+
assert.strictEqual(child.stdout, '');
149+
assert(child.stderr.includes(fixtures.path('run-script/invalid-schema/package.json')));
150+
assert.strictEqual(child.code, 1);
151+
}
152+
});
153+
56154
it('appends positional arguments', async () => {
57155
const child = await common.spawnPromisified(
58156
process.execPath,
@@ -120,7 +218,7 @@ describe('node --run [command]', () => {
120218
[ '--no-warnings', '--run', 'test'],
121219
{ cwd: fixtures.path('run-script/cannot-parse') },
122220
);
123-
assert.match(child.stderr, /Can't parse package\.json/);
221+
assert.match(child.stderr, /Can't parse/);
124222
assert.strictEqual(child.stdout, '');
125223
assert.strictEqual(child.code, 1);
126224
});
@@ -131,7 +229,7 @@ describe('node --run [command]', () => {
131229
[ '--no-warnings', '--run', 'test'],
132230
{ cwd: fixtures.path('run-script/cannot-find-script') },
133231
);
134-
assert.match(child.stderr, /Can't find "scripts" field in package\.json/);
232+
assert.match(child.stderr, /Can't find "scripts" field in/);
135233
assert.strictEqual(child.stdout, '');
136234
assert.strictEqual(child.code, 1);
137235
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.