From 55434d24637942fa3fe1bb3565bc86ace96df193 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Mon, 14 Sep 2026 02:03:33 +0200 Subject: [PATCH] feat(python): replay a lockfile on any target that installs it (#14847) A pylock.toml was reused only when the whole marker environment and the ordered wheel tag list were equal to the ones that produced it, so a kernel update invalidated a lockfile whose interpreter, wheel tags, requirements and packages were unchanged, and a frozen install on CI failed for it. Closes pnpm/pnpm#14843. A lockfile is now replayed when it still applies to the target: the requirements, index and requires-python are the ones it was resolved for, and every pinned wheel carries tags the interpreter accepts. The recorded environment is not compared. Whether the locked graph is still the one the markers select is settled by re-solving it against them, which the locked install already did; that check is exact where an equality check on the environment refused every kernel update. A wheel that installs here is the same wheel wherever it was chosen. An install that may resolve again does so when the locked graph no longer matches the markers, with a warning naming the lockfile and the resolver's explanation, instead of failing. A frozen install keeps failing on it, and keeps failing on a lockfile pinning a wheel the target cannot install, with the reason attached. The environments marker written to the lockfile names python_version and the marker variables the solved graph reads, instead of every variable of the resolving environment. PEP 751 requires an installer to refuse a lockfile whose environments none of its markers satisfy, so a marker over the whole environment made other installers refuse it after a kernel update too. The lockfile format is unchanged; tool.pnpm still records the resolving target, which a pnpr server's answer is checked against. The environments marker names python_full_version when a locked package's Requires-Python tells patch releases of the running minor apart, so a PEP 751 installer does not accept the lockfile on a patch release a package excludes. A fetch failure on a lockfile resolved for another target replaces the lockfile when the install may resolve again: such a lockfile may pin wheels the markers no longer reach, and was replaced without fetching anything before replay existed. A lockfile resolved for this exact target pins only wheels the target needs, so its fetch failures fail every install. --- .../python-lockfile-replays-across-targets.md | 7 + pnpm/crates/cli/tests/suite/python.rs | 108 +++++++++ .../cli/tests/suite/python/validation.rs | 125 ++++++++++ .../python-installer/src/environment.rs | 15 +- pnpm/crates/python-installer/src/lib.rs | 71 +----- pnpm/crates/python-installer/src/lockfile.rs | 148 ++++++++++++ pnpm/crates/python-installer/src/registry.rs | 10 +- pnpm/crates/python-installer/src/tests.rs | 2 +- pnpm/crates/python-resolver/src/lockfile.rs | 216 +++++++++++++++++- pnpm/crates/python-resolver/src/tests.rs | 169 +++++++++++++- pnpm/plans/PYTHON_SPIKE.md | 25 +- pnpr/crates/pnpr/src/resolver/pypi.rs | 1 + 12 files changed, 809 insertions(+), 88 deletions(-) create mode 100644 .changeset/python-lockfile-replays-across-targets.md create mode 100644 pnpm/crates/python-installer/src/lockfile.rs diff --git a/.changeset/python-lockfile-replays-across-targets.md b/.changeset/python-lockfile-replays-across-targets.md new file mode 100644 index 0000000000..6c25836b53 --- /dev/null +++ b/.changeset/python-lockfile-replays-across-targets.md @@ -0,0 +1,7 @@ +--- +"pacquet": minor +--- + +`pnpm install --frozen-lockfile` now replays a `pylock.toml` on any Python target that can install it. Previously the lockfile was reused only for the exact marker environment and wheel tag order that produced it, so a kernel update alone invalidated it. A lockfile is now reused when the project's requirements, index and `requires-python` are unchanged, every pinned wheel carries tags the interpreter accepts, and the locked packages are exactly what the interpreter's markers select [#14843](https://github.com/pnpm/pnpm/issues/14843). + +The `environments` marker written to `pylock.toml` now names only the interpreter version and the marker variables the locked dependency graph reads. When the lockfile is not frozen and its locked graph no longer matches the target, `pnpm install` warns and resolves the project again. diff --git a/pnpm/crates/cli/tests/suite/python.rs b/pnpm/crates/cli/tests/suite/python.rs index a76f0ad514..c7e6c28918 100644 --- a/pnpm/crates/cli/tests/suite/python.rs +++ b/pnpm/crates/cli/tests/suite/python.rs @@ -304,6 +304,114 @@ async fn installs_real_environment_with_ranges_extras_markers_scripts_and_offlin assert_eq!(lock, replayed_lock); } +/// The scenario of pnpm/pnpm#14843: an interpreter wrapper that reports the +/// kernel release `PNPM_TEST_KERNEL_RELEASE` names, with nothing else about +/// the interpreter, its wheel tags, or the project changing between runs. +#[cfg(unix)] +#[tokio::test] +async fn frozen_lockfile_replays_after_a_kernel_only_marker_change() { + use std::os::unix::fs::PermissionsExt; + let root = tempfile::tempdir().unwrap(); + let mut server = mockito::Server::new_async().await; + let _alpha = serve( + &mut server, + "alpha", + &[("1.0", wheel("alpha", "1.0", "Requires-Dist: beta; platform_release >= '9'", &[]))], + ) + .await; + let _beta = serve(&mut server, "beta", &[("1.0", wheel("beta", "1.0", "", &[]))]).await; + project(root.path(), &server.url(), &["alpha>=1"]); + let probe = root.path().join("python-probe"); + fs::write( + &probe, + concat!( + "#!/usr/bin/env python3\n", + "import os, platform, sys\n", + "args = sys.argv[1:]\n", + "if args and args[0] == '-I':\n", + " args = args[1:]\n", + "assert len(args) >= 2 and args[0] == '-c'\n", + "platform.release = lambda: os.environ['PNPM_TEST_KERNEL_RELEASE']\n", + "sys.argv = ['-c', *args[2:]]\n", + "exec(compile(args[1], '', 'exec'), {'__name__': '__main__'})\n", + ), + ) + .unwrap(); + fs::set_permissions(&probe, fs::Permissions::from_mode(0o755)).unwrap(); + let workspace = fs::read_to_string(root.path().join("pnpm-workspace.yaml")).unwrap(); + fs::write( + root.path().join("pnpm-workspace.yaml"), + workspace.replace( + "python:\n enabled: true\n", + &format!("python:\n enabled: true\n executable: '{}'\n", probe.display()), + ), + ) + .unwrap(); + let install = |args: &[&str], kernel: &str| { + let mut command = pacquet_in(root.path()); + command.args(args).env("PNPM_TEST_KERNEL_RELEASE", kernel); + command + }; + install(&["install"], "1.0.0").assert().success(); + let lock = fs::read_to_string(root.path().join("pylock.toml")).unwrap(); + eprintln!("LOCK:\n{lock}"); + let parsed: toml::Value = toml::from_str(&lock).unwrap(); + let environments = parsed["environments"].as_array().unwrap(); + assert_eq!(environments.len(), 1); + let marker = environments[0].as_str().unwrap(); + assert!(marker.contains("platform_release == '1.0.0'"), "{marker}"); + assert!(marker.contains("python_version == '"), "{marker}"); + assert!(!marker.contains("platform_version"), "{marker}"); + assert!(!marker.contains("sys_platform"), "{marker}"); + assert_eq!( + parsed["packages"] + .as_array() + .unwrap() + .len(), + 1, + ); + + pnpm_fs::remove_symlink_dir(&root.path().join(".venv")).unwrap(); + install(&["install", "--offline", "--frozen-lockfile"], "1.0.1").assert().success(); + assert_eq!(fs::read_to_string(root.path().join("pylock.toml")).unwrap(), lock); + python(root.path()) + .args(["-c", "import alpha"]) + .assert() + .success(); + + assert_failure_contains( + &mut install(&["install", "--offline", "--frozen-lockfile"], "9.0.0"), + "Python lockfile does not satisfy the project", + ); + assert_eq!(fs::read_to_string(root.path().join("pylock.toml")).unwrap(), lock); + + let output = install(&["install"], "9.0.0").output().unwrap(); + let stdout = String::from_utf8_lossy(&output.stdout); + eprintln!("stdout:\n{stdout}\nstderr:\n{}", String::from_utf8_lossy(&output.stderr)); + assert!(output.status.success()); + assert!(stdout.contains("[WARN] Ignoring Python lockfile"), "{stdout}"); + let relocked: toml::Value = + toml::from_str(&fs::read_to_string(root.path().join("pylock.toml")).unwrap()).unwrap(); + assert_eq!( + relocked["packages"] + .as_array() + .unwrap() + .len(), + 2, + ); + assert!( + relocked["environments"][0] + .as_str() + .unwrap() + .contains("platform_release == '9.0.0'"), + "{relocked}", + ); + python(root.path()) + .args(["-c", "import alpha, beta"]) + .assert() + .success(); +} + #[tokio::test] async fn add_updates_pyproject_and_lockfile_without_creating_node_metadata() { let root = tempfile::tempdir().unwrap(); diff --git a/pnpm/crates/cli/tests/suite/python/validation.rs b/pnpm/crates/cli/tests/suite/python/validation.rs index a7d3b1e113..4fc86c7687 100644 --- a/pnpm/crates/cli/tests/suite/python/validation.rs +++ b/pnpm/crates/cli/tests/suite/python/validation.rs @@ -443,3 +443,128 @@ async fn refuses_unmanaged_environment_and_rolls_back_add() { assert_eq!(manifest, fs::read(root.path().join("pyproject.toml")).unwrap()); assert_eq!(fs::read_to_string(root.path().join(".venv/owned-by-user")).unwrap(), "preserve"); } + +#[tokio::test] +async fn frozen_lockfile_rejects_wheels_the_target_cannot_install() { + let root = tempfile::tempdir().unwrap(); + let mut server = mockito::Server::new_async().await; + let _alpha = serve(&mut server, "alpha", &[("1.0", wheel("alpha", "1.0", "", &[]))]).await; + project(root.path(), &server.url(), &["alpha>=1"]); + pacquet_in(root.path()) + .arg("install") + .assert() + .success(); + let environment = pnpm_fs::read_symlink_dir(&root.path().join(".venv")).unwrap(); + let mut lock: toml::Value = + toml::from_str(&fs::read_to_string(root.path().join("pylock.toml")).unwrap()).unwrap(); + lock["packages"][0]["wheels"][0]["name"] = + toml::Value::String("alpha-1.0-cp27-cp27m-win32.whl".to_string()); + let foreign = toml::to_string(&lock).unwrap(); + fs::write(root.path().join("pylock.toml"), &foreign).unwrap(); + assert_failure_contains( + pacquet_in(root.path()).args(["install", "--offline", "--frozen-lockfile"]), + "frozen Python lockfile is missing or out of date", + ); + assert_failure_contains( + pacquet_in(root.path()).args(["install", "--offline", "--frozen-lockfile"]), + "Python wheel is incompatible with this interpreter: alpha-1.0-cp27-cp27m-win32.whl", + ); + assert_eq!(fs::read_to_string(root.path().join("pylock.toml")).unwrap(), foreign); + assert_eq!(environment, pnpm_fs::read_symlink_dir(&root.path().join(".venv")).unwrap()); +} + +#[tokio::test] +async fn an_install_resolves_again_when_the_lockfile_no_longer_satisfies_the_project() { + let root = tempfile::tempdir().unwrap(); + let mut server = mockito::Server::new_async().await; + let _alpha = serve( + &mut server, + "alpha", + &[("1.0", wheel("alpha", "1.0", "Requires-Dist: beta>=1", &[]))], + ) + .await; + let _beta = serve(&mut server, "beta", &[("1.0", wheel("beta", "1.0", "", &[]))]).await; + project(root.path(), &server.url(), &["alpha>=1"]); + pacquet_in(root.path()) + .arg("install") + .assert() + .success(); + let complete = fs::read_to_string(root.path().join("pylock.toml")).unwrap(); + let mut lock: toml::Value = toml::from_str(&complete).unwrap(); + lock["packages"] + .as_array_mut() + .unwrap() + .retain(|package| package["name"].as_str() != Some("beta")); + fs::write(root.path().join("pylock.toml"), toml::to_string(&lock).unwrap()).unwrap(); + let output = pacquet_in(root.path()) + .arg("install") + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&output.stdout); + eprintln!("stdout:\n{stdout}\nstderr:\n{}", String::from_utf8_lossy(&output.stderr)); + assert!(output.status.success()); + assert!(stdout.contains("[WARN] Ignoring Python lockfile"), "{stdout}"); + assert!(stdout.contains("does not satisfy the project"), "{stdout}"); + assert_eq!(fs::read_to_string(root.path().join("pylock.toml")).unwrap(), complete); + python(root.path()) + .args(["-c", "import alpha, beta"]) + .assert() + .success(); +} + +/// A lockfile resolved for another target may pin wheels the markers no +/// longer reach, so an install that may resolve again treats one it +/// cannot fetch as a lockfile to replace, not as a failure. A lockfile +/// resolved for this target pins only wheels the target needs. +#[tokio::test] +async fn an_unfetchable_wheel_fails_only_a_lockfile_resolved_for_this_target() { + let root = tempfile::tempdir().unwrap(); + let mut server = mockito::Server::new_async().await; + let _alpha = serve(&mut server, "alpha", &[("1.0", wheel("alpha", "1.0", "", &[]))]).await; + project(root.path(), &server.url(), &["alpha>=1"]); + pacquet_in(root.path()) + .arg("install") + .assert() + .success(); + let complete = fs::read_to_string(root.path().join("pylock.toml")).unwrap(); + let mut lock: toml::Value = toml::from_str(&complete).unwrap(); + lock["packages"].as_array_mut().unwrap().push( + toml::toml! { + name = "gamma" + version = "1.0" + [[wheels]] + name = "gamma-1.0-py3-none-any.whl" + url = "https://unused.invalid/gamma-1.0-py3-none-any.whl" + hashes = { sha256 = "0000000000000000000000000000000000000000000000000000000000000000" } + } + .into(), + ); + fs::write(root.path().join("pylock.toml"), toml::to_string(&lock).unwrap()).unwrap(); + assert_failure_contains( + pacquet_in(root.path()).args(["install", "--offline"]), + "gamma-1.0-py3-none-any.whl", + ); + + lock["tool"]["pnpm"]["environment"]["platform_release"] = + toml::Value::String("0.0.0-elsewhere".to_string()); + let foreign = toml::to_string(&lock).unwrap(); + fs::write(root.path().join("pylock.toml"), &foreign).unwrap(); + assert_failure_contains( + pacquet_in(root.path()).args(["install", "--offline", "--frozen-lockfile"]), + "gamma-1.0-py3-none-any.whl", + ); + assert_eq!(fs::read_to_string(root.path().join("pylock.toml")).unwrap(), foreign); + let output = pacquet_in(root.path()) + .args(["install", "--offline"]) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&output.stdout); + eprintln!("stdout:\n{stdout}\nstderr:\n{}", String::from_utf8_lossy(&output.stderr)); + assert!(output.status.success()); + assert!(stdout.contains("[WARN] Ignoring Python lockfile"), "{stdout}"); + assert_eq!(fs::read_to_string(root.path().join("pylock.toml")).unwrap(), complete); + python(root.path()) + .args(["-c", "import alpha"]) + .assert() + .success(); +} diff --git a/pnpm/crates/python-installer/src/environment.rs b/pnpm/crates/python-installer/src/environment.rs index 84eb82c717..401f93bbc7 100644 --- a/pnpm/crates/python-installer/src/environment.rs +++ b/pnpm/crates/python-installer/src/environment.rs @@ -18,13 +18,26 @@ pub(super) struct PythonPrepare<'a> { /// What [`PythonPrepare::lockfile`] needs about one project. pub(super) struct LockfileInputs<'a> { - /// The lockfile on disk, when it is still current for these inputs. + /// The lockfile on disk, when it still applies to these inputs on this + /// target. pub(super) existing: Option, + pub(super) lock_path: &'a Path, pub(super) requirements: &'a [pep508_rs::Requirement], pub(super) inputs: Inputs, pub(super) requires_python: Option, } +/// What [`PythonPrepare::replay_lockfile`] needs about the lockfile on +/// disk. +pub(super) struct LockfileReplay<'a> { + pub(super) lock: Lockfile, + pub(super) lock_path: &'a Path, + pub(super) requirements: &'a [pep508_rs::Requirement], + /// Whether the lockfile was resolved for this install's own target, + /// so that every wheel it pins is one this target needs. + pub(super) same_target: bool, +} + /// The lockfile beside the project, or `None` when it has none yet. pub(super) async fn read_existing_lock(lock_path: &Path) -> Result> { let contents = match tokio::fs::read_to_string(lock_path).await { diff --git a/pnpm/crates/python-installer/src/lib.rs b/pnpm/crates/python-installer/src/lib.rs index 8f7b3c7666..28f6c44b5c 100644 --- a/pnpm/crates/python-installer/src/lib.rs +++ b/pnpm/crates/python-installer/src/lib.rs @@ -4,14 +4,15 @@ pub use manifest::DependencySelection; mod add; mod environment; mod host; +mod lockfile; mod manifest; mod registry; mod resolver; use base64::{Engine as _, engine::general_purpose::STANDARD}; use environment::{ - LockfileInputs, PythonPrepare, accept_server_lockfile, ensure_environment_parent, publish_link, - read_existing_lock, resolve_via_pnpr, validate_environment_link, + LockfileInputs, PythonPrepare, ensure_environment_parent, publish_link, + validate_environment_link, }; use host::Interpreter; use miette::{IntoDiagnostic, Result, WrapErr, bail}; @@ -171,19 +172,14 @@ impl PythonPrepare<'_> { let inputs = Inputs::new(&requirements, &self.interpreter.target, self.index.as_str()); let mut registry = self.registry(); let lock_path = root.join("pylock.toml"); - let existing = read_existing_lock(&lock_path).await?; - let fresh = existing - .as_ref() - .is_some_and(|lock| { - lock.tool.pnpm == inputs && lock.requires_python == project.requires_python - }); - if self.context.frozen_lockfile && (!fresh || self.resolve) { - bail!("frozen Python lockfile is missing or out of date: {}", lock_path.display()); - } + let existing = + self.replayable_lockfile(&lock_path, &inputs, project.requires_python.as_deref()) + .await?; let lock = self.lockfile::( &mut registry, LockfileInputs { - existing: existing.filter(|_| fresh && !self.resolve), + existing, + lock_path: &lock_path, requirements: &requirements, inputs, requires_python: project.requires_python.clone(), @@ -243,57 +239,6 @@ impl PythonPrepare<'_> { Ok(()) } - /// The lockfile for one project: the one on disk when it still matches, - /// then the one the server resolves, and a local resolution last. - async fn lockfile( - &self, - registry: &mut Registry<'_>, - LockfileInputs { - existing, - requirements, - inputs, - requires_python, - }: LockfileInputs<'_>, - ) -> Result { - if let Some(lock) = existing { - self.accept_lockfile::(registry, lock, requirements).await - } else if let Some(lock) = resolve_via_pnpr( - self.context.config, - requirements, - &self.interpreter.target, - self.index.as_str(), - requires_python.clone(), - ) - .await? - { - accept_server_lockfile(&lock, &inputs, requires_python.as_deref())?; - self.accept_lockfile::(registry, lock, requirements).await - } else { - let solution = resolver::resolve::(registry, requirements).await?; - Lockfile::new( - ®istry.packages, - &self.interpreter.target, - solution, - inputs, - requires_python, - ) - } - } - - /// Fetch the wheels a ready-made lockfile pins and check that it still - /// covers the project's requirements. - async fn accept_lockfile( - &self, - registry: &mut Registry<'_>, - lock: Lockfile, - requirements: &[pep508_rs::Requirement], - ) -> Result { - lock.seed(&mut registry.packages)?; - registry.fetch_wheels::(&lock.packages).await?; - resolver::validate_locked(registry, requirements)?; - Ok(lock) - } - /// Install the locked wheels the project selects into a fresh /// environment generation. A lockfile-only run builds none. async fn environment( diff --git a/pnpm/crates/python-installer/src/lockfile.rs b/pnpm/crates/python-installer/src/lockfile.rs new file mode 100644 index 0000000000..bfa3b4ead8 --- /dev/null +++ b/pnpm/crates/python-installer/src/lockfile.rs @@ -0,0 +1,148 @@ +//! The lockfile a project installs from: the one on disk when it still +//! applies to the project on this target, else the one the pnpr server +//! resolves, else a local resolution. + +use super::{ + Inputs, Lockfile, Registry, + environment::{ + LockfileInputs, LockfileReplay, PythonPrepare, accept_server_lockfile, read_existing_lock, + resolve_via_pnpr, + }, + resolver, +}; +use miette::Result; +use pnpm_reporter::{GlobalLog, LogEvent, LogLevel, Reporter}; +use std::path::Path; + +impl PythonPrepare<'_> { + /// The lockfile on disk when this install may replay it: one that + /// applies to the project on this target, for an install that is not + /// adding a dependency. A frozen install fails on anything else. + pub(super) async fn replayable_lockfile( + &self, + lock_path: &Path, + inputs: &Inputs, + requires_python: Option<&str>, + ) -> Result> { + let existing = read_existing_lock(lock_path).await?; + let stale = if self.resolve { + Some(miette::miette!("adding a dependency resolves the project again")) + } else { + match &existing { + Some(lock) => { + lock.applies_to(inputs, requires_python, &self.interpreter.target).err() + } + None => Some(miette::miette!("the project has no lockfile")), + } + }; + match stale { + None => Ok(existing), + Some(reason) if self.context.frozen_lockfile => Err(reason.wrap_err(format!( + "frozen Python lockfile is missing or out of date: {}", + lock_path.display(), + ))), + Some(_) => Ok(None), + } + } + + /// The lockfile for one project: the one on disk when it still covers + /// the project on this target, then the one the server resolves, and a + /// local resolution last. + pub(super) async fn lockfile( + &self, + registry: &mut Registry<'_>, + inputs: LockfileInputs<'_>, + ) -> Result { + let LockfileInputs { + existing, + lock_path, + requirements, + inputs, + requires_python, + } = inputs; + if let Some(lock) = existing { + let replay = LockfileReplay { + same_target: lock.tool.pnpm == inputs, + lock, + lock_path, + requirements, + }; + if let Some(lock) = self.replay_lockfile::(registry, replay).await? { + return Ok(lock); + } + } + if let Some(lock) = resolve_via_pnpr( + self.context.config, + requirements, + &self.interpreter.target, + self.index.as_str(), + requires_python.clone(), + ) + .await? + { + accept_server_lockfile(&lock, &inputs, requires_python.as_deref())?; + self.accept_lockfile::(registry, lock, requirements).await + } else { + let solution = resolver::resolve::(registry, requirements).await?; + Lockfile::new( + ®istry.packages, + &self.interpreter.target, + requirements, + solution, + inputs, + requires_python, + ) + } + } + + /// Replay the lockfile on disk, or `None` when an install that may + /// resolve again should: its wheels install here but the interpreter's + /// markers no longer select its graph, or it was resolved for another + /// target and pins a wheel this install cannot fetch. A frozen install + /// fails on either, and every install fails on a wheel a lockfile + /// resolved for this very target cannot fetch: that lockfile pins only + /// wheels the target needs. + async fn replay_lockfile( + &self, + registry: &mut Registry<'_>, + LockfileReplay { + lock, + lock_path, + requirements, + same_target, + }: LockfileReplay<'_>, + ) -> Result> { + lock.seed(&mut registry.packages)?; + let replayed = match registry.fetch_wheels::(&lock.packages).await { + Ok(()) => resolver::validate_locked(registry, requirements), + Err(error) if same_target => return Err(error), + Err(error) => Err(error), + }; + let Err(error) = replayed else { + return Ok(Some(lock)); + }; + if self.context.frozen_lockfile { + return Err(error); + } + Reporter::emit(&LogEvent::Global(GlobalLog { + level: LogLevel::Warn, + message: format!("Ignoring Python lockfile {}: {error}", lock_path.display()), + })); + registry.packages = pnpm_python_resolver::Packages::new(); + Ok(None) + } + + /// Fetch the wheels a ready-made lockfile pins and check that it still + /// covers the project's requirements. + async fn accept_lockfile( + &self, + registry: &mut Registry<'_>, + lock: Lockfile, + requirements: &[pep508_rs::Requirement], + ) -> Result { + lock.seed(&mut registry.packages)?; + registry.fetch_wheels::(&lock.packages).await?; + resolver::validate_locked(registry, requirements)?; + Ok(lock) + } +} diff --git a/pnpm/crates/python-installer/src/registry.rs b/pnpm/crates/python-installer/src/registry.rs index 0ef8b67104..1d2ccbbf47 100644 --- a/pnpm/crates/python-installer/src/registry.rs +++ b/pnpm/crates/python-installer/src/registry.rs @@ -5,7 +5,7 @@ use pep440_rs::Version; use pep508_rs::PackageName; use pnpm_config::Config; use pnpm_network::{AuthHeaders, ThrottledClient}; -use pnpm_python_resolver::{LockedPackage, Packages, candidates_from_page, wheel_identity}; +use pnpm_python_resolver::{LockedPackage, Packages, candidates_from_page}; use pnpm_reporter::Reporter; use pnpm_tarball::{ArchiveStoreProjection, IngestZipArchiveToStore}; use serde::{Deserialize, Serialize}; @@ -242,11 +242,5 @@ fn validate_wheel_identity( version: &Version, ) -> Result<()> { pnpm_python_resolver::validate_url(&Url::parse(&wheel.url).into_diagnostic()?)?; - let Some((wheel_name, wheel_version, _)) = wheel_identity(&wheel.name, tags)? else { - bail!("Python wheel is incompatible with this interpreter: {}", wheel.name) - }; - if wheel_name != *name || wheel_version != *version { - bail!("Python lockfile wheel identity mismatch: {}", wheel.name); - } - Ok(()) + wheel.check_installable(tags, name, version) } diff --git a/pnpm/crates/python-installer/src/tests.rs b/pnpm/crates/python-installer/src/tests.rs index eba442712a..2df1bdd6c1 100644 --- a/pnpm/crates/python-installer/src/tests.rs +++ b/pnpm/crates/python-installer/src/tests.rs @@ -1,4 +1,4 @@ -use super::{accept_server_lockfile, resolve_via_pnpr}; +use super::environment::{accept_server_lockfile, resolve_via_pnpr}; use pnpm_config::Config; use pnpm_python_resolver::Target; diff --git a/pnpm/crates/python-resolver/src/lockfile.rs b/pnpm/crates/python-resolver/src/lockfile.rs index 13b4ba327d..b54116f64c 100644 --- a/pnpm/crates/python-resolver/src/lockfile.rs +++ b/pnpm/crates/python-resolver/src/lockfile.rs @@ -1,14 +1,17 @@ -use crate::packages::{Candidate, Packages}; +use crate::{ + candidates::{parse_requirement, wheel_identity}, + packages::{Candidate, Packages}, +}; use miette::{IntoDiagnostic, Result, bail}; -use pep440_rs::Version; -use pep508_rs::{MarkerEnvironment, PackageName, Requirement}; +use pep440_rs::{Operator, Version, VersionSpecifiers}; +use pep508_rs::{MarkerEnvironment, MarkerTree, MarkerTreeKind, PackageName, Requirement}; use serde::{Deserialize, Serialize}; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; /// What a resolution is for: the interpreter's marker environment and the /// wheel tags it accepts, in the order it prefers them. Both come from the -/// interpreter that will run the environment, so a lockfile records them -/// and is only reused for the same pair. +/// interpreter that will run the environment. A lockfile records the pair +/// it was resolved for and replays on any target that still installs it. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Target { pub environment: MarkerEnvironment, @@ -33,8 +36,10 @@ pub struct ToolMetadata { pub pnpm: Inputs, } -/// Everything a resolution depended on, so a lockfile can be reused only -/// for the inputs that produced it. +/// Everything a resolution depended on: what the project asked for, and +/// the target it was answered for. A server's answer is accepted only +/// when it was for exactly these; a lockfile on disk is replayed on +/// whatever target still installs it — see [`Lockfile::applies_to`]. #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case", deny_unknown_fields)] pub struct Inputs { @@ -88,6 +93,24 @@ pub struct LockedWheel { } impl LockedWheel { + /// Refuse a wheel that is not `name==version` in a build `tags` + /// accept: what a lockfile pins under a package has to be that + /// package, and installable where it is being replayed. + pub fn check_installable( + &self, + tags: &[String], + name: &PackageName, + version: &Version, + ) -> Result<()> { + let Some((wheel_name, wheel_version, _)) = wheel_identity(&self.name, tags)? else { + bail!("Python wheel is incompatible with this interpreter: {}", self.name) + }; + if wheel_name != *name || wheel_version != *version { + bail!("Python lockfile wheel identity mismatch: {}", self.name); + } + Ok(()) + } + /// The wheel's SHA-256 digest as an integrity string. A wheel with no /// SHA-256 is refused: it is the digest every index publishes and the /// only one a download is checked against. @@ -104,19 +127,30 @@ impl LockedWheel { impl Lockfile { /// The lockfile a solved project produces: one wheel per package, the - /// marker environment it was solved for, and the inputs that chose it. + /// markers it was solved under, and the inputs that chose it. + /// + /// The `environments` marker names the interpreter version and every + /// marker variable a requirement in the solved graph reads. Those are + /// the parts of the target the solution can depend on, so a PEP 751 + /// installer refuses the lockfile where they differ and nowhere else. + /// The version is the minor one, unless a locked package's + /// `Requires-Python` tells patch releases of it apart. pub fn new( packages: &Packages, target: &Target, + requirements: &[Requirement], solution: BTreeMap, inputs: Inputs, requires_python: Option, ) -> Result { + let referenced = + referenced_marker_keys(packages, requirements, &solution, &target.environment)?; let environment = serde_json::to_value(&target.environment).into_diagnostic()?; let marker = environment .as_object() .expect("marker environment serializes to an object") .iter() + .filter(|(key, _)| referenced.contains(key.as_str())) .map(|(key, value)| { let value = value.as_str().expect("marker environment values are strings"); if value.contains(['\'', '"', '\n', '\r']) { @@ -144,6 +178,40 @@ impl Lockfile { }) } + /// Refuse to replay this lockfile for a project or on a target it + /// does not cover: the requirements or index it was resolved for + /// changed, the interpreter range did, or a wheel it pins is one this + /// target cannot install. + /// + /// The markers it was resolved under are not compared. A wheel that + /// installs here is the same wheel wherever it was chosen, and + /// whether the locked graph is still the one the markers select is + /// settled by re-solving it against them ([`crate::validate_locked`]), + /// which is exact where an equality check on the whole environment + /// would refuse every kernel update. + pub fn applies_to( + &self, + inputs: &Inputs, + requires_python: Option<&str>, + target: &Target, + ) -> Result<()> { + if self.tool.pnpm.requirements != inputs.requirements { + bail!("the project's Python requirements changed"); + } + if self.tool.pnpm.index != inputs.index { + bail!("the Python index changed"); + } + if self.requires_python.as_deref() != requires_python { + bail!("the project's requires-python changed"); + } + for package in &self.packages { + for wheel in &package.wheels { + wheel.check_installable(&target.tags, &package.name, &package.version)?; + } + } + Ok(()) + } + /// Load this lockfile's packages as the only candidates a resolution /// may pick, so a locked install solves to exactly what was locked. pub fn seed(&self, packages: &mut Packages) -> Result<()> { @@ -171,3 +239,133 @@ impl Lockfile { Ok(()) } } + +/// The marker variables the solved graph reads, plus the interpreter +/// version: the variables in the markers of the root requirements and of +/// every solved package's `Requires-Dist`, which are all a target +/// contributes to the solution besides the wheels it accepts. +fn referenced_marker_keys( + packages: &Packages, + requirements: &[Requirement], + solution: &BTreeMap, + environment: &MarkerEnvironment, +) -> Result> { + let mut keys = BTreeSet::from(["python_version".to_string()]); + for requirement in requirements { + collect_marker_keys(&requirement.marker, &mut keys); + } + for (name, version) in solution { + let metadata = packages.metadata + .get(&(name.clone(), version.clone())) + .ok_or_else(|| { + miette::miette!("solved Python package {name} {version} was never read") + })?; + for requirement in &metadata.requires_dist { + collect_marker_keys(&parse_requirement(requirement)?.marker, &mut keys); + } + if let Some(requires_python) = &metadata.requires_python { + let specifiers: VersionSpecifiers = requires_python.parse().into_diagnostic()?; + if !admits_every_patch_release(&specifiers, &environment.python_full_version().version) + { + keys.insert("python_full_version".to_string()); + } + } + } + Ok(keys) +} + +/// Whether `specifiers`, which `running` satisfies, admit every patch +/// release of the minor version `running` belongs to. When they do, the +/// minor version says everything they can about a target; when they do +/// not, only the full version does. A bound in another minor version +/// cannot split this one; one in this minor version does unless it is +/// the minor version itself, taken whole. +fn admits_every_patch_release(specifiers: &VersionSpecifiers, running: &Version) -> bool { + let minor = |version: &Version| { + let release = version.release(); + [release.first().copied().unwrap_or(0), release.get(1).copied().unwrap_or(0)] + }; + let running_minor = minor(running); + specifiers + .iter() + .all(|specifier| { + if minor(specifier.version()) != running_minor { + return true; + } + if significant_release_segments(specifier) > 2 { + return false; + } + matches!( + specifier.operator(), + Operator::GreaterThanEqual + | Operator::TildeEqual + | Operator::EqualStar + | Operator::NotEqualStar + | Operator::LessThan, + ) + }) +} + +/// How many release segments of a specifier's version can tell versions +/// apart. PEP 440 zero-pads the ordered comparisons, so `>=3.12.0` is +/// `>=3.12`; a wildcard keeps every segment, so `==3.12.0.*` is not +/// `==3.12.*`; a compatible release is its lower bound and the wildcard +/// on all but its last segment, so `~=3.12.0.0` is `==3.12.0.*`. +fn significant_release_segments(specifier: &pep440_rs::VersionSpecifier) -> usize { + let release = specifier.version().release(); + let zero_padded = release + .iter() + .rposition(|&segment| segment != 0) + .map_or(0, |last| last + 1); + match specifier.operator() { + Operator::EqualStar | Operator::NotEqualStar => release.len(), + Operator::TildeEqual => zero_padded.max(release.len() - 1), + _ => zero_padded, + } +} + +fn collect_marker_keys(marker: &MarkerTree, keys: &mut BTreeSet) { + let (key, children) = marker_node(marker); + keys.extend(key); + for child in &children { + collect_marker_keys(child, keys); + } +} + +/// The environment variable a marker node reads, when it reads one, and +/// the nodes below it. +fn marker_node(marker: &MarkerTree) -> (Option, Vec) { + match marker.kind() { + MarkerTreeKind::True | MarkerTreeKind::False => (None, Vec::new()), + MarkerTreeKind::Version(node) => ( + Some(node.key().to_string()), + node.edges() + .map(|(_, child)| child) + .collect(), + ), + MarkerTreeKind::String(node) => ( + Some(node.key().to_string()), + node.children() + .map(|(_, child)| child) + .collect(), + ), + MarkerTreeKind::In(node) => ( + Some(node.key().to_string()), + node.children() + .map(|(_, child)| child) + .collect(), + ), + MarkerTreeKind::Contains(node) => ( + Some(node.key().to_string()), + node.children() + .map(|(_, child)| child) + .collect(), + ), + MarkerTreeKind::Extra(node) => ( + None, + node.children() + .map(|(_, child)| child) + .collect(), + ), + } +} diff --git a/pnpm/crates/python-resolver/src/tests.rs b/pnpm/crates/python-resolver/src/tests.rs index 52769d2340..4d5c556204 100644 --- a/pnpm/crates/python-resolver/src/tests.rs +++ b/pnpm/crates/python-resolver/src/tests.rs @@ -1,6 +1,6 @@ use crate::{ candidates::{candidates_from_page, wheel_identity}, - lockfile::Target, + lockfile::{Inputs, Lockfile, Target}, metadata::WheelMetadata, packages::Packages, resolve::{Step, step}, @@ -240,3 +240,170 @@ fn the_target_fixture_is_a_marker_environment() { let environment: &MarkerEnvironment = &target().environment; assert_eq!(environment.python_full_version().to_string(), "3.12.0"); } + +/// A solved one-package project: `demo 1.0.0`, whose `Requires-Dist` is +/// `requires_dist`, asked for by `requirement`. +fn solved_project( + requirement: &str, + requires_dist: &str, +) -> (Packages, Vec, BTreeMap) { + let target = target(); + let mut packages = Packages::new(); + packages.candidates.insert( + name("demo"), + candidates_from_page( + &page(&serde_json::json!([wheel("demo-1.0.0-py3-none-any.whl")])), + &index_url(), + &name("demo"), + &target, + ) + .expect("page parses"), + ); + packages.metadata.insert( + (name("demo"), Version::from_str("1.0.0").unwrap()), + WheelMetadata::parse(&format!("Name: demo\nVersion: 1.0.0\n{requires_dist}")) + .expect("metadata parses"), + ); + let requirements = vec![Requirement::from_str(requirement).expect("requirement fixture")]; + let solution = BTreeMap::from([(name("demo"), Version::from_str("1.0.0").unwrap())]); + (packages, requirements, solution) +} + +fn lockfile_for(requirement: &str, requires_dist: &str) -> Lockfile { + let target = target(); + let (packages, requirements, solution) = solved_project(requirement, requires_dist); + let inputs = Inputs::new(&requirements, &target, index_url().as_str()); + Lockfile::new(&packages, &target, &requirements, solution, inputs, Some(">=3.10".to_string())) + .expect("lockfile builds") +} + +#[test] +fn a_lockfile_names_the_interpreter_version_and_the_markers_its_graph_reads() { + let cases = [ + ("demo", "", "python_version == '3.12'"), + ( + "demo", + "Requires-Dist: helper; sys_platform == 'win32'\n", + "python_version == '3.12' and sys_platform == 'linux'", + ), + ( + "demo; platform_release >= '5'", + "Requires-Dist: helper; extra == 'fast' and os_name == 'nt'\n", + "os_name == 'posix' and platform_release == '6.1.0' and python_version == '3.12'", + ), + ]; + for (requirement, requires_dist, expected) in cases { + eprintln!("requirement {requirement:?}, requires-dist {requires_dist:?}"); + let lockfile = lockfile_for(requirement, requires_dist); + assert_eq!(lockfile.environments, [expected]); + } +} + +#[test] +fn a_lockfile_applies_wherever_its_wheels_install() { + let lockfile = lockfile_for("demo", ""); + let requirements = [Requirement::from_str("demo").unwrap()]; + let inputs = Inputs::new(&requirements, &target(), index_url().as_str()); + + let mut other_kernel = target(); + other_kernel.environment = serde_json::from_value(serde_json::json!({ + "implementation_name": "cpython", + "implementation_version": "3.12.4", + "os_name": "posix", + "platform_machine": "arm64", + "platform_release": "24.5.0", + "platform_system": "Darwin", + "platform_version": "Darwin Kernel Version 24.5.0", + "python_full_version": "3.12.4", + "platform_python_implementation": "CPython", + "python_version": "3.12", + "sys_platform": "darwin", + })) + .expect("marker environment fixture"); + other_kernel.tags = vec!["py3-none-any".to_string()]; + lockfile + .applies_to(&inputs, Some(">=3.10"), &other_kernel) + .expect("another environment that installs the same wheel"); + + let mut native_only = target(); + native_only.tags = vec!["cp312-cp312-manylinux_2_17_x86_64".to_string()]; + let error = lockfile + .applies_to(&inputs, Some(">=3.10"), &native_only) + .expect_err("no tag"); + assert!(error.to_string().contains("incompatible with this interpreter"), "{error}"); + + let other_requirements = [Requirement::from_str("demo>=1").unwrap()]; + let error = lockfile + .applies_to( + &Inputs::new(&other_requirements, &target(), index_url().as_str()), + Some(">=3.10"), + &target(), + ) + .expect_err("other requirements"); + assert!(error.to_string().contains("requirements changed"), "{error}"); + + let other_index = Inputs::new(&requirements, &target(), "https://other.test/simple/"); + let error = lockfile + .applies_to(&other_index, Some(">=3.10"), &target()) + .expect_err("index"); + assert!(error.to_string().contains("index changed"), "{error}"); + + let error = lockfile + .applies_to(&inputs, None, &target()) + .expect_err("requires-python"); + assert!(error.to_string().contains("requires-python changed"), "{error}"); +} + +#[test] +fn a_lockfile_pinning_another_distribution_under_a_package_is_refused() { + let mut lockfile = lockfile_for("demo", ""); + let requirements = [Requirement::from_str("demo").unwrap()]; + let inputs = Inputs::new(&requirements, &target(), index_url().as_str()); + lockfile.packages[0].wheels[0].name = "other-1.0.0-py3-none-any.whl".to_string(); + + let error = lockfile + .applies_to(&inputs, Some(">=3.10"), &target()) + .expect_err("wrong wheel"); + + assert!(error.to_string().contains("wheel identity mismatch"), "{error}"); +} + +#[test] +fn a_lockfile_pins_the_full_interpreter_version_when_a_package_tells_patch_releases_apart() { + let cases = [ + (">=3.10", false), + (">=3.8.1", false), + (">=3.12.1", true), + ("<3.13", false), + ("<3.13.1", false), + ("<3.12.9", true), + ("!=3.11.2", false), + ("~=3.12.2", true), + ("~=3.12", false), + ("==3.12.*", false), + ("!=3.11.*", false), + (">3.11", false), + (">3.12", true), + ("<=3.12", true), + ("==3.12", true), + ("!=3.12", true), + (">=3.10,<4", false), + (">=3.12.0", false), + ("<3.13.0", false), + ("~=3.12.0", false), + ("~=3.12.0.0", true), + ("==3.12.0", true), + ("==3.12.0.*", true), + ]; + for (requires_python, pins_full_version) in cases { + eprintln!("Requires-Python: {requires_python}"); + let lockfile = lockfile_for("demo", &format!("Requires-Python: {requires_python}\n")); + assert_eq!( + lockfile.environments[0].contains("python_full_version == '3.12.0'"), + pins_full_version, + "{}", + lockfile.environments[0], + ); + assert!(lockfile.environments[0].contains("python_version == '3.12'")); + } +} diff --git a/pnpm/plans/PYTHON_SPIKE.md b/pnpm/plans/PYTHON_SPIKE.md index 56ce4c5963..3c27e33a77 100644 --- a/pnpm/plans/PYTHON_SPIKE.md +++ b/pnpm/plans/PYTHON_SPIKE.md @@ -72,11 +72,26 @@ without changing the complete lockfile. `--lockfile-only` creates no environment ## Lockfile contract The standard [PEP 751 pylock format](https://packaging.python.org/en/latest/specifications/pylock-toml/) -stays separate from `pnpm-lock.yaml`. This implementation writes a single-target, -single-use lockfile with one compatible wheel per distribution. Environment -markers describe the target; `[tool.pnpm]` records resolver inputs for freshness. -Replay verifies artifacts and dependency closure. Cached Simple responses -also permit offline resolution if every selected wheel is in the shared store. +stays separate from `pnpm-lock.yaml`. This implementation writes a lockfile +resolved for one target, with one compatible wheel per distribution. +`[tool.pnpm]` records the resolver inputs, including the marker environment and +wheel tags the lockfile was resolved for. The `environments` marker names the interpreter +version and the marker variables the solved graph reads, which is what a PEP 751 +installer checks before installing it. The version is the minor one unless a +locked package's `Requires-Python` tells patch releases apart. + +A lockfile is replayed on any target that still installs it: the requirements, +index and `requires-python` must be the ones it was resolved for, every pinned +wheel must carry tags the interpreter accepts, and the locked graph must be +exactly what the interpreter's markers select. The recorded environment is not +compared, so a kernel update or a different tag order does not invalidate the +lockfile, while a requirement gated on `platform_release` still does when the +markers now select another graph. A lockfile whose graph no longer matches is +resolved again with a warning, and so is one resolved for another target that +pins a wheel the install cannot fetch; under `--frozen-lockfile` both are +errors. Replay +verifies artifacts and dependency closure. Cached Simple responses also permit +offline resolution if every selected wheel is in the shared store. `uv.lock` remains uv's project format. uv already [installs standard pylock files](https://docs.astral.sh/uv/pip/compile/). diff --git a/pnpr/crates/pnpr/src/resolver/pypi.rs b/pnpr/crates/pnpr/src/resolver/pypi.rs index d042d7d964..58d7ecf183 100644 --- a/pnpr/crates/pnpr/src/resolver/pypi.rs +++ b/pnpr/crates/pnpr/src/resolver/pypi.rs @@ -129,6 +129,7 @@ pub(super) async fn handle_resolve( match Lockfile::new( &packages.1, &request.target, + &requirements, solution, inputs, request.requires_python,