diff --git a/pacquet/Cargo.lock b/pacquet/Cargo.lock index bcaf14ae26..149845f1e9 100644 --- a/pacquet/Cargo.lock +++ b/pacquet/Cargo.lock @@ -2072,6 +2072,18 @@ dependencies = [ "tokio", ] +[[package]] +name = "pacquet-package-is-installable" +version = "0.0.1" +dependencies = [ + "derive_more", + "miette 7.6.0", + "node-semver", + "pacquet-testing-utils", + "pretty_assertions", + "serde", +] + [[package]] name = "pacquet-package-manager" version = "0.0.1" @@ -2093,6 +2105,7 @@ dependencies = [ "pacquet-lockfile", "pacquet-modules-yaml", "pacquet-network", + "pacquet-package-is-installable", "pacquet-package-manifest", "pacquet-patching", "pacquet-registry", diff --git a/pacquet/Cargo.toml b/pacquet/Cargo.toml index 6eab2a0c7a..02eaa4051b 100644 --- a/pacquet/Cargo.toml +++ b/pacquet/Cargo.toml @@ -13,26 +13,27 @@ repository = "https://github.com/pnpm/pacquet" [workspace.dependencies] # Crates -pacquet-cli = { path = "crates/cli" } -pacquet-cmd-shim = { path = "crates/cmd-shim" } -pacquet-fs = { path = "crates/fs" } -pacquet-registry = { path = "crates/registry" } -pacquet-tarball = { path = "crates/tarball" } -pacquet-testing-utils = { path = "crates/testing-utils" } -pacquet-package-manifest = { path = "crates/package-manifest" } -pacquet-package-manager = { path = "crates/package-manager" } -pacquet-lockfile = { path = "crates/lockfile" } -pacquet-modules-yaml = { path = "crates/modules-yaml" } -pacquet-network = { path = "crates/network" } -pacquet-config = { path = "crates/config" } -pacquet-executor = { path = "crates/executor" } -pacquet-git-fetcher = { path = "crates/git-fetcher" } -pacquet-diagnostics = { path = "crates/diagnostics" } -pacquet-graph-hasher = { path = "crates/graph-hasher" } -pacquet-store-dir = { path = "crates/store-dir" } -pacquet-reporter = { path = "crates/reporter" } -pacquet-patching = { path = "crates/patching" } -pacquet-real-hoist = { path = "crates/real-hoist" } +pacquet-cli = { path = "crates/cli" } +pacquet-cmd-shim = { path = "crates/cmd-shim" } +pacquet-fs = { path = "crates/fs" } +pacquet-registry = { path = "crates/registry" } +pacquet-tarball = { path = "crates/tarball" } +pacquet-testing-utils = { path = "crates/testing-utils" } +pacquet-package-manifest = { path = "crates/package-manifest" } +pacquet-package-manager = { path = "crates/package-manager" } +pacquet-package-is-installable = { path = "crates/package-is-installable" } +pacquet-lockfile = { path = "crates/lockfile" } +pacquet-modules-yaml = { path = "crates/modules-yaml" } +pacquet-network = { path = "crates/network" } +pacquet-config = { path = "crates/config" } +pacquet-executor = { path = "crates/executor" } +pacquet-git-fetcher = { path = "crates/git-fetcher" } +pacquet-diagnostics = { path = "crates/diagnostics" } +pacquet-graph-hasher = { path = "crates/graph-hasher" } +pacquet-store-dir = { path = "crates/store-dir" } +pacquet-reporter = { path = "crates/reporter" } +pacquet-patching = { path = "crates/patching" } +pacquet-real-hoist = { path = "crates/real-hoist" } # Tasks pacquet-registry-mock = { path = "tasks/registry-mock" } diff --git a/pacquet/crates/graph-hasher/src/engine_name.rs b/pacquet/crates/graph-hasher/src/engine_name.rs index 04a6d1cffd..71c4c55993 100644 --- a/pacquet/crates/graph-hasher/src/engine_name.rs +++ b/pacquet/crates/graph-hasher/src/engine_name.rs @@ -52,12 +52,31 @@ pub fn engine_name(node_major: u32, platform: Option<&str>, arch: Option<&str>) /// won't match any pnpm-written entry — safe) or skip the /// cache-read entirely when this returns `None`. pub fn detect_node_major() -> Option { + let raw = detect_node_version_raw()?; + parse_node_version_output(&raw) +} + +/// Discover the host Node binary's full version by spawning +/// `node --version` and stripping the leading `v`. Returns the +/// trimmed semver-shaped string (e.g. `"22.11.0"`) or `None` if +/// detection fails for any of the reasons listed on +/// [`detect_node_major`]. +/// +/// Used by `pacquet-package-is-installable`'s `check_engine` to +/// evaluate `engines.node` ranges. Pacquet's installability check +/// needs the full version, not just the major, because ranges like +/// `>=14.18.0` would otherwise spuriously reject `14.17.x`. +pub fn detect_node_version() -> Option { + let raw = detect_node_version_raw()?; + Some(raw.strip_prefix('v').unwrap_or(&raw).to_string()) +} + +fn detect_node_version_raw() -> Option { let output = std::process::Command::new("node").arg("--version").output().ok()?; if !output.status.success() { return None; } - let stdout = std::str::from_utf8(&output.stdout).ok()?.trim(); - parse_node_version_output(stdout) + Some(std::str::from_utf8(&output.stdout).ok()?.trim().to_string()) } /// Parse `v22.11.0`-style output from `node --version` to the major @@ -76,7 +95,7 @@ fn parse_node_version_output(stdout: &str) -> Option { /// `sunos` / `aix` / `android`. Rust uses `macos` / `linux` / /// `windows` / `freebsd` / `openbsd` / `solaris` / `aix` / /// `android`. Only `macos`, `windows`, and `solaris` differ. -fn host_platform() -> &'static str { +pub fn host_platform() -> &'static str { match std::env::consts::OS { "macos" => "darwin", "windows" => "win32", @@ -93,7 +112,7 @@ fn host_platform() -> &'static str { /// what Node itself emits on each target — anything left as /// passthrough (e.g. `arm`, `s390x`, `riscv64`) already matches /// between the two naming schemes. -fn host_arch() -> &'static str { +pub fn host_arch() -> &'static str { match std::env::consts::ARCH { "x86_64" => "x64", "aarch64" => "arm64", @@ -107,6 +126,22 @@ fn host_arch() -> &'static str { } } +/// Host libc family, mapped to the same string `detect-libc.familySync()` +/// returns upstream. Pacquet currently always reports `"unknown"`, +/// which matches non-Linux hosts (and any Linux host where the +/// upstream detection failed). `check_platform` treats `"unknown"` +/// as "skip libc constraint" — see the call site at +/// . +/// +/// TODO: wire real detection (the `detect-libc` Rust crate, or a +/// custom `ldd`-based probe) in slice 2 alongside `supportedArchitectures` +/// — the libc constraint becomes load-bearing on Alpine / musl +/// hosts that today over-install glibc-only optional binary +/// packages. +pub fn host_libc() -> &'static str { + "unknown" +} + #[cfg(test)] mod tests { use super::{engine_name, parse_node_version_output}; diff --git a/pacquet/crates/graph-hasher/src/lib.rs b/pacquet/crates/graph-hasher/src/lib.rs index 2ce843ae9d..35c58c0aa7 100644 --- a/pacquet/crates/graph-hasher/src/lib.rs +++ b/pacquet/crates/graph-hasher/src/lib.rs @@ -19,7 +19,9 @@ mod global_virtual_store_path; mod object_hasher; pub use dep_state::{CalcDepStateOptions, DepsGraphNode, DepsStateCache, calc_dep_state}; -pub use engine_name::{detect_node_major, engine_name}; +pub use engine_name::{ + detect_node_major, detect_node_version, engine_name, host_arch, host_libc, host_platform, +}; pub use global_virtual_store_path::{calc_graph_node_hash, format_global_virtual_store_path}; pub use object_hasher::{hash_object, hash_object_with_encoding, hash_object_without_sorting}; diff --git a/pacquet/crates/package-is-installable/Cargo.toml b/pacquet/crates/package-is-installable/Cargo.toml new file mode 100644 index 0000000000..b694800802 --- /dev/null +++ b/pacquet/crates/package-is-installable/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "pacquet-package-is-installable" +version = "0.0.1" +publish = false +authors.workspace = true +description.workspace = true +edition.workspace = true +homepage.workspace = true +keywords.workspace = true +license.workspace = true +repository.workspace = true + +[dependencies] +derive_more = { workspace = true } +miette = { workspace = true } +node-semver = { workspace = true } +serde = { workspace = true } + +[dev-dependencies] +pacquet-testing-utils = { workspace = true } +pretty_assertions = { workspace = true } diff --git a/pacquet/crates/package-is-installable/src/check_engine.rs b/pacquet/crates/package-is-installable/src/check_engine.rs new file mode 100644 index 0000000000..be75f4e926 --- /dev/null +++ b/pacquet/crates/package-is-installable/src/check_engine.rs @@ -0,0 +1,202 @@ +//! Port of `checkEngine.ts` from +//! . + +use derive_more::{Display, Error}; +use miette::Diagnostic; +use node_semver::{Range, Version}; +use serde::Serialize; + +/// Wanted engine versions declared by a package's `engines` field. +/// +/// Both members are optional and carry npm-style range strings. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize)] +pub struct WantedEngine { + pub node: Option, + pub pnpm: Option, +} + +/// Current runtime engine versions. `node` is mandatory (no install +/// without a node version on PATH or in config), `pnpm` is optional +/// — pacquet itself is not pnpm, so callers normally pass `None` +/// here, which matches upstream's behavior of skipping the pnpm +/// check entirely when `currentEngine.pnpm` is unset. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct Engine { + pub node: String, + pub pnpm: Option, +} + +/// Error returned by [`check_engine`] when the runtime fails to +/// satisfy a wanted range. Wire-compatible with pnpm's +/// `ERR_PNPM_UNSUPPORTED_ENGINE` (same code, same message shape). +#[derive(Debug, Display, Error, Diagnostic, Clone, PartialEq, Eq)] +#[display("Unsupported engine for {package_id}: wanted: {wanted_json} (current: {current_json})")] +#[diagnostic(code(ERR_PNPM_UNSUPPORTED_ENGINE))] +pub struct UnsupportedEngineError { + pub package_id: String, + pub wanted: WantedEngine, + pub current: Engine, + wanted_json: String, + current_json: String, +} + +impl UnsupportedEngineError { + fn new(package_id: String, wanted: WantedEngine, current: Engine) -> Self { + let wanted_json = engine_json(&wanted.node, &wanted.pnpm); + let current_json = engine_json(&Some(current.node.clone()), ¤t.pnpm); + Self { package_id, wanted, current, wanted_json, current_json } + } +} + +fn engine_json(node: &Option, pnpm: &Option) -> String { + let mut parts = Vec::new(); + if let Some(n) = node { + parts.push(format!("\"node\":{n:?}")); + } + if let Some(p) = pnpm { + parts.push(format!("\"pnpm\":{p:?}")); + } + format!("{{{}}}", parts.join(",")) +} + +/// Thrown when the configured `nodeVersion` is not a valid exact +/// semver version. Mirrors pnpm's `ERR_PNPM_INVALID_NODE_VERSION` at +/// . +#[derive(Debug, Display, Error, Diagnostic, Clone, PartialEq, Eq)] +#[display("The nodeVersion setting is \"{node_version}\", which is not exact semver version")] +#[diagnostic(code(ERR_PNPM_INVALID_NODE_VERSION))] +pub struct InvalidNodeVersionError { + pub node_version: String, +} + +/// Evaluate a wanted `engines` block against the current engine. +/// +/// Returns: +/// - `Ok(None)`: the runtime satisfies every declared range. +/// - `Ok(Some(UnsupportedEngineError))`: at least one declared range +/// was not satisfied; the error lists only the unsatisfied entries +/// in its `wanted` field. +/// - `Err(InvalidNodeVersionError)`: the supplied `current.node` is +/// not a parseable exact semver (e.g. user passed a range like +/// `>=20.0.0` into the `nodeVersion` config). +/// +/// The semver `satisfies` call uses `includePrerelease: true` upstream +/// (so a `21.0.0-nightly...` host satisfies `^14.18.0 || >=16.0.0`). +/// `node-semver`'s Rust port doesn't expose that flag, so this port +/// approximates the behavior — see `satisfies_with_prerelease` in +/// this module for the strategy + known divergences. +pub fn check_engine( + package_id: &str, + wanted: &WantedEngine, + current: &Engine, +) -> Result, InvalidNodeVersionError> { + let mut unsatisfied = WantedEngine::default(); + + if let Some(wanted_node) = wanted.node.as_ref() { + match node_satisfies(¤t.node, wanted_node) { + // Range matched — nothing to do. + Ok(true) => {} + // `current.node` is a valid version but doesn't satisfy + // the range — record the unsatisfied wanted entry. + // `node_satisfies` already parsed it once, so we don't + // re-parse here. + Ok(false) => unsatisfied.node = Some(wanted_node.clone()), + // `current.node` is not a parseable exact semver — this + // is the `ERR_PNPM_INVALID_NODE_VERSION` path upstream + // throws from inside `checkEngine`. + Err(InvalidVersion) => { + return Err(InvalidNodeVersionError { node_version: current.node.clone() }); + } + } + } + + if let (Some(current_pnpm), Some(wanted_pnpm)) = (current.pnpm.as_ref(), wanted.pnpm.as_ref()) { + let satisfied = match Version::parse(current_pnpm) { + Ok(version) => satisfies_with_prerelease(&version, wanted_pnpm), + Err(_) => false, + }; + if !satisfied { + unsatisfied.pnpm = Some(wanted_pnpm.clone()); + } + } + + if unsatisfied.node.is_some() || unsatisfied.pnpm.is_some() { + return Ok(Some(UnsupportedEngineError::new( + package_id.to_string(), + unsatisfied, + current.clone(), + ))); + } + Ok(None) +} + +struct InvalidVersion; + +fn node_satisfies(current: &str, wanted: &str) -> Result { + let version = Version::parse(current).map_err(|_| InvalidVersion)?; + Ok(satisfies_with_prerelease(&version, wanted)) +} + +/// Approximation of npm-semver's `satisfies(version, range, { includePrerelease: true })`. +/// +/// `node-semver` (the Rust crate) doesn't expose `includePrerelease`; +/// its `Range::satisfies` enforces strict semver prerelease compat, +/// where a prerelease version only matches a range with an explicit +/// prerelease bound at the same major.minor.patch. Upstream pnpm +/// always wants prereleases to count for engine checks, so a +/// `21.0.0-nightly...` host should satisfy `>=16.0.0`. +/// +/// Strategy: +/// 1. Try the strict check first. Most callers ship release versions +/// and this is the correct, byte-for-byte path. +/// 2. If the strict check fails AND the version is a prerelease, +/// re-check with the prerelease stripped (i.e. `21.0.0-nightly` +/// becomes `21.0.0`). This permits a prerelease to satisfy any +/// range its semantically-equivalent release would. +/// +/// The fallback is a controlled over-acceptance: a prerelease +/// `X.Y.Z-rc1` is taken as `X.Y.Z` for the bounded comparison. The +/// realistic engine ranges that use just a major (`>=N`, `^N`, plain +/// `N`) all behave correctly because `node-semver` expands those to +/// a partial-version range that already accepts prereleases of N at +/// the lower bound. +/// +/// Two known divergences from upstream's `includePrerelease: true`: +/// +/// - `>=X.Y.Z` (strict lower bound, fully specified mmp): upstream +/// rejects `X.Y.Z-rc1` because alpha-class prereleases sort below +/// the corresponding release in semver, so `X.Y.Z-rc1 < X.Y.Z`. +/// Pacquet's strip turns the version into `X.Y.Z` which then +/// satisfies `>=X.Y.Z` — over-acceptance. Pinned by the integration +/// test `pnpm_is_a_prerelease_version_strict_ge_full_version_does_not_satisfy` +/// under `known_failures`. +/// - ` bool { + let Ok(range) = Range::parse(wanted_range) else { + // Match upstream `semver.satisfies` returning `false` for + // an unparsable range rather than throwing. + return false; + }; + if range.satisfies(version) { + return true; + } + if !version.pre_release.is_empty() { + let stripped = Version { + major: version.major, + minor: version.minor, + patch: version.patch, + pre_release: Vec::new(), + build: version.build.clone(), + }; + return range.satisfies(&stripped); + } + false +} diff --git a/pacquet/crates/package-is-installable/src/check_platform.rs b/pacquet/crates/package-is-installable/src/check_platform.rs new file mode 100644 index 0000000000..9c4c31c7e6 --- /dev/null +++ b/pacquet/crates/package-is-installable/src/check_platform.rs @@ -0,0 +1,238 @@ +//! Port of `checkPlatform.ts` from +//! . + +use derive_more::{Display, Error}; +use miette::Diagnostic; +use serde::{Deserialize, Serialize}; + +/// Caller-supplied override for the `os` / `cpu` / `libc` triples +/// against which a package's wanted platform is evaluated. Each list +/// defaults to `['current']` at the call site (upstream reads the +/// config setting and falls back to `['current']` if absent). The +/// `'current'` sentinel is replaced with the host triple via +/// `dedupe_current` before the `os` / `cpu` / `libc` lists are +/// compared. Mirrors upstream's `SupportedArchitectures` at +/// . +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize, Serialize)] +pub struct SupportedArchitectures { + #[serde(skip_serializing_if = "Option::is_none")] + pub os: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub cpu: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub libc: Option>, +} + +/// Wanted platform triple as declared by a package's manifest +/// (`os`, `cpu`, `libc`). Each is optional; absent means "any". +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize)] +pub struct WantedPlatform { + pub os: Option>, + pub cpu: Option>, + pub libc: Option>, +} + +/// Borrow-only view of [`WantedPlatform`] used by [`check_platform`] +/// on the install hot path. Lets the caller pass `manifest.os.as_deref()` +/// (etc.) directly without cloning the manifest's owned `Vec`s +/// into a fresh `WantedPlatform` per snapshot. The owned form is +/// only materialised inside `check_platform` when an error is +/// produced (for diagnostic display). +/// +/// `Copy` so the recursive / per-snapshot call sites don't need an +/// extra reference layer; all three fields are already references. +#[derive(Debug, Clone, Copy, Default)] +pub struct WantedPlatformRef<'a> { + pub os: Option<&'a [String]>, + pub cpu: Option<&'a [String]>, + pub libc: Option<&'a [String]>, +} + +/// Current host platform triple, as reported (or overridden by +/// `supported_architectures`). +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct Platform { + pub os: Vec, + pub cpu: Vec, + pub libc: Vec, +} + +/// Error returned by [`check_platform`] when no entry in the host +/// triple satisfies a wanted list. Wire-compatible with pnpm's +/// `ERR_PNPM_UNSUPPORTED_PLATFORM` (same code, same message shape). +#[derive(Debug, Display, Error, Diagnostic, Clone, PartialEq, Eq)] +#[display("Unsupported platform for {package_id}: wanted {wanted_json} (current: {current_json})")] +#[diagnostic(code(ERR_PNPM_UNSUPPORTED_PLATFORM))] +pub struct UnsupportedPlatformError { + pub package_id: String, + pub wanted: WantedPlatform, + pub current: Platform, + wanted_json: String, + current_json: String, +} + +impl UnsupportedPlatformError { + fn new(package_id: String, wanted: WantedPlatform, current: Platform) -> Self { + let wanted_json = wanted_json(&wanted); + let current_json = current_json(¤t); + Self { package_id, wanted, current, wanted_json, current_json } + } +} + +fn wanted_json(w: &WantedPlatform) -> String { + // Mirror upstream's `JSON.stringify(wanted)` shape: only the + // fields actually set appear, and each is a JSON array. + let mut parts = Vec::new(); + if let Some(os) = &w.os { + parts.push(format!("\"os\":{}", json_string_array(os))); + } + if let Some(cpu) = &w.cpu { + parts.push(format!("\"cpu\":{}", json_string_array(cpu))); + } + if let Some(libc) = &w.libc { + parts.push(format!("\"libc\":{}", json_string_array(libc))); + } + format!("{{{}}}", parts.join(",")) +} + +fn current_json(c: &Platform) -> String { + // Upstream constructs `{ os: platform, cpu: arch, libc: currentLibc }` + // (single strings, not arrays). Mirror that shape. + fn single(values: &[String]) -> String { + values.first().cloned().unwrap_or_default() + } + format!( + "{{\"os\":{:?},\"cpu\":{:?},\"libc\":{:?}}}", + single(&c.os), + single(&c.cpu), + single(&c.libc), + ) +} + +fn json_string_array(values: &[String]) -> String { + let joined: Vec = values.iter().map(|s| format!("{s:?}")).collect(); + format!("[{}]", joined.join(",")) +} + +/// Evaluate a package's `os` / `cpu` / `libc` against the host. +/// +/// Returns `None` when the package is compatible, or +/// `Some(UnsupportedPlatformError)` when any constraint rejects the +/// host. Negation entries (`!foo`) and the special `any` sentinel are +/// honored exactly as upstream's `checkList`. +/// +/// The wanted axes are taken as `Option<&[String]>` slices so the +/// hot path doesn't allocate a `WantedPlatform` per snapshot — +/// callers can pass `manifest.os.as_deref()` directly. The owned +/// [`WantedPlatform`] form is only built when an error is returned +/// (for diagnostic display via the +/// [`UnsupportedPlatformError`]). +/// +/// `supported_architectures` substitutes for `['current']` per axis; +/// `'current'` entries are replaced with the host value before +/// comparison (see `dedupe_current` in this module), matching pnpm +/// at . +/// +/// `current_os`, `current_cpu`, and `current_libc` are passed in +/// rather than read from the environment so this function stays +/// trivially testable (and the upstream tests that mock `process.platform` +/// translate directly). +pub fn check_platform( + package_id: &str, + wanted: WantedPlatformRef<'_>, + supported: Option<&SupportedArchitectures>, + current_os: &str, + current_cpu: &str, + current_libc: &str, +) -> Option { + let default_current = vec!["current".to_string()]; + let os_supp = supported.and_then(|s| s.os.as_ref()).unwrap_or(&default_current); + let cpu_supp = supported.and_then(|s| s.cpu.as_ref()).unwrap_or(&default_current); + let libc_supp = supported.and_then(|s| s.libc.as_ref()).unwrap_or(&default_current); + + let current = Platform { + os: dedupe_current(current_os, os_supp), + cpu: dedupe_current(current_cpu, cpu_supp), + libc: dedupe_current(current_libc, libc_supp), + }; + + let mut os_ok = true; + let mut cpu_ok = true; + let mut libc_ok = true; + + if let Some(wanted_os) = wanted.os { + os_ok = check_list(¤t.os, wanted_os); + } + if let Some(wanted_cpu) = wanted.cpu { + cpu_ok = check_list(¤t.cpu, wanted_cpu); + } + // Upstream skips the libc check when the host returned 'unknown' + // from `detect-libc.familySync()`. Mirror that — non-Linux hosts + // (and any Linux host where detection failed) bypass libc. + if let Some(wanted_libc) = wanted.libc + && current_libc != "unknown" + { + libc_ok = check_list(¤t.libc, wanted_libc); + } + + if !os_ok || !cpu_ok || !libc_ok { + // Cold path. Only here do we materialise the owned + // `WantedPlatform` for the error payload — the rest of the + // pass borrows. + let owned_wanted = WantedPlatform { + os: wanted.os.map(<[String]>::to_vec), + cpu: wanted.cpu.map(<[String]>::to_vec), + libc: wanted.libc.map(<[String]>::to_vec), + }; + let real_current = Platform { + os: vec![current_os.to_string()], + cpu: vec![current_cpu.to_string()], + libc: vec![current_libc.to_string()], + }; + return Some(UnsupportedPlatformError::new( + package_id.to_string(), + owned_wanted, + real_current, + )); + } + None +} + +/// Replace the literal `current` sentinel in `supported` with the +/// concrete host value. Ports upstream's `dedupeCurrent` at +/// . +fn dedupe_current(current: &str, supported: &[String]) -> Vec { + supported.iter().map(|s| if s == "current" { current.to_string() } else { s.clone() }).collect() +} + +/// Decide whether any element of `value` is allowed by `list`. +/// +/// Ports `checkList` at +/// . +/// +/// Semantics: +/// - `list == ["any"]`: always accept. +/// - A `"!foo"` entry rejects when any element of `value` equals +/// `foo` (negation short-circuits to "fail"). +/// - A bare `"foo"` entry accepts when any element of `value` equals +/// `foo`. +/// - Empty positive set with all negations passing: accept (matches +/// upstream's `match || list.every(entry => entry[0] === '!')`). +fn check_list(value: &[String], list: &[String]) -> bool { + if list.len() == 1 && list[0] == "any" { + return true; + } + let mut matched = false; + for v in value { + for entry in list { + if let Some(stripped) = entry.strip_prefix('!') { + if stripped == v { + return false; + } + } else if entry == v { + matched = true; + } + } + } + matched || list.iter().all(|e| e.starts_with('!')) +} diff --git a/pacquet/crates/package-is-installable/src/lib.rs b/pacquet/crates/package-is-installable/src/lib.rs new file mode 100644 index 0000000000..cef5e3b1e5 --- /dev/null +++ b/pacquet/crates/package-is-installable/src/lib.rs @@ -0,0 +1,36 @@ +//! Port of `@pnpm/config.package-is-installable` from upstream pnpm. +//! +//! Mirrors the entry point and the two checker helpers +//! ([`check_engine()`], [`check_platform()`]) at +//! . +//! +//! Three exported functions: +//! - [`check_engine()`] — evaluates `engines.node` / `engines.pnpm` against +//! the current runtime. +//! - [`check_platform()`] — evaluates a package's `os` / `cpu` / `libc` +//! triple against the host (or a caller-supplied +//! [`SupportedArchitectures`] override). +//! - [`package_is_installable()`] — composes the two and produces a +//! tri-state verdict matching upstream's `boolean | null` return: +//! compatible, skip-as-optional, or proceed-with-warning. Caller +//! handles emitting `pnpm:install-check` and +//! `pnpm:skipped-optional-dependency` events. + +mod check_engine; +mod check_platform; +mod package_is_installable; + +#[cfg(test)] +mod tests; + +pub use check_engine::{ + Engine, InvalidNodeVersionError, UnsupportedEngineError, WantedEngine, check_engine, +}; +pub use check_platform::{ + Platform, SupportedArchitectures, UnsupportedPlatformError, WantedPlatform, WantedPlatformRef, + check_platform, +}; +pub use package_is_installable::{ + InstallabilityError, InstallabilityOptions, InstallabilityVerdict, + PackageInstallabilityManifest, SkipReason, check_package, package_is_installable, +}; diff --git a/pacquet/crates/package-is-installable/src/package_is_installable.rs b/pacquet/crates/package-is-installable/src/package_is_installable.rs new file mode 100644 index 0000000000..a23f5bfc55 --- /dev/null +++ b/pacquet/crates/package-is-installable/src/package_is_installable.rs @@ -0,0 +1,230 @@ +//! Port of `packageIsInstallable` from +//! . + +use derive_more::{Display, Error}; +use miette::Diagnostic; +use serde::Serialize; + +use crate::check_engine::{ + Engine, InvalidNodeVersionError, UnsupportedEngineError, WantedEngine, check_engine, +}; +use crate::check_platform::{ + SupportedArchitectures, UnsupportedPlatformError, WantedPlatformRef, check_platform, +}; + +/// Inputs from a package manifest (or lockfile metadata row) that +/// drive the installability check. +#[derive(Debug, Clone, Default)] +pub struct PackageInstallabilityManifest { + pub engines: Option, + pub cpu: Option>, + pub os: Option>, + pub libc: Option>, +} + +/// Discriminator on `pnpm:skipped-optional-dependency` payloads. +/// Matches upstream's `'unsupported_engine' | 'unsupported_platform'` +/// pair at +/// . +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum SkipReason { + UnsupportedEngine, + UnsupportedPlatform, +} + +/// Errors [`package_is_installable`] can surface. The first two +/// variants mirror upstream's `UnsupportedEngineError | +/// UnsupportedPlatformError` from `index.ts:81`. The third propagates +/// `checkEngine`'s `ERR_PNPM_INVALID_NODE_VERSION` throw at +/// +/// so callers keep the upstream error code instead of seeing it +/// collapsed into a misleading engine-mismatch error. +#[derive(Debug, Display, Error, Diagnostic, Clone, PartialEq, Eq)] +pub enum InstallabilityError { + #[display("{_0}")] + #[diagnostic(transparent)] + Engine(UnsupportedEngineError), + #[display("{_0}")] + #[diagnostic(transparent)] + Platform(UnsupportedPlatformError), + #[display("{_0}")] + #[diagnostic(transparent)] + InvalidNodeVersion(InvalidNodeVersionError), +} + +impl InstallabilityError { + /// Map the wrapped error variant to its `pnpm:skipped-optional-dependency` + /// reason, matching `index.ts:57`'s `'unsupported_engine' | + /// 'unsupported_platform'` ternary. + /// + /// `InvalidNodeVersion` is treated as an engine-class skip + /// because upstream's `checkEngine` throws it from inside the + /// engine evaluation; the reason discriminator on a + /// `pnpm:skipped-optional-dependency` event for an invalid node + /// version is therefore `unsupported_engine`, even though the + /// underlying error code is `ERR_PNPM_INVALID_NODE_VERSION`. + pub fn skip_reason(&self) -> SkipReason { + match self { + Self::Engine(_) | Self::InvalidNodeVersion(_) => SkipReason::UnsupportedEngine, + Self::Platform(_) => SkipReason::UnsupportedPlatform, + } + } +} + +/// Tri-state verdict mirroring upstream's `boolean | null` return at +/// `index.ts:38`. Returned by [`package_is_installable`]. +/// +/// - [`InstallabilityVerdict::Installable`]: maps to upstream `true`. +/// No warning, no skip, just install. +/// - [`InstallabilityVerdict::SkipOptional`]: maps to upstream `false`. +/// The package is incompatible and was declared optional; caller +/// should emit `pnpm:skipped-optional-dependency` and exclude the +/// package from the install set. +/// - [`InstallabilityVerdict::ProceedWithWarning`]: maps to upstream +/// `null`. The package is incompatible, not optional, and +/// `engineStrict` is off; caller emits `pnpm:install-check` warn +/// (or a tracing-level warning) and proceeds. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum InstallabilityVerdict { + Installable, + SkipOptional { + reason: SkipReason, + /// Details string the caller copies into the + /// `pnpm:skipped-optional-dependency` payload's `details` + /// field. Matches upstream `warn.toString()` at `index.ts:50`. + details: String, + }, + ProceedWithWarning { + /// Message body for the `pnpm:install-check` warn. Matches + /// upstream `warn.message` at `index.ts:44`. + message: String, + }, +} + +/// Options threaded into [`package_is_installable`] / [`check_package`]. +/// +/// `current_node_version` mirrors pnpm's `nodeVersion` config setting: +/// if present and parseable, it's used as the current node version; +/// if absent, the caller passes the actual runtime's version. +/// `pnpm_version` is normally `None` for pacquet (pacquet isn't pnpm); +/// upstream sets this from `getSystemPnpmVersion()` or a config +/// override. `engine_strict` defaults to false (pnpm's default at +/// ), +/// and `supported_architectures` is read from `pnpm-workspace.yaml` +/// when present. +/// +/// All string fields borrow so a caller running through many snapshots +/// in a row can build the host-derived part of the struct once and +/// only toggle `optional` per snapshot. +#[derive(Debug, Clone, Copy, Default)] +pub struct InstallabilityOptions<'a> { + pub engine_strict: bool, + pub optional: bool, + pub current_node_version: &'a str, + pub pnpm_version: Option<&'a str>, + pub current_os: &'a str, + pub current_cpu: &'a str, + pub current_libc: &'a str, + pub supported_architectures: Option<&'a SupportedArchitectures>, +} + +/// Pure compose of [`check_platform`] and [`check_engine`]. Returns +/// the first error a manifest produces, or `None` if compatible. +/// Mirrors upstream `checkPackage` at +/// . +/// +/// Platform is checked first (so an unsupported OS surfaces as a +/// `Platform` error even if the engine range would also reject). +pub fn check_package( + package_id: &str, + manifest: &PackageInstallabilityManifest, + options: &InstallabilityOptions<'_>, +) -> Result, InvalidNodeVersionError> { + // Pacquet-only optimization (still wire-compatible with upstream): + // upstream's `index.ts:82-86` defaults each absent platform axis + // to `['any']` before passing it down to `checkPlatform`. That + // shape is functionally equivalent to "no constraint", since + // `checkList` short-circuits a single-element `['any']` to + // accept. Pacquet's `check_platform` already skips an axis when + // the wanted slice is `None`, so we pass the manifest fields by + // reference straight through — no `vec!["any".to_string()]` per + // axis, no `WantedPlatform { ... }` clone of the manifest's + // owned vectors. The owned `WantedPlatform` only materialises + // inside `check_platform` when an error is returned. + let wanted = WantedPlatformRef { + os: manifest.os.as_deref(), + cpu: manifest.cpu.as_deref(), + libc: manifest.libc.as_deref(), + }; + if let Some(platform_err) = check_platform( + package_id, + wanted, + options.supported_architectures, + options.current_os, + options.current_cpu, + options.current_libc, + ) { + return Ok(Some(InstallabilityError::Platform(platform_err))); + } + + let Some(wanted_engines) = manifest.engines.as_ref() else { + return Ok(None); + }; + + let current = Engine { + node: options.current_node_version.to_string(), + pnpm: options.pnpm_version.map(str::to_string), + }; + match check_engine(package_id, wanted_engines, ¤t)? { + Some(engine_err) => Ok(Some(InstallabilityError::Engine(engine_err))), + None => Ok(None), + } +} + +/// Tri-state installability verdict, mirroring upstream +/// `packageIsInstallable` at +/// . +/// +/// Side effects (the `pnpm:install-check` warn and +/// `pnpm:skipped-optional-dependency` emit) are *not* performed here +/// — the caller composes them so log payloads can carry pacquet- +/// specific context (`prefix`, `requester`, etc.). +/// +/// `InstallabilityError` is large (200+ bytes) because it carries the +/// full wanted/current platform or engine state for diagnostic +/// rendering. Boxing the `Err` arm keeps `Result<_, _>` small enough +/// for clippy's `result-large-err` lint on installs where the error +/// path is rare. +pub fn package_is_installable( + package_id: &str, + manifest: &PackageInstallabilityManifest, + options: &InstallabilityOptions<'_>, +) -> Result> { + let warn = match check_package(package_id, manifest, options) { + Ok(maybe) => maybe, + Err(invalid_node) => { + // Upstream `checkEngine` `throw`s on invalid node version + // regardless of `engineStrict`. Surface as the dedicated + // `InvalidNodeVersion` variant so callers keep the + // `ERR_PNPM_INVALID_NODE_VERSION` code and message + // upstream uses, rather than a synthesized engine + // mismatch. + return Err(Box::new(InstallabilityError::InvalidNodeVersion(invalid_node))); + } + }; + let Some(warn) = warn else { return Ok(InstallabilityVerdict::Installable) }; + + if options.optional { + return Ok(InstallabilityVerdict::SkipOptional { + reason: warn.skip_reason(), + details: warn.to_string(), + }); + } + + if options.engine_strict { + return Err(Box::new(warn)); + } + + Ok(InstallabilityVerdict::ProceedWithWarning { message: warn.to_string() }) +} diff --git a/pacquet/crates/package-is-installable/src/tests.rs b/pacquet/crates/package-is-installable/src/tests.rs new file mode 100644 index 0000000000..4322154ac8 --- /dev/null +++ b/pacquet/crates/package-is-installable/src/tests.rs @@ -0,0 +1,9 @@ +//! Ports of upstream's unit tests: +//! - `config/package-is-installable/test/checkPlatform.ts` +//! - `config/package-is-installable/test/checkEngine.ts` +//! both at +//! . + +mod check_engine; +mod check_platform; +mod package_is_installable; diff --git a/pacquet/crates/package-is-installable/src/tests/check_engine.rs b/pacquet/crates/package-is-installable/src/tests/check_engine.rs new file mode 100644 index 0000000000..7b918a7b4d --- /dev/null +++ b/pacquet/crates/package-is-installable/src/tests/check_engine.rs @@ -0,0 +1,119 @@ +//! Port of `config/package-is-installable/test/checkEngine.ts` +//! at . + +use crate::{Engine, WantedEngine, check_engine}; + +const PACKAGE_ID: &str = "registry.npmjs.org/foo/1.0.0"; + +fn current(node: &str, pnpm: Option<&str>) -> Engine { + Engine { node: node.to_string(), pnpm: pnpm.map(str::to_string) } +} + +fn wanted(node: Option<&str>, pnpm: Option<&str>) -> WantedEngine { + WantedEngine { node: node.map(str::to_string), pnpm: pnpm.map(str::to_string) } +} + +#[test] +fn no_engine_defined() { + assert!( + check_engine(PACKAGE_ID, &wanted(None, None), ¤t("0.2.1", Some("1.1.2"))) + .expect("valid node version") + .is_none(), + ); +} + +#[test] +fn prerelease_node_version() { + assert!( + check_engine( + PACKAGE_ID, + &wanted(Some("^14.18.0 || >=16.0.0"), None), + ¤t("21.0.0-nightly20230429c968361829", None), + ) + .expect("valid node version") + .is_none(), + ); +} + +#[test] +fn node_version_too_old() { + let err = check_engine( + PACKAGE_ID, + &wanted(Some("0.10.24"), None), + ¤t("0.10.18", Some("1.1.2")), + ) + .expect("valid node version") + .expect("must report unsatisfied"); + assert_eq!(err.wanted.node.as_deref(), Some("0.10.24")); +} + +#[test] +fn node_range_passed_in_instead_of_version() { + // Upstream throws `ERR_PNPM_INVALID_NODE_VERSION` from inside + // `checkEngine` when the current node version isn't an exact + // semver. Pacquet returns the same condition as an `Err`. + let result = + check_engine(PACKAGE_ID, &wanted(Some("21.0.0"), None), ¤t(">=20.0.0", None)); + let err = result.expect_err("expected InvalidNodeVersionError"); + assert_eq!(err.node_version, ">=20.0.0"); +} + +#[test] +fn pnpm_version_too_old() { + let err = + check_engine(PACKAGE_ID, &wanted(None, Some("^1.4.6")), ¤t("0.2.1", Some("1.3.2"))) + .expect("valid node version") + .expect("must report unsatisfied"); + assert_eq!(err.wanted.pnpm.as_deref(), Some("^1.4.6")); +} + +#[test] +fn pnpm_is_a_prerelease_version_partial_major_only_satisfies() { + // `pnpm: '9'` matches `9.0.0-alpha.1` under upstream + // `includePrerelease: true`. Pacquet's strip-prerelease fallback + // gets this correctly because a partial major range parses as + // `>=9.0.0 <10.0.0-0`, and `9.0.0` satisfies. + assert!( + check_engine( + PACKAGE_ID, + &wanted(None, Some("9")), + ¤t("0.2.1", Some("9.0.0-alpha.1")) + ) + .expect("valid node version") + .is_none(), + ); +} + +#[test] +fn pnpm_is_a_prerelease_version_ge_major_satisfies() { + assert!( + check_engine( + PACKAGE_ID, + &wanted(None, Some(">=9")), + ¤t("0.2.1", Some("9.0.0-alpha.1")), + ) + .expect("valid node version") + .is_none(), + ); +} + +#[test] +fn engine_is_supported() { + assert!( + check_engine( + PACKAGE_ID, + &wanted(Some("10"), Some("1")), + ¤t("10.2.1", Some("1.3.2")), + ) + .expect("valid node version") + .is_none(), + ); +} + +// The strict-upper-bound prerelease semantics case lives in +// `tests/known_failures.rs` as an integration test so `just +// known-failures` (filter `^known_failures::`) picks it up. The +// helper expects the `known_failures` module to sit at the top of a +// test binary's path; a unit-test submodule would land at +// `tests::check_engine::known_failures::...` and fall outside that +// filter. diff --git a/pacquet/crates/package-is-installable/src/tests/check_platform.rs b/pacquet/crates/package-is-installable/src/tests/check_platform.rs new file mode 100644 index 0000000000..eb829ad946 --- /dev/null +++ b/pacquet/crates/package-is-installable/src/tests/check_platform.rs @@ -0,0 +1,179 @@ +//! Port of `config/package-is-installable/test/checkPlatform.ts` +//! at . +//! +//! The upstream tests mock `process.platform` / `process.arch` / +//! `detect-libc.familySync`. Pacquet's [`check_platform`] takes the +//! three values as parameters, so the ports pass them explicitly +//! rather than mutating any global state. + +use crate::{ + SupportedArchitectures, UnsupportedPlatformError, WantedPlatform, WantedPlatformRef, + check_platform, +}; + +const PACKAGE_ID: &str = "registry.npmjs.org/foo/1.0.0"; +const FAKE_LINUX: &str = "linux"; +const FAKE_X64: &str = "x64"; +const FAKE_MUSL: &str = "musl"; + +fn wanted(os: Option<&[&str]>, cpu: Option<&[&str]>, libc: Option<&[&str]>) -> WantedPlatform { + fn vec_opt(v: Option<&[&str]>) -> Option> { + v.map(|s| s.iter().map(|x| (*x).to_string()).collect()) + } + WantedPlatform { os: vec_opt(os), cpu: vec_opt(cpu), libc: vec_opt(libc) } +} + +/// Test-local convenience wrapper. The runtime `check_platform` takes +/// the wanted axes as `Option<&[String]>` slices so the install hot +/// path doesn't have to construct a `WantedPlatform` per snapshot; +/// the tests find it more ergonomic to build one and pass it by +/// reference, so this wrapper does the `.as_deref()` for each axis +/// in one place. +fn check_platform_w( + pkg: &str, + w: &WantedPlatform, + supp: Option<&SupportedArchitectures>, + os: &str, + cpu: &str, + libc: &str, +) -> Option { + let wanted = + WantedPlatformRef { os: w.os.as_deref(), cpu: w.cpu.as_deref(), libc: w.libc.as_deref() }; + check_platform(pkg, wanted, supp, os, cpu, libc) +} + +fn supported( + os: Option<&[&str]>, + cpu: Option<&[&str]>, + libc: Option<&[&str]>, +) -> SupportedArchitectures { + fn vec_opt(v: Option<&[&str]>) -> Option> { + v.map(|s| s.iter().map(|x| (*x).to_string()).collect()) + } + SupportedArchitectures { os: vec_opt(os), cpu: vec_opt(cpu), libc: vec_opt(libc) } +} + +#[test] +fn target_cpu_wrong() { + let w = wanted(Some(&["any"]), Some(&["enten-cpu"]), Some(&["any"])); + let err = check_platform_w(PACKAGE_ID, &w, None, FAKE_LINUX, FAKE_X64, FAKE_MUSL); + assert!(err.is_some()); +} + +#[test] +fn os_wrong() { + let w = wanted(Some(&["enten-os"]), Some(&["any"]), Some(&["any"])); + let err = check_platform_w(PACKAGE_ID, &w, None, FAKE_LINUX, FAKE_X64, FAKE_MUSL); + assert!(err.is_some()); +} + +#[test] +fn libc_wrong() { + let w = wanted(Some(&["any"]), Some(&["any"]), Some(&["enten-libc"])); + let err = check_platform_w(PACKAGE_ID, &w, None, FAKE_LINUX, FAKE_X64, FAKE_MUSL); + assert!(err.is_some()); +} + +#[test] +fn nothing_wrong() { + let w = wanted(Some(&["any"]), Some(&["any"]), Some(&["any"])); + let err = check_platform_w(PACKAGE_ID, &w, None, FAKE_LINUX, FAKE_X64, FAKE_MUSL); + assert!(err.is_none()); +} + +#[test] +fn everything_wrong_with_arrays() { + let w = wanted(Some(&["enten-os"]), Some(&["enten-cpu"]), Some(&["enten-libc"])); + let err = check_platform_w(PACKAGE_ID, &w, None, FAKE_LINUX, FAKE_X64, FAKE_MUSL); + assert!(err.is_some()); +} + +#[test] +fn os_wrong_negation() { + let w = wanted(Some(&["!linux"]), Some(&["any"]), Some(&["any"])); + let err = check_platform_w(PACKAGE_ID, &w, None, FAKE_LINUX, FAKE_X64, FAKE_MUSL); + assert!(err.is_some()); +} + +#[test] +fn nothing_wrong_negation() { + let w = wanted(Some(&["!enten-os"]), Some(&["!enten-cpu"]), Some(&["!enten-libc"])); + let err = check_platform_w(PACKAGE_ID, &w, None, FAKE_LINUX, FAKE_X64, FAKE_MUSL); + assert!(err.is_none()); +} + +#[test] +fn override_os() { + let s = supported(Some(&["win32"]), Some(&["current"]), Some(&["current"])); + let w = wanted(Some(&["win32"]), Some(&["any"]), Some(&["any"])); + let err = check_platform_w(PACKAGE_ID, &w, Some(&s), FAKE_LINUX, FAKE_X64, FAKE_MUSL); + assert!(err.is_none()); +} + +#[test] +fn accept_another_cpu() { + let s = supported(Some(&["current"]), Some(&["current", "x64"]), Some(&["current"])); + let w = wanted(Some(&["any"]), Some(&["x64"]), Some(&["any"])); + // Host arch is arm64, but `supported.cpu` adds `x64`. + let err = check_platform_w(PACKAGE_ID, &w, Some(&s), FAKE_LINUX, "arm64", FAKE_MUSL); + assert!(err.is_none()); +} + +#[test] +fn fail_when_cpu_is_different() { + let s = supported(Some(&["current"]), Some(&["arm64"]), Some(&["current"])); + let w = wanted(Some(&["any"]), Some(&["x64"]), Some(&["any"])); + let err = check_platform_w(PACKAGE_ID, &w, Some(&s), FAKE_LINUX, FAKE_X64, FAKE_MUSL); + assert!(err.is_some()); +} + +#[test] +fn override_libc() { + let s = supported(Some(&["current"]), Some(&["current"]), Some(&["glibc"])); + let w = wanted(Some(&["any"]), Some(&["any"]), Some(&["glibc"])); + let err = check_platform_w(PACKAGE_ID, &w, Some(&s), FAKE_LINUX, FAKE_X64, FAKE_MUSL); + assert!(err.is_none()); +} + +#[test] +fn accept_another_libc() { + let s = supported(Some(&["current"]), Some(&["current"]), Some(&["current", "glibc"])); + let w = wanted(Some(&["any"]), Some(&["any"]), Some(&["glibc"])); + let err = check_platform_w(PACKAGE_ID, &w, Some(&s), FAKE_LINUX, FAKE_X64, FAKE_MUSL); + assert!(err.is_none()); +} + +#[test] +fn accept_negated_os_with_multi_valued_supported() { + let s = supported(Some(&["linux", "current"]), Some(&["current"]), Some(&["current"])); + let w = wanted(Some(&["!win32"]), Some(&["any"]), Some(&["any"])); + let err = check_platform_w(PACKAGE_ID, &w, Some(&s), FAKE_LINUX, FAKE_X64, FAKE_MUSL); + assert!(err.is_none()); +} + +#[test] +fn accept_negated_cpu_with_multi_valued_supported() { + let s = supported(Some(&["current"]), Some(&["x64", "current"]), Some(&["current"])); + let w = wanted(Some(&["any"]), Some(&["!ia32"]), Some(&["any"])); + let err = check_platform_w(PACKAGE_ID, &w, Some(&s), FAKE_LINUX, FAKE_X64, FAKE_MUSL); + assert!(err.is_none()); +} + +#[test] +fn reject_negated_os_when_any_supported_value_matches_negation() { + let s = supported(Some(&["win32", "current"]), Some(&["current"]), Some(&["current"])); + let w = wanted(Some(&["!win32"]), Some(&["any"]), Some(&["any"])); + let err = check_platform_w(PACKAGE_ID, &w, Some(&s), "darwin", FAKE_X64, FAKE_MUSL); + assert!(err.is_some()); +} + +#[test] +fn libc_check_skipped_when_current_libc_is_unknown() { + // Pacquet-specific port of upstream's `currentLibc !== 'unknown'` + // guard. On non-Linux hosts (or any host where libc detection + // failed), libc constraints are unconditionally satisfied — + // mirrors `checkPlatform.ts:38`. + let w = wanted(Some(&["any"]), Some(&["any"]), Some(&["glibc"])); + let err = check_platform_w(PACKAGE_ID, &w, None, "darwin", FAKE_X64, "unknown"); + assert!(err.is_none()); +} diff --git a/pacquet/crates/package-is-installable/src/tests/package_is_installable.rs b/pacquet/crates/package-is-installable/src/tests/package_is_installable.rs new file mode 100644 index 0000000000..f6ae33f78e --- /dev/null +++ b/pacquet/crates/package-is-installable/src/tests/package_is_installable.rs @@ -0,0 +1,119 @@ +//! End-to-end tests for [`crate::package_is_installable`], exercising +//! the tri-state verdict and the optional / engine-strict branches. +//! +//! Upstream has no direct integration tests for this composer +//! (`packageIsInstallable` in `index.ts` is exercised end-to-end via +//! `installing/deps-installer/test/install/optionalDependencies.ts`). +//! These pacquet tests pin the contract at the function boundary so +//! callers can rely on it without spinning up the full install +//! pipeline. + +use crate::{ + InstallabilityOptions, InstallabilityVerdict, PackageInstallabilityManifest, SkipReason, + WantedEngine, package_is_installable, +}; + +fn host_linux_x64() -> InstallabilityOptions<'static> { + InstallabilityOptions { + engine_strict: false, + optional: false, + current_node_version: "20.10.0", + pnpm_version: None, + current_os: "linux", + current_cpu: "x64", + current_libc: "glibc", + supported_architectures: None, + } +} + +#[test] +fn compatible_manifest_is_installable() { + let manifest = PackageInstallabilityManifest::default(); + let verdict = package_is_installable("pkg", &manifest, &host_linux_x64()).unwrap(); + assert_eq!(verdict, InstallabilityVerdict::Installable); +} + +#[test] +fn incompatible_optional_is_skipped_with_platform_reason() { + let manifest = PackageInstallabilityManifest { + os: Some(vec!["this-os-does-not-exist".to_string()]), + ..Default::default() + }; + let mut opts = host_linux_x64(); + opts.optional = true; + + let verdict = package_is_installable("pkg", &manifest, &opts).unwrap(); + match verdict { + InstallabilityVerdict::SkipOptional { reason, .. } => { + assert_eq!(reason, SkipReason::UnsupportedPlatform); + } + other => panic!("expected SkipOptional, got {other:?}"), + } +} + +#[test] +fn incompatible_optional_engine_is_skipped_with_engine_reason() { + let manifest = PackageInstallabilityManifest { + engines: Some(WantedEngine { node: Some("0.10".to_string()), ..Default::default() }), + ..Default::default() + }; + let mut opts = host_linux_x64(); + opts.optional = true; + + let verdict = package_is_installable("pkg", &manifest, &opts).unwrap(); + match verdict { + InstallabilityVerdict::SkipOptional { reason, .. } => { + assert_eq!(reason, SkipReason::UnsupportedEngine); + } + other => panic!("expected SkipOptional, got {other:?}"), + } +} + +#[test] +fn incompatible_non_optional_proceeds_with_warning() { + let manifest = PackageInstallabilityManifest { + os: Some(vec!["this-os-does-not-exist".to_string()]), + ..Default::default() + }; + + let verdict = package_is_installable("pkg", &manifest, &host_linux_x64()).unwrap(); + match verdict { + InstallabilityVerdict::ProceedWithWarning { .. } => {} + other => panic!("expected ProceedWithWarning, got {other:?}"), + } +} + +#[test] +fn incompatible_non_optional_strict_returns_error() { + let manifest = PackageInstallabilityManifest { + os: Some(vec!["this-os-does-not-exist".to_string()]), + ..Default::default() + }; + let mut opts = host_linux_x64(); + opts.engine_strict = true; + + let err = package_is_installable("pkg", &manifest, &opts).expect_err("strict must error"); + assert_eq!(err.skip_reason(), SkipReason::UnsupportedPlatform); +} + +#[test] +fn platform_is_evaluated_before_engine() { + // A manifest that fails both platform and engine surfaces the + // platform error first — mirrors upstream `checkPackage`'s + // `checkPlatform ?? checkEngine` short-circuit. + let manifest = PackageInstallabilityManifest { + os: Some(vec!["this-os-does-not-exist".to_string()]), + engines: Some(WantedEngine { node: Some("0.10".to_string()), ..Default::default() }), + ..Default::default() + }; + let mut opts = host_linux_x64(); + opts.optional = true; + + let verdict = package_is_installable("pkg", &manifest, &opts).unwrap(); + match verdict { + InstallabilityVerdict::SkipOptional { reason, .. } => { + assert_eq!(reason, SkipReason::UnsupportedPlatform); + } + other => panic!("expected SkipOptional(UnsupportedPlatform), got {other:?}"), + } +} diff --git a/pacquet/crates/package-is-installable/tests/known_failures.rs b/pacquet/crates/package-is-installable/tests/known_failures.rs new file mode 100644 index 0000000000..9f6eefaa82 --- /dev/null +++ b/pacquet/crates/package-is-installable/tests/known_failures.rs @@ -0,0 +1,61 @@ +//! Integration-test boundary for ported tests whose subject under +//! test is not yet implemented. The module lives at the top of the +//! test binary so `just known-failures` (filter `^known_failures::`) +//! picks each test up. + +mod known_failures { + use pacquet_package_is_installable::{Engine, WantedEngine, check_engine}; + use pacquet_testing_utils::{ + allow_known_failure, + known_failure::{KnownFailure, KnownResult}, + }; + + const PACKAGE_ID: &str = "registry.npmjs.org/foo/1.0.0"; + + fn current(node: &str, pnpm: Option<&str>) -> Engine { + Engine { node: node.to_string(), pnpm: pnpm.map(str::to_string) } + } + + fn wanted(node: Option<&str>, pnpm: Option<&str>) -> WantedEngine { + WantedEngine { node: node.map(str::to_string), pnpm: pnpm.map(str::to_string) } + } + + /// Boundary helper for the strict-upper-bound prerelease semantics + /// pacquet's `satisfies_with_prerelease` doesn't fully implement. + fn semver_strict_upper_bound_prerelease_handled() -> KnownResult<()> { + Err(KnownFailure::new( + "pacquet's strip-prerelease fallback approximates npm-semver's \ + `includePrerelease: true`, but over-accepts on a strict upper-bounded \ + range. Upstream `>=9.0.0` rejects `9.0.0-alpha.1` (alpha.1 < 9.0.0 in \ + semver order, no implicit `-0` floor when fully specified); pacquet's \ + strip turns the version into `9.0.0` which then satisfies `>=9.0.0`. \ + Fix path: either add the `nodejs-semver` fork (which exposes \ + `satisfies_with_prerelease(include_prerelease: bool)`) or open-code the \ + strict-upper-bound carve-out. Engine ranges of this shape are vanishingly \ + rare in real package.json files.", + )) + } + + /// Ports the third assertion of `pnpm is a prerelease version` at + /// . + /// + /// Lives under `known_failures` because pacquet's + /// `satisfies_with_prerelease` accepts where upstream rejects. The + /// `allow_known_failure!` call at the boundary keeps the test + /// executable so the day pacquet implements byte-for-byte + /// `includePrerelease: true` semantics, this test will start + /// passing and lose its known-failure marker. + #[test] + fn pnpm_is_a_prerelease_version_strict_ge_full_version_does_not_satisfy() { + allow_known_failure!(semver_strict_upper_bound_prerelease_handled()); + + let err = check_engine( + PACKAGE_ID, + &wanted(None, Some(">=9.0.0")), + ¤t("0.2.1", Some("9.0.0-alpha.1")), + ) + .expect("valid node version") + .expect("must report unsatisfied"); + assert_eq!(err.wanted.pnpm.as_deref(), Some(">=9.0.0")); + } +} diff --git a/pacquet/crates/package-manager/Cargo.toml b/pacquet/crates/package-manager/Cargo.toml index 8a140ab306..0f89b4389f 100644 --- a/pacquet/crates/package-manager/Cargo.toml +++ b/pacquet/crates/package-manager/Cargo.toml @@ -11,21 +11,22 @@ license.workspace = true repository.workspace = true [dependencies] -pacquet-cmd-shim = { workspace = true } -pacquet-executor = { workspace = true } -pacquet-fs = { workspace = true } -pacquet-git-fetcher = { workspace = true } -pacquet-lockfile = { workspace = true } -pacquet-modules-yaml = { workspace = true } -pacquet-network = { workspace = true } -pacquet-config = { workspace = true } -pacquet-graph-hasher = { workspace = true } -pacquet-package-manifest = { workspace = true } -pacquet-patching = { workspace = true } -pacquet-registry = { workspace = true } -pacquet-reporter = { workspace = true } -pacquet-store-dir = { workspace = true } -pacquet-tarball = { workspace = true } +pacquet-cmd-shim = { workspace = true } +pacquet-executor = { workspace = true } +pacquet-fs = { workspace = true } +pacquet-git-fetcher = { workspace = true } +pacquet-lockfile = { workspace = true } +pacquet-modules-yaml = { workspace = true } +pacquet-network = { workspace = true } +pacquet-config = { workspace = true } +pacquet-graph-hasher = { workspace = true } +pacquet-package-manifest = { workspace = true } +pacquet-package-is-installable = { workspace = true } +pacquet-patching = { workspace = true } +pacquet-registry = { workspace = true } +pacquet-reporter = { workspace = true } +pacquet-store-dir = { workspace = true } +pacquet-tarball = { workspace = true } async-recursion = { workspace = true } dashmap = { workspace = true } diff --git a/pacquet/crates/package-manager/src/build_modules.rs b/pacquet/crates/package-manager/src/build_modules.rs index 15e276012f..ea40eb7256 100644 --- a/pacquet/crates/package-manager/src/build_modules.rs +++ b/pacquet/crates/package-manager/src/build_modules.rs @@ -1,3 +1,4 @@ +use crate::SkippedSnapshots; use crate::build_sequence::build_sequence; use crate::version_policy::{VersionPolicyError, expand_package_version_specs}; use derive_more::{Display, Error}; @@ -247,6 +248,13 @@ pub struct BuildModules<'a> { /// Floored to `1` to guarantee forward progress on /// resource-constrained hosts. pub child_concurrency: u32, + /// Snapshots the installability pass marked optional+incompatible. + /// Excluded from both `requires_build` computation and the + /// `build_sequence` input — pacquet does not run scripts (or + /// even check `binding.gyp`) for slots that don't exist on + /// disk. Mirrors pnpm's `lockfileToDepGraph` flow where skipped + /// snapshots never enter the build graph. + pub skipped: &'a SkippedSnapshots, } impl<'a> BuildModules<'a> { @@ -275,6 +283,7 @@ impl<'a> BuildModules<'a> { scripts_prepend_node_path, unsafe_perm, child_concurrency, + skipped, } = self; let Some(snapshots) = snapshots else { return Ok(Vec::new()) }; @@ -292,6 +301,12 @@ impl<'a> BuildModules<'a> { // moved to the build entry point. let requires_build_map: HashMap = snapshots .keys() + // Skip snapshots that never landed on disk. `pkg_requires_build` + // would just return `false` for a missing dir, but the + // walk would still spend a syscall per skipped key — the + // filter short-circuits that on installs with large + // optional fan-out. + .filter(|key| !skipped.contains(key)) .map(|key| { let pkg_dir = virtual_store_dir_for_key(virtual_store_dir, key); (key.clone(), pkg_requires_build(&pkg_dir)) @@ -353,7 +368,7 @@ impl<'a> BuildModules<'a> { let deps_state_cache: Mutex> = Mutex::new(pacquet_graph_hasher::DepsStateCache::new()); - let chunks = build_sequence(&requires_build_map, patches, snapshots, importers); + let chunks = build_sequence(&requires_build_map, patches, snapshots, importers, skipped); // Collect peer-stripped keys so the final list is unique and // sorted lexicographically — matches `dedupePackageNamesFromIgnoredBuilds`. diff --git a/pacquet/crates/package-manager/src/build_modules/tests.rs b/pacquet/crates/package-manager/src/build_modules/tests.rs index d751b8115b..16461c19cb 100644 --- a/pacquet/crates/package-manager/src/build_modules/tests.rs +++ b/pacquet/crates/package-manager/src/build_modules/tests.rs @@ -1,4 +1,5 @@ use super::{AllowBuildPolicy, BuildModules, parse_name_version_from_key}; +use crate::SkippedSnapshots; use pacquet_config::Config; use pacquet_executor::ScriptsPrependNodePath; use pacquet_lockfile::{ @@ -304,6 +305,7 @@ fn build_modules_collects_ignored_builds() { scripts_prepend_node_path: ScriptsPrependNodePath::Never, unsafe_perm: true, child_concurrency: 1, + skipped: &SkippedSnapshots::default(), } .run::() .expect("run BuildModules"); @@ -370,6 +372,7 @@ fn build_modules_collects_ignored_builds_under_concurrency() { scripts_prepend_node_path: ScriptsPrependNodePath::Never, unsafe_perm: true, child_concurrency: 2, + skipped: &SkippedSnapshots::default(), } .run::() .expect("run BuildModules under concurrency"); @@ -424,6 +427,7 @@ fn build_modules_excludes_explicit_deny_from_ignored() { scripts_prepend_node_path: ScriptsPrependNodePath::Never, unsafe_perm: true, child_concurrency: 1, + skipped: &SkippedSnapshots::default(), } .run::() .expect("run BuildModules"); @@ -502,6 +506,7 @@ fn do_not_fail_on_optional_dep_with_failing_postinstall() { scripts_prepend_node_path: ScriptsPrependNodePath::Never, unsafe_perm: true, child_concurrency: 1, + skipped: &SkippedSnapshots::default(), } .run::() .expect("optional build failure must NOT abort the install"); @@ -628,6 +633,7 @@ fn using_side_effects_cache_skips_rebuild() { scripts_prepend_node_path: ScriptsPrependNodePath::Never, unsafe_perm: true, child_concurrency: 1, + skipped: &SkippedSnapshots::default(), } .run::() .expect("install must succeed when the cache hit skips the rebuild"); @@ -689,6 +695,7 @@ fn side_effects_cache_disabled_bypasses_the_gate() { scripts_prepend_node_path: ScriptsPrependNodePath::Never, unsafe_perm: true, child_concurrency: 1, + skipped: &SkippedSnapshots::default(), } .run::() .expect_err("with cache disabled, the failing postinstall must run and the install must fail"); @@ -743,6 +750,7 @@ fn fail_when_failing_postinstall_is_required() { scripts_prepend_node_path: ScriptsPrependNodePath::Never, unsafe_perm: true, child_concurrency: 1, + skipped: &SkippedSnapshots::default(), } .run::() .expect_err("required build failure must propagate"); @@ -974,6 +982,7 @@ async fn write_path_populates_side_effects_row() { scripts_prepend_node_path: ScriptsPrependNodePath::Never, unsafe_perm: true, child_concurrency: 1, + skipped: &SkippedSnapshots::default(), } .run::() .expect("build modules must complete cleanly"); @@ -1080,6 +1089,7 @@ async fn write_path_disabled_skips_upload() { scripts_prepend_node_path: ScriptsPrependNodePath::Never, unsafe_perm: true, child_concurrency: 1, + skipped: &SkippedSnapshots::default(), } .run::() .expect("build modules must complete cleanly"); @@ -1195,6 +1205,7 @@ async fn upload_error_does_not_interrupt_install() { scripts_prepend_node_path: ScriptsPrependNodePath::Never, unsafe_perm: true, child_concurrency: 1, + skipped: &SkippedSnapshots::default(), } .run::() .expect("upload failure must not propagate; install continues"); @@ -1420,6 +1431,7 @@ new file mode 100644 scripts_prepend_node_path: ScriptsPrependNodePath::Never, unsafe_perm: true, child_concurrency: 1, + skipped: &SkippedSnapshots::default(), } .run::() .expect("build modules must complete cleanly"); @@ -1523,6 +1535,7 @@ new file mode 100644 scripts_prepend_node_path: ScriptsPrependNodePath::Never, unsafe_perm: true, child_concurrency: 1, + skipped: &SkippedSnapshots::default(), } .run::() .expect("build modules must complete cleanly"); @@ -1597,6 +1610,7 @@ async fn missing_patch_file_path_errors_with_diagnostic() { scripts_prepend_node_path: ScriptsPrependNodePath::Never, unsafe_perm: true, child_concurrency: 1, + skipped: &SkippedSnapshots::default(), } .run::() .expect_err("missing patch_file_path must surface as PatchFilePathMissing"); diff --git a/pacquet/crates/package-manager/src/build_sequence.rs b/pacquet/crates/package-manager/src/build_sequence.rs index 00117df102..0493f5918e 100644 --- a/pacquet/crates/package-manager/src/build_sequence.rs +++ b/pacquet/crates/package-manager/src/build_sequence.rs @@ -1,3 +1,4 @@ +use crate::SkippedSnapshots; use crate::graph_sequencer::{GraphSequencerResult, graph_sequencer}; use pacquet_lockfile::{PackageKey, ProjectSnapshot, SnapshotEntry}; use pacquet_patching::ExtendedPatchInfo; @@ -38,6 +39,7 @@ pub fn build_sequence( patches: Option<&HashMap>, snapshots: &HashMap, importers: &HashMap, + skipped: &SkippedSnapshots, ) -> Vec> { let children = build_children_map(snapshots); let root_dep_paths = collect_root_dep_paths(importers, snapshots); @@ -45,11 +47,10 @@ pub fn build_sequence( let mut nodes_to_build_set: HashSet = HashSet::new(); let mut nodes_to_build: Vec = Vec::new(); let mut walked: HashSet = HashSet::new(); + let ctx = GetSubgraphCtx { children: &children, requires_build, patches, skipped }; get_subgraph_to_build( &root_dep_paths, - &children, - requires_build, - patches, + &ctx, &mut nodes_to_build_set, &mut nodes_to_build, &mut walked, @@ -163,6 +164,17 @@ fn collect_root_dep_paths( roots } +/// Per-walk invariant inputs to [`get_subgraph_to_build`]. Bundled +/// into a struct so the recursive call doesn't have to thread eight +/// arguments through every level — the three mutable accumulators +/// stay as `&mut` params (one each because they're typed differently). +struct GetSubgraphCtx<'a> { + children: &'a HashMap>, + requires_build: &'a HashMap, + patches: Option<&'a HashMap>, + skipped: &'a SkippedSnapshots, +} + /// Walk the dep graph from `entry_nodes`, filling `nodes_to_build` with /// packages whose subtree (including themselves) contains a build candidate. /// @@ -170,41 +182,58 @@ fn collect_root_dep_paths( /// `https://github.com/pnpm/pnpm/blob/b4f8f47ac2/building/during-install/src/buildSequence.ts`. /// A node is a candidate when `requires_build` is set OR when an entry /// for the peer-stripped key is present in `patches` (mirrors -/// upstream's `node.requiresBuild || node.patch != null`). +/// upstream's `node.requiresBuild || node.patch != null`) — *unless* +/// the node is in `skipped`, in which case its virtual-store slot +/// was never created so neither the requires-build nor patch path +/// can run. Mirrors pnpm's `lockfileToDepGraph` flow where skipped +/// snapshots never enter the build graph at all (the patch lookup +/// upstream walks `pkgGraph[depPath]?.patch` and `depGraph` itself +/// excludes skipped nodes). /// /// Returns whether *any* of the entry nodes (or their subtrees) needs to build. fn get_subgraph_to_build( entry_nodes: &[PackageKey], - children: &HashMap>, - requires_build: &HashMap, - patches: Option<&HashMap>, + ctx: &GetSubgraphCtx<'_>, nodes_to_build_set: &mut HashSet, nodes_to_build: &mut Vec, walked: &mut HashSet, ) -> bool { let mut current_should_be_built = false; for dep_path in entry_nodes { - if !children.contains_key(dep_path) { + if !ctx.children.contains_key(dep_path) { continue; // already in node_modules / not part of this graph } if walked.contains(dep_path) { continue; } + + // A skipped snapshot never had its virtual-store slot + // created, so neither requires-build nor a configured + // patch can produce work. Mirrors pnpm's `lockfileToDepGraph` + // flow where skipped depPaths are dropped from `depGraph` + // entirely: a child reachable only via a skipped edge + // doesn't enter the build graph either. Gate *before* + // recursion so a skipped optional doesn't drag its + // transitive deps into the walk via an edge pnpm wouldn't + // see. + // + // A descendant of a skipped node that's ALSO reachable from + // a non-skipped root still gets visited normally on that + // other branch, because we don't poison `walked` for the + // child here — we just skip this edge. + if ctx.skipped.contains(dep_path) { + walked.insert(dep_path.clone()); + continue; + } + walked.insert(dep_path.clone()); - let child_paths = children.get(dep_path).cloned().unwrap_or_default(); - let child_should_be_built = get_subgraph_to_build( - &child_paths, - children, - requires_build, - patches, - nodes_to_build_set, - nodes_to_build, - walked, - ); + let child_paths = ctx.children.get(dep_path).cloned().unwrap_or_default(); + let child_should_be_built = + get_subgraph_to_build(&child_paths, ctx, nodes_to_build_set, nodes_to_build, walked); - let needs_build = requires_build.get(dep_path).copied().unwrap_or(false); - let has_patch = patches.is_some_and(|p| p.contains_key(&dep_path.without_peer())); + let needs_build = ctx.requires_build.get(dep_path).copied().unwrap_or(false); + let has_patch = ctx.patches.is_some_and(|p| p.contains_key(&dep_path.without_peer())); if child_should_be_built || needs_build || has_patch { if nodes_to_build_set.insert(dep_path.clone()) { diff --git a/pacquet/crates/package-manager/src/build_sequence/tests.rs b/pacquet/crates/package-manager/src/build_sequence/tests.rs index 9067ef5c87..f9be81e343 100644 --- a/pacquet/crates/package-manager/src/build_sequence/tests.rs +++ b/pacquet/crates/package-manager/src/build_sequence/tests.rs @@ -1,8 +1,10 @@ use super::build_sequence; +use crate::SkippedSnapshots; use pacquet_lockfile::{ PackageKey, PkgName, PkgVerPeer, ProjectSnapshot, ResolvedDependencyMap, ResolvedDependencySpec, SnapshotDepRef, SnapshotEntry, }; +use pacquet_patching::ExtendedPatchInfo; use pretty_assertions::assert_eq; use std::collections::HashMap; @@ -61,7 +63,13 @@ fn root_importers(deps: &[(&str, &str)]) -> HashMap { #[test] fn empty_inputs() { - let chunks = build_sequence(&HashMap::new(), None, &HashMap::new(), &HashMap::new()); + let chunks = build_sequence( + &HashMap::new(), + None, + &HashMap::new(), + &HashMap::new(), + &SkippedSnapshots::default(), + ); dbg!(&chunks); assert!(chunks.is_empty(), "empty inputs ⇒ no chunks: {chunks:?}"); } @@ -75,7 +83,8 @@ fn no_requires_build_yields_empty() { let requires_build = requires([(key("a", "1.0.0"), false), (key("b", "1.0.0"), false)]); let importers = root_importers(&[("a", "1.0.0")]); - let chunks = build_sequence(&requires_build, None, &snapshots, &importers); + let chunks = + build_sequence(&requires_build, None, &snapshots, &importers, &SkippedSnapshots::default()); dbg!(&chunks); assert!(chunks.is_empty(), "no requires_build ⇒ no chunks: {chunks:?}"); } @@ -92,7 +101,8 @@ fn leaf_with_requires_build_runs_first() { let requires_build = requires([(key("a", "1.0.0"), false), (key("b", "1.0.0"), true)]); let importers = root_importers(&[("a", "1.0.0")]); - let chunks = build_sequence(&requires_build, None, &snapshots, &importers); + let chunks = + build_sequence(&requires_build, None, &snapshots, &importers, &SkippedSnapshots::default()); assert_eq!(chunks, vec![vec![key("b", "1.0.0")], vec![key("a", "1.0.0")]]); } @@ -111,7 +121,8 @@ fn deep_chain_orders_leaf_first() { ]); let importers = root_importers(&[("a", "1.0.0")]); - let chunks = build_sequence(&requires_build, None, &snapshots, &importers); + let chunks = + build_sequence(&requires_build, None, &snapshots, &importers, &SkippedSnapshots::default()); assert_eq!( chunks, vec![vec![key("c", "1.0.0")], vec![key("b", "1.0.0")], vec![key("a", "1.0.0")]], @@ -136,7 +147,8 @@ fn unrelated_subgraph_excluded() { ]); let importers = root_importers(&[("a", "1.0.0")]); - let chunks = build_sequence(&requires_build, None, &snapshots, &importers); + let chunks = + build_sequence(&requires_build, None, &snapshots, &importers, &SkippedSnapshots::default()); let flat: Vec<_> = chunks.into_iter().flatten().collect(); dbg!(&flat); assert!(flat.contains(&key("a", "1.0.0")), "ancestor of build leaf must appear: {flat:?}"); @@ -165,7 +177,8 @@ fn parallel_build_leaves_share_chunk() { ]); let importers = root_importers(&[("root", "1.0.0")]); - let chunks = build_sequence(&requires_build, None, &snapshots, &importers); + let chunks = + build_sequence(&requires_build, None, &snapshots, &importers, &SkippedSnapshots::default()); assert_eq!(chunks.len(), 2); let mut leaves = chunks[0].clone(); leaves.sort_by_key(|k| k.to_string()); @@ -202,6 +215,127 @@ fn non_builder_importer_with_shared_builder_child_is_trimmed() { ]); let importers = root_importers(&[("a", "1.0.0"), ("b", "1.0.0")]); - let chunks = build_sequence(&requires_build, None, &snapshots, &importers); + let chunks = + build_sequence(&requires_build, None, &snapshots, &importers, &SkippedSnapshots::default()); assert_eq!(chunks, vec![vec![key("c", "1.0.0")], vec![key("a", "1.0.0")]]); } + +/// A snapshot marked in the skip set must NOT enter the build queue +/// even if it carries a configured patch. Upstream's `lockfileToDepGraph` +/// excludes skipped nodes from the depGraph entirely, so the patch +/// lookup never finds them. Without this gate, `build_one_snapshot`'s +/// `pkg_dir.exists()` defensive return would still suppress the +/// actual build attempt — but the snapshot would have been queued +/// and the graph walked through it. The gate makes the exclusion +/// correct-by-construction. +#[test] +fn skipped_patched_snapshot_does_not_enter_build_queue() { + use std::collections::HashSet; + use std::path::PathBuf; + + let a_key = key("a", "1.0.0"); + let snapshots = HashMap::from([(a_key.clone(), snap(&[]))]); + let requires_build = requires([(a_key.clone(), false)]); + let importers = root_importers(&[("a", "1.0.0")]); + + // `a@1.0.0` has a patch configured AND would normally trigger + // a build via `has_patch` alone (requires_build = false). + let patches = HashMap::from([( + a_key.clone(), + ExtendedPatchInfo { + hash: "fake-hash".to_string(), + patch_file_path: Some(PathBuf::from("/dev/null")), + key: "a@1.0.0".to_string(), + }, + )]); + + let skipped = SkippedSnapshots::from_set(HashSet::from([a_key.clone()])); + + let chunks = build_sequence(&requires_build, Some(&patches), &snapshots, &importers, &skipped); + + assert!( + chunks.is_empty(), + "skipped+patched snapshot must not be queued for build, got {chunks:?}", + ); +} + +/// A snapshot reachable *only* via a skipped optional parent must not +/// enter the build queue, even if it requires a build. Pnpm's +/// `lockfileToDepGraph` removes skipped depPaths from the graph +/// entirely, so descendants reachable only via that edge are +/// effectively orphans in the build phase. +/// +/// Setup: root → S (skipped) → C (requires_build). Without the +/// skip-before-recurse gate, the walk would step through S into C, +/// see C as buildable, and queue both C and ancestors that look like +/// they need to be sequenced before C. With the gate, S's subtree +/// isn't visited; C never enters the queue. +#[test] +fn skipped_parent_does_not_drag_descendants_into_build_queue() { + use std::collections::HashSet; + + let root_key = key("root", "1.0.0"); + let s_key = key("s", "1.0.0"); + let c_key = key("c", "1.0.0"); + let snapshots = HashMap::from([ + (root_key.clone(), snap(&[("s", "1.0.0")])), + (s_key.clone(), snap(&[("c", "1.0.0")])), + (c_key.clone(), snap(&[])), + ]); + let requires_build = + requires([(root_key.clone(), false), (s_key.clone(), false), (c_key.clone(), true)]); + let importers = root_importers(&[("root", "1.0.0")]); + + let skipped = SkippedSnapshots::from_set(HashSet::from([s_key])); + + let chunks = build_sequence(&requires_build, None, &snapshots, &importers, &skipped); + + assert!( + chunks.is_empty(), + "C (buildable) reachable only via skipped S must not be queued, got {chunks:?}", + ); +} + +/// A snapshot reachable via BOTH a skipped parent and a non-skipped +/// parent must still enter the build queue if it requires building — +/// pnpm doesn't propagate "skipped" status to descendants reached by +/// any other (non-skipped) path. This pins that the +/// skip-before-recurse gate doesn't accidentally poison `walked` for +/// the alternate branch. +/// +/// Setup: root → {S (skipped), B}, both S and B → C (requires_build). +/// Even though S is skipped, B still pulls C into the build graph. +#[test] +fn descendant_with_non_skipped_parent_still_builds() { + use std::collections::HashSet; + + let root_key = key("root", "1.0.0"); + let s_key = key("s", "1.0.0"); + let b_key = key("b", "1.0.0"); + let c_key = key("c", "1.0.0"); + let snapshots = HashMap::from([ + (root_key.clone(), snap(&[("s", "1.0.0"), ("b", "1.0.0")])), + (s_key.clone(), snap(&[("c", "1.0.0")])), + (b_key.clone(), snap(&[("c", "1.0.0")])), + (c_key.clone(), snap(&[])), + ]); + let requires_build = requires([ + (root_key.clone(), false), + (s_key.clone(), false), + (b_key.clone(), false), + (c_key.clone(), true), + ]); + let importers = root_importers(&[("root", "1.0.0")]); + + let skipped = SkippedSnapshots::from_set(HashSet::from([s_key])); + + let chunks = build_sequence(&requires_build, None, &snapshots, &importers, &skipped); + + let flat: Vec<_> = chunks.into_iter().flatten().collect(); + assert!(flat.contains(&c_key), "C reached via non-skipped B must build, got {flat:?}"); + assert!(flat.contains(&b_key), "B (ancestor of buildable C) must appear, got {flat:?}"); + assert!( + flat.contains(&root_key), + "root (ancestor of buildable subtree) must appear, got {flat:?}", + ); +} diff --git a/pacquet/crates/package-manager/src/create_virtual_store.rs b/pacquet/crates/package-manager/src/create_virtual_store.rs index d9d761cdaa..695e94ce6a 100644 --- a/pacquet/crates/package-manager/src/create_virtual_store.rs +++ b/pacquet/crates/package-manager/src/create_virtual_store.rs @@ -1,5 +1,6 @@ use crate::{ - InstallPackageBySnapshot, InstallPackageBySnapshotError, store_init::init_store_dir_best_effort, + InstallPackageBySnapshot, InstallPackageBySnapshotError, SkippedSnapshots, + store_init::init_store_dir_best_effort, }; use derive_more::{Display, Error}; use futures_util::future; @@ -94,6 +95,14 @@ pub struct CreateVirtualStore<'a> { /// allowlisted. Computed once per install in /// [`crate::InstallFrozenLockfile::run`]. pub allow_build_policy: &'a crate::AllowBuildPolicy, + /// Snapshots the installability pass marked optional+incompatible + /// on this host. Their virtual-store slots are not created — the + /// warm/cold partition skips them, and the bundled-manifest + + /// side-effects-cache lookups they would feed downstream phases + /// are likewise omitted. Mirrors pnpm's `lockfileToDepGraph` + /// behavior of materializing only non-skipped snapshots in the + /// graph passed to the build phase. + pub skipped: &'a SkippedSnapshots, } /// Error type of [`CreateVirtualStore`]. @@ -134,6 +143,7 @@ impl<'a> CreateVirtualStore<'a> { requester, store_index_writer, allow_build_policy, + skipped, } = self; let Some(snapshots) = snapshots else { @@ -248,31 +258,43 @@ impl<'a> CreateVirtualStore<'a> { // Sort + dedup the prefetch input so `prefetch_cas_paths` // doesn't redo identical SELECT + integrity-check work for // every peer variant. - // Per-snapshot skip pass: for every snapshot the previous - // install also installed (`current_snapshots`) with the same - // dependency wiring + integrity, *and* whose virtual-store - // slot still exists on disk, drop it entirely from the install - // graph. The current-lockfile write at end-of-install captures - // the full wanted graph regardless, so the next install sees - // the same skip surface even after partial graph deltas. + // Per-snapshot skip pass: drop snapshots that don't need + // installing. // - // Mirrors upstream's gate at - // . - // When the cache key matches but the directory is gone (user - // ran `rm -rf /...`, antivirus quarantine, - // etc.) we emit the `_broken_node_modules` debug event and - // fall through to the full install path for that snapshot. + // Two reasons a snapshot can be dropped from the install graph: + // + // 1. **Installability skip (this PR)** — `SkippedSnapshots` + // contains it because the host's `engines` / `cpu` / `os` + // / `libc` don't satisfy the package's constraints and the + // snapshot is `optional`. Mirrors pnpm's `lockfileToDepGraph` + // behavior at + // + // where skipped depPaths are dropped from the graph the + // builder iterates. These snapshots also stay out of the + // `skipped_entries` cache-key pass — they were never + // supposed to be installed, so there are no store-index + // rows to keep alive. + // + // 2. **Current-lockfile skip (main #442)** — the previous + // install also installed this snapshot (`current_snapshots`) + // with the same dependency wiring + integrity, AND its + // virtual-store slot still exists on disk. Mirrors + // upstream's gate at + // . + // These DO land in `skipped_entries` so `BuildModules`'s + // `is_built` cache lookup can short-circuit re-runs of + // allowed-build scripts on warm reinstalls. // // Run this *before* deriving cache keys so unchanged // directory-backed snapshots aren't tripped by - // `snapshot_cache_key`'s `UnsupportedResolution`. Once - // directory / git resolutions land they'll still survive the - // skip (their slot existing on disk is all the check needs) - // and the cache-key step won't see them either (review on - // #442). + // `snapshot_cache_key`'s `UnsupportedResolution`. let virtual_store_dir = &config.virtual_store_dir; let survivors: Vec<(&PackageKey, &SnapshotEntry)> = snapshots .iter() + // Reason 1: installability skip. Drop entirely. + .filter(|(snapshot_key, _)| !skipped.contains(snapshot_key)) + // Reason 2: current-lockfile skip. Drop survivors that + // already match the previous install. .filter(|(snapshot_key, snapshot)| { let Some(current_snapshots) = current_snapshots else { return true }; let Some(current_snapshot) = current_snapshots.get(*snapshot_key) else { @@ -348,6 +370,13 @@ impl<'a> CreateVirtualStore<'a> { let skipped_entries: Vec> = snapshots .iter() .filter(|(snapshot_key, _)| !survivor_keys.contains(snapshot_key)) + // Installability-skipped snapshots are excluded from + // `skipped_entries` too — they were never installed, so + // there's no store-index row to keep warm for the + // build-cache lookup. Only the current-lockfile-skip + // path (`survivors` filtered above) should contribute + // here. + .filter(|(snapshot_key, _)| !skipped.contains(snapshot_key)) .map(|(snapshot_key, snapshot)| { let cache_key = snapshot_cache_key(snapshot_key, packages).ok().flatten(); (snapshot_key, snapshot, cache_key) diff --git a/pacquet/crates/package-manager/src/install_frozen_lockfile.rs b/pacquet/crates/package-manager/src/install_frozen_lockfile.rs index 43dde7c1a7..c504e29239 100644 --- a/pacquet/crates/package-manager/src/install_frozen_lockfile.rs +++ b/pacquet/crates/package-manager/src/install_frozen_lockfile.rs @@ -1,7 +1,8 @@ use crate::{ AllowBuildPolicy, BuildModules, BuildModulesError, CreateVirtualStore, CreateVirtualStoreError, - CreateVirtualStoreOutput, LinkVirtualStoreBins, LinkVirtualStoreBinsError, - SymlinkDirectDependencies, SymlinkDirectDependenciesError, VersionPolicyError, + CreateVirtualStoreOutput, InstallabilityHost, LinkVirtualStoreBins, LinkVirtualStoreBinsError, + SkippedSnapshots, SymlinkDirectDependencies, SymlinkDirectDependenciesError, + VersionPolicyError, any_installability_constraint, compute_skipped_snapshots, }; use derive_more::{Display, Error}; use miette::Diagnostic; @@ -84,6 +85,29 @@ pub enum InstallFrozenLockfileError { /// See . #[diagnostic(transparent)] VersionPolicy(#[error(source)] VersionPolicyError), + + /// Wraps any error `compute_skipped_snapshots` surfaces from the + /// installability pass. Three sources, all reachable under + /// today's default config: + /// + /// - `InstallabilityError::InvalidNodeVersion` — the resolved + /// `current_node_version` isn't a parseable exact semver. + /// Pacquet falls back to a synthetic `99999.0.0` when + /// `node --version` fails, so this is currently unreachable + /// from production — but a future `nodeVersion` config wiring + /// (slice 2) will surface user-supplied bad values here, + /// mirroring upstream's `ERR_PNPM_INVALID_NODE_VERSION` throw + /// at . + /// - `InstallabilityError::Engine` / `InstallabilityError::Platform` + /// from a non-optional incompatible snapshot with + /// `engine_strict = true`. Pacquet's default has + /// `engine_strict = false`, so this path is currently + /// unreachable from production either — wired through so the + /// slice that lands the config setting doesn't churn the + /// error enum again. Mirrors upstream's `throw warn` at + /// . + #[diagnostic(transparent)] + Installability(#[error(source)] Box), } impl<'a, DependencyGroupList> InstallFrozenLockfile<'a, DependencyGroupList> @@ -126,6 +150,62 @@ where // existing best-effort stance on cache writes. let (store_index_writer, writer_task) = StoreIndexWriter::spawn(&config.store_dir); + // Caller-side fast-path for the installability check. The + // common case (no lockfile metadata row declares an + // `engines` / `cpu` / `os` / `libc` constraint) lets us skip + // both [`InstallabilityHost::detect`] and + // [`compute_skipped_snapshots`] entirely. Spawning + // `node --version` here would otherwise serialize the + // node-binary startup with `CreateVirtualStore::run` (the + // dominant cost of a cold install), giving up the overlap + // pacquet had before — see the previous benchmark regression + // on this PR. + // + // When constraints DO exist, the host is needed before + // extraction (so `CreateVirtualStore` can suppress slots for + // skipped snapshots), and the spawn cost is unavoidable. + let needs_installability_check = match (snapshots, packages) { + (Some(snaps), Some(pkgs)) if !snaps.is_empty() => any_installability_constraint(pkgs), + _ => false, + }; + + // Build the per-install [`SkippedSnapshots`] set. For every + // lockfile snapshot, run the installability check against + // the host triple; optional+incompatible entries land in + // the set and fire `pnpm:skipped-optional-dependency`. + // Mirrors pnpm's headless re-check at + // . + // + // `host` is built only when needed. The detection path runs + // `node --version` on the blocking pool so it doesn't stall + // the reactor thread. + let (skipped, host_node) = if needs_installability_check { + let host = tokio::task::spawn_blocking(InstallabilityHost::detect) + .await + .unwrap_or_else(|_| InstallabilityHost { + node_version: "99999.0.0".to_string(), + node_detected: false, + os: pacquet_graph_hasher::host_platform(), + cpu: pacquet_graph_hasher::host_arch(), + libc: pacquet_graph_hasher::host_libc(), + supported_architectures: None, + engine_strict: false, + }); + let s = compute_skipped_snapshots::( + snapshots.expect("guarded by needs_installability_check"), + packages.expect("guarded by needs_installability_check"), + &host, + requester, + ) + .map_err(InstallFrozenLockfileError::Installability)?; + // Preserve `node_detected` + `node_version` for the + // engine-name derivation below. Dropping the rest of the + // host struct frees the allocations early. + (s, Some((host.node_detected, host.node_version))) + } else { + (SkippedSnapshots::new(), None) + }; + let CreateVirtualStoreOutput { package_manifests, side_effects_maps_by_snapshot } = CreateVirtualStore { http_client, @@ -138,31 +218,48 @@ where requester, store_index_writer: &store_index_writer, allow_build_policy: &allow_build_policy, + skipped: &skipped, } .run::() .await .map_err(InstallFrozenLockfileError::CreateVirtualStore)?; - // Detect the host `node` major version once per install, - // not per snapshot. Threaded into `BuildModules` so the - // side-effects-cache lookup can compose the right cache - // key. `None` (no `node` on PATH) means the cache gate - // falls through to "rebuild" — safe. + // `engine_name` for the side-effects-cache lookup. // - // `detect_node_major` spawns `node --version` synchronously, - // so run it on a blocking thread to keep the async install - // driver from stalling. - let engine_name: Option = tokio::task::spawn_blocking(|| { - pacquet_graph_hasher::detect_node_major() - .map(|major| pacquet_graph_hasher::engine_name(major, None, None)) - }) - .await - .ok() - .flatten(); + // Two paths: + // - We already detected the host for the installability + // check (constraint-bearing lockfile): reuse the cached + // version. The synthetic-fallback case (`node_detected = false`) + // yields `None` so a bogus `99999.0.0`-derived key can't + // poison the cache. + // - We skipped the installability check (constraint-free + // lockfile, the common case): no cached version. Fall + // back to the legacy `detect_node_major` spawn — run + // after `CreateVirtualStore::run` so it overlaps with + // nothing on the critical path. This is the same + // placement upstream used to have. + let engine_name: Option = match &host_node { + Some((true, ver)) => parse_major_from_version(ver) + .map(|major| pacquet_graph_hasher::engine_name(major, None, None)), + Some((false, _)) => None, + None => tokio::task::spawn_blocking(|| { + pacquet_graph_hasher::detect_node_major() + .map(|major| pacquet_graph_hasher::engine_name(major, None, None)) + }) + .await + .ok() + .flatten(), + }; - SymlinkDirectDependencies { config, importers, dependency_groups, requester } - .run::() - .map_err(InstallFrozenLockfileError::SymlinkDirectDependencies)?; + SymlinkDirectDependencies { + config, + importers, + dependency_groups, + requester, + skipped: &skipped, + } + .run::() + .map_err(InstallFrozenLockfileError::SymlinkDirectDependencies)?; // Link the bins of each virtual-store slot's children into the // slot's own `node_modules/.bin`. Pnpm runs this from @@ -179,6 +276,7 @@ where snapshots, packages, package_manifests: &package_manifests, + skipped: &skipped, } .run() .map_err(InstallFrozenLockfileError::LinkVirtualStoreBins)?; @@ -275,6 +373,7 @@ where scripts_prepend_node_path, unsafe_perm: config.unsafe_perm, child_concurrency: config.child_concurrency, + skipped: &skipped, } .run::() .map_err(InstallFrozenLockfileError::BuildModules)?; @@ -312,3 +411,13 @@ where Ok(()) } } + +/// Pull the leading major-version digits out of a semver string like +/// `"22.11.0"`. Returns `None` if the leading token isn't parseable +/// as `u32`. Used to derive the engine-name string upstream's +/// side-effects cache lookup expects without re-spawning +/// `node --version`. +fn parse_major_from_version(version: &str) -> Option { + let after_v = version.strip_prefix('v').unwrap_or(version); + after_v.split('.').next()?.parse().ok() +} diff --git a/pacquet/crates/package-manager/src/install_without_lockfile.rs b/pacquet/crates/package-manager/src/install_without_lockfile.rs index a9c4319320..789917d04d 100644 --- a/pacquet/crates/package-manager/src/install_without_lockfile.rs +++ b/pacquet/crates/package-manager/src/install_without_lockfile.rs @@ -206,11 +206,16 @@ impl<'a, DependencyGroupList> InstallWithoutLockfile<'a, DependencyGroupList> { // disk). The frozen-lockfile path skips both via // [`LinkVirtualStoreBins::snapshots`] / `package_manifests`. let empty_manifests = std::collections::HashMap::new(); + let empty_skipped = crate::SkippedSnapshots::new(); LinkVirtualStoreBins { virtual_store_dir: &config.virtual_store_dir, snapshots: None, packages: None, package_manifests: &empty_manifests, + // The without-lockfile path has no installability check + // (no `packages:` metadata to evaluate constraints + // against), so the skip set is empty by definition. + skipped: &empty_skipped, } .run() .map_err(InstallWithoutLockfileError::LinkVirtualStoreBins)?; diff --git a/pacquet/crates/package-manager/src/installability.rs b/pacquet/crates/package-manager/src/installability.rs new file mode 100644 index 0000000000..e3f2de109b --- /dev/null +++ b/pacquet/crates/package-manager/src/installability.rs @@ -0,0 +1,348 @@ +//! Per-install installability pass. +//! +//! For each snapshot in a frozen-lockfile install, run +//! `pacquet-package-is-installable`'s `check_package` against the +//! matching `PackageMetadata` and the host environment, build the +//! [`SkippedSnapshots`] set, and emit +//! `pnpm:skipped-optional-dependency` for every optional+incompatible +//! one. +//! +//! Mirrors the union of upstream's: +//! - The resolver-side gate at +//! . +//! - The headless re-check at +//! . +//! +//! Pacquet's install path is lockfile-driven and has no resolver, so +//! the headless re-check is the only relevant emit site. Running it +//! every install also means the set is recomputed against the current +//! host — pnpm's `lockfileToDepGraph` does exactly the same, and the +//! comment at upstream's `:194-215` calls out that the host arch may +//! have changed since the previous install wrote `.modules.yaml`. + +use std::collections::{HashMap, HashSet}; + +use pacquet_lockfile::{PackageKey, PackageMetadata, SnapshotEntry}; +use pacquet_package_is_installable::{ + InstallabilityError, InstallabilityOptions, PackageInstallabilityManifest, SkipReason, + SupportedArchitectures, WantedEngine, check_package, +}; +use pacquet_reporter::{ + LogEvent, LogLevel, Reporter, SkippedOptionalDependencyLog, SkippedOptionalPackage, + SkippedOptionalReason, +}; + +/// The set of snapshot keys skipped on this host. +#[derive(Debug, Default, Clone)] +pub struct SkippedSnapshots { + set: HashSet, +} + +impl SkippedSnapshots { + pub fn new() -> Self { + Self { set: HashSet::new() } + } + + /// Construct a [`SkippedSnapshots`] from an existing set. Test + /// helper for callers that want to drive build-sequence / + /// virtual-store filtering against a known skip set without + /// running the full installability pass. + #[cfg(test)] + pub(crate) fn from_set(set: HashSet) -> Self { + Self { set } + } + + pub fn contains(&self, key: &PackageKey) -> bool { + self.set.contains(key) + } + + pub fn len(&self) -> usize { + self.set.len() + } + + pub fn is_empty(&self) -> bool { + self.set.is_empty() + } + + pub fn iter(&self) -> impl Iterator + '_ { + self.set.iter() + } +} + +/// Host context for the installability check. Built once per install +/// so the per-snapshot calls don't each re-spawn `node --version` +/// or re-read `std::env::consts::OS`. +pub struct InstallabilityHost { + pub node_version: String, + /// `true` when `node_version` was discovered by spawning + /// `node --version`; `false` when the field carries the synthetic + /// fallback. The side-effects-cache key derives from this — a + /// fallback version must not seed the cache because subsequent + /// installs would key on the actual node major and miss every + /// row written under the fallback. + pub node_detected: bool, + pub os: &'static str, + pub cpu: &'static str, + pub libc: &'static str, + pub supported_architectures: Option, + pub engine_strict: bool, +} + +impl InstallabilityHost { + /// Resolve the host context from the running process. + /// + /// `node_version` is detected via + /// [`pacquet_graph_hasher::detect_node_version`]; when detection + /// fails (no `node` on PATH), pacquet falls back to a synthetic + /// `99999.0.0` so `engines.node` ranges keep accepting packages. + /// The alternative `0.0.0` would falsely-skip every optional + /// dependency targeting any concrete node range, which is worse + /// than the over-acceptance the very-high fallback produces. + /// `node_detected` records which path was taken so callers can + /// suppress side-effects-cache lookups when the version is + /// synthetic. Slice 2 will wire a proper `nodeVersion` config + /// setting and surface `ERR_PNPM_INVALID_NODE_VERSION` to match + /// upstream's throw-on-detection-failure behavior. + pub fn detect() -> Self { + let detected = pacquet_graph_hasher::detect_node_version(); + let node_detected = detected.is_some(); + let node_version = detected.unwrap_or_else(|| "99999.0.0".to_string()); + Self { + node_version, + node_detected, + os: pacquet_graph_hasher::host_platform(), + cpu: pacquet_graph_hasher::host_arch(), + libc: pacquet_graph_hasher::host_libc(), + supported_architectures: None, + engine_strict: false, + } + } +} + +/// Compute the [`SkippedSnapshots`] set for a frozen-lockfile install. +/// +/// For each `(snapshot_key, snapshot)`: +/// 1. Look up the matching `PackageMetadata` (skipping snapshots +/// without one — `CreateVirtualStore` will error on them +/// separately). +/// 2. Build a [`PackageInstallabilityManifest`] from `metadata.engines`, +/// `metadata.cpu`, `metadata.os`, `metadata.libc`. +/// 3. Run `check_package` against the host triple. +/// 4. Apply the per-snapshot dispatch: +/// - `Ok(None)`: compatible, nothing to do. +/// - `Ok(Some(err))` + `snapshot.optional`: add to the set; emit +/// `pnpm:skipped-optional-dependency`. +/// - `Ok(Some(err))` + `engine_strict`: return as the install +/// error. Pacquet's default has `engine_strict = false`, so +/// this path is currently unreachable from production — wired +/// for the slice that lands the config setting. +/// - `Ok(Some(err))` otherwise: emit `tracing::warn!` and proceed. +/// Upstream uses `pnpm:install-check` here, which pacquet's +/// reporter does not yet expose — slice 1 follow-up. +/// - `Err(InvalidNodeVersionError)`: surface as +/// `ERR_PNPM_INVALID_NODE_VERSION`. +pub fn compute_skipped_snapshots( + snapshots: &HashMap, + packages: &HashMap, + host: &InstallabilityHost, + prefix: &str, +) -> Result> { + // Fast path: if no package in the lockfile declares any + // installability constraint, every snapshot is trivially + // installable. Skip the per-snapshot + // `without_peer()` / `to_string()` / `check_package` loop + // entirely. Pacquet has no resolver so the lockfile's packages + // map is fixed for the duration of the install; one linear scan + // early is much cheaper than walking the snapshots map and + // decomposing each metadata row only to find no constraints to + // evaluate. + // + // Concretely on the integrated benchmark (1352 packages with no + // platform / engine constraints): drops ~1352 `String` and + // `PackageKey` allocations and the matching number of + // `check_package` calls. The scan is O(N) on `packages` — same + // shape as the loop it short-circuits — but does at most four + // `Option::is_some` checks per row and short-circuits on the + // first declared constraint. + if !any_installability_constraint(packages) { + return Ok(SkippedSnapshots::new()); + } + + let mut skipped = SkippedSnapshots::new(); + let mut seen_emit: HashSet = HashSet::new(); + + // Build the host-derived part of the options once. Only the + // (`engine_strict`-irrelevant) `optional` flag varies per + // snapshot, but the result of [`check_package`] — "does this + // manifest satisfy the host?" — does not. We compute and cache + // the check verdict per peer-stripped `metadata_key`; the + // per-snapshot loop then only needs to apply the + // optional / engine-strict dispatch. + // + // The cache pays off on lockfiles with peer-resolved variants of + // the same package (`react-dom@17(react@17)` / + // `react-dom@17(react@18)`, etc.) — every variant shares the + // same `metadata_key`, so the check only runs once. + // `InstallabilityOptions` borrows its string fields for exactly + // this reuse pattern. + let base_options = InstallabilityOptions { + engine_strict: host.engine_strict, + // Cache-shared check: `optional` is applied per-snapshot + // below, not inside `check_package`. + optional: false, + current_node_version: host.node_version.as_str(), + pnpm_version: None, + current_os: host.os, + current_cpu: host.cpu, + current_libc: host.libc, + supported_architectures: host.supported_architectures.as_ref(), + }; + + // `None` = compatible. `Some(err)` = incompatible, with the + // diagnostic the caller would surface (used as both the + // `SkipOptional` details payload and the `ProceedWithWarning` + // message body, matching upstream's `warn.toString()` / `warn.message` + // at `index.ts:50` / `:44`). + let mut check_cache: HashMap> = HashMap::new(); + + for (snapshot_key, snapshot) in snapshots { + let metadata_key = snapshot_key.without_peer(); + let Some(metadata) = packages.get(&metadata_key) else { continue }; + + // Cache miss → run `check_package` once for this metadata + // row. The clone-on-insert is a single `Option` + // (small) and only happens on the first peer-variant of each + // package. Subsequent peer-variants land in the `else` arm + // and read back the cached verdict. + let warn = if let Some(cached) = check_cache.get(&metadata_key) { + cached.clone() + } else { + let manifest = manifest_from_metadata(metadata); + let pkg_id = metadata_key.to_string(); + let result = check_package(&pkg_id, &manifest, &base_options) + .map_err(|invalid| Box::new(InstallabilityError::InvalidNodeVersion(invalid)))?; + check_cache.insert(metadata_key.clone(), result.clone()); + result + }; + + let Some(warn) = warn else { continue }; + + if snapshot.optional { + skipped.set.insert(snapshot_key.clone()); + // Dedup events per metadata key, matching upstream's + // emit-per-pkgId at `index.ts:49-58`. + if seen_emit.insert(metadata_key.clone()) { + emit_skipped::( + &metadata_key.to_string(), + warn.skip_reason(), + warn.to_string(), + prefix, + ); + } + continue; + } + + if host.engine_strict { + return Err(Box::new(warn)); + } + + // Non-optional, non-strict: upstream emits `pnpm:install-check` + // warn (TODO: add channel to the reporter). For now the + // tracing-level warning is the user-visible signal that an + // incompatible non-optional dep slipped through. + tracing::warn!( + target: "pacquet::install", + package = %metadata_key, + "{}", + warn, + ); + } + + Ok(skipped) +} + +/// True if any package metadata row in the lockfile declares an +/// `engines` / `cpu` / `os` / `libc` constraint pacquet would need +/// to evaluate. Short-circuits on the first hit. When this returns +/// false, both [`compute_skipped_snapshots`] and the caller can +/// short-circuit: no need to spawn `node --version` or build the +/// host context, because the verdict is unconditionally an empty +/// skip set. +/// +/// `pub` so `install_frozen_lockfile` can gate the host detection +/// on it — the spawn is otherwise on the critical path of +/// `CreateVirtualStore::run` and serializes ~100ms of node-binary +/// startup with extraction it used to overlap with. +pub fn any_installability_constraint(packages: &HashMap) -> bool { + packages.values().any(metadata_has_meaningful_constraint) +} + +/// True if a single metadata row carries a constraint pacquet would +/// actually evaluate. Distinguishes "field present" from "field present +/// AND meaningful": +/// +/// - `engines`: only `node` / `pnpm` keys matter. A package that +/// declares `engines.npm = ">=8"` (and nothing else) has no +/// constraint pacquet evaluates — pacquet isn't npm. +/// - `cpu` / `os` / `libc`: a `["any"]` value short-circuits to +/// "accept" inside `check_platform`'s `check_list`, and an empty +/// list cannot exclude the host either. Treat both as no-constraint. +fn metadata_has_meaningful_constraint(m: &PackageMetadata) -> bool { + let engines_meaningful = + m.engines.as_ref().is_some_and(|e| e.contains_key("node") || e.contains_key("pnpm")); + engines_meaningful + || platform_axis_meaningful(m.cpu.as_deref()) + || platform_axis_meaningful(m.os.as_deref()) + || platform_axis_meaningful(m.libc.as_deref()) +} + +/// One axis of `cpu` / `os` / `libc` carries no constraint when the +/// list is absent, empty, or exactly the `["any"]` sentinel that +/// `check_list` short-circuits as "accept everything". +fn platform_axis_meaningful(axis: Option<&[String]>) -> bool { + match axis { + None | Some([]) => false, + Some([only]) if only == "any" => false, + Some(_) => true, + } +} + +fn manifest_from_metadata(metadata: &PackageMetadata) -> PackageInstallabilityManifest { + PackageInstallabilityManifest { + engines: metadata + .engines + .as_ref() + .map(|m| WantedEngine { node: m.get("node").cloned(), pnpm: m.get("pnpm").cloned() }), + cpu: metadata.cpu.clone(), + os: metadata.os.clone(), + libc: metadata.libc.clone(), + } +} + +fn emit_skipped(pkg_id: &str, reason: SkipReason, details: String, prefix: &str) { + let (name, version) = split_name_version(pkg_id); + let wire_reason = match reason { + SkipReason::UnsupportedEngine => SkippedOptionalReason::UnsupportedEngine, + SkipReason::UnsupportedPlatform => SkippedOptionalReason::UnsupportedPlatform, + }; + R::emit(&LogEvent::SkippedOptionalDependency(SkippedOptionalDependencyLog { + level: LogLevel::Debug, + details: Some(details), + package: SkippedOptionalPackage { id: pkg_id.to_string(), name, version }, + prefix: prefix.to_string(), + reason: wire_reason, + })); +} + +/// Split a `name@version` (with possible leading `@` for scoped +/// packages) into `(name, version)`. Mirrors the `lastIndexOf('@')` +/// rule pacquet's manifest parser already uses. +fn split_name_version(pkg_id: &str) -> (String, String) { + match pkg_id.rfind('@') { + Some(idx) if idx > 0 => (pkg_id[..idx].to_string(), pkg_id[idx + 1..].to_string()), + _ => (pkg_id.to_string(), String::new()), + } +} + +#[cfg(test)] +mod tests; diff --git a/pacquet/crates/package-manager/src/installability/tests.rs b/pacquet/crates/package-manager/src/installability/tests.rs new file mode 100644 index 0000000000..4862bb8c88 --- /dev/null +++ b/pacquet/crates/package-manager/src/installability/tests.rs @@ -0,0 +1,346 @@ +//! Unit tests for [`crate::installability::compute_skipped_snapshots`]. + +use std::cell::RefCell; +use std::collections::HashMap; + +use pacquet_lockfile::{ + LockfileResolution, PackageKey, PackageMetadata, PkgNameVerPeer, SnapshotEntry, + TarballResolution, +}; +use pacquet_reporter::{LogEvent, Reporter, SkippedOptionalReason}; +use pretty_assertions::assert_eq; + +use crate::installability::{ + InstallabilityHost, any_installability_constraint, compute_skipped_snapshots, +}; + +// Thread-local recording so the cargo-default parallel test runner +// can fan out without tests polluting each other's event stream. +// `Reporter::emit` is a free function; the captured buffer has to +// live in static storage somewhere — thread-local trades a small +// allocation per test thread for zero cross-test contention. +thread_local! { + static RECORDED_EVENTS: RefCell> = const { RefCell::new(Vec::new()) }; +} + +struct RecordingReporter; + +impl Reporter for RecordingReporter { + fn emit(event: &LogEvent) { + RECORDED_EVENTS.with(|cell| cell.borrow_mut().push(event.clone())); + } +} + +fn take_events() -> Vec { + RECORDED_EVENTS.with(|cell| std::mem::take(&mut *cell.borrow_mut())) +} + +fn reset_events() { + RECORDED_EVENTS.with(|cell| cell.borrow_mut().clear()); +} + +fn snapshot_key(name_at_version: &str) -> PackageKey { + name_at_version.parse::().expect("valid package key") +} + +fn synthetic_metadata( + engines: Option<&[(&str, &str)]>, + cpu: Option<&[&str]>, + os: Option<&[&str]>, + libc: Option<&[&str]>, +) -> PackageMetadata { + // Tarball resolution — the installability check ignores the + // resolution shape entirely, but every `PackageMetadata` must + // carry one. + PackageMetadata { + resolution: LockfileResolution::Tarball(TarballResolution { + integrity: None, + tarball: "https://example.test/pkg.tgz".to_string(), + git_hosted: None, + }), + engines: engines + .map(|e| e.iter().map(|(k, v)| ((*k).to_string(), (*v).to_string())).collect()), + cpu: cpu.map(|v| v.iter().map(|s| (*s).to_string()).collect()), + os: os.map(|v| v.iter().map(|s| (*s).to_string()).collect()), + libc: libc.map(|v| v.iter().map(|s| (*s).to_string()).collect()), + deprecated: None, + has_bin: None, + prepare: None, + bundled_dependencies: None, + peer_dependencies: None, + peer_dependencies_meta: None, + } +} + +fn host(node_version: &str, os: &'static str, cpu: &'static str) -> InstallabilityHost { + InstallabilityHost { + node_version: node_version.to_string(), + node_detected: true, + os, + cpu, + libc: "unknown", + supported_architectures: None, + engine_strict: false, + } +} + +/// Mirrors `optionalDependencies.ts:74` `skip optional dependency that +/// does not support the current OS`: an optional package whose `os` +/// list excludes the host is skipped, and the +/// `pnpm:skipped-optional-dependency` event carries the +/// `unsupported_platform` reason. +#[test] +fn skip_optional_with_wrong_os() { + reset_events(); + let key = snapshot_key("not-compatible-with-any-os@1.0.0"); + let mut snapshots = HashMap::new(); + snapshots.insert(key.clone(), SnapshotEntry { optional: true, ..Default::default() }); + let mut packages = HashMap::new(); + packages.insert( + key.clone(), + synthetic_metadata(None, None, Some(&["this-os-does-not-exist"]), None), + ); + + let skipped = compute_skipped_snapshots::( + &snapshots, + &packages, + &host("20.10.0", "darwin", "arm64"), + "/proj", + ) + .unwrap(); + + assert_eq!(skipped.len(), 1); + assert!(skipped.contains(&key)); + + let events = take_events(); + let skipped_events: Vec<_> = + events.iter().filter(|e| matches!(e, LogEvent::SkippedOptionalDependency(_))).collect(); + assert_eq!(skipped_events.len(), 1); + if let LogEvent::SkippedOptionalDependency(log) = skipped_events[0] { + assert_eq!(log.reason, SkippedOptionalReason::UnsupportedPlatform); + assert_eq!(log.package.name, "not-compatible-with-any-os"); + assert_eq!(log.package.version, "1.0.0"); + assert_eq!(log.prefix, "/proj"); + } +} + +/// Mirrors `optionalDependencies.ts:143` `skip optional dependency +/// that does not support the current Node version`. The engine +/// rejection surfaces as `unsupported_engine`. +#[test] +fn skip_optional_with_wrong_node_engine() { + reset_events(); + let key = snapshot_key("for-legacy-node@1.0.0"); + let mut snapshots = HashMap::new(); + snapshots.insert(key.clone(), SnapshotEntry { optional: true, ..Default::default() }); + let mut packages = HashMap::new(); + packages.insert(key.clone(), synthetic_metadata(Some(&[("node", "0.10")]), None, None, None)); + + let skipped = compute_skipped_snapshots::( + &snapshots, + &packages, + &host("20.10.0", "darwin", "arm64"), + "/proj", + ) + .unwrap(); + + assert!(skipped.contains(&key)); + let events = take_events(); + let skipped_events: Vec<_> = + events.iter().filter(|e| matches!(e, LogEvent::SkippedOptionalDependency(_))).collect(); + assert_eq!(skipped_events.len(), 1); + if let LogEvent::SkippedOptionalDependency(log) = skipped_events[0] { + assert_eq!(log.reason, SkippedOptionalReason::UnsupportedEngine); + } +} + +/// Compatible snapshots stay out of the skip set and trigger no events. +#[test] +fn compatible_snapshots_are_not_skipped() { + reset_events(); + let key = snapshot_key("compat@1.0.0"); + let mut snapshots = HashMap::new(); + snapshots.insert(key.clone(), SnapshotEntry { optional: true, ..Default::default() }); + let mut packages = HashMap::new(); + packages.insert(key, synthetic_metadata(None, None, Some(&["darwin", "linux"]), None)); + + let skipped = compute_skipped_snapshots::( + &snapshots, + &packages, + &host("20.10.0", "darwin", "arm64"), + "/proj", + ) + .unwrap(); + + assert!(skipped.is_empty()); + let events = take_events(); + assert!( + events.iter().all(|e| !matches!(e, LogEvent::SkippedOptionalDependency(_))), + "expected no skipped-optional events, got {events:?}", + ); +} + +/// A non-optional incompatible package does NOT get skipped — it +/// surfaces a tracing-level warning and proceeds, matching pnpm's +/// non-engineStrict default. Verifies the skip set stays empty in +/// that case. +#[test] +fn non_optional_incompatible_is_not_skipped() { + reset_events(); + let key = snapshot_key("non-optional-but-wrong-os@1.0.0"); + let mut snapshots = HashMap::new(); + snapshots.insert(key.clone(), SnapshotEntry { optional: false, ..Default::default() }); + let mut packages = HashMap::new(); + packages.insert(key, synthetic_metadata(None, None, Some(&["this-os-does-not-exist"]), None)); + + let skipped = compute_skipped_snapshots::( + &snapshots, + &packages, + &host("20.10.0", "darwin", "arm64"), + "/proj", + ) + .unwrap(); + + assert!(skipped.is_empty()); + let events = take_events(); + assert!( + events.iter().all(|e| !matches!(e, LogEvent::SkippedOptionalDependency(_))), + "non-optional must not fire skipped-optional events", + ); +} + +/// Fast path: a lockfile where no metadata row declares any +/// installability constraint skips the per-snapshot pass entirely. +/// Verifies the optimization triggers and produces the same +/// observable behavior as the slow path (empty skip set, no events). +#[test] +fn no_constraints_skips_the_per_snapshot_pass() { + reset_events(); + let key = snapshot_key("no-constraints@1.0.0"); + let mut snapshots = HashMap::new(); + snapshots.insert(key.clone(), SnapshotEntry { optional: true, ..Default::default() }); + let mut packages = HashMap::new(); + // No engines / cpu / os / libc — the fast path returns an + // empty SkippedSnapshots without inspecting individual + // snapshots. + packages.insert(key, synthetic_metadata(None, None, None, None)); + + let skipped = compute_skipped_snapshots::( + &snapshots, + &packages, + &host("20.10.0", "darwin", "arm64"), + "/proj", + ) + .unwrap(); + + assert!(skipped.is_empty()); + let events = take_events(); + assert!( + events.iter().all(|e| !matches!(e, LogEvent::SkippedOptionalDependency(_))), + "fast path must not fire skipped-optional events", + ); +} + +/// `engines` block with no `node` / `pnpm` key (e.g. only `npm`) +/// does NOT trigger the slow path. Pacquet doesn't evaluate the npm +/// engine, so a package declaring `engines.npm` alone is no +/// constraint as far as installability is concerned. +#[test] +fn engines_without_node_or_pnpm_does_not_count_as_constraint() { + let key = snapshot_key("npm-engine-only@1.0.0"); + let mut packages = HashMap::new(); + packages.insert(key, synthetic_metadata(Some(&[("npm", ">=8")]), None, None, None)); + assert!( + !any_installability_constraint(&packages), + "engines.npm alone should not block the fast path", + ); +} + +/// `cpu` / `os` / `libc` set to the `["any"]` sentinel is a no-op +/// in `check_platform`'s `check_list`, so it must not trigger the +/// slow path either. +#[test] +fn platform_any_sentinel_does_not_count_as_constraint() { + let key = snapshot_key("any-platforms@1.0.0"); + let mut packages = HashMap::new(); + packages.insert(key, synthetic_metadata(None, Some(&["any"]), Some(&["any"]), Some(&["any"]))); + assert!( + !any_installability_constraint(&packages), + "cpu/os/libc = [\"any\"] should not block the fast path", + ); +} + +/// Empty `cpu` / `os` / `libc` lists carry no exclusion either — +/// they cannot reject any host value. Should not block the fast +/// path. +#[test] +fn empty_platform_lists_do_not_count_as_constraint() { + let key = snapshot_key("empty-platforms@1.0.0"); + let mut packages = HashMap::new(); + packages.insert(key, synthetic_metadata(None, Some(&[]), Some(&[]), Some(&[]))); + assert!( + !any_installability_constraint(&packages), + "empty platform lists should not block the fast path", + ); +} + +/// A meaningful `engines.node` triggers the slow path. Sanity check +/// the predicate doesn't over-aggressively fast-path. +#[test] +fn meaningful_engines_node_triggers_slow_path() { + let key = snapshot_key("for-legacy-node@1.0.0"); + let mut packages = HashMap::new(); + packages.insert(key, synthetic_metadata(Some(&[("node", "0.10")]), None, None, None)); + assert!(any_installability_constraint(&packages), "engines.node must trigger the slow path"); +} + +/// A meaningful non-`any` platform value triggers the slow path. +#[test] +fn meaningful_platform_value_triggers_slow_path() { + let key = snapshot_key("not-compatible-with-any-os@1.0.0"); + let mut packages = HashMap::new(); + packages.insert(key, synthetic_metadata(None, None, Some(&["this-os-does-not-exist"]), None)); + assert!(any_installability_constraint(&packages), "non-any os must trigger the slow path"); +} + +/// Peer-resolved variants of the same metadata row (e.g. +/// `react-dom@17.0.2(react@17.0.2)` vs the same against `react@18`) +/// must dedup at the reporter — upstream emits one event per +/// `pkgId`, not per snapshot. +#[test] +fn duplicate_metadata_dedupes_reporter_events() { + reset_events(); + let metadata_key = snapshot_key("not-compatible-with-any-os@1.0.0"); + let snapshot_key_a = snapshot_key("not-compatible-with-any-os@1.0.0(react@17.0.2)"); + let snapshot_key_b = snapshot_key("not-compatible-with-any-os@1.0.0(react@18.0.0)"); + + let mut snapshots = HashMap::new(); + snapshots + .insert(snapshot_key_a.clone(), SnapshotEntry { optional: true, ..Default::default() }); + snapshots + .insert(snapshot_key_b.clone(), SnapshotEntry { optional: true, ..Default::default() }); + let mut packages = HashMap::new(); + packages.insert( + metadata_key, + synthetic_metadata(None, None, Some(&["this-os-does-not-exist"]), None), + ); + + let skipped = compute_skipped_snapshots::( + &snapshots, + &packages, + &host("20.10.0", "darwin", "arm64"), + "/proj", + ) + .unwrap(); + + // Both snapshot variants land in the skip set (each variant has + // its own virtual-store slot to suppress). + assert!(skipped.contains(&snapshot_key_a)); + assert!(skipped.contains(&snapshot_key_b)); + + // ...but the reporter only sees one event for the metadata row. + let events = take_events(); + let skipped_events: Vec<_> = + events.iter().filter(|e| matches!(e, LogEvent::SkippedOptionalDependency(_))).collect(); + assert_eq!(skipped_events.len(), 1, "must dedup per metadata row"); +} diff --git a/pacquet/crates/package-manager/src/lib.rs b/pacquet/crates/package-manager/src/lib.rs index d194d067b2..4b3ffd72ac 100644 --- a/pacquet/crates/package-manager/src/lib.rs +++ b/pacquet/crates/package-manager/src/lib.rs @@ -13,6 +13,7 @@ mod install_frozen_lockfile; mod install_package_by_snapshot; mod install_package_from_registry; mod install_without_lockfile; +mod installability; mod link_bins; mod link_file; mod retry_config; @@ -37,6 +38,7 @@ pub use install_frozen_lockfile::*; pub use install_package_by_snapshot::*; pub use install_package_from_registry::*; pub use install_without_lockfile::*; +pub use installability::*; pub use link_bins::*; pub use link_file::*; pub use symlink_direct_dependencies::*; diff --git a/pacquet/crates/package-manager/src/link_bins.rs b/pacquet/crates/package-manager/src/link_bins.rs index dda6448208..314a2fa926 100644 --- a/pacquet/crates/package-manager/src/link_bins.rs +++ b/pacquet/crates/package-manager/src/link_bins.rs @@ -1,4 +1,4 @@ -use crate::PackageManifests; +use crate::{PackageManifests, SkippedSnapshots}; use derive_more::{Display, Error}; use miette::Diagnostic; use pacquet_cmd_shim::{ @@ -138,6 +138,13 @@ pub struct LinkVirtualStoreBins<'a> { /// installed earlier in the same run still get their bins /// linked. pub package_manifests: &'a PackageManifests, + /// Snapshots the installability pass marked optional+incompatible. + /// Their slots were never created by [`crate::CreateVirtualStore`], + /// so the bin linker has nothing to walk for them — and any + /// child-manifest disk read against their non-existent + /// `/node_modules/` would fail. Excluding them up + /// front matches the rest of the install pipeline's filtering. + pub skipped: &'a SkippedSnapshots, } impl<'a> LinkVirtualStoreBins<'a> { @@ -162,8 +169,13 @@ impl<'a> LinkVirtualStoreBins<'a> { + FsSetExecutable + FsEnsureExecutableBits, { - let LinkVirtualStoreBins { virtual_store_dir, snapshots, packages, package_manifests } = - self; + let LinkVirtualStoreBins { + virtual_store_dir, + snapshots, + packages, + package_manifests, + skipped, + } = self; if let Some(snapshots) = snapshots { let has_bin_set = build_has_bin_set(packages); run_lockfile_driven::( @@ -171,6 +183,7 @@ impl<'a> LinkVirtualStoreBins<'a> { snapshots, has_bin_set.as_ref(), package_manifests, + skipped, ) } else { run_with_readdir::(virtual_store_dir) @@ -227,6 +240,7 @@ fn run_lockfile_driven( snapshots: &HashMap, has_bin_set: Option<&HashSet>, package_manifests: &PackageManifests, + skipped: &SkippedSnapshots, ) -> Result<(), LinkVirtualStoreBinsError> where Api: FsReadFile @@ -249,7 +263,18 @@ where // a `HashMap` directly with `par_iter` would require collecting // anyway, and explicit collection here keeps the parallelism // contract obvious. - let slot_entries: Vec<(&PackageKey, &SnapshotEntry)> = snapshots.iter().collect(); + // + // Filter out installability-skipped snapshots here: their + // virtual-store slot was never created (see + // [`crate::CreateVirtualStore::run`]'s `survivors` filter), so + // attempting to walk the snapshot's `dependencies` / + // `optional_dependencies` for bin linking would either fall + // through to a cold-batch disk read against a non-existent + // `/node_modules/` (returning `None` harmlessly but + // wasting work) or — worse — create a `/.../node_modules/.bin` + // directory under a slot that doesn't exist on disk. + let slot_entries: Vec<(&PackageKey, &SnapshotEntry)> = + snapshots.iter().filter(|(slot_key, _)| !skipped.contains(slot_key)).collect(); slot_entries.par_iter().try_for_each(|(slot_key, snapshot)| { let children = snapshot .dependencies diff --git a/pacquet/crates/package-manager/src/link_bins/tests.rs b/pacquet/crates/package-manager/src/link_bins/tests.rs index a5ca84caaf..f13c14b551 100644 --- a/pacquet/crates/package-manager/src/link_bins/tests.rs +++ b/pacquet/crates/package-manager/src/link_bins/tests.rs @@ -1,4 +1,5 @@ use super::{LinkVirtualStoreBins, LinkVirtualStoreBinsError, link_direct_dep_bins}; +use crate::SkippedSnapshots; use pacquet_cmd_shim::is_shim_pointing_at; use serde_json::json; use std::{ @@ -44,6 +45,7 @@ fn writes_child_bins_into_slot_own_package_node_modules() { snapshots: None, packages: None, package_manifests: &Default::default(), + skipped: &SkippedSnapshots::default(), } .run() .unwrap(); @@ -111,6 +113,7 @@ fn skips_slot_own_package_when_walking_children() { snapshots: None, packages: None, package_manifests: &Default::default(), + skipped: &SkippedSnapshots::default(), } .run() .unwrap(); @@ -135,6 +138,7 @@ fn link_virtual_store_bins_no_op_when_dir_missing() { snapshots: None, packages: None, package_manifests: &Default::default(), + skipped: &SkippedSnapshots::default(), } .run() .expect("missing dir is Ok"); @@ -172,6 +176,7 @@ fn link_virtual_store_bins_handles_scoped_slot_name() { snapshots: None, packages: None, package_manifests: &Default::default(), + skipped: &SkippedSnapshots::default(), } .run() .unwrap(); @@ -222,6 +227,7 @@ fn link_virtual_store_bins_handles_peer_resolved_slot_name() { snapshots: None, packages: None, package_manifests: &Default::default(), + skipped: &SkippedSnapshots::default(), } .run() .unwrap(); @@ -271,6 +277,7 @@ fn link_virtual_store_bins_handles_unscoped_name_with_plus() { snapshots: None, packages: None, package_manifests: &Default::default(), + skipped: &SkippedSnapshots::default(), } .run() .unwrap(); @@ -295,6 +302,7 @@ fn link_virtual_store_bins_skips_slot_without_node_modules() { snapshots: None, packages: None, package_manifests: &Default::default(), + skipped: &SkippedSnapshots::default(), } .run() .unwrap(); @@ -321,6 +329,7 @@ fn link_virtual_store_bins_skips_slot_without_own_package_dir() { snapshots: None, packages: None, package_manifests: &Default::default(), + skipped: &SkippedSnapshots::default(), } .run() .expect("missing own-package dir is skipped silently"); @@ -468,6 +477,7 @@ fn link_virtual_store_bins_propagates_read_error_via_di() { snapshots: None, packages: None, package_manifests: &Default::default(), + skipped: &SkippedSnapshots::default(), } .run_with::() .expect_err("read_dir error must propagate"); diff --git a/pacquet/crates/package-manager/src/symlink_direct_dependencies.rs b/pacquet/crates/package-manager/src/symlink_direct_dependencies.rs index 0dddca5883..63b41f2a43 100644 --- a/pacquet/crates/package-manager/src/symlink_direct_dependencies.rs +++ b/pacquet/crates/package-manager/src/symlink_direct_dependencies.rs @@ -1,4 +1,4 @@ -use crate::{link_direct_dep_bins, symlink_package}; +use crate::{SkippedSnapshots, link_direct_dep_bins, symlink_package}; use derive_more::{Display, Error}; use miette::Diagnostic; use pacquet_cmd_shim::LinkBinsError; @@ -28,6 +28,13 @@ where /// Install root, threaded into the `pnpm:root` `prefix` field. /// Same value as the `prefix` in [`pacquet_reporter::StageLog`]. pub requester: &'a str, + /// Snapshots the installability pass marked optional+incompatible. + /// A direct dep whose resolved snapshot key is in this set is + /// omitted from `node_modules/` (no symlink, no + /// `pnpm:root added` event, no bin linking). Mirrors pnpm's + /// `linkDirectDeps` walk skipping entries whose `depPath` is + /// in `skipPkgIds`. + pub skipped: &'a SkippedSnapshots, } /// Error type of [`SymlinkDirectDependencies`]. @@ -49,7 +56,8 @@ where { /// Execute the subroutine. pub fn run(self) -> Result<(), SymlinkDirectDependenciesError> { - let SymlinkDirectDependencies { config, importers, dependency_groups, requester } = self; + let SymlinkDirectDependencies { config, importers, dependency_groups, requester, skipped } = + self; let project_snapshot = importers.get(Lockfile::ROOT_IMPORTER_KEY).ok_or_else(|| { SymlinkDirectDependenciesError::MissingRootImporter { @@ -99,6 +107,16 @@ where .map(move |(name, spec)| (name, spec, group)) }) .filter(|(name, _, _)| seen.insert(*name)) + // Drop direct deps whose resolved snapshot landed in the + // skipped set. Without this filter, the symlink would + // either dangle (no virtual-store slot was created) or + // — worse — point at a half-installed slot from a prior + // install. Mirrors upstream's `linkDirectDeps` walk + // skipping entries whose `depPath` is in `skipPkgIds`. + .filter(|(name, spec, _)| { + let resolved = PkgNameVerPeer::new(PkgName::clone(name), spec.version.clone()); + !skipped.contains(&resolved) + }) .collect(); entries.par_iter().for_each(|(name, spec, group)| { diff --git a/pacquet/crates/package-manager/src/symlink_direct_dependencies/tests.rs b/pacquet/crates/package-manager/src/symlink_direct_dependencies/tests.rs index 16ac635cb1..33dd2bf4f9 100644 --- a/pacquet/crates/package-manager/src/symlink_direct_dependencies/tests.rs +++ b/pacquet/crates/package-manager/src/symlink_direct_dependencies/tests.rs @@ -1,4 +1,5 @@ use super::{SymlinkDirectDependencies, SymlinkDirectDependenciesError}; +use crate::SkippedSnapshots; use pacquet_config::Config; use pacquet_lockfile::{Lockfile, ProjectSnapshot, ResolvedDependencyMap, ResolvedDependencySpec}; use pacquet_package_manifest::DependencyGroup; @@ -85,6 +86,7 @@ fn emits_pnpm_root_added_per_direct_dependency() { importers: &importers, dependency_groups: [DependencyGroup::Prod, DependencyGroup::Dev], requester, + skipped: &SkippedSnapshots::default(), } .run::() .expect("symlink should succeed"); @@ -198,6 +200,7 @@ fn duplicate_dep_across_groups_collapses_to_one_entry() { // Prod first → first-wins gives `dependencyType: prod`. dependency_groups: [DependencyGroup::Prod, DependencyGroup::Optional], requester: "/proj", + skipped: &SkippedSnapshots::default(), } .run::() .expect("symlink should succeed"); @@ -237,6 +240,7 @@ fn missing_root_importer_surfaces_as_error() { importers: &importers, dependency_groups: [DependencyGroup::Prod], requester: "/proj", + skipped: &SkippedSnapshots::default(), } .run::();