mirror of
https://github.com/pnpm/pnpm.git
synced 2026-07-19 04:02:32 -04:00
* 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<String> }` 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).
This commit is contained in:
@@ -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<PlatformAssetResolution>,
|
||||
}
|
||||
|
||||
/// 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<String>,
|
||||
}
|
||||
|
||||
/// 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")]
|
||||
|
||||
@@ -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<PlatformAssetTarget>) -> 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
|
||||
/// <https://github.com/pnpm/pnpm/blob/94240bc046/resolving/resolver-base/src/index.ts#L100-L107>:
|
||||
/// `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")));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user