From 1a0a6be71a771683aabaa518f74a32d42495138c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 22:23:02 +0000 Subject: [PATCH 1/4] Start fix for #1104 Assisted-by: Claude Code:claude-opus-5-5 From ba77e574409e3dd4d34cbbe3835d7bc46108f5b0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 22:45:38 +0000 Subject: [PATCH 2/4] Vendor pip marker splits across -r includes A project that keeps a version-specific pin in a separate requirements file, e.g. `six==1.16.0 ; python_version < "3.12"` in requirements.txt and `six==1.17.0 ; python_version >= "3.12"` in an included base.txt, could not vendor a patch for that package: the run exited 1 with a false "six is not pinned to ==1.16.0", and a fresh install stayed unpatched. pip merges the root file and its -r includes into one requirement set, but the marker-branch check from #928 was made one file at a time, so the file holding only the other version's branch looked ambiguous. Make that decision once over the whole include tree. The vendored branch is rewritten where it lives and the other branch is left alone; an unmarked target pin, or no target pin at all, still refuses. Fixes #1104 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/vendor/pypi_requirements.rs | 247 ++++++++++++++---- 1 file changed, 195 insertions(+), 52 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/pypi_requirements.rs b/crates/socket-patch-core/src/vendor/pypi_requirements.rs index d52a78e68..16b38f6f3 100644 --- a/crates/socket-patch-core/src/vendor/pypi_requirements.rs +++ b/crates/socket-patch-core/src/vendor/pypi_requirements.rs @@ -35,7 +35,10 @@ use super::common::refuse_symlinked; use super::state::{VendorEntry, WiringAction, WiringRecord}; use super::{RevertOutcome, VendorWarning}; -/// Classification of the target package within the requirements tree. +/// Classification of the target package within one requirements file read +/// as the whole requirement set ([`find_pin`], the tests' view of +/// [`scan_pins`]). +#[cfg(test)] #[derive(Debug, PartialEq, Eq)] enum PinSearch { /// A clean `name==version` pin (no extras). `line_start` / `line_count` @@ -64,18 +67,42 @@ enum PinSearch { /// marker to carry over, and whether the pin carried `--hash` options. type PinSpan = (usize, usize, Option, bool); -/// Scan one file for the target package: every clean exact -/// `canon_name==version` pin, plus whether any occurrence carries extras or -/// names the package ambiguously. +/// What one file says about the target package ([`scan_pins`]). +struct PinScan { + /// Every clean exact `canon_name==version` pin. + exact: Vec, + /// Some occurrence carries extras. + extras: bool, + /// Some occurrence names the package ambiguously on its own. + range: bool, + /// Some occurrence exactly pins a DIFFERENT version on its own + /// environment-marker branch; whether that is ambiguous depends on the + /// target pins of the whole requirement set ([`marker_split_ok`]). + other_branches: bool, +} + +/// Whether the other version's marker branches (if any) are harmless given +/// the target pins: every target pin must carry a marker too, and there +/// must be one. That is the shape `uv pip compile --universal` writes when +/// a package resolves differently per Python (#928), and pip installs +/// exactly one branch. Only the target branches are rewritten; the other +/// branch is left alone, as hosted requirements and vendored pylock already +/// do. Without markers on both sides pip could see both pins at once, so it +/// stays a refusal. /// -/// An exact pin of a DIFFERENT version is not ambiguous when it sits on its -/// own environment-marker branch and every target pin does too: that is -/// the shape `uv pip compile --universal` writes when a package resolves -/// differently per Python (#928), and pip installs exactly one branch. -/// Only the target branches are rewritten; the other branch is left alone, -/// as hosted requirements and vendored pylock already do. Without markers -/// on both sides pip could see both pins at once, so it stays a refusal. -fn scan_pins(content: &str, canon_name: &str, version: &str) -> (Vec, bool, bool) { +/// pip merges the root file and its `-r` includes into one requirement set, +/// so [`plan_requirements`] asks this once over every file's pins: the two +/// branches may sit in different files (#1104). +fn marker_split_ok<'a>(other_branches: bool, exact: impl Iterator) -> bool { + let mut exact = exact.peekable(); + !other_branches || (exact.peek().is_some() && exact.all(|(_, _, marker, _)| marker.is_some())) +} + +/// Scan one file for the target package: every clean exact +/// `canon_name==version` pin, plus whether any occurrence carries extras, +/// names the package ambiguously, or pins another version on its own +/// marker branch ([`marker_split_ok`]). +fn scan_pins(content: &str, canon_name: &str, version: &str) -> PinScan { let mut exact: Vec = Vec::new(); let mut found_extras = false; let mut found_range = false; @@ -128,10 +155,12 @@ fn scan_pins(content: &str, canon_name: &str, version: &str) -> (Vec, b found_range = true; } } - if other_branches && (exact.is_empty() || exact.iter().any(|(_, _, m, _)| m.is_none())) { - found_range = true; + PinScan { + exact, + extras: found_extras, + range: found_range, + other_branches, } - (exact, found_extras, found_range) } /// Whether one requirements file's content names the package at all (any @@ -143,19 +172,21 @@ pub(super) fn names_package(content: &str, canon_name: &str) -> bool { }) } -/// Find the target pin in one file's content. Precedence is fail-closed: -/// any extras occurrence wins over any non-pin occurrence wins over a clean -/// exact pin — a file that names the package ambiguously is never rewritten -/// (a marker-split other version is not ambiguous; see [`scan_pins`]). +/// Find the target pin in one file's content, read as the whole +/// requirement set. Precedence is fail-closed: any extras occurrence wins +/// over any non-pin occurrence wins over a clean exact pin — a file that +/// names the package ambiguously is never rewritten (a marker-split other +/// version is not ambiguous; see [`marker_split_ok`]). +#[cfg(test)] fn find_pin(content: &str, canon_name: &str, version: &str) -> PinSearch { - let (exact, found_extras, found_range) = scan_pins(content, canon_name, version); - if found_extras { + let scan = scan_pins(content, canon_name, version); + if scan.extras { return PinSearch::Extras; } - if found_range { + if scan.range || !marker_split_ok(scan.other_branches, scan.exact.iter()) { return PinSearch::Range; } - match exact.into_iter().next() { + match scan.exact.into_iter().next() { Some((line_start, line_count, marker, hashed)) => PinSearch::Exact { line_start, line_count, @@ -709,31 +740,43 @@ async fn plan_requirements( let mut planned: Vec = Vec::new(); let mut rewrote_any = false; - for file in &files { - match find_pin(&file.content, canon_name, version) { - PinSearch::Extras => { - return Err(( - "pypi_extras_unsupported", - format!( - "{}: the {canon_name} pin declares extras, which a vendored wheel path \ - line cannot express; remove the extras or use agent mode \ - (`scan --mode agent` + `socket-patch apply`) instead", - file.rel - ), - )); - } - PinSearch::Range => { - return Err(( - "pypi_requirement_not_pinned", - format!( - "{}: {canon_name} is not pinned to =={version}; pin it exactly or use \ - agent mode (`scan --mode agent` + `socket-patch apply`) instead", - file.rel - ), - )); - } - PinSearch::Absent => continue, - PinSearch::Exact { .. } => {} + let scans: Vec = files + .iter() + .map(|f| scan_pins(&f.content, canon_name, version)) + .collect(); + // pip reads the root and its includes as one requirement set, so the + // other version's marker branch may sit in a different file than the + // target's (#1104): decide that over the whole tree, and name the file + // holding the other version when it is ambiguous. + let split_ok = marker_split_ok( + scans.iter().any(|s| s.other_branches), + scans.iter().flat_map(|s| s.exact.iter()), + ); + + for (file, scan) in files.iter().zip(scans) { + if scan.extras { + return Err(( + "pypi_extras_unsupported", + format!( + "{}: the {canon_name} pin declares extras, which a vendored wheel path \ + line cannot express; remove the extras or use agent mode \ + (`scan --mode agent` + `socket-patch apply`) instead", + file.rel + ), + )); + } + if scan.range || (scan.other_branches && !split_ok) { + return Err(( + "pypi_requirement_not_pinned", + format!( + "{}: {canon_name} is not pinned to =={version}; pin it exactly or use \ + agent mode (`scan --mode agent` + `socket-patch apply`) instead", + file.rel + ), + )); + } + if scan.exact.is_empty() { + continue; } if !file.editable { // SECURITY/scope: an include outside the project root cannot be @@ -752,10 +795,7 @@ async fn plan_requirements( // Rewrite EVERY exact-pin occurrence in this file, bottom-up so the // recorded spans (against the original content) stay valid. - let (spans, _, _) = scan_pins(&file.content, canon_name, version); - if spans.is_empty() { - continue; - } + let spans = scan.exact; let original_lines = split_lines(&file.content); let mut lines = original_lines.clone(); let mut records = Vec::new(); @@ -2036,6 +2076,109 @@ mod tests { ); } + /// #1104: pip merges the root and its `-r` includes into one + /// requirement set, so a marker split is a split across the whole tree, + /// not per file. Either way round, the vendored version's branch is + /// rewritten where it lives (marker kept), the other branch is left + /// alone, and revert is byte-identical. + #[tokio::test] + async fn marker_split_across_an_include_rewrites_only_the_vendored_branch() { + let target = "six==1.16.0 ; python_version < \"3.12\"\n"; + let other = "six==1.17.0 ; python_version >= \"3.12\"\n"; + let rewritten = format!( + "./{REL_WHEEL} ; python_version < \"3.12\" # socket-patch vendor: six==1.16.0\n" + ); + // (root body after `-r base.txt`, base.txt) and the expected rewrites. + let layouts = [ + (target, format!("idna==3.7\n{other}"), true), + (other, format!("idna==3.7\n{target}"), false), + ]; + for (root_pin, base, target_in_root) in layouts { + let root_original = format!("-r base.txt\n{root_pin}"); + let tmp = write_root(&root_original).await; + tokio::fs::write(tmp.path().join("base.txt"), &base) + .await + .unwrap(); + assert!( + matches!( + preflight_requirements(tmp.path(), "six", "1.16.0", "u").await, + Ok(RequirementsTarget::Fresh) + ), + "target in root: {target_in_root}" + ); + assert!(sibling_pin_wirable(tmp.path(), "six", "1.16.0").await); + let wiring = wire_requirements(tmp.path(), "six", "1.16.0", REL_WHEEL, "") + .await + .unwrap(); + assert_eq!(wiring.len(), 1); + assert_eq!(wiring[0].action, WiringAction::Rewritten); + let base_now = tokio::fs::read_to_string(tmp.path().join("base.txt")) + .await + .unwrap(); + if target_in_root { + assert_eq!( + read_root(tmp.path()).await, + format!("-r base.txt\n{rewritten}") + ); + assert_eq!(base_now, base); + } else { + assert_eq!(read_root(tmp.path()).await, root_original); + assert_eq!(base_now, format!("idna==3.7\n{rewritten}")); + } + + let outcome = revert_requirements(&entry_for(wiring), tmp.path(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert_eq!(read_root(tmp.path()).await, root_original); + assert_eq!( + tokio::fs::read_to_string(tmp.path().join("base.txt")) + .await + .unwrap(), + base + ); + } + } + + /// #1104's guard rails: across the tree, the other version's branch is + /// only tolerated when every target pin carries a marker too. An + /// unmarked target pin, or no target pin at all, means pip could pick + /// the other version, so the vendor still refuses (naming the file + /// that pins the other version) and writes nothing. + #[tokio::test] + async fn marker_split_across_an_include_still_refuses_an_unmarked_or_missing_target() { + let other = "six==1.17.0 ; python_version >= \"3.12\"\n"; + let cases = [ + // An unmarked target pin in the root next to a marked other + // branch in the include. + ("-r base.txt\nsix==1.16.0\n", other.to_string(), "base.txt"), + // The unmarked target in the include, the other branch in the root. + ( + &*format!("-r base.txt\n{other}"), + "six==1.16.0\n".to_string(), + "requirements.txt", + ), + // Only the other version's branch anywhere in the tree. + ("-r base.txt\nidna==3.7\n", other.to_string(), "base.txt"), + ]; + for (root, base, named) in cases { + let tmp = write_root(root).await; + tokio::fs::write(tmp.path().join("base.txt"), &base) + .await + .unwrap(); + let err = wire_requirements(tmp.path(), "six", "1.16.0", REL_WHEEL, SHA) + .await + .unwrap_err(); + assert_eq!(err.0, "pypi_requirement_not_pinned", "{root:?} / {base:?}"); + assert!(err.1.starts_with(&format!("{named}: ")), "{}", err.1); + assert_eq!(read_root(tmp.path()).await, root); + assert_eq!( + tokio::fs::read_to_string(tmp.path().join("base.txt")) + .await + .unwrap(), + base + ); + } + } + #[tokio::test] async fn absent_package_appends_managed_transitive_line() { let tmp = write_root("python-dateutil==2.8.2\n").await; From 89e8337cc21158318696f29b47c045e98760bb45 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 22:45:38 +0000 Subject: [PATCH 3/4] Test pip marker split across includes end to end Vendor a six marker split whose branches sit in different files of the -r tree, in both layouts, then install the result with pip --no-index in a fresh checkout and check the patched module is imported, and that revert restores both files byte for byte. Refs #1104 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_vendor_pypi_build.rs | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs index 820f76795..3ea6424db 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs @@ -1874,6 +1874,119 @@ fn pip_vendored_requirements_evaluate_environment_markers() { } } +/// #1104: the #928 marker split with its branches in different files of +/// the `-r` tree (a layered `base.txt` plus a root file). pip merges the +/// tree into one requirement set, so vendoring must rewrite the 1.16.0 +/// branch where it lives, leave the other version's branch alone in either +/// layout, install the patched wheel from a fresh `--no-index` checkout, +/// and revert byte-identically. Before the fix the file holding only the +/// other branch made vendored refuse with a false +/// `pypi_requirement_not_pinned`. +#[test] +#[serial_test::serial] +fn pip_vendored_requirements_marker_split_across_includes() { + let python = + find_python().expect("Python is required for the pip cross-file marker-split regression"); + let target = "six==1.16.0 ; python_full_version >= '3'\n"; + // Never matches the running interpreter. + let other = "six==1.17.0 ; python_full_version < '3'\n"; + for (label, root_pin, include_pin) in [("root", target, other), ("include", other, target)] { + let tmp = tempfile::tempdir().unwrap(); + let project = tmp.path().join("proj"); + std::fs::create_dir_all(&project).unwrap(); + assert_tool_ok( + &tool(Path::new(python), &project, &["-m", "venv", ".venv"], &[]), + "create source venv", + ); + let venv = project.join(".venv"); + assert_tool_ok( + &tool( + &venv.join("bin/pip"), + &project, + &["install", "--disable-pip-version-check", "six==1.16.0"], + &[], + ), + "install upstream six", + ); + let patched = stage_patch(&project, &site_packages(&venv).join("six.py")); + let root_original = format!("-r base.txt\n{root_pin}"); + let base_original = include_pin.to_string(); + std::fs::write(project.join("requirements.txt"), &root_original).unwrap(); + std::fs::write(project.join("base.txt"), &base_original).unwrap(); + + let (code, stdout, stderr) = run_vendored(&VendorDriver::VendorOffline, &project); + assert_eq!(code, 0, "[{label}] vendor failed: {stdout}\n{stderr}"); + assert_vendored_applied(&parse_envelope(&stdout)); + let (wired, untouched, untouched_original) = if label == "root" { + ("requirements.txt", "base.txt", &base_original) + } else { + ("base.txt", "requirements.txt", &root_original) + }; + assert_eq!( + &std::fs::read_to_string(project.join(untouched)).unwrap(), + untouched_original, + "[{label}] the other branch's file must stay untouched" + ); + let wired_now = std::fs::read_to_string(project.join(wired)).unwrap(); + let vendor_line = wired_now.lines().last().unwrap(); + assert!( + vendor_line.starts_with("./.socket/vendor/pypi/") + && vendor_line.contains(" ; python_full_version >= '3' # socket-patch vendor:"), + "[{label}] the 1.16.0 branch becomes a marker-carrying vendor line: {vendor_line}" + ); + + let fresh = project.join("fresh"); + std::fs::create_dir_all(&fresh).unwrap(); + for file in ["requirements.txt", "base.txt"] { + std::fs::copy(project.join(file), fresh.join(file)).unwrap(); + } + copy_dir_recursive(&project.join(".socket"), &fresh.join(".socket")); + assert_tool_ok( + &tool(Path::new(python), &fresh, &["-m", "venv", ".venv"], &[]), + "create fresh venv", + ); + let fresh_venv = fresh.join(".venv"); + assert_tool_ok( + &tool( + &fresh_venv.join("bin/pip"), + &fresh, + &[ + "install", + "--disable-pip-version-check", + "--no-index", + "-r", + "requirements.txt", + ], + &[], + ), + "install vendored cross-file marker-split requirements", + ); + assert_eq!(python_oracle(&fresh_venv, &fresh), "1", "[{label}]"); + // The VEX steps un-wire by restoring the root file, which holds the + // vendor line only in this layout. + if label == "root" { + manifestless_vex( + &fresh, + "pip vendored cross-file marker split", + &patched, + root_original.as_bytes(), + ); + } + + let (code, stdout, stderr) = + run_socket(&project, &["vendor", "--revert", "--offline", "--json"]); + assert_eq!(code, 0, "[{label}] revert failed: {stdout}\n{stderr}"); + assert_eq!( + std::fs::read_to_string(project.join("requirements.txt")).unwrap(), + root_original + ); + assert_eq!( + std::fs::read_to_string(project.join("base.txt")).unwrap(), + base_original + ); + } +} + /// #928: `uv pip compile --universal` splits a package across marker /// branches when it resolves differently per Python. Vendoring the patched /// 1.16.0 must rewrite only that branch (marker and hash kept) and leave From e317089335de96ed6ce55c712aebab7859488ae9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 23:03:20 +0000 Subject: [PATCH 4/4] Explain refused marker splits across -r files When a requirements tree pins another version of a package on its own environment-marker branch but the patched version's pin has no marker (or there is none), vendoring refuses because pip may install either version. The error used to say the package "is not pinned", which reads as false when an exact pin is right there in another file. It now says that the other version's marker branch needs a marker-branch pin of the patched version beside it. The per-file refusal order now lives in one helper shared by planning and its tests, and the end-to-end test also checks VEX when the vendor line lands in an include. Refs #1104 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_vendor_pypi_build.rs | 34 +++-- .../src/vendor/pypi_requirements.rs | 128 ++++++++++++------ 2 files changed, 108 insertions(+), 54 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs index 3ea6424db..42fbfee62 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs @@ -89,6 +89,19 @@ const VEX_VULNS: &[(&str, &[&str])] = &[("GHSA-vend-pypi-real", &["CVE-2024-8888 /// API: the staged patch's `six.py` afterHash + advisory), `original` /// being the registry requirements the revert step restores. fn manifestless_vex(fresh: &Path, what: &str, patched: &[u8], original: &[u8]) { + manifestless_vex_reverting(fresh, what, patched, &|p: &Path| { + std::fs::write(p.join("requirements.txt"), original).unwrap() + }); +} + +/// [`manifestless_vex`] for a wiring that `revert` un-does (the vendor line +/// may sit in a `-r` include rather than the root file). +fn manifestless_vex_reverting( + fresh: &Path, + what: &str, + patched: &[u8], + revert: &(dyn Fn(&Path) + Sync), +) { let view = vex_e2e_common::patch_view(UUID, PURL, &[("six.py", &git_sha256(patched))], VEX_VULNS); vex_pipenv_pip_steps::run_manifestless_steps(&vex_pipenv_pip_steps::Steps { @@ -101,7 +114,7 @@ fn manifestless_vex(fresh: &Path, what: &str, patched: &[u8], original: &[u8]) { records: vex_pipenv_pip_steps::Records::Mock(vec![(UUID.to_string(), view)]), patch_server_url: None, product: "pkg:pypi/app@0.1.0", - revert: &|p: &Path| std::fs::write(p.join("requirements.txt"), original).unwrap(), + revert, envs: Vec::new(), on_step: None, expect_verified: true, @@ -1962,16 +1975,15 @@ fn pip_vendored_requirements_marker_split_across_includes() { "install vendored cross-file marker-split requirements", ); assert_eq!(python_oracle(&fresh_venv, &fresh), "1", "[{label}]"); - // The VEX steps un-wire by restoring the root file, which holds the - // vendor line only in this layout. - if label == "root" { - manifestless_vex( - &fresh, - "pip vendored cross-file marker split", - &patched, - root_original.as_bytes(), - ); - } + manifestless_vex_reverting( + &fresh, + &format!("pip vendored cross-file marker split {label}"), + &patched, + &|p: &Path| { + std::fs::write(p.join("requirements.txt"), &root_original).unwrap(); + std::fs::write(p.join("base.txt"), &base_original).unwrap(); + }, + ); let (code, stdout, stderr) = run_socket(&project, &["vendor", "--revert", "--offline", "--json"]); diff --git a/crates/socket-patch-core/src/vendor/pypi_requirements.rs b/crates/socket-patch-core/src/vendor/pypi_requirements.rs index 16b38f6f3..0df16aeae 100644 --- a/crates/socket-patch-core/src/vendor/pypi_requirements.rs +++ b/crates/socket-patch-core/src/vendor/pypi_requirements.rs @@ -81,9 +81,9 @@ struct PinScan { other_branches: bool, } -/// Whether the other version's marker branches (if any) are harmless given -/// the target pins: every target pin must carry a marker too, and there -/// must be one. That is the shape `uv pip compile --universal` writes when +/// Whether the other version's marker branches (if any) in a requirement +/// set are harmless given its target pins: every target pin must carry a +/// marker too, and there must be one. That is the shape `uv pip compile --universal` writes when /// a package resolves differently per Python (#928), and pip installs /// exactly one branch. Only the target branches are rewritten; the other /// branch is left alone, as hosted requirements and vendored pylock already @@ -91,11 +91,35 @@ struct PinScan { /// stays a refusal. /// /// pip merges the root file and its `-r` includes into one requirement set, -/// so [`plan_requirements`] asks this once over every file's pins: the two +/// so [`plan_requirements`] asks this once over every file's scan: the two /// branches may sit in different files (#1104). -fn marker_split_ok<'a>(other_branches: bool, exact: impl Iterator) -> bool { - let mut exact = exact.peekable(); - !other_branches || (exact.peek().is_some() && exact.all(|(_, _, marker, _)| marker.is_some())) +fn marker_split_ok(scans: &[PinScan]) -> bool { + let mut exact = scans.iter().flat_map(|s| s.exact.iter()).peekable(); + !scans.iter().any(|s| s.other_branches) + || (exact.peek().is_some() && exact.all(|(_, _, marker, _)| marker.is_some())) +} + +/// Why one file's pins refuse the vendor, in fail-closed precedence: any +/// extras occurrence wins over any ambiguous occurrence wins over the other +/// version's marker branch when the requirement set's target pins do not +/// all carry a marker (`split_ok` from [`marker_split_ok`]). +#[derive(Debug, PartialEq, Eq)] +enum PinRefusal { + Extras, + NotPinned, + UnmarkedSplit, +} + +fn pin_refusal(scan: &PinScan, split_ok: bool) -> Option { + if scan.extras { + Some(PinRefusal::Extras) + } else if scan.range { + Some(PinRefusal::NotPinned) + } else if scan.other_branches && !split_ok { + Some(PinRefusal::UnmarkedSplit) + } else { + None + } } /// Scan one file for the target package: every clean exact @@ -173,18 +197,18 @@ pub(super) fn names_package(content: &str, canon_name: &str) -> bool { } /// Find the target pin in one file's content, read as the whole -/// requirement set. Precedence is fail-closed: any extras occurrence wins -/// over any non-pin occurrence wins over a clean exact pin — a file that -/// names the package ambiguously is never rewritten (a marker-split other -/// version is not ambiguous; see [`marker_split_ok`]). +/// requirement set, through the same [`pin_refusal`] precedence +/// [`plan_requirements`] applies: a file that names the package ambiguously +/// is never rewritten (a marker-split other version is not ambiguous; see +/// [`marker_split_ok`]). #[cfg(test)] fn find_pin(content: &str, canon_name: &str, version: &str) -> PinSearch { let scan = scan_pins(content, canon_name, version); - if scan.extras { - return PinSearch::Extras; - } - if scan.range || !marker_split_ok(scan.other_branches, scan.exact.iter()) { - return PinSearch::Range; + let split_ok = marker_split_ok(std::slice::from_ref(&scan)); + match pin_refusal(&scan, split_ok) { + Some(PinRefusal::Extras) => return PinSearch::Extras, + Some(PinRefusal::NotPinned | PinRefusal::UnmarkedSplit) => return PinSearch::Range, + None => {} } match scan.exact.into_iter().next() { Some((line_start, line_count, marker, hashed)) => PinSearch::Exact { @@ -746,34 +770,46 @@ async fn plan_requirements( .collect(); // pip reads the root and its includes as one requirement set, so the // other version's marker branch may sit in a different file than the - // target's (#1104): decide that over the whole tree, and name the file - // holding the other version when it is ambiguous. - let split_ok = marker_split_ok( - scans.iter().any(|s| s.other_branches), - scans.iter().flat_map(|s| s.exact.iter()), - ); + // target's (#1104): decide that over the whole tree. + let split_ok = marker_split_ok(&scans); for (file, scan) in files.iter().zip(scans) { - if scan.extras { - return Err(( - "pypi_extras_unsupported", - format!( - "{}: the {canon_name} pin declares extras, which a vendored wheel path \ - line cannot express; remove the extras or use agent mode \ - (`scan --mode agent` + `socket-patch apply`) instead", - file.rel - ), - )); - } - if scan.range || (scan.other_branches && !split_ok) { - return Err(( - "pypi_requirement_not_pinned", - format!( - "{}: {canon_name} is not pinned to =={version}; pin it exactly or use \ - agent mode (`scan --mode agent` + `socket-patch apply`) instead", - file.rel - ), - )); + match pin_refusal(&scan, split_ok) { + Some(PinRefusal::Extras) => { + return Err(( + "pypi_extras_unsupported", + format!( + "{}: the {canon_name} pin declares extras, which a vendored wheel path \ + line cannot express; remove the extras or use agent mode \ + (`scan --mode agent` + `socket-patch apply`) instead", + file.rel + ), + )); + } + Some(PinRefusal::NotPinned) => { + return Err(( + "pypi_requirement_not_pinned", + format!( + "{}: {canon_name} is not pinned to =={version}; pin it exactly or use \ + agent mode (`scan --mode agent` + `socket-patch apply`) instead", + file.rel + ), + )); + } + Some(PinRefusal::UnmarkedSplit) => { + return Err(( + "pypi_requirement_not_pinned", + format!( + "{}: pins another {canon_name} version on an environment-marker \ + branch, but not every =={version} pin in the requirements tree sits \ + on a marker branch of its own (or there is none), so pip may install \ + either version; pin =={version} on its own marker branch or use agent \ + mode (`scan --mode agent` + `socket-patch apply`) instead", + file.rel + ), + )); + } + None => {} } if scan.exact.is_empty() { continue; @@ -2168,7 +2204,13 @@ mod tests { .await .unwrap_err(); assert_eq!(err.0, "pypi_requirement_not_pinned", "{root:?} / {base:?}"); - assert!(err.1.starts_with(&format!("{named}: ")), "{}", err.1); + assert!( + err.1 + .starts_with(&format!("{named}: pins another six version")) + && err.1.contains("pin ==1.16.0 on its own marker branch"), + "{}", + err.1 + ); assert_eq!(read_root(tmp.path()).await, root); assert_eq!( tokio::fs::read_to_string(tmp.path().join("base.txt"))