Skip to content

Commit

Permalink
fix(task): regression where npx <command> sometimes couldn't find c…
Browse files Browse the repository at this point in the history
…ommand (denoland#23730)

Closes denoland#23724
  • Loading branch information
dsherret authored May 9, 2024
1 parent 47f7bed commit 263b6b9
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 9 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ deno_lockfile.workspace = true
deno_npm = "=0.18.0"
deno_runtime = { workspace = true, features = ["include_js_files_for_snapshotting"] }
deno_semver = "=0.5.4"
deno_task_shell = "=0.16.0"
deno_task_shell = "=0.16.1"
deno_terminal.workspace = true
eszip = "=0.68.2"
napi_sym.workspace = true
Expand Down
26 changes: 21 additions & 5 deletions cli/tools/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use deno_core::futures;
use deno_core::futures::future::LocalBoxFuture;
use deno_runtime::deno_node::NodeResolver;
use deno_semver::package::PackageNv;
use deno_task_shell::ExecutableCommand;
use deno_task_shell::ExecuteResult;
use deno_task_shell::ShellCommand;
use deno_task_shell::ShellCommandContext;
Expand Down Expand Up @@ -236,7 +237,15 @@ fn prepend_to_path(env_vars: &mut HashMap<String, String>, value: String) {

fn collect_env_vars() -> HashMap<String, String> {
// get the starting env vars (the PWD env var will be set by deno_task_shell)
let mut env_vars = std::env::vars().collect::<HashMap<String, String>>();
let mut env_vars = std::env::vars()
.map(|(k, v)| {
if cfg!(windows) {
(k.to_uppercase(), v)
} else {
(k, v)
}
})
.collect::<HashMap<String, String>>();
const INIT_CWD_NAME: &str = "INIT_CWD";
if !env_vars.contains_key(INIT_CWD_NAME) {
if let Ok(cwd) = std::env::current_dir() {
Expand Down Expand Up @@ -318,10 +327,17 @@ impl ShellCommand for NpxCommand {
};
command.execute(context)
} else {
let _ = context
.stderr
.write_line(&format!("npx: could not resolve command '{first_arg}'"));
Box::pin(futures::future::ready(ExecuteResult::from_exit_code(1)))
// can't find the command, so fallback to running the real npx command
let npx_path = match context.resolve_command_path("npx") {
Ok(npx) => npx,
Err(err) => {
let _ = context.stderr.write_line(&format!("{}", err));
return Box::pin(futures::future::ready(
ExecuteResult::from_exit_code(err.exit_code()),
));
}
};
ExecutableCommand::new("npx".to_string(), npx_path).execute(context)
}
} else {
let _ = context.stderr.write_line("npx: missing command");
Expand Down
11 changes: 11 additions & 0 deletions tests/specs/task/npx_installed_pkg_non_byonm/__test__.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"tempDir": true,
"steps": [{
"commandName": "npm",
"args": "install",
"output": "[WILDCARD]"
}, {
"args": "task say",
"output": "main.out"
}]
}
7 changes: 7 additions & 0 deletions tests/specs/task/npx_installed_pkg_non_byonm/deno.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
// not byonm
"nodeModulesDir": true,
"tasks": {
"say": "npx cowsay moo"
}
}
9 changes: 9 additions & 0 deletions tests/specs/task/npx_installed_pkg_non_byonm/main.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Task say npx cowsay moo
_____
< moo >
-----
\ ^__^
\ (oo)\_______
(__)\ )\/\
||----w |
|| ||
5 changes: 5 additions & 0 deletions tests/specs/task/npx_installed_pkg_non_byonm/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": {
"cowsay": "*"
}
}
4 changes: 3 additions & 1 deletion tests/testdata/task/npx/non_existent.out
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
Task non-existent npx this-command-should-not-exist-for-you
npx: could not resolve command 'this-command-should-not-exist-for-you'
npm ERR! code E404
npm ERR! 404 Not Found - GET https://localhost:4260/this-command-should-not-exist-for-you
[WILDCARD]

0 comments on commit 263b6b9

Please sign in to comment.