Repository: denoland/deno
Pull request: fix(node): Ignore broken default install scripts

NPM inserts a default install script when a package has a `binding.gyp` file.

It's possible, however, for the package to exclude the `binding.gyp` file when they publish, and in this case the install script will never succeed for a user of the package.

This happens with `fsevents`, for instance. They don't include the `binding.gyp` file in their published tarball, but the default install script appears in the manifest served by `npm`.

This causes us to warn that `fsevents` has an install script, but when you try to run it it fails due to `binding.gyp` not existing.

Changed files:
filename: cli/npm/managed/resolvers/local.rs
patch: @@ -384,9 +384,24 @@ fn can_run_scripts(
   }
 }
 
-fn has_lifecycle_scripts(package: &NpmResolutionPackage) -> bool {
+// 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("install")
     || package.scripts.contains_key("postinstall")
 }
 
@@ -504,14 +519,14 @@ async fn sync_resolution_with_fs(
       });
     }
 
-    if has_lifecycle_scripts(package) {
+    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) {
       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() {
-          let sub_node_modules = folder_path.join("node_modules");
-          let package_path =
-            join_package_name(&sub_node_modules, &package.id.nv.name);
           packages_with_scripts.push((
             package.clone(),
             package_path,
@@ -685,6 +700,11 @@ async fn sync_resolution_with_fs(
       )?;
       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,

filename: tests/specs/npm/lifecycle_scripts/__test__.jsonc
patch: @@ -104,6 +104,21 @@
           "output": "conflicting_bin.out"
         }
       ]
+    },
+    "fsevents_default_install_script": {
+      "tempDir": true,
+      "steps": [
+        {
+          "if": "mac",
+          "args": "cache fsevents.js",
+          "output": "fsevents.out"
+        },
+        {
+          "if": "mac",
+          "args": "cache --allow-scripts fsevents.js",
+          "output": ""
+        }
+      ]
     }
   }
 }

filename: tests/specs/npm/lifecycle_scripts/fsevents.js
patch: @@ -0,0 +1 @@
+import {} from "npm:fsevents";

filename: tests/specs/npm/lifecycle_scripts/fsevents.out
patch: @@ -0,0 +1,3 @@
+Download http://localhost:4260/fsevents
+Download http://localhost:4260/fsevents/fsevents-2.3.3.tgz
+Initialize fsevents@2.3.3