Skip to content

Commit

Permalink
fix(node): Ignore broken default install scripts (#24534)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nathanwhit committed Jul 11, 2024
1 parent ba63740 commit 3d0e1b6
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 6 deletions.
32 changes: 26 additions & 6 deletions cli/npm/managed/resolvers/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Binary file added tests/registry/npm/fsevents/fsevents-2.3.3.tgz
Binary file not shown.
1 change: 1 addition & 0 deletions tests/registry/npm/fsevents/registry.json

Large diffs are not rendered by default.

15 changes: 15 additions & 0 deletions tests/specs/npm/lifecycle_scripts/__test__.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -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": ""
}
]
}
}
}
1 change: 1 addition & 0 deletions tests/specs/npm/lifecycle_scripts/fsevents.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import {} from "npm:fsevents";
3 changes: 3 additions & 0 deletions tests/specs/npm/lifecycle_scripts/fsevents.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Download http://localhost:4260/fsevents
Download http://localhost:4260/fsevents/fsevents-2.3.3.tgz
Initialize [email protected]

0 comments on commit 3d0e1b6

Please sign in to comment.