Skip to content

Commit aa50086

Browse files
anonrigtpoisseau
anonrig
authored and
tpoisseau
committed
cli: ensure --run has proper pwd
PR-URL: nodejs#54949 Refs: nodejs#53600 Reviewed-By: Matthew Aitken <[email protected]> Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
1 parent ac529d1 commit aa50086

File tree

9 files changed

+180
-36
lines changed

9 files changed

+180
-36
lines changed

‎doc/api/cli.md‎

Lines changed: 2 additions & 0 deletions
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

‎src/node_task_runner.cc‎

Lines 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;

‎src/node_task_runner.h‎

Lines 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);

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

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

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

Lines 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+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{}

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

Lines 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
}

‎test/message/node_run_non_existent.out‎

Lines 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

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

Lines 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)