Skip to content

Commit

Permalink
feat(node): Support executing npm package lifecycle scripts (preinsta…
Browse files Browse the repository at this point in the history
…ll/install/postinstall) (#24487)

Adds support for running npm package lifecycle scripts, opted into via a
new `--allow-scripts` flag.

With this PR, when running `deno cache` (or `DENO_FUTURE=1 deno
install`) you can specify the `--allow-scripts=pkg1,pkg2` flag to run
lifecycle scripts attached to the given packages.

Note at the moment this only works when `nodeModulesDir` is true (using
the local resolver).

When a package with un-run lifecycle scripts is encountered, we emit a
warning suggesting things may not work and to try running lifecycle
scripts. Additionally, if a package script implicitly requires
`node-gyp` and it's not found on the system, we emit a warning.

Extra things in this PR:
- Extracted out bits of `task.rs` into a separate module for reuse
- Added a couple fields to `process.config` in order to support
`node-gyp` (it relies on a few variables being there)
- Drive by fix to downloading new npm packages to test registry

---

TODO:
- [x] validation for allow-scripts args (make sure it looks like an npm
package)
- [x] make allow-scripts matching smarter
- [ ] figure out what issues this closes

---
Review notes:
- This adds a bunch of deps to our test registry due to using
`node-gyp`, so it's pretty noisy
  • Loading branch information
nathanwhit committed Jul 10, 2024
1 parent eb46296 commit ce7dc2b
Show file tree
Hide file tree
Showing 207 changed files with 1,408 additions and 472 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ twox-hash = "=1.6.3"
url = { version = "< 2.5.0", features = ["serde", "expose_internals"] }
uuid = { version = "1.3.0", features = ["v4"] }
webpki-roots = "0.26"
which = "4.2.5"
zeromq = { version = "=0.3.4", default-features = false, features = ["tcp-transport", "tokio-runtime"] }
zstd = "=0.12.4"

Expand Down
1 change: 1 addition & 0 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ tower-lsp.workspace = true
twox-hash.workspace = true
typed-arena = "=2.0.1"
uuid = { workspace = true, features = ["serde"] }
which.workspace = true
zeromq.workspace = true
zstd.workspace = true

Expand Down
98 changes: 97 additions & 1 deletion cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,30 @@ pub enum CaData {
Bytes(Vec<u8>),
}

// Info needed to run NPM lifecycle scripts
#[derive(Clone, Debug, Eq, PartialEq, Default)]
pub struct LifecycleScriptsConfig {
pub allowed: PackagesAllowedScripts,
pub initial_cwd: Option<PathBuf>,
}

#[derive(Debug, Clone, Eq, PartialEq, Default)]
/// The set of npm packages that are allowed to run lifecycle scripts.
pub enum PackagesAllowedScripts {
All,
Some(Vec<String>),
#[default]
None,
}

fn parse_packages_allowed_scripts(s: &str) -> Result<String, AnyError> {
if !s.starts_with("npm:") {
bail!("Invalid package for --allow-scripts: '{}'. An 'npm:' specifier is required", s);
} else {
Ok(s.into())
}
}

#[derive(
Clone, Default, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize,
)]
Expand Down Expand Up @@ -562,6 +586,7 @@ pub struct Flags {
pub v8_flags: Vec<String>,
pub code_cache_enabled: bool,
pub permissions: PermissionFlags,
pub allow_scripts: PackagesAllowedScripts,
}

#[derive(Clone, Debug, Eq, PartialEq, Default, Serialize, Deserialize)]
Expand Down Expand Up @@ -1502,6 +1527,7 @@ Future runs of this module will trigger no downloads or compilation unless
.value_hint(ValueHint::FilePath),
)
.arg(frozen_lockfile_arg())
.arg(allow_scripts_arg())
})
}

Expand Down Expand Up @@ -2213,7 +2239,7 @@ The installation root is determined, in order of precedence:
These must be added to the path manually if required.")
.defer(|cmd| {
let cmd = runtime_args(cmd, true, true).arg(check_arg(true));
let cmd = runtime_args(cmd, true, true).arg(check_arg(true)).arg(allow_scripts_arg());
install_args(cmd, true)
})
}
Expand Down Expand Up @@ -3728,6 +3754,28 @@ fn unsafely_ignore_certificate_errors_arg() -> Arg {
.value_parser(flags_net::validator)
}

fn allow_scripts_arg() -> Arg {
Arg::new("allow-scripts")
.long("allow-scripts")
.num_args(0..)
.use_value_delimiter(true)
.require_equals(true)
.value_name("PACKAGE")
.value_parser(parse_packages_allowed_scripts)
.help("Allow running npm lifecycle scripts for the given packages. Note: Scripts will only be executed when using a node_modules directory (`--node-modules-dir`)")
}

fn allow_scripts_arg_parse(flags: &mut Flags, matches: &mut ArgMatches) {
let Some(parts) = matches.remove_many::<String>("allow-scripts") else {
return;
};
if parts.len() == 0 {
flags.allow_scripts = PackagesAllowedScripts::All;
} else {
flags.allow_scripts = PackagesAllowedScripts::Some(parts.collect());
}
}

fn add_parse(flags: &mut Flags, matches: &mut ArgMatches) {
flags.subcommand = DenoSubcommand::Add(add_parse_inner(matches, None));
}
Expand Down Expand Up @@ -3810,6 +3858,7 @@ fn bundle_parse(flags: &mut Flags, matches: &mut ArgMatches) {
fn cache_parse(flags: &mut Flags, matches: &mut ArgMatches) {
compile_args_parse(flags, matches);
frozen_lockfile_arg_parse(flags, matches);
allow_scripts_arg_parse(flags, matches);
let files = matches.remove_many::<String>("file").unwrap().collect();
flags.subcommand = DenoSubcommand::Cache(CacheFlags { files });
}
Expand Down Expand Up @@ -4096,6 +4145,7 @@ fn install_parse(flags: &mut Flags, matches: &mut ArgMatches) {
let local_flags = matches
.remove_many("cmd")
.map(|packages| add_parse_inner(matches, Some(packages)));
allow_scripts_arg_parse(flags, matches);
flags.subcommand = DenoSubcommand::Install(InstallFlags {
global,
kind: InstallKind::Local(local_flags),
Expand Down Expand Up @@ -9969,4 +10019,50 @@ mod tests {
);
}
}

#[test]
fn allow_scripts() {
let cases = [
(Some("--allow-scripts"), Ok(PackagesAllowedScripts::All)),
(None, Ok(PackagesAllowedScripts::None)),
(
Some("--allow-scripts=npm:foo"),
Ok(PackagesAllowedScripts::Some(svec!["npm:foo"])),
),
(
Some("--allow-scripts=npm:foo,npm:bar"),
Ok(PackagesAllowedScripts::Some(svec!["npm:foo", "npm:bar"])),
),
(Some("--allow-scripts=foo"), Err("Invalid package")),
];
for (flag, value) in cases {
let mut args = svec!["deno", "cache"];
if let Some(flag) = flag {
args.push(flag.into());
}
args.push("script.ts".into());
let r = flags_from_vec(args);
match value {
Ok(value) => {
assert_eq!(
r.unwrap(),
Flags {
subcommand: DenoSubcommand::Cache(CacheFlags {
files: svec!["script.ts"],
}),
allow_scripts: value,
..Flags::default()
}
);
}
Err(e) => {
let err = r.unwrap_err();
assert!(
err.to_string().contains(e),
"expected to contain '{e}' got '{err}'"
);
}
}
}
}
}
14 changes: 14 additions & 0 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1720,6 +1720,20 @@ impl CliOptions {
}
full_paths
}

pub fn lifecycle_scripts_config(&self) -> LifecycleScriptsConfig {
LifecycleScriptsConfig {
allowed: self.flags.allow_scripts.clone(),
initial_cwd: if matches!(
self.flags.allow_scripts,
PackagesAllowedScripts::None
) {
None
} else {
Some(self.initial_cwd.clone())
},
}
}
}

/// Resolves the path to use for a local node_modules folder.
Expand Down
3 changes: 2 additions & 1 deletion cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,8 @@ impl CliFactory {
&self.options.workspace,
)),
npm_system_info: self.options.npm_system_info(),
npmrc: self.options.npmrc().clone()
npmrc: self.options.npmrc().clone(),
lifecycle_scripts: self.options.lifecycle_scripts_config(),
})
}).await
}.boxed_local())
Expand Down
1 change: 1 addition & 0 deletions cli/lsp/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,7 @@ async fn create_npm_resolver(
.and_then(|d| d.npmrc.clone())
.unwrap_or_else(create_default_npmrc),
npm_system_info: NpmSystemInfo::default(),
lifecycle_scripts: Default::default(),
})
};
Some(create_cli_npm_resolver_for_lsp(options).await)
Expand Down
1 change: 1 addition & 0 deletions cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ mod npm;
mod ops;
mod resolver;
mod standalone;
mod task_runner;
mod tools;
mod tsc;
mod util;
Expand Down
1 change: 1 addition & 0 deletions cli/mainrt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ mod js;
mod node;
mod npm;
mod resolver;
mod task_runner;
mod util;
mod version;
mod worker;
Expand Down
12 changes: 12 additions & 0 deletions cli/npm/managed/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use deno_semver::package::PackageReq;
use resolution::AddPkgReqsResult;

use crate::args::CliLockfile;
use crate::args::LifecycleScriptsConfig;
use crate::args::NpmProcessState;
use crate::args::NpmProcessStateKind;
use crate::args::PackageJsonInstallDepsProvider;
Expand Down Expand Up @@ -70,6 +71,7 @@ pub struct CliNpmResolverManagedCreateOptions {
pub npm_system_info: NpmSystemInfo,
pub package_json_deps_provider: Arc<PackageJsonInstallDepsProvider>,
pub npmrc: Arc<ResolvedNpmRc>,
pub lifecycle_scripts: LifecycleScriptsConfig,
}

pub async fn create_managed_npm_resolver_for_lsp(
Expand Down Expand Up @@ -98,6 +100,7 @@ pub async fn create_managed_npm_resolver_for_lsp(
options.maybe_node_modules_path,
options.npm_system_info,
snapshot,
options.lifecycle_scripts,
)
})
.await
Expand All @@ -122,6 +125,7 @@ pub async fn create_managed_npm_resolver(
options.maybe_node_modules_path,
options.npm_system_info,
snapshot,
options.lifecycle_scripts,
))
}

Expand All @@ -138,6 +142,7 @@ fn create_inner(
node_modules_dir_path: Option<PathBuf>,
npm_system_info: NpmSystemInfo,
snapshot: Option<ValidSerializedNpmResolutionSnapshot>,
lifecycle_scripts: LifecycleScriptsConfig,
) -> Arc<dyn CliNpmResolver> {
let resolution = Arc::new(NpmResolution::from_serialized(
npm_api.clone(),
Expand All @@ -160,6 +165,7 @@ fn create_inner(
tarball_cache.clone(),
node_modules_dir_path,
npm_system_info.clone(),
lifecycle_scripts.clone(),
);
Arc::new(ManagedCliNpmResolver::new(
fs,
Expand All @@ -172,6 +178,7 @@ fn create_inner(
tarball_cache,
text_only_progress_bar,
npm_system_info,
lifecycle_scripts,
))
}

Expand Down Expand Up @@ -258,6 +265,7 @@ pub struct ManagedCliNpmResolver {
text_only_progress_bar: ProgressBar,
npm_system_info: NpmSystemInfo,
top_level_install_flag: AtomicFlag,
lifecycle_scripts: LifecycleScriptsConfig,
}

impl std::fmt::Debug for ManagedCliNpmResolver {
Expand All @@ -281,6 +289,7 @@ impl ManagedCliNpmResolver {
tarball_cache: Arc<TarballCache>,
text_only_progress_bar: ProgressBar,
npm_system_info: NpmSystemInfo,
lifecycle_scripts: LifecycleScriptsConfig,
) -> Self {
Self {
fs,
Expand All @@ -294,6 +303,7 @@ impl ManagedCliNpmResolver {
tarball_cache,
npm_system_info,
top_level_install_flag: Default::default(),
lifecycle_scripts,
}
}

Expand Down Expand Up @@ -578,6 +588,7 @@ impl CliNpmResolver for ManagedCliNpmResolver {
self.tarball_cache.clone(),
self.root_node_modules_path().map(ToOwned::to_owned),
self.npm_system_info.clone(),
self.lifecycle_scripts.clone(),
),
self.maybe_lockfile.clone(),
self.npm_api.clone(),
Expand All @@ -587,6 +598,7 @@ impl CliNpmResolver for ManagedCliNpmResolver {
self.tarball_cache.clone(),
self.text_only_progress_bar.clone(),
self.npm_system_info.clone(),
self.lifecycle_scripts.clone(),
))
}

Expand Down
Loading

0 comments on commit ce7dc2b

Please sign in to comment.