fix(cli): Warn on not-run lifecycle scripts with global cache (#25786)

Refactors the lifecycle scripts code to extract out the common
functionality and then uses that to provide a warning in the global
resolver.

While ideally we would still support them with the global cache, for now
a warning is at least better than the status quo (where people are
unaware why their packages aren't working).

cli/args/flags.rs

@@ -544,7 +544,8 @@ pub enum CaData {
 #[derive(Clone, Debug, Eq, PartialEq, Default)]
 pub struct LifecycleScriptsConfig {
   pub allowed: PackagesAllowedScripts,
-  pub initial_cwd: Option<PathBuf>,
+  pub initial_cwd: PathBuf,
+  pub root_dir: PathBuf,
 }
 
 #[derive(Debug, Clone, Eq, PartialEq, Default)]

cli/args/mod.rs

@@ -1652,14 +1652,8 @@ impl CliOptions {
   pub fn lifecycle_scripts_config(&self) -> LifecycleScriptsConfig {
     LifecycleScriptsConfig {
       allowed: self.flags.allow_scripts.clone(),
-      initial_cwd: if matches!(
+      initial_cwd: self.initial_cwd.clone(),
-        self.flags.allow_scripts,
+      root_dir: self.workspace().root_dir_path(),
-        PackagesAllowedScripts::None
-      ) {
-        None
-      } else {
-        Some(self.initial_cwd.clone())
-      },
     }
   }
 }

cli/npm/managed/resolvers/common.rs

@@ -1,5 +1,8 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
+pub mod bin_entries;
+pub mod lifecycle_scripts;
+
 use std::collections::HashMap;
 use std::io::ErrorKind;
 use std::path::Path;
@@ -134,7 +137,7 @@ impl RegistryReadPermissionChecker {
 
 /// Caches all the packages in parallel.
 pub async fn cache_packages(
-  packages: Vec<NpmResolutionPackage>,
+  packages: &[NpmResolutionPackage],
   tarball_cache: &Arc<TarballCache>,
 ) -> Result<(), AnyError> {
   let mut futures_unordered = futures::stream::FuturesUnordered::new();

cli/npm/managed/resolvers/common/bin_entries.rs

@@ -12,12 +12,12 @@ use std::path::Path;
 use std::path::PathBuf;
 
 #[derive(Default)]
-pub(super) struct BinEntries {
+pub struct BinEntries<'a> {
   /// Packages that have colliding bin names
-  collisions: HashSet<NpmPackageId>,
+  collisions: HashSet<&'a NpmPackageId>,
-  seen_names: HashMap<String, NpmPackageId>,
+  seen_names: HashMap<&'a str, &'a NpmPackageId>,
   /// The bin entries
-  entries: Vec<(NpmResolutionPackage, PathBuf)>,
+  entries: Vec<(&'a NpmResolutionPackage, PathBuf)>,
 }
 
 /// Returns the name of the default binary for the given package.
@@ -31,37 +31,32 @@ fn default_bin_name(package: &NpmResolutionPackage) -> &str {
     .map_or(package.id.nv.name.as_str(), |(_, name)| name)
 }
 
-impl BinEntries {
+impl<'a> BinEntries<'a> {
-  pub(super) fn new() -> Self {
+  pub fn new() -> Self {
     Self::default()
   }
 
   /// Add a new bin entry (package with a bin field)
-  pub(super) fn add(
+  pub fn add(
     &mut self,
-    package: NpmResolutionPackage,
+    package: &'a NpmResolutionPackage,
     package_path: PathBuf,
   ) {
     // check for a new collision, if we haven't already
     // found one
     match package.bin.as_ref().unwrap() {
       deno_npm::registry::NpmPackageVersionBinEntry::String(_) => {
-        let bin_name = default_bin_name(&package);
+        let bin_name = default_bin_name(package);
 
-        if let Some(other) = self
+        if let Some(other) = self.seen_names.insert(bin_name, &package.id) {
-          .seen_names
+          self.collisions.insert(&package.id);
-          .insert(bin_name.to_string(), package.id.clone())
-        {
-          self.collisions.insert(package.id.clone());
           self.collisions.insert(other);
         }
       }
       deno_npm::registry::NpmPackageVersionBinEntry::Map(entries) => {
         for name in entries.keys() {
-          if let Some(other) =
+          if let Some(other) = self.seen_names.insert(name, &package.id) {
-            self.seen_names.insert(name.to_string(), package.id.clone())
+            self.collisions.insert(&package.id);
-          {
-            self.collisions.insert(package.id.clone());
             self.collisions.insert(other);
           }
         }
@@ -117,7 +112,7 @@ impl BinEntries {
   }
 
   /// Collect the bin entries into a vec of (name, script path)
-  pub(super) fn into_bin_files(
+  pub fn into_bin_files(
     mut self,
     snapshot: &NpmResolutionSnapshot,
   ) -> Vec<(String, PathBuf)> {
@@ -133,7 +128,7 @@ impl BinEntries {
 
   /// Finish setting up the bin entries, writing the necessary files
   /// to disk.
-  pub(super) fn finish(
+  pub fn finish(
     mut self,
     snapshot: &NpmResolutionSnapshot,
     bin_node_modules_dir_path: &Path,
@@ -162,8 +157,8 @@ impl BinEntries {
 // that has a bin entry, then sort them by depth
 fn sort_by_depth(
   snapshot: &NpmResolutionSnapshot,
-  bin_entries: &mut [(NpmResolutionPackage, PathBuf)],
+  bin_entries: &mut [(&NpmResolutionPackage, PathBuf)],
-  collisions: &mut HashSet<NpmPackageId>,
+  collisions: &mut HashSet<&NpmPackageId>,
 ) {
   enum Entry<'a> {
     Pkg(&'a NpmPackageId),
@@ -217,7 +212,7 @@ fn sort_by_depth(
   });
 }
 
-pub(super) fn set_up_bin_entry(
+pub fn set_up_bin_entry(
   package: &NpmResolutionPackage,
   bin_name: &str,
   #[allow(unused_variables)] bin_script: &str,

cli/npm/managed/resolvers/common/lifecycle_scripts.rs

@@ -0,0 +1,306 @@
+// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
+
+use super::bin_entries::BinEntries;
+use crate::args::LifecycleScriptsConfig;
+use deno_npm::resolution::NpmResolutionSnapshot;
+use deno_semver::package::PackageNv;
+use std::borrow::Cow;
+use std::rc::Rc;
+
+use std::path::Path;
+use std::path::PathBuf;
+
+use deno_core::error::AnyError;
+use deno_npm::NpmResolutionPackage;
+
+pub trait LifecycleScriptsStrategy {
+  fn can_run_scripts(&self) -> bool {
+    true
+  }
+  fn package_path(&self, package: &NpmResolutionPackage) -> PathBuf;
+
+  fn warn_on_scripts_not_run(
+    &self,
+    packages: &[(&NpmResolutionPackage, PathBuf)],
+  ) -> Result<(), AnyError>;
+
+  fn has_warned(&self, package: &NpmResolutionPackage) -> bool;
+
+  fn has_run(&self, package: &NpmResolutionPackage) -> bool;
+
+  fn did_run_scripts(
+    &self,
+    package: &NpmResolutionPackage,
+  ) -> Result<(), AnyError>;
+}
+
+pub struct LifecycleScripts<'a> {
+  packages_with_scripts: Vec<(&'a NpmResolutionPackage, PathBuf)>,
+  packages_with_scripts_not_run: Vec<(&'a NpmResolutionPackage, PathBuf)>,
+
+  config: &'a LifecycleScriptsConfig,
+  strategy: Box<dyn LifecycleScriptsStrategy + 'a>,
+}
+
+impl<'a> LifecycleScripts<'a> {
+  pub fn new<T: LifecycleScriptsStrategy + 'a>(
+    config: &'a LifecycleScriptsConfig,
+    strategy: T,
+  ) -> Self {
+    Self {
+      config,
+      packages_with_scripts: Vec::new(),
+      packages_with_scripts_not_run: Vec::new(),
+      strategy: Box::new(strategy),
+    }
+  }
+}
+
+fn has_lifecycle_scripts(
+  package: &NpmResolutionPackage,
+  package_path: &Path,
+) -> bool {
+  if let Some(install) = package.scripts.get("install") {
+    // default script
+    if !is_broken_default_install_script(install, package_path) {
+      return true;
+    }
+  }
+  package.scripts.contains_key("preinstall")
+    || package.scripts.contains_key("postinstall")
+}
+
+// npm defaults to running `node-gyp rebuild` if there is a `binding.gyp` file
+// but it always fails if the package excludes the `binding.gyp` file when they publish.
+// (for example, `fsevents` hits this)
+fn is_broken_default_install_script(script: &str, package_path: &Path) -> bool {
+  script == "node-gyp rebuild" && !package_path.join("binding.gyp").exists()
+}
+
+impl<'a> LifecycleScripts<'a> {
+  fn can_run_scripts(&self, package_nv: &PackageNv) -> bool {
+    if !self.strategy.can_run_scripts() {
+      return false;
+    }
+    use crate::args::PackagesAllowedScripts;
+    match &self.config.allowed {
+      PackagesAllowedScripts::All => true,
+      // TODO: make this more correct
+      PackagesAllowedScripts::Some(allow_list) => allow_list.iter().any(|s| {
+        let s = s.strip_prefix("npm:").unwrap_or(s);
+        s == package_nv.name || s == package_nv.to_string()
+      }),
+      PackagesAllowedScripts::None => false,
+    }
+  }
+  /// Register a package for running lifecycle scripts, if applicable.
+  ///
+  /// `package_path` is the path containing the package's code (its root dir).
+  /// `package_meta_path` is the path to serve as the base directory for lifecycle
+  /// script-related metadata (e.g. to store whether the scripts have been run already)
+  pub fn add(
+    &mut self,
+    package: &'a NpmResolutionPackage,
+    package_path: Cow<Path>,
+  ) {
+    if has_lifecycle_scripts(package, &package_path) {
+      if self.can_run_scripts(&package.id.nv) {
+        if !self.strategy.has_run(package) {
+          self
+            .packages_with_scripts
+            .push((package, package_path.into_owned()));
+        }
+      } else if !self.strategy.has_run(package)
+        && !self.strategy.has_warned(package)
+      {
+        self
+          .packages_with_scripts_not_run
+          .push((package, package_path.into_owned()));
+      }
+    }
+  }
+
+  pub fn warn_not_run_scripts(&self) -> Result<(), AnyError> {
+    if !self.packages_with_scripts_not_run.is_empty() {
+      self
+        .strategy
+        .warn_on_scripts_not_run(&self.packages_with_scripts_not_run)?;
+    }
+    Ok(())
+  }
+
+  pub async fn finish(
+    self,
+    snapshot: &NpmResolutionSnapshot,
+    packages: &[NpmResolutionPackage],
+    root_node_modules_dir_path: Option<&Path>,
+  ) -> Result<(), AnyError> {
+    self.warn_not_run_scripts()?;
+    let get_package_path =
+      |p: &NpmResolutionPackage| self.strategy.package_path(p);
+    let mut failed_packages = Vec::new();
+    if !self.packages_with_scripts.is_empty() {
+      // get custom commands for each bin available in the node_modules dir (essentially
+      // the scripts that are in `node_modules/.bin`)
+      let base =
+        resolve_baseline_custom_commands(snapshot, packages, get_package_path)?;
+      let init_cwd = &self.config.initial_cwd;
+      let process_state = crate::npm::managed::npm_process_state(
+        snapshot.as_valid_serialized(),
+        root_node_modules_dir_path,
+      );
+
+      let mut env_vars = crate::task_runner::real_env_vars();
+      env_vars.insert(
+        crate::args::NPM_RESOLUTION_STATE_ENV_VAR_NAME.to_string(),
+        process_state,
+      );
+      for (package, package_path) in self.packages_with_scripts {
+        // add custom commands for binaries from the package's dependencies. this will take precedence over the
+        // baseline commands, so if the package relies on a bin that conflicts with one higher in the dependency tree, the
+        // correct bin will be used.
+        let custom_commands = resolve_custom_commands_from_deps(
+          base.clone(),
+          package,
+          snapshot,
+          get_package_path,
+        )?;
+        for script_name in ["preinstall", "install", "postinstall"] {
+          if let Some(script) = package.scripts.get(script_name) {
+            if script_name == "install"
+              && is_broken_default_install_script(script, &package_path)
+            {
+              continue;
+            }
+            let exit_code = crate::task_runner::run_task(
+              crate::task_runner::RunTaskOptions {
+                task_name: script_name,
+                script,
+                cwd: &package_path,
+                env_vars: env_vars.clone(),
+                custom_commands: custom_commands.clone(),
+                init_cwd,
+                argv: &[],
+                root_node_modules_dir: root_node_modules_dir_path,
+              },
+            )
+            .await?;
+            if exit_code != 0 {
+              log::warn!(
+                "error: script '{}' in '{}' failed with exit code {}",
+                script_name,
+                package.id.nv,
+                exit_code,
+              );
+              failed_packages.push(&package.id.nv);
+              // assume if earlier script fails, later ones will fail too
+              break;
+            }
+          }
+        }
+        self.strategy.did_run_scripts(package)?;
+      }
+    }
+    if failed_packages.is_empty() {
+      Ok(())
+    } else {
+      Err(AnyError::msg(format!(
+        "failed to run scripts for packages: {}",
+        failed_packages
+          .iter()
+          .map(|p| p.to_string())
+          .collect::<Vec<_>>()
+          .join(", ")
+      )))
+    }
+  }
+}
+
+// take in all (non copy) packages from snapshot,
+// and resolve the set of available binaries to create
+// custom commands available to the task runner
+fn resolve_baseline_custom_commands(
+  snapshot: &NpmResolutionSnapshot,
+  packages: &[NpmResolutionPackage],
+  get_package_path: impl Fn(&NpmResolutionPackage) -> PathBuf,
+) -> Result<crate::task_runner::TaskCustomCommands, AnyError> {
+  let mut custom_commands = crate::task_runner::TaskCustomCommands::new();
+  custom_commands
+    .insert("npx".to_string(), Rc::new(crate::task_runner::NpxCommand));
+
+  custom_commands
+    .insert("npm".to_string(), Rc::new(crate::task_runner::NpmCommand));
+
+  custom_commands
+    .insert("node".to_string(), Rc::new(crate::task_runner::NodeCommand));
+
+  custom_commands.insert(
+    "node-gyp".to_string(),
+    Rc::new(crate::task_runner::NodeGypCommand),
+  );
+
+  // TODO: this recreates the bin entries which could be redoing some work, but the ones
+  // we compute earlier in `sync_resolution_with_fs` may not be exhaustive (because we skip
+  // doing it for packages that are set up already.
+  // realistically, scripts won't be run very often so it probably isn't too big of an issue.
+  resolve_custom_commands_from_packages(
+    custom_commands,
+    snapshot,
+    packages,
+    get_package_path,
+  )
+}
+
+// resolves the custom commands from an iterator of packages
+// and adds them to the existing custom commands.
+// note that this will overwrite any existing custom commands
+fn resolve_custom_commands_from_packages<
+  'a,
+  P: IntoIterator<Item = &'a NpmResolutionPackage>,
+>(
+  mut commands: crate::task_runner::TaskCustomCommands,
+  snapshot: &'a NpmResolutionSnapshot,
+  packages: P,
+  get_package_path: impl Fn(&'a NpmResolutionPackage) -> PathBuf,
+) -> Result<crate::task_runner::TaskCustomCommands, AnyError> {
+  let mut bin_entries = BinEntries::new();
+  for package in packages {
+    let package_path = get_package_path(package);
+
+    if package.bin.is_some() {
+      bin_entries.add(package, package_path);
+    }
+  }
+  let bins = bin_entries.into_bin_files(snapshot);
+  for (bin_name, script_path) in bins {
+    commands.insert(
+      bin_name.clone(),
+      Rc::new(crate::task_runner::NodeModulesFileRunCommand {
+        command_name: bin_name,
+        path: script_path,
+      }),
+    );
+  }
+
+  Ok(commands)
+}
+
+// resolves the custom commands from the dependencies of a package
+// and adds them to the existing custom commands.
+// note that this will overwrite any existing custom commands.
+fn resolve_custom_commands_from_deps(
+  baseline: crate::task_runner::TaskCustomCommands,
+  package: &NpmResolutionPackage,
+  snapshot: &NpmResolutionSnapshot,
+  get_package_path: impl Fn(&NpmResolutionPackage) -> PathBuf,
+) -> Result<crate::task_runner::TaskCustomCommands, AnyError> {
+  resolve_custom_commands_from_packages(
+    baseline,
+    snapshot,
+    package
+      .dependencies
+      .values()
+      .map(|id| snapshot.package_from_id(id).unwrap()),
+    get_package_path,
+  )
+}

cli/npm/managed/resolvers/global.rs

@@ -2,16 +2,19 @@
 
 //! Code for global npm cache resolution.
 
+use std::borrow::Cow;
 use std::path::Path;
 use std::path::PathBuf;
 use std::sync::Arc;
 
+use crate::colors;
 use async_trait::async_trait;
 use deno_ast::ModuleSpecifier;
 use deno_core::error::AnyError;
 use deno_core::url::Url;
 use deno_npm::NpmPackageCacheFolderId;
 use deno_npm::NpmPackageId;
+use deno_npm::NpmResolutionPackage;
 use deno_npm::NpmSystemInfo;
 use deno_runtime::deno_fs::FileSystem;
 use deno_runtime::deno_node::NodePermissions;
@@ -19,10 +22,14 @@ use node_resolver::errors::PackageFolderResolveError;
 use node_resolver::errors::PackageNotFoundError;
 use node_resolver::errors::ReferrerNotFoundError;
 
+use crate::args::LifecycleScriptsConfig;
+use crate::cache::FastInsecureHasher;
+
 use super::super::cache::NpmCache;
 use super::super::cache::TarballCache;
 use super::super::resolution::NpmResolution;
 use super::common::cache_packages;
+use super::common::lifecycle_scripts::LifecycleScriptsStrategy;
 use super::common::NpmPackageFsResolver;
 use super::common::RegistryReadPermissionChecker;
 
@@ -34,6 +41,7 @@ pub struct GlobalNpmPackageResolver {
   resolution: Arc<NpmResolution>,
   system_info: NpmSystemInfo,
   registry_read_permission_checker: RegistryReadPermissionChecker,
+  lifecycle_scripts: LifecycleScriptsConfig,
 }
 
 impl GlobalNpmPackageResolver {
@@ -43,6 +51,7 @@ impl GlobalNpmPackageResolver {
     tarball_cache: Arc<TarballCache>,
     resolution: Arc<NpmResolution>,
     system_info: NpmSystemInfo,
+    lifecycle_scripts: LifecycleScriptsConfig,
   ) -> Self {
     Self {
       registry_read_permission_checker: RegistryReadPermissionChecker::new(
@@ -53,6 +62,7 @@ impl GlobalNpmPackageResolver {
       tarball_cache,
       resolution,
       system_info,
+      lifecycle_scripts,
     }
   }
 }
@@ -149,8 +159,7 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
     let package_partitions = self
       .resolution
       .all_system_packages_partitioned(&self.system_info);
-
+    cache_packages(&package_partitions.packages, &self.tarball_cache).await?;
-    cache_packages(package_partitions.packages, &self.tarball_cache).await?;
 
     // create the copy package folders
     for copy in package_partitions.copy_packages {
@@ -159,6 +168,18 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
         .ensure_copy_package(&copy.get_package_cache_folder_id())?;
     }
 
+    let mut lifecycle_scripts =
+      super::common::lifecycle_scripts::LifecycleScripts::new(
+        &self.lifecycle_scripts,
+        GlobalLifecycleScripts::new(self, &self.lifecycle_scripts.root_dir),
+      );
+    for package in &package_partitions.packages {
+      let package_folder = self.cache.package_folder_for_nv(&package.id.nv);
+      lifecycle_scripts.add(package, Cow::Borrowed(&package_folder));
+    }
+
+    lifecycle_scripts.warn_not_run_scripts()?;
+
     Ok(())
   }
 
@@ -172,3 +193,78 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
       .ensure_registry_read_permission(permissions, path)
   }
 }
+
+struct GlobalLifecycleScripts<'a> {
+  resolver: &'a GlobalNpmPackageResolver,
+  path_hash: u64,
+}
+
+impl<'a> GlobalLifecycleScripts<'a> {
+  fn new(resolver: &'a GlobalNpmPackageResolver, root_dir: &Path) -> Self {
+    let mut hasher = FastInsecureHasher::new_without_deno_version();
+    hasher.write(root_dir.to_string_lossy().as_bytes());
+    let path_hash = hasher.finish();
+    Self {
+      resolver,
+      path_hash,
+    }
+  }
+
+  fn warned_scripts_file(&self, package: &NpmResolutionPackage) -> PathBuf {
+    self
+      .package_path(package)
+      .join(format!(".scripts-warned-{}", self.path_hash))
+  }
+}
+
+impl<'a> super::common::lifecycle_scripts::LifecycleScriptsStrategy
+  for GlobalLifecycleScripts<'a>
+{
+  fn can_run_scripts(&self) -> bool {
+    false
+  }
+  fn package_path(&self, package: &NpmResolutionPackage) -> PathBuf {
+    self.resolver.cache.package_folder_for_nv(&package.id.nv)
+  }
+
+  fn warn_on_scripts_not_run(
+    &self,
+    packages: &[(&NpmResolutionPackage, PathBuf)],
+  ) -> std::result::Result<(), deno_core::anyhow::Error> {
+    log::warn!("{} The following packages contained npm lifecycle scripts ({}) that were not executed:", colors::yellow("Warning"), colors::gray("preinstall/install/postinstall"));
+    for (package, _) in packages {
+      log::warn!("┠─ {}", colors::gray(format!("npm:{}", package.id.nv)));
+    }
+    log::warn!("┃");
+    log::warn!(
+      "┠─ {}",
+      colors::italic("This may cause the packages to not work correctly.")
+    );
+    log::warn!("┠─ {}", colors::italic("Lifecycle scripts are only supported when using a `node_modules` directory."));
+    log::warn!(
+      "┠─ {}",
+      colors::italic("Enable it in your deno config file:")
+    );
+    log::warn!("┖─ {}", colors::bold("\"nodeModulesDir\": \"auto\""));
+
+    for (package, _) in packages {
+      std::fs::write(self.warned_scripts_file(package), "")?;
+    }
+    Ok(())
+  }
+
+  fn did_run_scripts(
+    &self,
+    _package: &NpmResolutionPackage,
+  ) -> std::result::Result<(), deno_core::anyhow::Error> {
+    Ok(())
+  }
+
+  fn has_warned(&self, package: &NpmResolutionPackage) -> bool {
+    self.warned_scripts_file(package).exists()
+  }
+
+  fn has_run(&self, _package: &NpmResolutionPackage) -> bool {
+    false
+  }
+}

cli/npm/managed/resolvers/local.rs

@@ -2,8 +2,6 @@
 
 //! Code for local node_modules resolution.
 
-mod bin_entries;
-
 use std::borrow::Cow;
 use std::cell::RefCell;
 use std::cmp::Ordering;
@@ -18,11 +16,9 @@ use std::rc::Rc;
 use std::sync::Arc;
 
 use crate::args::LifecycleScriptsConfig;
-use crate::args::PackagesAllowedScripts;
 use crate::colors;
 use async_trait::async_trait;
 use deno_ast::ModuleSpecifier;
-use deno_core::anyhow;
 use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
 use deno_core::futures::stream::FuturesUnordered;
@@ -272,77 +268,10 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver {
   }
 }
 
-// take in all (non copy) packages from snapshot,
+/// `node_modules/.deno/<package>/node_modules/<package_name>`
-// and resolve the set of available binaries to create
+///
-// custom commands available to the task runner
+/// Where the actual package is stored.
-fn resolve_baseline_custom_commands(
+fn local_node_modules_package_contents_path(
-  snapshot: &NpmResolutionSnapshot,
-  packages: &[NpmResolutionPackage],
-  local_registry_dir: &Path,
-) -> Result<crate::task_runner::TaskCustomCommands, AnyError> {
-  let mut custom_commands = crate::task_runner::TaskCustomCommands::new();
-  custom_commands
-    .insert("npx".to_string(), Rc::new(crate::task_runner::NpxCommand));
-
-  custom_commands
-    .insert("npm".to_string(), Rc::new(crate::task_runner::NpmCommand));
-
-  custom_commands
-    .insert("node".to_string(), Rc::new(crate::task_runner::NodeCommand));
-
-  custom_commands.insert(
-    "node-gyp".to_string(),
-    Rc::new(crate::task_runner::NodeGypCommand),
-  );
-
-  // TODO: this recreates the bin entries which could be redoing some work, but the ones
-  // we compute earlier in `sync_resolution_with_fs` may not be exhaustive (because we skip
-  // doing it for packages that are set up already.
-  // realistically, scripts won't be run very often so it probably isn't too big of an issue.
-  resolve_custom_commands_from_packages(
-    custom_commands,
-    snapshot,
-    packages,
-    local_registry_dir,
-  )
-}
-
-// resolves the custom commands from an iterator of packages
-// and adds them to the existing custom commands.
-// note that this will overwrite any existing custom commands
-fn resolve_custom_commands_from_packages<
-  'a,
-  P: IntoIterator<Item = &'a NpmResolutionPackage>,
->(
-  mut commands: crate::task_runner::TaskCustomCommands,
-  snapshot: &'a NpmResolutionSnapshot,
-  packages: P,
-  local_registry_dir: &Path,
-) -> Result<crate::task_runner::TaskCustomCommands, AnyError> {
-  let mut bin_entries = bin_entries::BinEntries::new();
-  for package in packages {
-    let package_path =
-      local_node_modules_package_path(local_registry_dir, package);
-
-    if package.bin.is_some() {
-      bin_entries.add(package.clone(), package_path);
-    }
-  }
-  let bins = bin_entries.into_bin_files(snapshot);
-  for (bin_name, script_path) in bins {
-    commands.insert(
-      bin_name.clone(),
-      Rc::new(crate::task_runner::NodeModulesFileRunCommand {
-        command_name: bin_name,
-        path: script_path,
-      }),
-    );
-  }
-
-  Ok(commands)
-}
-
-fn local_node_modules_package_path(
   local_registry_dir: &Path,
   package: &NpmResolutionPackage,
 ) -> PathBuf {
@@ -354,62 +283,6 @@ fn local_node_modules_package_path(
     .join(&package.id.nv.name)
 }
 
-// resolves the custom commands from the dependencies of a package
-// and adds them to the existing custom commands.
-// note that this will overwrite any existing custom commands.
-fn resolve_custom_commands_from_deps(
-  baseline: crate::task_runner::TaskCustomCommands,
-  package: &NpmResolutionPackage,
-  snapshot: &NpmResolutionSnapshot,
-  local_registry_dir: &Path,
-) -> Result<crate::task_runner::TaskCustomCommands, AnyError> {
-  resolve_custom_commands_from_packages(
-    baseline,
-    snapshot,
-    package
-      .dependencies
-      .values()
-      .map(|id| snapshot.package_from_id(id).unwrap()),
-    local_registry_dir,
-  )
-}
-
-fn can_run_scripts(
-  allow_scripts: &PackagesAllowedScripts,
-  package_nv: &PackageNv,
-) -> bool {
-  match allow_scripts {
-    PackagesAllowedScripts::All => true,
-    // TODO: make this more correct
-    PackagesAllowedScripts::Some(allow_list) => allow_list.iter().any(|s| {
-      let s = s.strip_prefix("npm:").unwrap_or(s);
-      s == package_nv.name || s == package_nv.to_string()
-    }),
-    PackagesAllowedScripts::None => false,
-  }
-}
-
-// npm defaults to running `node-gyp rebuild` if there is a `binding.gyp` file
-// but it always fails if the package excludes the `binding.gyp` file when they publish.
-// (for example, `fsevents` hits this)
-fn is_broken_default_install_script(script: &str, package_path: &Path) -> bool {
-  script == "node-gyp rebuild" && !package_path.join("binding.gyp").exists()
-}
-
-fn has_lifecycle_scripts(
-  package: &NpmResolutionPackage,
-  package_path: &Path,
-) -> bool {
-  if let Some(install) = package.scripts.get("install") {
-    // default script
-    if !is_broken_default_install_script(install, package_path) {
-      return true;
-    }
-  }
-  package.scripts.contains_key("preinstall")
-    || package.scripts.contains_key("postinstall")
-}
-
 /// Creates a pnpm style folder structure.
 #[allow(clippy::too_many_arguments)]
 async fn sync_resolution_with_fs(
@@ -460,9 +333,15 @@ async fn sync_resolution_with_fs(
   let mut cache_futures = FuturesUnordered::new();
   let mut newest_packages_by_name: HashMap<&String, &NpmResolutionPackage> =
     HashMap::with_capacity(package_partitions.packages.len());
-  let bin_entries = Rc::new(RefCell::new(bin_entries::BinEntries::new()));
+  let bin_entries =
-  let mut packages_with_scripts = Vec::with_capacity(2);
+    Rc::new(RefCell::new(super::common::bin_entries::BinEntries::new()));
-  let mut packages_with_scripts_not_run = Vec::new();
+  let mut lifecycle_scripts =
+    super::common::lifecycle_scripts::LifecycleScripts::new(
+      lifecycle_scripts,
+      LocalLifecycleScripts {
+        deno_local_registry_dir: &deno_local_registry_dir,
+      },
+    );
   let packages_with_deprecation_warnings = Arc::new(Mutex::new(Vec::new()));
   for package in &package_partitions.packages {
     if let Some(current_pkg) =
@@ -518,9 +397,7 @@ async fn sync_resolution_with_fs(
         .await??;
 
         if package.bin.is_some() {
-          bin_entries_to_setup
+          bin_entries_to_setup.borrow_mut().add(package, package_path);
-            .borrow_mut()
-            .add(package.clone(), package_path);
         }
 
         if let Some(deprecated) = &package.deprecated {
@@ -538,21 +415,7 @@ async fn sync_resolution_with_fs(
     let sub_node_modules = folder_path.join("node_modules");
     let package_path =
       join_package_name(&sub_node_modules, &package.id.nv.name);
-    if has_lifecycle_scripts(package, &package_path) {
+    lifecycle_scripts.add(package, package_path.into());
-      let scripts_run = folder_path.join(".scripts-run");
-      let has_warned = folder_path.join(".scripts-warned");
-      if can_run_scripts(&lifecycle_scripts.allowed, &package.id.nv) {
-        if !scripts_run.exists() {
-          packages_with_scripts.push((
-            package.clone(),
-            package_path,
-            scripts_run,
-          ));
-        }
-      } else if !scripts_run.exists() && !has_warned.exists() {
-        packages_with_scripts_not_run.push((has_warned, package.id.nv.clone()));
-      }
-    }
   }
 
   while let Some(result) = cache_futures.next().await {
@@ -789,74 +652,12 @@ async fn sync_resolution_with_fs(
     }
   }
 
-  if !packages_with_scripts.is_empty() {
-    // get custom commands for each bin available in the node_modules dir (essentially
-    // the scripts that are in `node_modules/.bin`)
-    let base = resolve_baseline_custom_commands(
-      snapshot,
-      &package_partitions.packages,
-      &deno_local_registry_dir,
-    )?;
-    let init_cwd = lifecycle_scripts.initial_cwd.as_deref().unwrap();
-    let process_state = crate::npm::managed::npm_process_state(
-      snapshot.as_valid_serialized(),
-      Some(root_node_modules_dir_path),
-    );
-
-    let mut env_vars = crate::task_runner::real_env_vars();
-    env_vars.insert(
-      crate::args::NPM_RESOLUTION_STATE_ENV_VAR_NAME.to_string(),
-      process_state,
-    );
-    for (package, package_path, scripts_run_path) in packages_with_scripts {
-      // add custom commands for binaries from the package's dependencies. this will take precedence over the
-      // baseline commands, so if the package relies on a bin that conflicts with one higher in the dependency tree, the
-      // correct bin will be used.
-      let custom_commands = resolve_custom_commands_from_deps(
-        base.clone(),
-        &package,
-        snapshot,
-        &deno_local_registry_dir,
-      )?;
-      for script_name in ["preinstall", "install", "postinstall"] {
-        if let Some(script) = package.scripts.get(script_name) {
-          if script_name == "install"
-            && is_broken_default_install_script(script, &package_path)
-          {
-            continue;
-          }
-          let exit_code =
-            crate::task_runner::run_task(crate::task_runner::RunTaskOptions {
-              task_name: script_name,
-              script,
-              cwd: &package_path,
-              env_vars: env_vars.clone(),
-              custom_commands: custom_commands.clone(),
-              init_cwd,
-              argv: &[],
-              root_node_modules_dir: Some(root_node_modules_dir_path),
-            })
-            .await?;
-          if exit_code != 0 {
-            anyhow::bail!(
-              "script '{}' in '{}' failed with exit code {}",
-              script_name,
-              package.id.nv,
-              exit_code,
-            );
-          }
-        }
-      }
-      fs::write(scripts_run_path, "")?;
-    }
-  }
-
   {
     let packages_with_deprecation_warnings =
       packages_with_deprecation_warnings.lock();
     if !packages_with_deprecation_warnings.is_empty() {
       log::warn!(
-        "{} Following packages are deprecated:",
+        "{} The following packages are deprecated:",
         colors::yellow("Warning")
       );
       let len = packages_with_deprecation_warnings.len();
@@ -870,7 +671,7 @@ async fn sync_resolution_with_fs(
           );
         } else {
           log::warn!(
-            "┗─ {}",
+            "┖─ {}",
             colors::gray(format!("npm:{:?} ({})", package_id, msg))
           );
         }
@@ -878,36 +679,13 @@ async fn sync_resolution_with_fs(
     }
   }
 
-  if !packages_with_scripts_not_run.is_empty() {
+  lifecycle_scripts
-    log::warn!("{} Following packages contained npm lifecycle scripts ({}) that were not executed:", colors::yellow("Warning"), colors::gray("preinstall/install/postinstall"));
+    .finish(
-
+      snapshot,
-    for (_, package_nv) in packages_with_scripts_not_run.iter() {
+      &package_partitions.packages,
-      log::warn!("┠─ {}", colors::gray(format!("npm:{package_nv}")));
+      Some(root_node_modules_dir_path),
-    }
+    )
-
+    .await?;
-    log::warn!("┃");
-    log::warn!(
-      "┠─ {}",
-      colors::italic("This may cause the packages to not work correctly.")
-    );
-    log::warn!("┗─ {}", colors::italic("To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:"));
-    let packages_comma_separated = packages_with_scripts_not_run
-      .iter()
-      .map(|(_, p)| format!("npm:{p}"))
-      .collect::<Vec<_>>()
-      .join(",");
-    log::warn!(
-      "   {}",
-      colors::bold(format!(
-        "deno install --allow-scripts={}",
-        packages_comma_separated
-      ))
-    );
-
-    for (scripts_warned_path, _) in packages_with_scripts_not_run {
-      let _ignore_err = fs::write(scripts_warned_path, "");
-    }
-  }
 
   setup_cache.save();
   drop(single_process_lock);
@@ -916,6 +694,98 @@ async fn sync_resolution_with_fs(
   Ok(())
 }
 
+/// `node_modules/.deno/<package>/`
+fn local_node_modules_package_folder(
+  local_registry_dir: &Path,
+  package: &NpmResolutionPackage,
+) -> PathBuf {
+  local_registry_dir.join(get_package_folder_id_folder_name(
+    &package.get_package_cache_folder_id(),
+  ))
+}
+
+struct LocalLifecycleScripts<'a> {
+  deno_local_registry_dir: &'a Path,
+}
+
+impl<'a> LocalLifecycleScripts<'a> {
+  /// `node_modules/.deno/<package>/.scripts-run`
+  fn ran_scripts_file(&self, package: &NpmResolutionPackage) -> PathBuf {
+    local_node_modules_package_folder(self.deno_local_registry_dir, package)
+      .join(".scripts-run")
+  }
+
+  /// `node_modules/.deno/<package>/.scripts-warned`
+  fn warned_scripts_file(&self, package: &NpmResolutionPackage) -> PathBuf {
+    local_node_modules_package_folder(self.deno_local_registry_dir, package)
+      .join(".scripts-warned")
+  }
+}
+
+impl<'a> super::common::lifecycle_scripts::LifecycleScriptsStrategy
+  for LocalLifecycleScripts<'a>
+{
+  fn package_path(&self, package: &NpmResolutionPackage) -> PathBuf {
+    local_node_modules_package_contents_path(
+      self.deno_local_registry_dir,
+      package,
+    )
+  }
+
+  fn did_run_scripts(
+    &self,
+    package: &NpmResolutionPackage,
+  ) -> std::result::Result<(), deno_core::anyhow::Error> {
+    std::fs::write(self.ran_scripts_file(package), "")?;
+    Ok(())
+  }
+
+  fn warn_on_scripts_not_run(
+    &self,
+    packages: &[(&NpmResolutionPackage, std::path::PathBuf)],
+  ) -> Result<(), AnyError> {
+    if !packages.is_empty() {
+      log::warn!("{} The following packages contained npm lifecycle scripts ({}) that were not executed:", colors::yellow("Warning"), colors::gray("preinstall/install/postinstall"));
+
+      for (package, _) in packages {
+        log::warn!("┠─ {}", colors::gray(format!("npm:{}", package.id.nv)));
+      }
+
+      log::warn!("┃");
+      log::warn!(
+        "┠─ {}",
+        colors::italic("This may cause the packages to not work correctly.")
+      );
+      log::warn!("┖─ {}", colors::italic("To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:"));
+      let packages_comma_separated = packages
+        .iter()
+        .map(|(p, _)| format!("npm:{}", p.id.nv))
+        .collect::<Vec<_>>()
+        .join(",");
+      log::warn!(
+        "   {}",
+        colors::bold(format!(
+          "deno install --allow-scripts={}",
+          packages_comma_separated
+        ))
+      );
+
+      for (package, _) in packages {
+        let _ignore_err = fs::write(self.warned_scripts_file(package), "");
+      }
+    }
+    Ok(())
+  }
+
+  fn has_warned(&self, package: &NpmResolutionPackage) -> bool {
+    self.warned_scripts_file(package).exists()
+  }
+
+  fn has_run(&self, package: &NpmResolutionPackage) -> bool {
+    self.ran_scripts_file(package).exists()
+  }
+}
+
 // Uses BTreeMap to preserve the ordering of the elements in memory, to ensure
 // the file generated from this datastructure is deterministic.
 // See: https://github.com/denoland/deno/issues/24479

cli/npm/managed/resolvers/mod.rs

@@ -54,6 +54,7 @@ pub fn create_npm_fs_resolver(
       tarball_cache,
       resolution,
       system_info,
+      lifecycle_scripts,
     )),
   }
 }

tests/specs/install/install_deprecated_package/install.out

@@ -2,5 +2,5 @@ Add npm:@denotest/deprecated-package@1.0.0
 Download http://localhost:4260/@denotest/deprecated-package
 Download http://localhost:4260/@denotest/deprecated-package/1.0.0.tgz
 Initialize @denotest/deprecated-package@1.0.0
-Warning Following packages are deprecated:
+Warning The following packages are deprecated:
-┗─ npm:@denotest/deprecated-package@1.0.0 (Deprecated version)
+┖─ npm:@denotest/deprecated-package@1.0.0 (Deprecated version)

tests/specs/npm/lifecycle_scripts/__test__.jsonc

@@ -1,7 +1,7 @@
 {
+  "tempDir": true,
   "tests": {
     "node_gyp": {
-      "tempDir": true,
       "steps": [
         {
           "args": "cache --allow-scripts=npm:@denotest/node-addon main.js",
@@ -14,7 +14,6 @@
       ]
     },
     "run_without_scripts": {
-      "tempDir": true,
       "steps": [
         {
           "args": "run -A main.js",
@@ -24,11 +23,9 @@
       ]
     },
     "implicit_node_gyp": {
-      "tempDir": true,
       "steps": [
         {
           "envs": {
-            // I don't think this will work on windows
             "PATH": ""
           },
           "args": "cache --allow-scripts implicit_node_gyp.js",
@@ -38,7 +35,6 @@
       ]
     },
     "lifecycle_scripts": {
-      "tempDir": true,
       "steps": [
         {
           // without running scripts (should warn)
@@ -55,8 +51,25 @@
         }
       ]
     },
+    "global_lifecycle_scripts": {
+      "steps": [
+        {
+          "args": ["eval", "Deno.removeSync('deno.json')"],
+          "output": ""
+        },
+        {
+          // without running scripts (should warn)
+          "args": "install -e all_lifecycles.js",
+          "output": "all_lifecycles_not_run_global.out"
+        },
+        {
+          // should not warn
+          "args": "install -e all_lifecycles.js",
+          "output": ""
+        }
+      ]
+    },
     "only_warns_first": {
-      "tempDir": true,
       "steps": [
         {
           // without running scripts (should warn)
@@ -96,7 +109,6 @@
       ]
     },
     "lifecycle_scripts_conflicting_bin": {
-      "tempDir": true,
       "steps": [
         {
           // we import @denotest/says-hello-on-install, which executes `say-hello` from `@denotest/say-hello` in its
@@ -110,7 +122,6 @@
       ]
     },
     "fsevents_default_install_script": {
-      "tempDir": true,
       "steps": [
         {
           "if": "mac",
@@ -125,7 +136,6 @@
       ]
     },
     "lifecycle_scripts_no_deno_json": {
-      "tempDir": true,
       "steps": [
         {
           "args": ["eval", "Deno.removeSync('deno.json')"],
@@ -138,7 +148,6 @@
       ]
     },
     "lifecycle_scripts_no_deno_json_conflicting_bin": {
-      "tempDir": true,
       "steps": [
         {
           "args": ["eval", "Deno.removeSync('deno.json')"],

tests/specs/npm/lifecycle_scripts/all_lifecycles_not_run.out

@@ -6,9 +6,9 @@ Download http://localhost:4260/@denotest/bin/1.0.0.tgz
 Initialize @denotest/node-lifecycle-scripts@1.0.0
 Initialize @denotest/bin@1.0.0
 [UNORDERED_END]
-Warning Following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:
+Warning The following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:
 ┠─ npm:@denotest/node-lifecycle-scripts@1.0.0
 ┃
 ┠─ This may cause the packages to not work correctly.
-┗─ To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:
+┖─ To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:
    deno install --allow-scripts=npm:@denotest/node-lifecycle-scripts@1.0.0

tests/specs/npm/lifecycle_scripts/all_lifecycles_not_run_global.out

@@ -0,0 +1,13 @@
+[UNORDERED_START]
+Download http://localhost:4260/@denotest/node-lifecycle-scripts
+Download http://localhost:4260/@denotest/bin
+Download http://localhost:4260/@denotest/node-lifecycle-scripts/1.0.0.tgz
+Download http://localhost:4260/@denotest/bin/1.0.0.tgz
+[UNORDERED_END]
+Warning The following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:
+┠─ npm:@denotest/node-lifecycle-scripts@1.0.0
+┃
+┠─ This may cause the packages to not work correctly.
+┠─ Lifecycle scripts are only supported when using a `node_modules` directory.
+┠─ Enable it in your deno config file:
+┖─ "nodeModulesDir": "auto"

tests/specs/npm/lifecycle_scripts/future_install_all_lifecycles_not_run.out

@@ -6,9 +6,9 @@ Download http://localhost:4260/@denotest/bin/1.0.0.tgz
 Initialize @denotest/node-lifecycle-scripts@1.0.0
 Initialize @denotest/bin@1.0.0
 [UNORDERED_END]
-Warning Following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:
+Warning The following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:
 ┠─ npm:@denotest/node-lifecycle-scripts@1.0.0
 ┃
 ┠─ This may cause the packages to not work correctly.
-┗─ To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:
+┖─ To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:
    deno install --allow-scripts=npm:@denotest/node-lifecycle-scripts@1.0.0

tests/specs/npm/lifecycle_scripts/node_gyp_not_found.out

@@ -6,3 +6,4 @@ Initialize @denotest/node-addon-implicit-node-gyp@1.0.0
 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
+error: failed to run scripts for packages: @denotest/node-addon-implicit-node-gyp@1.0.0

tests/specs/npm/lifecycle_scripts/node_gyp_not_run.out

@@ -1,11 +1,11 @@
 Download http://localhost:4260/@denotest/node-addon
 Download http://localhost:4260/node-gyp
 [WILDCARD]
-Warning Following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:
+Warning The following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:
 ┠─ npm:@denotest/node-addon@1.0.0
 ┃
 ┠─ This may cause the packages to not work correctly.
-┗─ To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:
+┖─ To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:
    deno install --allow-scripts=npm:@denotest/node-addon@1.0.0
 error: Uncaught (in promise) Error: Cannot find module './build/Release/node_addon'
 [WILDCARD]

tests/specs/npm/lifecycle_scripts/only_warns_first1.out

@@ -6,11 +6,11 @@ Download http://localhost:4260/@denotest/bin/1.0.0.tgz
 Initialize @denotest/node-lifecycle-scripts@1.0.0
 Initialize @denotest/bin@1.0.0
 [UNORDERED_END]
-Warning Following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:
+Warning The following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:
 ┠─ npm:@denotest/node-lifecycle-scripts@1.0.0
 ┃
 ┠─ This may cause the packages to not work correctly.
-┗─ To run lifecycle scripts, use the `--allow-scripts` flag with `deno install`:
+┖─ 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]