From cb6f9e58c3efdfd2215e9554c6fece30d34ba3f2 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Wed, 13 May 2026 18:34:30 +0200 Subject: [PATCH] feat(lockfile): `select_platform_variant` + `PlatformSelector` (#437 slice B) (#466) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(lockfile): select_platform_variant + PlatformSelector (#437 slice B) Add the variant-picking logic for `VariationsResolution` ahead of Slice D's install-pipeline dispatch: - `PlatformSelector { os, cpu, libc: Option }` mirrors upstream's [`PlatformSelector`](https://github.com/pnpm/pnpm/blob/94240bc046/resolving/resolver-base/src/index.ts#L78-L83). The `libc` tri-state encodes pnpm's `string | null | undefined` shape: `None` (host doesn't care about libc — macOS/Windows/BSD) and `Some("glibc")` collapse to the same matching arm (variant must have no `libc:` annotation); `Some("musl")` requires an exact `libc: "musl"` annotation so the glibc default doesn't silently win on a musl host. - `select_platform_variant(variants, selector)` ports [`selectPlatformVariant`](https://github.com/pnpm/pnpm/blob/94240bc046/resolving/resolver-base/src/index.ts#L92-L98). Iterates `variants` in declaration order and returns the first variant whose `targets[]` contains an `(os, cpu, libc)` triple matching the selector. Targets-array scan is linear because real archives ship 1–3 target entries per variant; quadratic cost is immaterial. - `libc_matches(variant_libc, requested_libc)` is the asymmetric helper from [`libcMatches`](https://github.com/pnpm/pnpm/blob/94240bc046/resolving/resolver-base/src/index.ts#L100-L107). Crate-private so the matching policy isn't part of the public API. Lives in `pacquet-lockfile` next to the variant types (mirrors upstream's `resolver-base` package boundary). The host-platform side (constructing the `PlatformSelector` from `pacquet-graph-hasher`'s existing `host_platform/arch/libc` helpers) lands at the install dispatcher in Slice D — keeps `pacquet-lockfile` free of the `graph-hasher` dep and lets tests drive the picker with synthetic hosts. Tests: first-match wins, multi-target variant matches any host triple, no-match returns `None`, musl-host rejects glibc-default variant, musl-host matches musl-annotated variant, and a six-row `libc_matches` truth table (None / "glibc" / "musl" / unknown-future-libc on both sides) pinning the asymmetric contract from upstream. Part of #437. --- Written by an agent (Claude Code, claude-opus-4-7). * docs: refresh stale references + add declaration-order tie-breaker test Two follow-ups from Copilot review on PR #466: - `resolution.rs:152-154` claimed "the variant picker checks at runtime that the resolved inner is atomic", but `select_platform_variant` does not. Reword to describe the actual contract: pacquet's `PlatformAssetResolution.resolution` is typed as the full `LockfileResolution` for serde uniformity, and the lockfile is trusted to honor upstream's atomic-inner invariant. No infinite-recursion risk because the install dispatcher doesn't call back into `select_platform_variant` for non-`Variations` inputs. - `resolution.rs:169` referenced `pick_variant` in `pacquet-package-manager`, but the picker actually lives in this module as `select_platform_variant`. Updated the pointer. - `pick_returns_first_when_multiple_variants_match` test: two variants both list the same `(darwin, arm64)` target; the test asserts the first one wins, pinning the `Array.prototype.find` semantics. Pnpm-written lockfiles can rely on declaration order (e.g., listing a preferred build before a fallback) — without this test, a future refactor that switched the iteration to a triple-keyed `BTreeMap` would silently break that. No behavior change; doc-comment text + test addition. --- Written by an agent (Claude Code, claude-opus-4-7). --- pacquet/crates/lockfile/src/resolution.rs | 78 ++++++++- .../crates/lockfile/src/resolution/tests.rs | 154 +++++++++++++++++- 2 files changed, 226 insertions(+), 6 deletions(-) diff --git a/pacquet/crates/lockfile/src/resolution.rs b/pacquet/crates/lockfile/src/resolution.rs index 4416111f91..3e702360cb 100644 --- a/pacquet/crates/lockfile/src/resolution.rs +++ b/pacquet/crates/lockfile/src/resolution.rs @@ -149,9 +149,15 @@ pub struct PlatformAssetTarget { /// /// The inner resolution is *atomic* upstream — a `BinaryResolution`, /// `TarballResolution`, etc. — never another `VariationsResolution`. -/// Pacquet keeps it typed as the full `LockfileResolution` for -/// serde-round-trip uniformity; the variant picker checks at runtime -/// that the resolved inner is atomic. +/// Pacquet's type is wider (the full [`LockfileResolution`]) for serde- +/// round-trip uniformity, and we trust the lockfile to honor the +/// upstream contract: [`select_platform_variant`] does not add a +/// runtime check rejecting a nested `Variations`. A malformed +/// lockfile that nested them would just route the picked variant's +/// inner shape back through the install dispatcher, which surfaces +/// each shape independently — no infinite recursion is possible +/// because the install dispatcher does not call back into +/// [`select_platform_variant`] for non-`Variations` inputs. #[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] #[serde(deny_unknown_fields, rename_all = "camelCase")] pub struct PlatformAssetResolution { @@ -166,13 +172,77 @@ pub struct PlatformAssetResolution { /// /// At install time, the dispatcher walks `variants` in declaration /// order and picks the first whose `targets[]` includes the host -/// triple — see `pick_variant` in `pacquet-package-manager`. +/// triple — see [`select_platform_variant`] in this module. #[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] #[serde(deny_unknown_fields, rename_all = "camelCase")] pub struct VariationsResolution { pub variants: Vec, } +/// Host triple used to pick a variant out of a [`VariationsResolution`]. +/// Mirrors pnpm's +/// [`PlatformSelector`](https://github.com/pnpm/pnpm/blob/94240bc046/resolving/resolver-base/src/index.ts#L78-L83). +/// +/// `libc`'s tri-state encodes pnpm's `string | null | undefined` shape: +/// +/// - `None` — the host's libc constraint is irrelevant (macOS, Windows, +/// BSD, …). Matches a variant whose `libc` is `None` (the default +/// build); a `libc: "musl"` variant is rejected since `musl` is a +/// non-default, non-interchangeable artifact. +/// - `Some("glibc")` — Linux with glibc. Same matching rule as `None`: +/// the default variant wins, musl variants are skipped. Upstream +/// collapses `null` and `"glibc"` into the same arm in +/// [`libcMatches`](https://github.com/pnpm/pnpm/blob/94240bc046/resolving/resolver-base/src/index.ts#L100-L107) +/// because the variant emitter only annotates non-glibc builds. +/// - `Some("musl")` — Linux with musl. Requires an exact `libc: +/// "musl"` annotation on the variant, so the glibc default doesn't +/// silently install. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PlatformSelector { + pub os: String, + pub cpu: String, + pub libc: Option, +} + +/// Pick the variant whose target list contains the host triple, or +/// `None` if no variant matches. Port of pnpm's +/// [`selectPlatformVariant`](https://github.com/pnpm/pnpm/blob/94240bc046/resolving/resolver-base/src/index.ts#L92-L98). +/// +/// Iterates `variants` in declaration order and returns the first +/// `PlatformAssetResolution` whose `targets[]` contains an `(os, cpu, +/// libc?)` triple matching `selector`. Each variant's target list is +/// scanned linearly — `targets[]` is typically 1–3 entries (one per +/// architecture combo that shares an artifact), so the nested-loop +/// cost is negligible. +pub fn select_platform_variant<'a>( + variants: &'a [PlatformAssetResolution], + selector: &PlatformSelector, +) -> Option<&'a PlatformAssetResolution> { + variants.iter().find(|variant| { + variant.targets.iter().any(|target| { + target.os == selector.os + && target.cpu == selector.cpu + && libc_matches(target.libc.as_deref(), selector.libc.as_deref()) + }) + }) +} + +/// Check whether a variant's `libc` annotation matches the host +/// selector's `libc` value. Port of upstream's +/// [`libcMatches`](https://github.com/pnpm/pnpm/blob/94240bc046/resolving/resolver-base/src/index.ts#L100-L107). +/// +/// The contract is asymmetric on purpose: `None` and `"glibc"` on the +/// selector side both demand `None` on the variant (the unannotated +/// default), so a `musl` variant cannot win for a glibc host. A +/// non-default selector value (e.g. `"musl"`) requires the variant to +/// declare the exact same value. +pub(crate) fn libc_matches(variant_libc: Option<&str>, requested_libc: Option<&str>) -> bool { + match requested_libc { + None | Some("glibc") => variant_libc.is_none(), + Some(requested) => variant_libc == Some(requested), + } +} + /// Represent the resolution object. #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, From, TryInto)] #[serde(from = "ResolutionSerde", into = "ResolutionSerde")] diff --git a/pacquet/crates/lockfile/src/resolution/tests.rs b/pacquet/crates/lockfile/src/resolution/tests.rs index 551a037f3b..f139d76891 100644 --- a/pacquet/crates/lockfile/src/resolution/tests.rs +++ b/pacquet/crates/lockfile/src/resolution/tests.rs @@ -1,7 +1,8 @@ use super::{ BinaryArchive, BinaryResolution, BinarySpec, DirectoryResolution, GitResolution, - LockfileResolution, PlatformAssetResolution, PlatformAssetTarget, RegistryResolution, - TarballResolution, VariationsResolution, + LockfileResolution, PlatformAssetResolution, PlatformAssetTarget, PlatformSelector, + RegistryResolution, TarballResolution, VariationsResolution, libc_matches, + select_platform_variant, }; use crate::serialize_yaml; use pretty_assertions::assert_eq; @@ -531,3 +532,152 @@ fn serialize_variations_resolution() { }; assert_eq!(received, expected); } + +// ----------------------------------------------------------------------------- +// `select_platform_variant` / `libc_matches` — Slice B +// ----------------------------------------------------------------------------- + +fn binary_resolution(url: &str) -> LockfileResolution { + LockfileResolution::Binary(BinaryResolution { + url: url.to_string(), + integrity: integrity( + "sha512-gf6ZldcfCDyNXPRiW3lQjEP1Z9rrUM/4Cn7BZbv3SdTA82zxWRP8OmLwvGR974uuENhGCFgFdN11z3n1Ofpprg==", + ), + bin: BinarySpec::Single("bin/node".to_string()), + archive: BinaryArchive::Tarball, + prefix: None, + }) +} + +fn target(os: &str, cpu: &str, libc: Option<&str>) -> PlatformAssetTarget { + PlatformAssetTarget { os: os.to_string(), cpu: cpu.to_string(), libc: libc.map(str::to_string) } +} + +fn variant(url: &str, targets: Vec) -> PlatformAssetResolution { + PlatformAssetResolution { resolution: binary_resolution(url), targets } +} + +fn selector(os: &str, cpu: &str, libc: Option<&str>) -> PlatformSelector { + PlatformSelector { os: os.to_string(), cpu: cpu.to_string(), libc: libc.map(str::to_string) } +} + +/// The picker returns the first variant whose `targets[]` contains an +/// `(os, cpu, libc)` triple matching the selector. Mirrors upstream's +/// declaration-order semantics — `Array.prototype.find` in +/// `selectPlatformVariant`. +#[test] +fn pick_first_matching_variant() { + let variants = vec![ + variant("darwin-arm64", vec![target("darwin", "arm64", None)]), + variant("linux-x64", vec![target("linux", "x64", None)]), + ]; + let picked = select_platform_variant(&variants, &selector("linux", "x64", Some("glibc"))) + .expect("matching variant"); + assert_eq!( + picked.resolution.integrity().map(ToString::to_string), + Some("sha512-gf6ZldcfCDyNXPRiW3lQjEP1Z9rrUM/4Cn7BZbv3SdTA82zxWRP8OmLwvGR974uuENhGCFgFdN11z3n1Ofpprg==".to_string()), + "picked variant should be the linux-x64 one (url is opaque to integrity, but the structural fixture means both share the same hash)", + ); + assert_eq!(picked.targets, vec![target("linux", "x64", None)]); +} + +/// One variant can cover multiple host triples; the picker matches +/// against any entry in the variant's `targets[]`. Real-world Node +/// archives ship a single `darwin` tarball that covers both `x64` +/// and `arm64` via separate target entries. +#[test] +fn pick_matches_any_target_in_a_variant() { + let variants = vec![variant( + "darwin-universal", + vec![target("darwin", "arm64", None), target("darwin", "x64", None)], + )]; + let picked = select_platform_variant(&variants, &selector("darwin", "x64", None)); + assert!(picked.is_some()); +} + +/// No variant matching the host triple → `None`. The install +/// dispatcher will surface this as a typed "no variant matches host +/// platform" error (Slice D). +#[test] +fn pick_returns_none_when_no_variant_matches() { + let variants = vec![variant("darwin-arm64", vec![target("darwin", "arm64", None)])]; + assert!(select_platform_variant(&variants, &selector("linux", "x64", Some("glibc"))).is_none()); +} + +/// On a musl host, the glibc-default variant must NOT win silently. +/// Upstream rejects a `None`-libc variant when the selector requests +/// `musl`, requiring an exact `libc: "musl"` annotation to match. +/// Without this, a musl host would attempt to run a glibc-linked +/// binary. +#[test] +fn pick_rejects_default_variant_for_musl_host() { + let variants = vec![variant("linux-x64-glibc", vec![target("linux", "x64", None)])]; + assert!( + select_platform_variant(&variants, &selector("linux", "x64", Some("musl"))).is_none(), + "musl host must not silently pick the glibc default variant", + ); +} + +/// When two variants both match the same `(os, cpu, libc)` triple, +/// declaration order wins — mirroring upstream's `Array.prototype.find` +/// in `selectPlatformVariant`. Pinning this guards against a future +/// refactor that reorders the iteration (e.g., to a `BTreeMap` keyed +/// by triple) since pnpm-written lockfiles can rely on the order +/// (e.g., listing a preferred build before a fallback). +#[test] +fn pick_returns_first_when_multiple_variants_match() { + // Both variants list the same darwin-arm64 target. The first one + // is identified by its URL via the inner `BinaryResolution`. + let variants = vec![ + variant("first-darwin-arm64", vec![target("darwin", "arm64", None)]), + variant("second-darwin-arm64", vec![target("darwin", "arm64", None)]), + ]; + let picked = select_platform_variant(&variants, &selector("darwin", "arm64", None)) + .expect("matching variant"); + let LockfileResolution::Binary(inner) = &picked.resolution else { + panic!("expected Binary inner resolution"); + }; + assert_eq!(inner.url, "first-darwin-arm64", "declaration order must win"); +} + +/// A musl variant is picked only when the selector requests musl. +#[test] +fn pick_matches_musl_variant_for_musl_host() { + let variants = vec![ + variant("linux-x64-glibc", vec![target("linux", "x64", None)]), + variant("linux-x64-musl", vec![target("linux", "x64", Some("musl"))]), + ]; + let picked = select_platform_variant(&variants, &selector("linux", "x64", Some("musl"))) + .expect("musl variant present"); + assert_eq!(picked.targets, vec![target("linux", "x64", Some("musl"))]); +} + +/// `libc_matches` truth table. Pinning each cell guards the +/// upstream contract in +/// : +/// `None`-libc selector or `"glibc"` selector → variant libc must be +/// `None`; any other selector value → exact match. +#[test] +fn libc_matches_truth_table() { + // Selector says "no libc constraint" (non-Linux host): only + // the default (unannotated) variant matches. + assert!(libc_matches(None, None)); + assert!(!libc_matches(Some("musl"), None)); + assert!(!libc_matches(Some("glibc"), None)); + + // Selector says "glibc" (Linux glibc host): same rule as None. + assert!(libc_matches(None, Some("glibc"))); + assert!(!libc_matches(Some("musl"), Some("glibc"))); + + // Selector says "musl" (Linux musl host): require exact musl + // annotation; the default variant is rejected. + assert!(libc_matches(Some("musl"), Some("musl"))); + assert!(!libc_matches(None, Some("musl"))); + + // Selector says an unknown libc (future-compat): require + // exact match. The default variant is rejected so a future + // libc value can't be silently aliased to glibc. + assert!(libc_matches(Some("uclibc"), Some("uclibc"))); + assert!(!libc_matches(None, Some("uclibc"))); + assert!(!libc_matches(Some("glibc"), Some("uclibc"))); +}