From cf1482252895210cc0f56bf8c30c39bb98e0a248 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 15:27:05 +0000 Subject: [PATCH 1/3] Start fix for #694, #695 Assisted-by: Claude Code:claude-opus-5-5 From bc6d815fd70c1c6947665ba5b00d075a004a13ed Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 15:42:07 +0000 Subject: [PATCH 2/3] Share the Poetry/PDM lock splice engine poetry.lock and pdm.lock rewrites each carried their own copy of the engine that splices a rewritten package back into the original lock, and the copies disagreed on line endings. On a lock that mixes CRLF and LF lines, Poetry turned the patched package's lines all CRLF while PDM turned them all LF, so the same input churned in opposite ways. Both rewriters now use one engine in utils/lock_fragments.rs. Each replaced fragment takes the line ending most of the original fragment's lines had, so lines outside the patched package never change. LF-only and CRLF-only locks come out byte-identical to before. Fixes #694, #695. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/utils/lock_fragments.rs | 338 ++++++++++++++++++ crates/socket-patch-core/src/utils/mod.rs | 3 +- .../socket-patch-core/src/utils/pdm_lock.rs | 242 ++++++------- .../src/utils/poetry_lock.rs | 216 +++++------ 4 files changed, 538 insertions(+), 261 deletions(-) create mode 100644 crates/socket-patch-core/src/utils/lock_fragments.rs diff --git a/crates/socket-patch-core/src/utils/lock_fragments.rs b/crates/socket-patch-core/src/utils/lock_fragments.rs new file mode 100644 index 000000000..06b8f45f9 --- /dev/null +++ b/crates/socket-patch-core/src/utils/lock_fragments.rs @@ -0,0 +1,338 @@ +//! The fragment-splice engine shared by the `poetry.lock` and `pdm.lock` +//! rewriters. +//! +//! Both rewriters mutate a parsed `toml_edit` document, take the package's +//! verbatim text fragments from the original and from the rendering, pair +//! them, and splice the changed ones into the ORIGINAL text, so untouched +//! bytes survive and rollback can replay the recorded fragments in reverse. +//! What differs per format (which fragments a package owns, what the rewrite +//! refuses) stays in `poetry_lock` / `pdm_lock`; everything around it lives +//! here, once. + +use toml_edit::Table; + +use crate::utils::line_endings::majority_terminator; + +/// Takes `name`'s fragments of `text` from its (spanned) parse. +pub(crate) type FragmentsIn = + fn(&toml_edit::Document, &str, &str) -> Result, String>; + +/// The parse of the lock text last seen or produced, carried between calls +/// so a caller working through one lock dep by dep parses each state once: +/// [`Self::parsed`] reuses it for byte-identical text, and a rewrite hands on +/// the parse of its own output (which it takes anyway, for the output's +/// fragments) when the splice reproduced that output byte for byte. +/// `DocumentMut`'s own parser is exactly `Document::parse(..).into_mut()`, so +/// every result is the fresh parse's. +#[derive(Default)] +pub struct LockParse { + doc: Option>, +} + +impl LockParse { + /// The parse of `text`, reusing the held one when it is of these bytes. + pub fn parsed(&mut self, text: &str) -> Result<&Table, toml_edit::TomlError> { + if self.doc.as_ref().is_none_or(|doc| doc.raw() != text) { + self.doc = None; + self.doc = Some(toml_edit::Document::parse(text.to_owned())?); + } + Ok(self.doc.as_ref().expect("parsed just above").as_table()) + } + + /// The held parse of `text` (taken out), or a fresh one; `what` names the + /// lock in the parse error. + pub(crate) fn take( + &mut self, + text: &str, + what: &str, + ) -> Result, String> { + match self.doc.take() { + Some(doc) if doc.raw() == text => Ok(doc), + _ => toml_edit::Document::parse(text.to_owned()) + .map_err(|e| format!("invalid {what} lock: {e}")), + } + } + + /// Hand back a parse of the text it was taken for, unchanged (a refusal + /// before any mutation). + pub(crate) fn restore(&mut self, doc: toml_edit::Document) { + self.doc = Some(doc); + } +} + +/// A successful fragment-spliced lock rewrite. +pub struct FragmentRewrite<'a> { + /// The rewritten lock text. + pub text: String, + pub(crate) original: &'a str, + pub(crate) name: &'a str, + /// The format's lock name, for error messages. + pub(crate) what: &'static str, + pub(crate) fragments_in: FragmentsIn, + /// The original's fragments, already taken to build `text`. + pub(crate) before: Vec, + /// The fragment edits, when `text` is byte-identical to the rendered + /// document they were derived against (the common case: toml_edit + /// round-trips the untouched bytes) — then they are also the edits + /// against `text`. + pub(crate) known_edits: Option>, +} + +impl FragmentRewrite<'_> { + /// Exactly the format's `*_lock_edits(original, &self.text, name)`, + /// without re-deriving what the rewrite already did. + pub fn edits(&self) -> Result, String> { + if let Some(edits) = &self.known_edits { + return Ok(edits.clone()); + } + let after = fragments_of(&self.text, self.name, self.fragments_in)?; + pair_fragments(self.what, self.original, &self.before, &self.text, after) + } +} + +/// `name`'s fragments of `text`, parsing it first. +pub(crate) fn fragments_of( + text: &str, + name: &str, + fragments_in: FragmentsIn, +) -> Result, String> { + let lock = toml_edit::Document::parse(text.to_owned()).map_err(|e| e.to_string())?; + fragments_in(&lock, text, name) +} + +/// The verbatim `(original, replacement)` fragment pairs that differ between +/// two sides. Each fragment must occur exactly once on its side, so a +/// textual splice (and its rollback) can never hit the wrong place, and both +/// sides must have the same shape. +pub(crate) fn pair_fragments( + what: &str, + original: &str, + before: &[String], + rewritten: &str, + after: Vec, +) -> Result, String> { + if before.len() != after.len() { + return Err(format!("{what} package fragments changed shape")); + } + let mut edits = Vec::new(); + for (old, new) in before.iter().zip(after) { + if *old == new { + continue; + } + if original.matches(old.as_str()).count() != 1 || rewritten.matches(&new).count() != 1 { + return Err(format!("ambiguous {what} rollback fragment")); + } + edits.push((old.clone(), new)); + } + Ok(edits) +} + +/// Widen `span` over `table`'s own span and every key, value and sub-table +/// inside it. +pub(crate) fn extend_span(table: &Table, span: &mut std::ops::Range) { + if let Some(own) = table.span() { + span.start = span.start.min(own.start); + span.end = span.end.max(own.end); + } + for (_, item) in table.iter() { + if let Some(own) = item.span() { + span.start = span.start.min(own.start); + span.end = span.end.max(own.end); + } + if let Some(child) = item.as_table() { + extend_span(child, span); + } + } +} + +/// End (exclusive, before its line break) of the first top-level TOML header +/// line at or after `from`, skipping blank lines and comments; `text.len()` +/// at EOF; `from` itself when the next non-blank line is not a header (a +/// shape neither Poetry nor PDM writes — the fragment is then not extended). +pub(crate) fn next_header_end(text: &str, from: usize) -> usize { + let mut pos = from; + for line in text[from..].split_inclusive('\n') { + let content = line.trim_end_matches(['\r', '\n']); + // Blank lines and comments sit between units (toml_edit clones carry + // the file's leading comment as decor); they belong to the boundary. + if content.trim().is_empty() || content.trim_start().starts_with('#') { + pos += line.len(); + continue; + } + if content.starts_with('[') { + return pos + content.len(); + } + return from; + } + text.len() +} + +/// `new` (the rendering's fragment) with every line break spelled the way +/// most of `old`'s (the original fragment it replaces) are, or +/// `file_terminator` when `old` has none. +/// +/// A fragment anchored at the line break before it (Poetry's legacy +/// integrity entry) starts with that break's `\n`, whose `\r`, if any, lies +/// outside the fragment: that leading `\n` is kept as is on both sides. +fn respell(old: &str, new: &str, file_terminator: &str) -> String { + let (lead, old, new) = match (old.strip_prefix('\n'), new.strip_prefix('\n')) { + (Some(old), Some(new)) => ("\n", old, new), + _ => ("", old, new), + }; + let terminator = if old.contains('\n') { + majority_terminator(old) + } else { + file_terminator + }; + let lf = new.replace("\r\n", "\n"); + let body = if terminator == "\n" { + lf + } else { + lf.replace('\n', terminator) + }; + format!("{lead}{body}") +} + +/// Finish a rewrite whose mutated document rendered to `rendered`: take the +/// rendering's fragments, give each the line ending that dominates the +/// original fragment it replaces (toml_edit renders every break as LF), and +/// splice the changed ones into `text`. +/// +/// The ending rule is per fragment, so no line outside the spliced bytes +/// changes and the replaced unit keeps its own style: an LF-only or +/// CRLF-only lock comes back in that style, and a mixed-ending lock's unit +/// gets the ending most of its own lines had (the file's majority when the +/// original fragment has no break at all). +pub(crate) fn finish<'a>( + what: &'static str, + parse: &mut LockParse, + text: &'a str, + name: &'a str, + rendered: String, + before: Result, String>, + fragments_in: FragmentsIn, +) -> Result, String> { + let before = before?; + let rendered = crate::utils::python_lock::preserve_line_endings(text, rendered); + let after_doc = toml_edit::Document::parse(rendered).map_err(|e| e.to_string())?; + let rendered = after_doc.raw(); + let after = fragments_in(&after_doc, rendered, name)?; + if before.len() != after.len() { + return Err(format!("{what} package fragments changed shape")); + } + let file_terminator = majority_terminator(text); + let after: Vec = before + .iter() + .zip(after) + .map(|(old, new)| respell(old, &new, file_terminator)) + .collect(); + let mut edits = Vec::new(); + for (old, new) in before.iter().zip(after) { + if *old == new { + continue; + } + if text.matches(old.as_str()).count() != 1 { + return Err(format!("ambiguous {what} rollback fragment")); + } + edits.push((old.clone(), new)); + } + let mut result = text.to_string(); + for (old, new) in &edits { + result = result.replacen(old, new, 1); + } + if edits + .iter() + .any(|(_, new)| result.matches(new.as_str()).count() != 1) + { + return Err(format!("ambiguous {what} rollback fragment")); + } + let known_edits = (result == rendered).then_some(edits); + if known_edits.is_some() { + // The output IS the rendered text just parsed: the next dep's input. + parse.restore(after_doc); + } + Ok(FragmentRewrite { + text: result, + original: text, + name, + what, + fragments_in, + before, + known_edits, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn whole( + lock: &toml_edit::Document, + text: &str, + _name: &str, + ) -> Result, String> { + let span = lock + .get("unit") + .and_then(|item| item.as_table()) + .and_then(Table::span) + .ok_or("missing unit")?; + let mut span = span; + extend_span(lock["unit"].as_table().unwrap(), &mut span); + Ok(vec![text[span].to_string()]) + } + + #[test] + fn pair_fragments_refuses_a_shape_change() { + for what in ["Poetry", "PDM"] { + let error = pair_fragments(what, "a", &["a".into()], "b", vec![]).unwrap_err(); + assert_eq!(error, format!("{what} package fragments changed shape")); + let error = pair_fragments(what, "a", &[], "b", vec!["b".into()]).unwrap_err(); + assert_eq!(error, format!("{what} package fragments changed shape")); + } + } + + #[test] + fn finish_spells_each_fragment_like_the_one_it_replaces() { + // A CRLF file whose unit is mostly LF, and an LF file whose unit is + // mostly CRLF: the unit keeps its own majority, the rest is untouched. + for (text, unit_break) in [ + ("[head]\r\na = 1\r\n\r\n[unit]\nb = 2\nc = 3\r\ne = 5\n", "\n"), + ("[head]\na = 1\n\n[unit]\r\nb = 2\r\nc = 3\ne = 5\r\n", "\r\n"), + ] { + let mut doc = toml_edit::Document::parse(text.to_owned()).unwrap(); + let before = whole(&doc, text, ""); + let mut mutable = + std::mem::replace(&mut doc, toml_edit::Document::parse(String::new()).unwrap()) + .into_mut(); + mutable["unit"]["d"] = toml_edit::value(4); + let rewrite = finish( + "Test", + &mut LockParse::default(), + text, + "", + mutable.to_string(), + before, + whole, + ) + .unwrap(); + let head = text.split("[unit]").next().unwrap(); + assert!(rewrite.text.starts_with(head), "{:?}", rewrite.text); + let unit = &rewrite.text[head.len()..]; + assert!(unit.contains("d = 4"), "{unit:?}"); + let crlf = unit.matches("\r\n").count(); + let lf = unit.matches('\n').count() - crlf; + if unit_break == "\n" { + assert_eq!(crlf, 0, "{unit:?}"); + } else { + assert_eq!(lf, 0, "{unit:?}"); + } + // The recorded edit replays the original back byte for byte. + let edits = rewrite.edits().unwrap(); + let mut back = rewrite.text.clone(); + for (old, new) in &edits { + back = back.replacen(new.as_str(), old, 1); + } + assert_eq!(back, text); + } + } +} diff --git a/crates/socket-patch-core/src/utils/mod.rs b/crates/socket-patch-core/src/utils/mod.rs index e3ade4d63..8c17de3bd 100644 --- a/crates/socket-patch-core/src/utils/mod.rs +++ b/crates/socket-patch-core/src/utils/mod.rs @@ -7,9 +7,10 @@ pub mod env_compat; pub mod failpoint; pub mod fs; pub mod group_commit; -pub mod notice; pub(crate) mod http; pub(crate) mod line_endings; +pub mod lock_fragments; +pub mod notice; pub mod pdm_lock; pub(crate) mod pep440; pub mod pipenv; diff --git a/crates/socket-patch-core/src/utils/pdm_lock.rs b/crates/socket-patch-core/src/utils/pdm_lock.rs index 9c6485cc1..dca8e88be 100644 --- a/crates/socket-patch-core/src/utils/pdm_lock.rs +++ b/crates/socket-patch-core/src/utils/pdm_lock.rs @@ -3,7 +3,10 @@ use toml_edit::DocumentMut; use toml_edit::{value, Array, InlineTable, Item, Table, Value}; use crate::crawlers::python_crawler::canonicalize_pypi_name; -use crate::utils::python_lock::{is_prior_hosted_url, preserve_line_endings}; +use crate::utils::lock_fragments::{ + extend_span, finish, fragments_of, next_header_end, pair_fragments, FragmentRewrite, LockParse, +}; +use crate::utils::python_lock::is_prior_hosted_url; pub fn lock_version(lock: &Table) -> Result<&str, String> { let version = lock @@ -131,32 +134,10 @@ pub fn rewrite_pdm_lock( Ok(rewrite_pdm_lock_with_edits(text, name, version, source, filename, sha256)?.text) } -/// A successful [`rewrite_pdm_lock_with_edits`]. -pub struct PdmLockRewrite<'a> { - /// The rewritten lock text. - pub text: String, - original: &'a str, - name: &'a str, - /// The original's fragments, already taken to build `text`. - before: Vec, - /// The fragment edits, when `text` is byte-identical to the rendered - /// document they were derived against (the common case: toml_edit - /// round-trips the untouched bytes) — then they are also the edits - /// against `text`. - known_edits: Option>, -} - -impl PdmLockRewrite<'_> { - /// Exactly `pdm_lock_edits(original, &self.text, name)`, without - /// re-deriving what the rewrite already did. - pub fn edits(&self) -> Result, String> { - if let Some(edits) = &self.known_edits { - return Ok(edits.clone()); - } - let after = pdm_lock_fragments(&self.text, self.name)?; - pair_pdm_lock_fragments(self.original, &self.before, &self.text, after) - } -} +/// A successful [`rewrite_pdm_lock_with_edits`]; its +/// [`edits`](FragmentRewrite::edits) are exactly +/// `pdm_lock_edits(original, &text, name)`. +pub type PdmLockRewrite<'a> = FragmentRewrite<'a>; /// [`rewrite_pdm_lock`], also handing back what the caller needs to record /// the rewrite's fragment edits ([`PdmLockRewrite::edits`]). @@ -179,28 +160,9 @@ pub fn rewrite_pdm_lock_with_edits<'a>( ) } -/// The parse of the lock text last seen or produced, carried between calls -/// so a caller working through one `pdm.lock` dep by dep parses each state -/// once: [`Self::parsed`] reuses it for byte-identical text, and a rewrite -/// hands on the parse of its own output (which it takes anyway, for the -/// output's fragments) when the splice reproduced that output byte for byte. -/// `DocumentMut`'s own parser is exactly `Document::parse(..).into_mut()`, so -/// every result is the fresh parse's. -#[derive(Default)] -pub struct PdmLockParse { - doc: Option>, -} - -impl PdmLockParse { - /// The parse of `text`, reusing the held one when it is of these bytes. - pub fn parsed(&mut self, text: &str) -> Result<&Table, toml_edit::TomlError> { - if self.doc.as_ref().is_none_or(|doc| doc.raw() != text) { - self.doc = None; - self.doc = Some(toml_edit::Document::parse(text.to_owned())?); - } - Ok(self.doc.as_ref().expect("parsed just above").as_table()) - } -} +/// The parse of the `pdm.lock` text last seen or produced, carried between +/// calls (see [`LockParse`]). +pub type PdmLockParse = LockParse; /// [`rewrite_pdm_lock_with_edits`], reusing (and refreshing) `parse`. pub fn rewrite_pdm_lock_in<'a>( @@ -222,16 +184,12 @@ pub fn rewrite_pdm_lock_in<'a>( if !crate::vendor::pypi_distribution::matches(filename, name, version) { return Err("PDM patch wheel does not match package".into()); } - let doc = match parse.doc.take() { - Some(doc) if doc.raw() == text => doc, - _ => toml_edit::Document::parse(text.to_owned()) - .map_err(|e| format!("invalid PDM lock: {e}"))?, - }; + let doc = parse.take(text, "PDM")?; let edits = match plan_pdm_rewrite(&doc, name, version, kind, location) { Ok(edits) => edits, Err(detail) => { // Nothing was mutated: the parse still describes `text`. - parse.doc = Some(doc); + parse.restore(doc); return Err(detail); } }; @@ -265,28 +223,15 @@ pub fn rewrite_pdm_lock_in<'a>( table.insert(&files_key, value(files)); } } - let rendered = preserve_line_endings(text, lock.to_string()); - let before = before?; - let after_doc = toml_edit::Document::parse(rendered).map_err(|e| e.to_string())?; - let rendered = after_doc.raw(); - let after = pdm_lock_fragments_in(&after_doc, rendered, name)?; - let edits = pair_pdm_lock_fragments(text, &before, rendered, after)?; - let mut result = text.to_string(); - for (old, new) in &edits { - result = result.replacen(old, new, 1); - } - let known_edits = (result == rendered).then_some(edits); - if known_edits.is_some() { - // The output IS the rendered text just parsed: the next dep's input. - parse.doc = Some(after_doc); - } - Ok(PdmLockRewrite { - text: result, - original: text, + finish( + "PDM", + parse, + text, name, + lock.to_string(), before, - known_edits, - }) + pdm_lock_fragments_in::, + ) } /// Every refusal of [`rewrite_pdm_lock_in`], read from the parsed lock before @@ -395,27 +340,6 @@ fn plan_pdm_rewrite( Ok(edits) } -/// End (exclusive, before its line break) of the first top-level TOML header -/// line at or after `from`, skipping blank/comment lines; `text.len()` at EOF; -/// `from` itself when the next non-blank line is not a header (a shape PDM never -/// writes — the fragment is then not extended). Kept local, like this -/// module's own `extend_span`. -fn next_header_end(text: &str, from: usize) -> usize { - let mut pos = from; - for line in text[from..].split_inclusive('\n') { - let content = line.trim_end_matches(['\r', '\n']); - if content.trim().is_empty() || content.trim_start().starts_with('#') { - pos += line.len(); - continue; - } - if content.starts_with('[') { - return pos + content.len(); - } - return from; - } - text.len() -} - pub fn pdm_lock_edits( original: &str, rewritten: &str, @@ -423,14 +347,13 @@ pub fn pdm_lock_edits( ) -> Result, String> { let before = pdm_lock_fragments(original, name)?; let after = pdm_lock_fragments(rewritten, name)?; - pair_pdm_lock_fragments(original, &before, rewritten, after) + pair_fragments("PDM", original, &before, rewritten, after) } /// The fragments of `text` for `name` that [`pdm_lock_edits`] pairs up: /// every `[[package]]` unit and each legacy `[metadata.files]` entry. fn pdm_lock_fragments(text: &str, name: &str) -> Result, String> { - let lock = toml_edit::Document::parse(text).map_err(|e| e.to_string())?; - pdm_lock_fragments_in(&lock, text, name) + fragments_of(text, name, pdm_lock_fragments_in::) } /// [`pdm_lock_fragments`] of `text` from its (spanned) parse `lock`. @@ -453,17 +376,6 @@ fn pdm_lock_fragments_in( canonicalize_pypi_name(candidate) == canonicalize_pypi_name(name) }) }) { - fn extend_span(table: &Table, span: &mut std::ops::Range) { - for (_, item) in table.iter() { - if let Some(own) = item.span() { - span.start = span.start.min(own.start); - span.end = span.end.max(own.end); - } - if let Some(child) = item.as_table() { - extend_span(child, span); - } - } - } let mut span = package.span().ok_or("missing PDM package span")?; extend_span(package, &mut span); span.end += text[span.end..] @@ -507,31 +419,8 @@ fn pdm_lock_fragments_in( Ok(result) } -/// [`pdm_lock_edits`] over fragments already taken from both sides. -fn pair_pdm_lock_fragments( - original: &str, - before: &[String], - rewritten: &str, - after: Vec, -) -> Result, String> { - if before.len() != after.len() { - return Err("PDM package fragments changed shape".into()); - } - let mut edits = Vec::new(); - for (old, new) in before.iter().zip(after) { - if *old == new { - continue; - } - if original.matches(old.as_str()).count() != 1 || rewritten.matches(&new).count() != 1 { - return Err("ambiguous PDM rollback fragment".into()); - } - edits.push((old.clone(), new)); - } - Ok(edits) -} - #[cfg(test)] -mod tests { +pub(crate) mod tests { use super::*; const WHEEL: &str = "urllib3-1.26.18-py2.py3-none-any.whl"; @@ -837,6 +726,91 @@ mod tests { /// line-ending lock: the renderer keeps no CRLF, the splice leaves the /// untouched CRLF lines alone), the rewrite's own edits are not kept and /// `edits()` re-derives them against the output. + /// `text` with every line break spelled `base`, except the patched + /// unit's `name = "urllib3"` line(s), spelled `odd`. + pub(crate) fn mixed_endings(text: &str, base: &str, odd: &str) -> String { + text.replace("\r\n", "\n") + .split_inclusive('\n') + .map(|line| { + let ending = if line.starts_with("name = \"urllib3\"") { + odd + } else { + base + }; + line.replace('\n', ending) + }) + .collect() + } + + /// Breaks spelled `ending` (bare `\n` for LF) in `text`. + pub(crate) fn count_breaks(text: &str, ending: &str) -> usize { + let crlf = text.matches("\r\n").count(); + if ending == "\r\n" { + crlf + } else { + text.matches('\n').count() - crlf + } + } + + /// #695: a mixed-ending lock's rewritten unit takes the ending most of + /// its own lines had, every other line keeps its own, and the recorded + /// edits replay back byte for byte (hosted `url` and vendored `path`). + #[test] + fn mixed_line_ending_unit_keeps_its_majority_ending() { + let url = "https://patch.socket.dev/patch/pypi/urllib3/1.26.18/7e52b8b6-53f2-4dc8-860a-1ae7ebd8be0e/e828efa5-5c6d-43f3-9909-03f5ac232b98/urllib3-1.26.18-py2.py3-none-any.whl"; + for version in [ + "0.12.3", + "2.8.2", + "2.11.2", + "2.17.3", + "2.29.2", + "2.29.2-extras", + ] { + for (base, odd) in [("\r\n", "\n"), ("\n", "\r\n")] { + let original = mixed_endings(&fixture(version), base, odd); + assert!(count_breaks(&original, odd) > 0); + for source in [("path", PATH), ("url", url)] { + let rewrite = rewrite_pdm_lock_with_edits( + &original, + "urllib3", + "1.26.18", + source, + WHEEL, + &"a".repeat(64), + ) + .unwrap(); + assert_eq!( + count_breaks(&rewrite.text, odd), + 0, + "{version} {source:?} base {base:?}: {:?}", + rewrite.text + ); + let edits = rewrite.edits().unwrap(); + assert_eq!( + edits, + pdm_lock_edits(&original, &rewrite.text, "urllib3").unwrap() + ); + let mut reverted = rewrite.text.clone(); + for (before, after) in edits.into_iter().rev() { + reverted = reverted.replacen(&after, &before, 1); + } + assert_eq!(reverted, original, "{version} {source:?}"); + } + } + } + } + + /// #694: the shared pairing refuses fragments that changed shape. + #[test] + fn pairing_refuses_a_fragment_shape_change() { + let original = fixture("2.29.2"); + let before = pdm_lock_fragments(&original, "urllib3").unwrap(); + assert_eq!( + pair_fragments("PDM", &original, &before, &original, Vec::new()), + Err("PDM package fragments changed shape".to_string()) + ); + } + #[test] fn rewrite_edits_are_rederived_when_the_splice_differs_from_the_document() { let mut rederived = 0; diff --git a/crates/socket-patch-core/src/utils/poetry_lock.rs b/crates/socket-patch-core/src/utils/poetry_lock.rs index 8502fb5c8..39f3e70f8 100644 --- a/crates/socket-patch-core/src/utils/poetry_lock.rs +++ b/crates/socket-patch-core/src/utils/poetry_lock.rs @@ -15,6 +15,9 @@ use toml_edit::{value, Array, DocumentMut, InlineTable, Item, Table, TableLike, Value}; use crate::crawlers::python_crawler::canonicalize_pypi_name; +use crate::utils::lock_fragments::{ + extend_span, finish, fragments_of, next_header_end, pair_fragments, FragmentRewrite, LockParse, +}; use crate::utils::python_lock::{is_prior_hosted_url, table_likes}; /// The `{file, hash}` tables Poetry records in `package`'s own @@ -195,32 +198,10 @@ pub fn rewrite_poetry_lock( .map(|rewrite| rewrite.text)) } -/// A successful [`rewrite_poetry_lock_with_edits`]. -pub struct PoetryLockRewrite<'a> { - /// The rewritten lock text. - pub text: String, - original: &'a str, - name: &'a str, - /// The original's fragments, already taken to build `text`. - before: Vec, - /// The fragment edits, when `text` is byte-identical to the serialized - /// document they were derived against (the common case: toml_edit - /// round-trips the untouched bytes) — then they are also the edits - /// against `text`. - known_edits: Option>, -} - -impl PoetryLockRewrite<'_> { - /// Exactly `poetry_lock_edits(original, &self.text, name)`, without - /// re-deriving what the rewrite already did. - pub fn edits(&self) -> Result, String> { - if let Some(edits) = &self.known_edits { - return Ok(edits.clone()); - } - let after = poetry_lock_fragments(&self.text, self.name)?; - pair_poetry_lock_fragments(self.original, &self.before, &self.text, after) - } -} +/// A successful [`rewrite_poetry_lock_with_edits`]; its +/// [`edits`](FragmentRewrite::edits) are exactly +/// `poetry_lock_edits(original, &text, name)`. +pub type PoetryLockRewrite<'a> = FragmentRewrite<'a>; /// [`rewrite_poetry_lock`], also handing back what the caller needs to /// record the rewrite's fragment edits ([`PoetryLockRewrite::edits`]). @@ -245,17 +226,9 @@ pub fn rewrite_poetry_lock_with_edits<'a>( ) } -/// The parse of the lock text a [`rewrite_poetry_lock_in`] call last saw or -/// produced, handed to the next call so a caller rewriting one lock dep by -/// dep parses each state once: a rewrite parses its own output anyway (to -/// take the output's fragments), and when the splice reproduces that output -/// byte for byte — the common case — it is the next dep's input. Reused only -/// for byte-identical text, and `DocumentMut`'s own parser is exactly -/// `Document::parse(..).into_mut()`, so every result is the fresh parse's. -#[derive(Default)] -pub struct PoetryLockParse { - doc: Option>, -} +/// The parse of the `poetry.lock` text a [`rewrite_poetry_lock_in`] call +/// last saw or produced, handed to the next call (see [`LockParse`]). +pub type PoetryLockParse = LockParse; /// Where [`rewrite_poetry_lock_in`] rewrites, settled before it mutates. struct PoetryLockPlan { @@ -289,16 +262,12 @@ pub fn rewrite_poetry_lock_in<'a>( if !crate::vendor::pypi_distribution::matches(filename, name, version) { return Err("Poetry patch wheel does not match the locked package".into()); } - let doc = match parse.doc.take() { - Some(doc) if doc.raw() == text => doc, - _ => toml_edit::Document::parse(text.to_owned()) - .map_err(|e| format!("invalid Poetry lock: {e}"))?, - }; + let doc = parse.take(text, "Poetry")?; let plan = match plan_poetry_rewrite(&doc, name, version, source_type, source_url, &sha256) { Ok(Some(plan)) => plan, verdict => { // Nothing was mutated: the parse still describes `text`. - parse.doc = Some(doc); + parse.restore(doc); return verdict.map(|_| None); } }; @@ -363,31 +332,16 @@ pub fn rewrite_poetry_lock_in<'a>( table.insert(&package_name, entry); } } - let mut rewritten = lock.to_string(); - if text.contains("\r\n") { - rewritten = rewritten.replace("\r\n", "\n").replace('\n', "\r\n"); - } - let before = before?; - let after_doc = toml_edit::Document::parse(rewritten).map_err(|e| e.to_string())?; - let rewritten = after_doc.raw(); - let after = poetry_lock_fragments_in(&after_doc, rewritten, name)?; - let edits = pair_poetry_lock_fragments(text, &before, rewritten, after)?; - let mut result = text.to_string(); - for (original, replacement) in &edits { - result = result.replacen(original, replacement, 1); - } - let known_edits = (result == rewritten).then_some(edits); - if known_edits.is_some() { - // The output IS the rendered text just parsed: the next dep's input. - parse.doc = Some(after_doc); - } - Ok(Some(PoetryLockRewrite { - text: result, - original: text, + finish( + "Poetry", + parse, + text, name, + lock.to_string(), before, - known_edits, - })) + poetry_lock_fragments_in::, + ) + .map(Some) } /// Every refusal and not-applicable verdict of [`rewrite_poetry_lock_in`] @@ -468,28 +422,6 @@ fn plan_poetry_rewrite( })) } -/// End (exclusive, before its line break) of the first top-level TOML header -/// line at or after `from`, skipping blank lines; `text.len()` at EOF; `from` -/// itself when the next non-blank line is not a header (a shape Poetry never -/// writes — the fragment is then not extended). -fn next_header_end(text: &str, from: usize) -> usize { - let mut pos = from; - for line in text[from..].split_inclusive('\n') { - let content = line.trim_end_matches(['\r', '\n']); - // Blank lines and comments sit between units (toml_edit clones carry - // the file's leading comment as decor); they belong to the boundary. - if content.trim().is_empty() || content.trim_start().starts_with('#') { - pos += line.len(); - continue; - } - if content.starts_with('[') { - return pos + content.len(); - } - return from; - } - text.len() -} - /// The verbatim `(original, replacement)` fragments that turn `original` into /// `rewritten` for `name`: the package's `[[package]]` unit (with its /// sub-tables) and, for legacy formats, its `[metadata.files]` / @@ -503,14 +435,13 @@ pub fn poetry_lock_edits( ) -> Result, String> { let before = poetry_lock_fragments(original, name)?; let after = poetry_lock_fragments(rewritten, name)?; - pair_poetry_lock_fragments(original, &before, rewritten, after) + pair_fragments("Poetry", original, &before, rewritten, after) } /// The fragments of `text` for `name` that [`poetry_lock_edits`] pairs up: /// the `[[package]]` unit and, for legacy formats, the integrity entry. fn poetry_lock_fragments(text: &str, name: &str) -> Result, String> { - let lock = toml_edit::Document::parse(text).map_err(|e| e.to_string())?; - poetry_lock_fragments_in(&lock, text, name) + fragments_of(text, name, poetry_lock_fragments_in::) } /// [`poetry_lock_fragments`] of `text` from its (spanned) parse `lock`. @@ -533,21 +464,6 @@ fn poetry_lock_fragments_in( }) }) .ok_or("missing Poetry package")?; - fn extend_span(table: &Table, span: &mut std::ops::Range) { - if let Some(own) = table.span() { - span.start = span.start.min(own.start); - span.end = span.end.max(own.end); - } - for (_, item) in table.iter() { - if let Some(own) = item.span() { - span.start = span.start.min(own.start); - span.end = span.end.max(own.end); - } - if let Some(child) = item.as_table() { - extend_span(child, span); - } - } - } let mut span = package.span().ok_or("missing Poetry package span")?; extend_span(package, &mut span); span.end += text[span.end..] @@ -602,26 +518,6 @@ fn poetry_lock_fragments_in( Ok(result) } -/// [`poetry_lock_edits`] over fragments already taken from both sides. -fn pair_poetry_lock_fragments( - original: &str, - before: &[String], - rewritten: &str, - after: Vec, -) -> Result, String> { - let mut edits = Vec::new(); - for (old, new) in before.iter().zip(after) { - if *old == new { - continue; - } - if original.matches(old.as_str()).count() != 1 || rewritten.matches(&new).count() != 1 { - return Err("ambiguous Poetry rollback fragment".into()); - } - edits.push((old.clone(), new)); - } - Ok(edits) -} - #[cfg(test)] mod tests { use super::*; @@ -646,6 +542,74 @@ mod tests { rewrite_poetry_lock(text, "urllib3", "1.26.18", "url", URL, WHEEL, &sha()) } + /// #695: a mixed-ending lock's rewritten unit (and legacy integrity + /// entry) takes the ending most of its own lines had, every other line + /// keeps its own, and the recorded edits replay back byte for byte + /// (hosted `url` and vendored `file`). + #[test] + fn mixed_line_ending_unit_keeps_its_majority_ending() { + use crate::utils::pdm_lock::tests::{count_breaks, mixed_endings}; + let path = "./.socket/vendor/pypi/e828efa5-5c6d-43f3-9909-03f5ac232b98/urllib3-1.26.18-py2.py3-none-any.whl"; + for version in [ + "0.12.17", "1.0.10", "1.1.15", "1.2.2", "1.8.5", "2.0.1", "2.4.3", + ] { + for (base, odd) in [("\r\n", "\n"), ("\n", "\r\n")] { + let original = mixed_endings(&fixture(version), base, odd); + assert!(count_breaks(&original, odd) > 0); + for (kind, source) in [("file", path), ("url", URL)] { + if version == "0.12.17" && kind == "url" { + continue; + } + let rewrite = rewrite_poetry_lock_with_edits( + &original, + "urllib3", + "1.26.18", + kind, + source, + WHEEL, + &sha(), + ) + .unwrap() + .unwrap(); + assert_eq!( + count_breaks(&rewrite.text, odd), + 0, + "{version} {kind} base {base:?}: {:?}", + rewrite.text + ); + let edits = rewrite.edits().unwrap(); + assert_eq!( + edits, + poetry_lock_edits(&original, &rewrite.text, "urllib3").unwrap() + ); + let mut reverted = rewrite.text.clone(); + for (before, after) in edits.into_iter().rev() { + reverted = reverted.replacen(&after, &before, 1); + } + assert_eq!(reverted, original, "{version} {kind}"); + } + } + } + } + + /// #694: the shared pairing refuses fragments that changed shape. + #[test] + fn pairing_refuses_a_fragment_shape_change() { + let original = fixture("1.2.2"); + let before = poetry_lock_fragments(&original, "urllib3").unwrap(); + assert_eq!(before.len(), 2); + assert_eq!( + pair_fragments( + "Poetry", + &original, + &before, + &original, + before[..1].to_vec() + ), + Err("Poetry package fragments changed shape".to_string()) + ); + } + /// A user-editable lock with a malformed integrity table must be refused, /// never panic (no `IndexMut` straight into `metadata.files`). #[test] From b4d8e129f8ee1b966a44f1dfd0577184f5f3a9a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 15:42:19 +0000 Subject: [PATCH 3/3] Format the shared lock splice tests Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/utils/lock_fragments.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-core/src/utils/lock_fragments.rs b/crates/socket-patch-core/src/utils/lock_fragments.rs index 06b8f45f9..c78ca2004 100644 --- a/crates/socket-patch-core/src/utils/lock_fragments.rs +++ b/crates/socket-patch-core/src/utils/lock_fragments.rs @@ -296,8 +296,14 @@ mod tests { // A CRLF file whose unit is mostly LF, and an LF file whose unit is // mostly CRLF: the unit keeps its own majority, the rest is untouched. for (text, unit_break) in [ - ("[head]\r\na = 1\r\n\r\n[unit]\nb = 2\nc = 3\r\ne = 5\n", "\n"), - ("[head]\na = 1\n\n[unit]\r\nb = 2\r\nc = 3\ne = 5\r\n", "\r\n"), + ( + "[head]\r\na = 1\r\n\r\n[unit]\nb = 2\nc = 3\r\ne = 5\n", + "\n", + ), + ( + "[head]\na = 1\n\n[unit]\r\nb = 2\r\nc = 3\ne = 5\r\n", + "\r\n", + ), ] { let mut doc = toml_edit::Document::parse(text.to_owned()).unwrap(); let before = whole(&doc, text, "");