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,