1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-21 15:04:11 -05:00

feat(npm): support --allow-scripts on deno run (and deno add, deno test, etc) (#26075)

Fixes https://github.com/denoland/deno/issues/25533. Fixes
https://github.com/denoland/deno/issues/25396.

Previously we only supported it on `deno install` and `deno cache`,
which is annoying if you're using `nodeModulesDir: auto`.

Also changes from printing output of lifecycle scripts directly to
capturing the output and only printing it on error.
This commit is contained in:
Nathan Whitaker 2024-10-12 12:14:32 -07:00 committed by GitHub
parent 8b2c6fc2d2
commit 7a990d9d42
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
21 changed files with 235 additions and 106 deletions

View file

@ -1342,7 +1342,7 @@ pub fn flags_from_vec(args: Vec<OsString>) -> clap::error::Result<Flags> {
}
match subcommand.as_str() {
"add" => add_parse(&mut flags, &mut m),
"add" => add_parse(&mut flags, &mut m)?,
"remove" => remove_parse(&mut flags, &mut m),
"bench" => bench_parse(&mut flags, &mut m)?,
"bundle" => bundle_parse(&mut flags, &mut m),
@ -1675,6 +1675,7 @@ You can add multiple dependencies at once:
.action(ArgAction::Append),
)
.arg(add_dev_arg())
.arg(allow_scripts_arg())
})
}
@ -1717,7 +1718,7 @@ If you specify a directory instead of a file, the path is expanded to all contai
UnstableArgsConfig::ResolutionAndRuntime,
)
.defer(|cmd| {
runtime_args(cmd, true, false)
runtime_args(cmd, true, false, true)
.arg(check_arg(true))
.arg(
Arg::new("json")
@ -1881,7 +1882,7 @@ On the first invocation with deno will download the proper binary and cache it i
UnstableArgsConfig::ResolutionAndRuntime,
)
.defer(|cmd| {
runtime_args(cmd, true, false)
runtime_args(cmd, true, false, true)
.arg(check_arg(true))
.arg(
Arg::new("include")
@ -2202,7 +2203,7 @@ This command has implicit access to all permissions.
UnstableArgsConfig::ResolutionAndRuntime,
)
.defer(|cmd| {
runtime_args(cmd, false, true)
runtime_args(cmd, false, true, true)
.arg(check_arg(false))
.arg(executable_ext_arg())
.arg(
@ -2501,7 +2502,7 @@ The installation root is determined, in order of precedence:
These must be added to the path manually if required."), UnstableArgsConfig::ResolutionAndRuntime)
.visible_alias("i")
.defer(|cmd| {
permission_args(runtime_args(cmd, false, true), Some("global"))
permission_args(runtime_args(cmd, false, true, false), Some("global"))
.arg(check_arg(true))
.arg(allow_scripts_arg())
.arg(
@ -2767,7 +2768,7 @@ It is especially useful for quick prototyping and checking snippets of code.
TypeScript is supported, however it is not type-checked, only transpiled."
), UnstableArgsConfig::ResolutionAndRuntime)
.defer(|cmd| runtime_args(cmd, true, true)
.defer(|cmd| runtime_args(cmd, true, true, true)
.arg(check_arg(false))
.arg(
Arg::new("eval-file")
@ -2799,7 +2800,7 @@ TypeScript is supported, however it is not type-checked, only transpiled."
}
fn run_args(command: Command, top_level: bool) -> Command {
runtime_args(command, true, true)
runtime_args(command, true, true, true)
.arg(check_arg(false))
.arg(watch_arg(true))
.arg(hmr_arg(true))
@ -2855,7 +2856,7 @@ Start a server defined in server.ts:
Start a server defined in server.ts, watching for changes and running on port 5050:
<p(245)>deno serve --watch --port 5050 server.ts</>
<y>Read more:</> <c>https://docs.deno.com/go/serve</>"), UnstableArgsConfig::ResolutionAndRuntime), true, true)
<y>Read more:</> <c>https://docs.deno.com/go/serve</>"), UnstableArgsConfig::ResolutionAndRuntime), true, true, true)
.arg(
Arg::new("port")
.long("port")
@ -2929,7 +2930,7 @@ or <c>**/__tests__/**</>:
UnstableArgsConfig::ResolutionAndRuntime
)
.defer(|cmd|
runtime_args(cmd, true, true)
runtime_args(cmd, true, true, true)
.arg(check_arg(true))
.arg(
Arg::new("ignore")
@ -3642,6 +3643,7 @@ fn runtime_args(
app: Command,
include_perms: bool,
include_inspector: bool,
include_allow_scripts: bool,
) -> Command {
let app = compile_args(app);
let app = if include_perms {
@ -3654,6 +3656,11 @@ fn runtime_args(
} else {
app
};
let app = if include_allow_scripts {
app.arg(allow_scripts_arg())
} else {
app
};
app
.arg(frozen_lockfile_arg())
.arg(cached_only_arg())
@ -4235,8 +4242,13 @@ fn allow_scripts_arg_parse(
Ok(())
}
fn add_parse(flags: &mut Flags, matches: &mut ArgMatches) {
fn add_parse(
flags: &mut Flags,
matches: &mut ArgMatches,
) -> clap::error::Result<()> {
allow_scripts_arg_parse(flags, matches)?;
flags.subcommand = DenoSubcommand::Add(add_parse_inner(matches, None));
Ok(())
}
fn add_parse_inner(
@ -4262,7 +4274,7 @@ fn bench_parse(
) -> clap::error::Result<()> {
flags.type_check_mode = TypeCheckMode::Local;
runtime_args_parse(flags, matches, true, false)?;
runtime_args_parse(flags, matches, true, false, true)?;
ext_arg_parse(flags, matches);
// NOTE: `deno bench` always uses `--no-prompt`, tests shouldn't ever do
@ -4352,7 +4364,7 @@ fn compile_parse(
matches: &mut ArgMatches,
) -> clap::error::Result<()> {
flags.type_check_mode = TypeCheckMode::Local;
runtime_args_parse(flags, matches, true, false)?;
runtime_args_parse(flags, matches, true, false, true)?;
let mut script = matches.remove_many::<String>("script_arg").unwrap();
let source_file = script.next().unwrap();
@ -4527,7 +4539,7 @@ fn eval_parse(
flags: &mut Flags,
matches: &mut ArgMatches,
) -> clap::error::Result<()> {
runtime_args_parse(flags, matches, false, true)?;
runtime_args_parse(flags, matches, false, true, false)?;
unstable_args_parse(flags, matches, UnstableArgsConfig::ResolutionAndRuntime);
flags.allow_all();
@ -4620,7 +4632,7 @@ fn install_parse(
flags: &mut Flags,
matches: &mut ArgMatches,
) -> clap::error::Result<()> {
runtime_args_parse(flags, matches, true, true)?;
runtime_args_parse(flags, matches, true, true, false)?;
let global = matches.get_flag("global");
if global {
@ -4846,7 +4858,7 @@ fn repl_parse(
flags: &mut Flags,
matches: &mut ArgMatches,
) -> clap::error::Result<()> {
runtime_args_parse(flags, matches, true, true)?;
runtime_args_parse(flags, matches, true, true, true)?;
unsafely_ignore_certificate_errors_parse(flags, matches);
let eval_files = matches
@ -4879,7 +4891,7 @@ fn run_parse(
mut app: Command,
bare: bool,
) -> clap::error::Result<()> {
runtime_args_parse(flags, matches, true, true)?;
runtime_args_parse(flags, matches, true, true, true)?;
ext_arg_parse(flags, matches);
flags.code_cache_enabled = !matches.get_flag("no-code-cache");
@ -4920,7 +4932,7 @@ fn serve_parse(
let worker_count = parallel_arg_parse(matches).map(|v| v.get());
runtime_args_parse(flags, matches, true, true)?;
runtime_args_parse(flags, matches, true, true, true)?;
// If the user didn't pass --allow-net, add this port to the network
// allowlist. If the host is 0.0.0.0, we add :{port} and allow the same network perms
// as if it was passed to --allow-net directly.
@ -5015,7 +5027,7 @@ fn test_parse(
matches: &mut ArgMatches,
) -> clap::error::Result<()> {
flags.type_check_mode = TypeCheckMode::Local;
runtime_args_parse(flags, matches, true, true)?;
runtime_args_parse(flags, matches, true, true, true)?;
ext_arg_parse(flags, matches);
// NOTE: `deno test` always uses `--no-prompt`, tests shouldn't ever do
@ -5380,6 +5392,7 @@ fn runtime_args_parse(
matches: &mut ArgMatches,
include_perms: bool,
include_inspector: bool,
include_allow_scripts: bool,
) -> clap::error::Result<()> {
unstable_args_parse(flags, matches, UnstableArgsConfig::ResolutionAndRuntime);
compile_args_parse(flags, matches)?;
@ -5391,6 +5404,9 @@ fn runtime_args_parse(
if include_inspector {
inspect_arg_parse(flags, matches);
}
if include_allow_scripts {
allow_scripts_arg_parse(flags, matches)?;
}
location_arg_parse(flags, matches);
v8_flags_arg_parse(flags, matches);
seed_arg_parse(flags, matches);
@ -8862,8 +8878,12 @@ mod tests {
#[test]
fn test_no_colon_in_value_name() {
let app =
runtime_args(Command::new("test_inspect_completion_value"), true, true);
let app = runtime_args(
Command::new("test_inspect_completion_value"),
true,
true,
false,
);
let inspect_args = app
.get_arguments()
.filter(|arg| arg.get_id() == "inspect")

View file

@ -2,6 +2,8 @@
use super::bin_entries::BinEntries;
use crate::args::LifecycleScriptsConfig;
use crate::task_runner::TaskStdio;
use crate::util::progress_bar::ProgressBar;
use deno_core::anyhow::Context;
use deno_npm::resolution::NpmResolutionSnapshot;
use deno_runtime::deno_io::FromRawIoHandle;
@ -148,6 +150,7 @@ impl<'a> LifecycleScripts<'a> {
snapshot: &NpmResolutionSnapshot,
packages: &[NpmResolutionPackage],
root_node_modules_dir_path: Option<&Path>,
progress_bar: &ProgressBar,
) -> Result<(), AnyError> {
self.warn_not_run_scripts()?;
let get_package_path =
@ -201,7 +204,15 @@ impl<'a> LifecycleScripts<'a> {
{
continue;
}
let exit_code = crate::task_runner::run_task(
let _guard = progress_bar.update_with_prompt(
crate::util::progress_bar::ProgressMessagePrompt::Initialize,
&format!("{}: running '{script_name}' script", package.id.nv),
);
let crate::task_runner::TaskResult {
exit_code,
stderr,
stdout,
} = crate::task_runner::run_task(
crate::task_runner::RunTaskOptions {
task_name: script_name,
script,
@ -211,15 +222,37 @@ impl<'a> LifecycleScripts<'a> {
init_cwd,
argv: &[],
root_node_modules_dir: root_node_modules_dir_path,
stdio: Some(crate::task_runner::TaskIo {
stderr: TaskStdio::piped(),
stdout: TaskStdio::piped(),
}),
},
)
.await?;
let stdout = stdout.unwrap();
let stderr = stderr.unwrap();
if exit_code != 0 {
log::warn!(
"error: script '{}' in '{}' failed with exit code {}",
"error: script '{}' in '{}' failed with exit code {}{}{}",
script_name,
package.id.nv,
exit_code,
if !stdout.trim_ascii().is_empty() {
format!(
"\nstdout:\n{}\n",
String::from_utf8_lossy(&stdout).trim()
)
} else {
String::new()
},
if !stderr.trim_ascii().is_empty() {
format!(
"\nstderr:\n{}\n",
String::from_utf8_lossy(&stderr).trim()
)
} else {
String::new()
},
);
failed_packages.push(&package.id.nv);
// assume if earlier script fails, later ones will fail too

View file

@ -713,6 +713,7 @@ async fn sync_resolution_with_fs(
snapshot,
&package_partitions.packages,
Some(root_node_modules_dir_path),
progress_bar,
)
.await?;

View file

@ -16,8 +16,11 @@ use deno_task_shell::ExecutableCommand;
use deno_task_shell::ExecuteResult;
use deno_task_shell::ShellCommand;
use deno_task_shell::ShellCommandContext;
use deno_task_shell::ShellPipeReader;
use deno_task_shell::ShellPipeWriter;
use lazy_regex::Lazy;
use regex::Regex;
use tokio::task::JoinHandle;
use tokio::task::LocalSet;
use crate::npm::CliNpmResolver;
@ -36,6 +39,35 @@ pub fn get_script_with_args(script: &str, argv: &[String]) -> String {
script.trim().to_owned()
}
pub struct TaskStdio(Option<ShellPipeReader>, ShellPipeWriter);
impl TaskStdio {
pub fn stdout() -> Self {
Self(None, ShellPipeWriter::stdout())
}
pub fn stderr() -> Self {
Self(None, ShellPipeWriter::stderr())
}
pub fn piped() -> Self {
let (r, w) = deno_task_shell::pipe();
Self(Some(r), w)
}
}
pub struct TaskIo {
pub stdout: TaskStdio,
pub stderr: TaskStdio,
}
impl Default for TaskIo {
fn default() -> Self {
Self {
stderr: TaskStdio::stderr(),
stdout: TaskStdio::stdout(),
}
}
}
pub struct RunTaskOptions<'a> {
pub task_name: &'a str,
pub script: &'a str,
@ -45,24 +77,69 @@ pub struct RunTaskOptions<'a> {
pub argv: &'a [String],
pub custom_commands: HashMap<String, Rc<dyn ShellCommand>>,
pub root_node_modules_dir: Option<&'a Path>,
pub stdio: Option<TaskIo>,
}
pub type TaskCustomCommands = HashMap<String, Rc<dyn ShellCommand>>;
pub async fn run_task(opts: RunTaskOptions<'_>) -> Result<i32, AnyError> {
pub struct TaskResult {
pub exit_code: i32,
pub stdout: Option<Vec<u8>>,
pub stderr: Option<Vec<u8>>,
}
pub async fn run_task(
opts: RunTaskOptions<'_>,
) -> Result<TaskResult, AnyError> {
let script = get_script_with_args(opts.script, opts.argv);
let seq_list = deno_task_shell::parser::parse(&script)
.with_context(|| format!("Error parsing script '{}'.", opts.task_name))?;
let env_vars =
prepare_env_vars(opts.env_vars, opts.init_cwd, opts.root_node_modules_dir);
let state =
deno_task_shell::ShellState::new(env_vars, opts.cwd, opts.custom_commands);
let stdio = opts.stdio.unwrap_or_default();
let (
TaskStdio(stdout_read, stdout_write),
TaskStdio(stderr_read, stderr_write),
) = (stdio.stdout, stdio.stderr);
fn read(reader: ShellPipeReader) -> JoinHandle<Result<Vec<u8>, AnyError>> {
tokio::task::spawn_blocking(move || {
let mut buf = Vec::new();
reader.pipe_to(&mut buf)?;
Ok(buf)
})
}
let stdout = stdout_read.map(read);
let stderr = stderr_read.map(read);
let local = LocalSet::new();
let future = deno_task_shell::execute(
let future = async move {
let exit_code = deno_task_shell::execute_with_pipes(
seq_list,
env_vars,
opts.cwd,
opts.custom_commands,
);
Ok(local.run_until(future).await)
state,
ShellPipeReader::stdin(),
stdout_write,
stderr_write,
)
.await;
Ok::<_, AnyError>(TaskResult {
exit_code,
stdout: if let Some(stdout) = stdout {
Some(stdout.await??)
} else {
None
},
stderr: if let Some(stderr) = stderr {
Some(stderr.await??)
} else {
None
},
})
};
local.run_until(future).await
}
fn prepare_env_vars(

View file

@ -182,6 +182,7 @@ async fn run_task(opts: RunTaskOptions<'_>) -> Result<i32, AnyError> {
&task_runner::get_script_with_args(script, cli_options.argv()),
);
Ok(
task_runner::run_task(task_runner::RunTaskOptions {
task_name,
script,
@ -191,8 +192,11 @@ async fn run_task(opts: RunTaskOptions<'_>) -> Result<i32, AnyError> {
init_cwd: opts.cli_options.initial_cwd(),
argv: cli_options.argv(),
root_node_modules_dir: npm_resolver.root_node_modules_path(),
stdio: None,
})
.await
.await?
.exit_code,
)
}
fn output_task(task_name: &str, script: &str) {

View file

@ -1,5 +1,3 @@
modules.export = {
module.exports = {
value: 42
};
console.log('index.js', modules.export.value);

View file

@ -6,6 +6,7 @@
"install": "echo install && cli-esm 'hello from install script'",
"postinstall": "echo postinstall && npx cowsay postinstall"
},
"exports": "./index.js",
"dependencies": {
"@denotest/bin": "1.0.0"
}

View file

@ -1,2 +1,5 @@
import { sayHello } from "./index.js";
console.log(sayHello());
import path from "node:path";
import fs from "node:fs";
fs.writeSync(fs.openSync(path.join(process.env.INIT_CWD, "say-hello-output.txt"), "w"), sayHello());

View file

@ -5,18 +5,7 @@ Download http://localhost:4260/@denotest/node-lifecycle-scripts/1.0.0.tgz
Download http://localhost:4260/@denotest/bin/1.0.0.tgz
Initialize @denotest/node-lifecycle-scripts@1.0.0
Initialize @denotest/bin@1.0.0
Initialize @denotest/node-lifecycle-scripts@1.0.0: running 'preinstall' script
Initialize @denotest/node-lifecycle-scripts@1.0.0: running 'install' script
Initialize @denotest/node-lifecycle-scripts@1.0.0: running 'postinstall' script
[UNORDERED_END]
preinstall
deno preinstall.js
node preinstall.js
install
hello from install script
postinstall[WILDCARD]
_____________
< postinstall >
-------------
\ ^__^
\ (oo)\_______
(__)\ )\/\
||----w |
|| ||

View file

@ -5,7 +5,7 @@
"steps": [
{
"args": "cache --allow-scripts=npm:@denotest/node-addon main.js",
"output": "[WILDCARD]gyp info ok \n"
"output": "[WILDCARD]Initialize @denotest/node-addon@1.0.0: running 'install' script\n"
},
{
"args": "run -A main.js",
@ -38,7 +38,7 @@
"steps": [
{
// without running scripts (should warn)
"args": "cache all_lifecycles.js",
"args": "run all_lifecycles.js",
"output": "all_lifecycles_not_run.out"
},
{
@ -51,6 +51,23 @@
}
]
},
"deno_run_lifecycle_scripts": {
"steps": [
{
// without running scripts (should warn)
"args": "run all_lifecycles.js",
"output": "all_lifecycles_not_run.out"
},
{
// now run scripts
"args": "run --allow-scripts all_lifecycles.js",
// this test package covers running preinstall, install, and postinstall scripts
// it also exercises running bin packages (esbuild in this case), using `node` in a script
// (with and without node-only CLI flags), and using `npx` in a script
"output": "all_lifecycles_deno_run.out"
}
]
},
"global_lifecycle_scripts": {
"steps": [
{
@ -79,14 +96,12 @@
{
// without running scripts (should warn)
"args": "run all_lifecycles.js",
"output": "only_warns_first1.out",
"exitCode": 1
"output": "only_warns_first1.out"
},
{
// without running scripts (does not warn)
"args": "run all_lifecycles.js",
"output": "only_warns_first2.out",
"exitCode": 1
"output": "only_warns_first2.out"
},
{
// should warn because this is an explicit install
@ -128,6 +143,13 @@
// we run the install script, we should use the correct binary (relative to the package)
"args": "cache --allow-scripts conflicting_bin.js",
"output": "conflicting_bin.out"
},
{
"args": [
"eval",
"console.log(Deno.readTextFileSync('./say-hello-output.txt'))"
],
"output": "conflicting_bin2.out"
}
]
},

View file

@ -1,14 +1,3 @@
preinstall
deno preinstall.js
node preinstall.js
install
hello from install script
postinstall[WILDCARD]
_____________
< postinstall >
-------------
\ ^__^
\ (oo)\_______
(__)\ )\/\
||----w |
|| ||
Initialize @denotest/node-lifecycle-scripts@1.0.0: running 'preinstall' script
Initialize @denotest/node-lifecycle-scripts@1.0.0: running 'install' script
Initialize @denotest/node-lifecycle-scripts@1.0.0: running 'postinstall' script

View file

@ -0,0 +1,4 @@
Initialize @denotest/node-lifecycle-scripts@1.0.0: running 'preinstall' script
Initialize @denotest/node-lifecycle-scripts@1.0.0: running 'install' script
Initialize @denotest/node-lifecycle-scripts@1.0.0: running 'postinstall' script
value is 42

View file

@ -12,3 +12,4 @@ Warning The following packages contained npm lifecycle scripts (preinstall/insta
┠─ This may cause the packages to not work correctly.
┖─ To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:
deno install --allow-scripts=npm:@denotest/node-lifecycle-scripts@1.0.0
value is 42

View file

@ -8,6 +8,5 @@ Download http://localhost:4260/@denotest/say-hello/1.0.0.tgz
Initialize @denotest/better-say-hello@1.0.0
Initialize @denotest/say-hello-on-install@1.0.0
Initialize @denotest/say-hello@1.0.0
Initialize @denotest/say-hello-on-install@1.0.0: running 'install' script
[UNORDERED_END]
install script
@denotest/say-hello says hello!

View file

@ -0,0 +1 @@
@denotest/say-hello says hello!

View file

@ -1,15 +1,3 @@
preinstall
deno preinstall.js
node preinstall.js
install
hello from install script
postinstall
[WILDCARD]
_____________
< postinstall >
-------------
\ ^__^
\ (oo)\_______
(__)\ )\/\
||----w |
|| ||
Initialize @denotest/node-lifecycle-scripts@1.0.0: running 'preinstall' script
Initialize @denotest/node-lifecycle-scripts@1.0.0: running 'install' script
Initialize @denotest/node-lifecycle-scripts@1.0.0: running 'postinstall' script

View file

@ -5,7 +5,6 @@ Download http://localhost:4260/@denotest/lifecycle-scripts-cjs/1.0.0.tgz
Download http://localhost:4260/@denotest/bin/1.0.0.tgz
Initialize @denotest/lifecycle-scripts-cjs@1.0.0
Initialize @denotest/bin@1.0.0
Initialize @denotest/lifecycle-scripts-cjs@1.0.0: running 'preinstall' script
Initialize @denotest/lifecycle-scripts-cjs@1.0.0: running 'install' script
[UNORDERED_END]
preinstall
install
hello from install script

View file

@ -2,8 +2,11 @@
Download http://localhost:4260/@denotest/node-addon-implicit-node-gyp
Download http://localhost:4260/@denotest/node-addon-implicit-node-gyp/1.0.0.tgz
Initialize @denotest/node-addon-implicit-node-gyp@1.0.0
Initialize @denotest/node-addon-implicit-node-gyp@1.0.0: running 'install' script
[UNORDERED_END]
Warning node-gyp was used in a script, but was not listed as a dependency. Either add it as a dependency or install it globally (e.g. `npm install -g node-gyp`)
[WILDCARD]
error: script 'install' in '@denotest/node-addon-implicit-node-gyp@1.0.0' failed with exit code 1
stderr:
Error launching 'node-gyp': [WILDCARD]
error: failed to run scripts for packages: @denotest/node-addon-implicit-node-gyp@1.0.0

View file

@ -12,5 +12,4 @@ Warning The following packages contained npm lifecycle scripts (preinstall/insta
┠─ This may cause the packages to not work correctly.
┖─ To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:
deno install --allow-scripts=npm:@denotest/node-lifecycle-scripts@1.0.0
error: Uncaught SyntaxError: The requested module 'npm:@denotest/node-lifecycle-scripts' does not provide an export named 'value'
[WILDCARD]
value is 42

View file

@ -1,3 +1 @@
[# note no warning]
error: Uncaught SyntaxError: The requested module 'npm:@denotest/node-lifecycle-scripts' does not provide an export named 'value'
[WILDCARD]
value is 42