Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(node): Support executing npm package lifecycle scripts (preinstall/install/postinstall) #24487

Merged
merged 29 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
4ed43a2
Extract out task runner
nathanwhit Jul 4, 2024
f39e5fe
Add `--allow-scripts` flag
nathanwhit Jul 4, 2024
d32b75b
Fix downloading to test registry
nathanwhit Jul 8, 2024
214326a
Fix default bin entry resolution
nathanwhit Jul 8, 2024
92dad6a
Thread allow-scripts through
nathanwhit Jul 8, 2024
2f3ed54
First go at scripts
nathanwhit Jul 8, 2024
49bdbe2
Add variables required by node-gyp to process.config
nathanwhit Jul 8, 2024
f8f157a
Add node-gyp install script test
nathanwhit Jul 9, 2024
25df480
Cleanup, add warning on un-run scripts
nathanwhit Jul 9, 2024
c17242b
Add warning if `node-gyp` implicitly required but not found
nathanwhit Jul 9, 2024
ead6948
Add test
nathanwhit Jul 9, 2024
e680015
Rm dev logs
nathanwhit Jul 9, 2024
c665831
Only run simple node invocations with deno
nathanwhit Jul 9, 2024
384aea0
Cleanup msg
nathanwhit Jul 9, 2024
9a8f46b
Add more tests
nathanwhit Jul 9, 2024
a2d8d60
Fmt + lint
nathanwhit Jul 9, 2024
666260b
bikeshed
nathanwhit Jul 9, 2024
8ee353f
Rm references to npm registry
nathanwhit Jul 9, 2024
db0a0e0
Don't run w deno if it's not a path we understand
nathanwhit Jul 9, 2024
84449df
Fmt
nathanwhit Jul 9, 2024
aa6ea37
Make checking allowed a bit smarter
nathanwhit Jul 9, 2024
9004f59
llvm_version can just be 0.0
nathanwhit Jul 9, 2024
119b750
Basic validation, arg tests, clarify help
nathanwhit Jul 9, 2024
7323af7
make registry files check less sensitive
nathanwhit Jul 9, 2024
808660b
Fmt
nathanwhit Jul 9, 2024
4acfdaf
Fix output
nathanwhit Jul 9, 2024
bdfd148
Use correct bin despite conflicts + fix windows
nathanwhit Jul 9, 2024
46d93c6
Add test for conflicting bins
nathanwhit Jul 9, 2024
1a277ec
Thread init cwd through
nathanwhit Jul 10, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -506,6 +506,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 @@ -561,6 +585,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 @@ -1501,6 +1526,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 @@ -2205,7 +2231,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 @@ -3715,6 +3741,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 @@ -3797,6 +3845,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 @@ -4082,6 +4131,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 @@ -9924,4 +9974,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 @@ -1730,6 +1730,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