diff --git a/crates/socket-patch-core/src/utils/lock_fragments.rs b/crates/socket-patch-core/src/utils/lock_fragments.rs index fb2455757..cbee7f34a 100644 --- a/crates/socket-patch-core/src/utils/lock_fragments.rs +++ b/crates/socket-patch-core/src/utils/lock_fragments.rs @@ -6,12 +6,13 @@ //! 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 +//! refuses, how it mutates the lock) is a [`LockFormat`] in `poetry_lock` / +//! `pdm_lock`; the drivers around it ([`rewrite_in`], [`rewrite_all`]) live //! here, once. use std::sync::Arc; -use toml_edit::Table; +use toml_edit::{DocumentMut, Table}; use crate::utils::line_endings::majority_terminator; use crate::vendor::parse_memo::ParseMemo; @@ -149,7 +150,7 @@ pub struct FragmentRewrite<'a> { /// The rewritten lock text. pub text: String, pub(crate) original: &'a str, - pub(crate) name: &'a str, + pub(crate) name: String, /// The format's lock name, for error messages. pub(crate) what: &'static str, pub(crate) fragments_in: FragmentsIn, @@ -169,7 +170,7 @@ impl FragmentRewrite<'_> { if let Some(edits) = &self.known_edits { return Ok(edits.clone()); } - let after = fragments_of(&self.text, self.name, self.fragments_in)?; + let after = fragments_of(&self.text, &self.name, self.fragments_in)?; pair_fragments(self.what, self.original, &self.before, &self.text, after) } } @@ -194,21 +195,47 @@ pub(crate) fn pair_fragments( before: &[String], rewritten: &str, after: Vec, +) -> Result, String> { + let edits = changed_pairs(what, before, after, |_, new| new)?; + for (old, new) in &edits { + sole_in(what, original, old)?; + sole_in(what, rewritten, new)?; + } + Ok(edits) +} + +/// The `(old, new)` pairs of `before` and `after` (each `new` passed through +/// `spell` with its `old`) that differ, in order; an error when the two +/// sides have a different number of fragments. Every pairing in this module +/// goes through here. +fn changed_pairs( + what: &str, + before: &[String], + after: Vec, + mut spell: impl FnMut(&str, String) -> String, ) -> 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(before + .iter() + .zip(after) + .map(|(old, new)| { + let new = spell(old, new); + (old.clone(), new) + }) + .filter(|(old, new)| old != new) + .collect()) +} + +/// `Ok` when `fragment` occurs exactly once in `text` (non-overlapping), so +/// a splice of it, or its rollback, cannot hit the wrong place. +fn sole_in(what: &str, text: &str, fragment: &str) -> Result<(), String> { + if text.matches(fragment).count() == 1 { + Ok(()) + } else { + Err(format!("ambiguous {what} rollback fragment")) } - Ok(edits) } /// Widen `span` over `table`'s own span and every key, value and sub-table @@ -291,7 +318,7 @@ pub(crate) fn finish<'a>( what: &'static str, parse: &mut LockParse, text: &'a str, - name: &'a str, + name: &str, rendered: String, before: Result, String>, fragments_in: FragmentsIn, @@ -303,34 +330,19 @@ pub(crate) fn finish<'a>( 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 edits = changed_pairs(what, &before, after, |old, new| { + respell(old, &new, file_terminator) + })?; + for (old, _) in &edits { + sole_in(what, text, old)?; } 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")); + for (_, new) in &edits { + sole_in(what, &result, new)?; } let known_edits = (result == rendered).then_some(edits); if known_edits.is_some() { @@ -340,7 +352,7 @@ pub(crate) fn finish<'a>( Ok(FragmentRewrite { text: result, original: text, - name, + name: name.to_string(), what, fragments_in, before, @@ -409,38 +421,160 @@ pub struct LockBatch { pub steps: Vec, } +/// One lock format's rewrite, as [`rewrite_in`] and [`rewrite_all`] drive +/// it: what a dep is, what it refuses before and after the lock is read, +/// and how a planned dep mutates the lock. +pub(crate) trait LockFormat { + /// The format's lock name, for error messages ("Poetry", "PDM"). + const WHAT: &'static str; + /// Takes a dep's fragments of a (spanned) lock parse. + const FRAGMENTS_IN: FragmentsIn; + /// One dep's rewrite arguments. + type Dep<'a>; + /// Where a dep's rewrite lands, settled before anything is mutated. + type Plan; + /// The dep's package name. + fn name<'d>(dep: &'d Self::Dep<'_>) -> &'d str; + /// The dep's own refusals, settled before the lock is read. + fn check(dep: &Self::Dep<'_>) -> Result<(), String>; + /// Every refusal (`Err`) and not-found verdict (`Ok(None)`) that comes + /// before the first mutation, read from the parsed lock. + fn plan(&self, lock: &Table, dep: &Self::Dep<'_>) -> Result, String>; + /// Apply a planned dep to the parsed lock. + fn mutate(lock: &mut DocumentMut, plan: Self::Plan, dep: &Self::Dep<'_>) -> Result<(), String>; +} + +/// One dep's rewrite of `text`, reusing (and refreshing) `parse`: `Ok(None)` +/// when the lock has no unit for the dep, `Err` for a refusal, which leaves +/// the lock as it was. +pub(crate) fn rewrite_in<'a, F: LockFormat>( + format: &F, + parse: &mut LockParse, + text: &'a str, + dep: &F::Dep<'_>, +) -> Result>, String> { + F::check(dep)?; + let name = F::name(dep); + let doc = parse.take(text, F::WHAT)?; + let plan = match format.plan(&doc, dep) { + Ok(Some(plan)) => plan, + verdict => { + // Nothing was mutated: the parse still describes `text`. + parse.restore(doc); + return verdict.map(|_| None); + } + }; + // The original's fragments come from the same parse; an error surfaces + // where a fresh parse would raise it, after the rewrite. + let before = (F::FRAGMENTS_IN)(&doc, text, name); + let mut lock = doc.into_mut(); + F::mutate(&mut lock, plan, dep)?; + finish( + F::WHAT, + parse, + text, + name, + lock.to_string(), + before, + F::FRAGMENTS_IN, + ) + .map(Some) +} + +/// Every dep's [`rewrite_in`] over `text`, each against the previous one's +/// output: one parse and one render of the lock for all of them when +/// [`rewrite_batch`] can vouch for the step-by-step result, else step by +/// step. +pub(crate) fn rewrite_all(format: &F, text: &str, deps: &[F::Dep<'_>]) -> LockBatch { + rewrite_batch(format, text, deps).unwrap_or_else(|| rewrite_steps(format, text, deps)) +} + +/// [`rewrite_all`] one dep at a time. +pub(crate) fn rewrite_steps( + format: &F, + text: &str, + deps: &[F::Dep<'_>], +) -> LockBatch { + let mut content = text.to_string(); + let mut parse = LockParse::default(); + let mut steps = Vec::with_capacity(deps.len()); + for dep in deps { + let mut landed = None; + let step = match rewrite_in(format, &mut parse, &content, dep) { + Ok(Some(rewrite)) if rewrite.text != content => match rewrite.edits() { + Ok(edits) => { + landed = Some(rewrite.text); + LockStep::Rewritten(edits) + } + Err(detail) => LockStep::Refused(detail), + }, + Ok(Some(_)) => LockStep::Unchanged, + Ok(None) => LockStep::NotFound, + Err(detail) => LockStep::Refused(detail), + }; + if let Some(text) = landed { + content = text; + } + steps.push(step); + } + LockBatch { + text: content, + steps, + } +} + +/// The equivalence every format's batch tests pin (#760, #762): whenever +/// [`rewrite_batch`] answers for `deps` over `text`, its text and every +/// dep's step are [`rewrite_steps`]' ones, and [`rewrite_all`] is always the +/// step-by-step result. Hands back that result and the batch's answer. +#[cfg(test)] +pub(crate) fn assert_batch_matches_steps( + text: &str, + deps: &[F::Dep<'_>], + what: &str, +) -> (LockBatch, Option) { + let steps = rewrite_steps(&F::default(), text, deps); + let batch = rewrite_batch(&F::default(), text, deps); + if let Some(batch) = &batch { + assert_eq!(batch, &steps, "{what}"); + } + assert_eq!(rewrite_all(&F::default(), text, deps), steps, "{what}"); + (steps, batch) +} + /// Every dep's rewrite of `text` over ONE parse and ONE render, with exactly /// the outcome the step-by-step rewrite (one [`finish`] per dep, each parsing /// the previous dep's output) would have had; `None` whenever this cannot /// vouch for that, and the caller then goes step by step. /// -/// `plan(i, lock)` settles dep `i` against the document as the previous deps -/// left it, so a refusal or not-found verdict is the one the step-by-step -/// rewrite would reach; `mutate` then applies a planned dep. Each rewritten -/// dep's fragments are taken from the original text and from the single -/// rendering, and spliced into the original text. That is only the +/// Each dep is checked and planned against the document as the previous +/// deps left it, so a refusal or not-found verdict is the one the +/// step-by-step rewrite would reach; a planned dep is then mutated in. +/// Each rewritten dep's fragments are taken from the original text and from +/// the single rendering, and spliced into the original text. That is only the /// step-by-step result when the deps rewrite distinct packages (a second /// rewrite of a package would start from the first one's output) and the /// lock has one line-ending style (a mixed lock's majority, which spells /// each spliced fragment, can shift as deps land), and the combined splice /// must reproduce the rendering byte for byte. -pub(crate) fn rewrite_batch

( +pub(crate) fn rewrite_batch( + format: &F, text: &str, - names: &[&str], - mut plan: impl FnMut(usize, &Table) -> Result, String>, - mut mutate: impl FnMut(&mut toml_edit::DocumentMut, P) -> Result<(), String>, - fragments_in: FragmentsIn, + deps: &[F::Dep<'_>], ) -> Option { if text.contains("\r\n") && text.replace("\r\n", "").contains('\n') { return None; } + let names: Vec<&str> = deps.iter().map(F::name).collect(); + let fragments_in = F::FRAGMENTS_IN; let original = parse_lock(text).ok()?; let mut lock = (*original).clone().into_mut(); let mut steps = Vec::with_capacity(names.len()); let mut rewritten: Vec<(usize, Vec)> = Vec::new(); let mut packages = std::collections::BTreeSet::new(); - for (index, name) in names.iter().enumerate() { - match plan(index, lock.as_table()) { + for (index, (dep, name)) in deps.iter().zip(&names).enumerate() { + let plan = F::check(dep).and_then(|()| format.plan(lock.as_table(), dep)); + match plan { Err(detail) => steps.push(LockStep::Refused(detail)), Ok(None) => steps.push(LockStep::NotFound), Ok(Some(planned)) => { @@ -450,7 +584,7 @@ pub(crate) fn rewrite_batch

( return None; } let before = fragments_in(&original, text, name).ok()?; - mutate(&mut lock, planned).ok()?; + F::mutate(&mut lock, planned, dep).ok()?; rewritten.push((index, before)); steps.push(LockStep::Unchanged); } @@ -473,16 +607,10 @@ pub(crate) fn rewrite_batch

( let mut edits_of: Vec<(usize, Vec<(String, String)>)> = Vec::new(); for (index, before) in &rewritten { let after = fragments_in(&after_doc, rendered, names[*index]).ok()?; - if before.len() != after.len() { - return None; - } - let mut edits = Vec::new(); - for (old, new) in before.iter().zip(after) { - let new = respell(old, &new, file_terminator); - if *old != new { - edits.push((old.clone(), new)); - } - } + let edits = changed_pairs(F::WHAT, before, after, |old, new| { + respell(old, &new, file_terminator) + }) + .ok()?; edits_of.push((*index, edits)); } // Where each changed fragment sits in `text`, all found in one pass; a @@ -591,6 +719,8 @@ mod tests { assert_eq!(sole_occurrences("aaa", &["aa"]), None); } + /// #694: the shared pairing refuses fragments that changed shape, for + /// every format. #[test] fn pair_fragments_refuses_a_shape_change() { for what in ["Poetry", "PDM"] { diff --git a/crates/socket-patch-core/src/utils/pdm_lock.rs b/crates/socket-patch-core/src/utils/pdm_lock.rs index 5ff2ffc65..f063d8448 100644 --- a/crates/socket-patch-core/src/utils/pdm_lock.rs +++ b/crates/socket-patch-core/src/utils/pdm_lock.rs @@ -1,11 +1,10 @@ -#[cfg(test)] use toml_edit::DocumentMut; use toml_edit::{value, Array, InlineTable, Item, Table, Value}; use crate::crawlers::python_crawler::canonicalize_pypi_name; use crate::utils::lock_fragments::{ - extend_span, finish, fragments_of, next_header_end, pair_fragments, rewrite_batch, - FragmentRewrite, LockParse, + extend_span, fragments_of, next_header_end, pair_fragments, rewrite_all, rewrite_in, + FragmentRewrite, FragmentsIn, LockFormat, LockParse, }; pub use crate::utils::lock_fragments::{LockBatch, LockStep}; use crate::utils::python_lock::is_prior_hosted_url; @@ -144,16 +143,11 @@ 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`]; 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`]). pub fn rewrite_pdm_lock_with_edits<'a>( text: &'a str, - name: &'a str, + name: &str, version: &str, source: (&str, &str), filename: &str, @@ -170,6 +164,11 @@ pub fn rewrite_pdm_lock_with_edits<'a>( ) } +/// 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>; + /// The parse of the `pdm.lock` text last seen or produced, carried between /// calls (see [`LockParse`]). pub type PdmLockParse = LockParse; @@ -178,37 +177,57 @@ pub type PdmLockParse = LockParse; pub fn rewrite_pdm_lock_in<'a>( parse: &mut PdmLockParse, text: &'a str, - name: &'a str, + name: &str, version: &str, source: (&str, &str), filename: &str, sha256: &str, ) -> Result, String> { - let (kind, location) = source; - check_pdm_artifact(name, version, kind, filename, sha256)?; - 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.restore(doc); - return Err(detail); - } - }; - // The original's fragments come from the same parse; an error surfaces - // where a fresh parse would raise it, after the rewrite. - let before = pdm_lock_fragments_in(&doc, text, name); - let mut lock = doc.into_mut(); - mutate_pdm_lock(&mut lock, edits, source, filename, sha256)?; - finish( - "PDM", - parse, - text, + let dep = PdmLockDep { name, - lock.to_string(), - before, - pdm_lock_fragments_in::, - ) + version, + source, + filename, + sha256, + }; + // PDM refuses a package its lock lacks (its plan is never `None`); the + // fallback only keeps that a refusal should the plan ever change. + rewrite_in(&Pdm, parse, text, &dep)?.ok_or_else(|| "missing PDM package".into()) +} + +/// `pdm.lock` as the shared [`lock_fragments`](crate::utils::lock_fragments) +/// driver rewrites it. +#[derive(Default)] +struct Pdm; + +impl LockFormat for Pdm { + const WHAT: &'static str = "PDM"; + const FRAGMENTS_IN: FragmentsIn = pdm_lock_fragments_in::; + type Dep<'a> = PdmLockDep<'a>; + /// The `(package index, inline files, legacy files key)` of each unit. + type Plan = Vec<(usize, bool, String)>; + + fn name<'d>(dep: &'d PdmLockDep) -> &'d str { + dep.name + } + + fn check(dep: &PdmLockDep) -> Result<(), String> { + check_pdm_artifact( + dep.name, + dep.version, + dep.source.0, + dep.filename, + dep.sha256, + ) + } + + fn plan(&self, lock: &Table, dep: &PdmLockDep) -> Result, String> { + plan_pdm_rewrite(lock, dep.name, dep.version, dep.source.0, dep.source.1).map(Some) + } + + fn mutate(lock: &mut DocumentMut, plan: Self::Plan, dep: &PdmLockDep) -> Result<(), String> { + mutate_pdm_lock(lock, plan, dep.source, dep.filename, dep.sha256) + } } /// The rewrite's own refusals, settled before the lock is read. @@ -279,75 +298,11 @@ pub struct PdmLockDep<'a> { pub sha256: &'a str, } -/// Every dep's [`rewrite_pdm_lock_in`] over `text`, each against the -/// previous one's output: one parse and one render of the lock for all of -/// them when the batch can vouch for the step-by-step result, else step by -/// step. A dep's step is never [`LockStep::NotFound`]: PDM refuses a -/// package its lock lacks. +/// Every dep's rewrite over `text`, each against the previous one's output +/// (see [`rewrite_all`]). A dep's step is never [`LockStep::NotFound`]: PDM +/// refuses a package its lock lacks. pub fn rewrite_pdm_lock_all(text: &str, deps: &[PdmLockDep]) -> LockBatch { - rewrite_pdm_lock_batch(text, deps).unwrap_or_else(|| rewrite_pdm_lock_steps(text, deps)) -} - -/// [`rewrite_pdm_lock_all`] over one parse and one render, or `None` (see -/// [`rewrite_batch`]). -fn rewrite_pdm_lock_batch(text: &str, deps: &[PdmLockDep]) -> Option { - let names: Vec<&str> = deps.iter().map(|dep| dep.name).collect(); - rewrite_batch( - text, - &names, - |index, lock| { - let dep = &deps[index]; - check_pdm_artifact( - dep.name, - dep.version, - dep.source.0, - dep.filename, - dep.sha256, - )?; - let units = plan_pdm_rewrite(lock, dep.name, dep.version, dep.source.0, dep.source.1)?; - Ok(Some((units, index))) - }, - |lock, (units, index)| { - let dep = &deps[index]; - mutate_pdm_lock(lock, units, dep.source, dep.filename, dep.sha256) - }, - pdm_lock_fragments_in::, - ) -} - -/// [`rewrite_pdm_lock_all`] one dep at a time. -fn rewrite_pdm_lock_steps(text: &str, deps: &[PdmLockDep]) -> LockBatch { - let mut content = text.to_string(); - let mut parse = PdmLockParse::default(); - let mut steps = Vec::with_capacity(deps.len()); - for dep in deps { - let rewrite = rewrite_pdm_lock_in( - &mut parse, - &content, - dep.name, - dep.version, - dep.source, - dep.filename, - dep.sha256, - ); - let step = match rewrite.and_then(|rewrite| Ok((rewrite.edits()?, rewrite.text))) { - Ok((edits, rewritten)) => { - let step = if rewritten == content { - LockStep::Unchanged - } else { - LockStep::Rewritten(edits) - }; - content = rewritten; - step - } - Err(detail) => LockStep::Refused(detail), - }; - steps.push(step); - } - LockBatch { - text: content, - steps, - } + rewrite_all(&Pdm, text, deps) } /// Every refusal of [`rewrite_pdm_lock_in`], read from the parsed lock before @@ -825,7 +780,7 @@ pub(crate) mod tests { rewritten += 1; let want = pdm_lock_edits(&text, &rewrite.text, "urllib3"); assert_eq!(rewrite.edits(), want, "{name} crlf={crlf}"); - let rederived = PdmLockRewrite { + let rederived = FragmentRewrite { known_edits: None, ..rewrite }; @@ -916,17 +871,6 @@ pub(crate) mod tests { } } - /// #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; @@ -1108,7 +1052,7 @@ pub(crate) mod parse_reuse_tests { lock = lock.replace('\n', "\r\n"); } let mut got_text = lock; - let mut parse = PdmLockParse::default(); + let mut parse = LockParse::default(); for (name, version, (kind, location), sha) in steps(extra).iter() { let filename = location.rsplit('/').next().unwrap(); let source = (kind.as_str(), location.as_str()); @@ -1135,7 +1079,7 @@ pub(crate) mod parse_reuse_tests { #[test] fn reused_parse_misses_on_changed_text() { let lock = grown(&fixtures().pop().unwrap().1.replace("\r\n", "\n"), 2); - let mut parse = PdmLockParse::default(); + let mut parse = LockParse::default(); let url = "https://patch.socket.dev/patch/pypi/pkg0/a/pkg0-1.26.18-py3-none-any.whl"; let first = rewrite_pdm_lock_in( &mut parse, @@ -1160,7 +1104,7 @@ pub(crate) mod parse_reuse_tests { ); assert_eq!(got.err().as_deref(), Some("missing PDM package")); // `parsed` likewise re-parses changed text. - let names = |parse: &mut PdmLockParse, text: &str| -> Vec { + let names = |parse: &mut LockParse, text: &str| -> Vec { parse.parsed(text).unwrap()["package"] .as_array_of_tables() .unwrap() @@ -1180,6 +1124,7 @@ mod batch_equivalence_tests { //! every dep's step are the step-by-step ones. use super::parse_reuse_tests::{fixtures, grown}; use super::*; + use crate::utils::lock_fragments::{assert_batch_matches_steps, rewrite_steps}; const SHA: &str = "34b97092d7e0a3a8cf7cd10e386f401b3737364026c45e622aa02903dffe0f07"; @@ -1270,7 +1215,7 @@ mod batch_equivalence_tests { for (mix, (deps, batchable)) in mixes(extra).into_iter().enumerate() { let deps = lock_deps(&deps); let what = format!("{fixture} extra={extra} {style} mix={mix}"); - let first = rewrite_pdm_lock_steps(&lock, &deps); + let first = rewrite_steps(&Pdm, &lock, &deps); let lands = first .steps .iter() @@ -1279,14 +1224,11 @@ mod batch_equivalence_tests { for (rerun, text) in [lock.clone(), first.text.clone()].into_iter().enumerate() { - let steps = rewrite_pdm_lock_steps(&text, &deps); - let batch = rewrite_pdm_lock_batch(&text, &deps); - if let Some(batch) = &batch { + let (_, batch) = assert_batch_matches_steps::(&text, &deps, &what); + if batch.is_some() { batched += 1; rendered += usize::from(lands); - assert_eq!(batch, &steps, "{what}"); } - assert_eq!(rewrite_pdm_lock_all(&text, &deps), steps, "{what}"); if !lands { continue; // an unsupported generation: all refused } diff --git a/crates/socket-patch-core/src/utils/poetry_lock.rs b/crates/socket-patch-core/src/utils/poetry_lock.rs index 8e06c0c58..ce6ca8a7c 100644 --- a/crates/socket-patch-core/src/utils/poetry_lock.rs +++ b/crates/socket-patch-core/src/utils/poetry_lock.rs @@ -18,8 +18,8 @@ use toml_edit::{value, Array, DocumentMut, InlineTable, Item, Table, TableLike, use crate::crawlers::python_crawler::canonicalize_pypi_name; use crate::utils::lock_fragments::{ - extend_span, finish, fragments_of, next_header_end, pair_fragments, rewrite_batch, - FragmentRewrite, LockParse, + extend_span, fragments_of, next_header_end, pair_fragments, rewrite_all, rewrite_in, + FragmentRewrite, FragmentsIn, LockFormat, LockParse, }; pub use crate::utils::lock_fragments::{LockBatch, LockStep}; use crate::utils::python_lock::{is_prior_hosted_url, table_likes}; @@ -453,7 +453,7 @@ pub type PoetryLockRewrite<'a> = FragmentRewrite<'a>; /// record the rewrite's fragment edits ([`PoetryLockRewrite::edits`]). pub fn rewrite_poetry_lock_with_edits<'a>( text: &'a str, - name: &'a str, + name: &str, version: &str, source_type: &str, source_url: &str, @@ -476,62 +476,103 @@ pub fn rewrite_poetry_lock_with_edits<'a>( /// 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 { - format: String, - effective_url: String, - index: usize, - package_name: String, -} - /// [`rewrite_poetry_lock_with_edits`], reusing (and refreshing) `parse`. #[allow(clippy::too_many_arguments)] pub fn rewrite_poetry_lock_in<'a>( parse: &mut PoetryLockParse, text: &'a str, - name: &'a str, + name: &str, version: &str, source_type: &str, source_url: &str, filename: &str, sha256: &str, ) -> Result>, String> { - let sha256 = checked_sha256(name, version, source_type, filename, sha256)?; - 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.restore(doc); - return verdict.map(|_| None); - } - }; - // The original's fragments come from the same parse; an error surfaces - // where a fresh parse would raise it, after the rewrite. - let before = poetry_lock_fragments_in(&doc, text, name); - let mut lock = doc.into_mut(); - mutate_poetry_lock(&mut lock, plan, source_type, filename, &sha256)?; - finish( - "Poetry", - parse, - text, + let dep = PoetryLockDep { name, - lock.to_string(), - before, - poetry_lock_fragments_in::, - ) - .map(Some) + version, + source_type, + source_url, + filename, + sha256, + }; + rewrite_in(&Poetry::default(), parse, text, &dep) +} + +/// Where a Poetry rewrite lands, settled before it mutates. +struct PoetryLockPlan { + format: String, + effective_url: String, + index: usize, + package_name: String, + /// The lowercase digest to pin. + sha256: String, +} + +/// `poetry.lock` as the shared [`lock_fragments`](crate::utils::lock_fragments) +/// driver rewrites it. +#[derive(Default)] +struct Poetry { + /// The lock's `lock_version`, from the first parse a plan read it from. + lock_version: std::cell::OnceCell, +} + +impl LockFormat for Poetry { + const WHAT: &'static str = "Poetry"; + const FRAGMENTS_IN: FragmentsIn = poetry_lock_fragments_in::; + type Dep<'a> = PoetryLockDep<'a>; + type Plan = PoetryLockPlan; + + fn name<'d>(dep: &'d PoetryLockDep) -> &'d str { + dep.name + } + + fn check(dep: &PoetryLockDep) -> Result<(), String> { + check_poetry_artifact( + dep.name, + dep.version, + dep.source_type, + dep.filename, + dep.sha256, + ) + } + + fn plan(&self, lock: &Table, dep: &PoetryLockDep) -> Result, String> { + if self.lock_version.get().is_none() { + if let Ok(version) = lock_version(lock) { + let _ = self.lock_version.set(version.to_string()); + } + } + plan_poetry_rewrite( + lock, + dep.name, + dep.version, + dep.source_type, + dep.source_url, + // Poetry compares the lock's `sha256:` against `hashlib`'s + // lowercase hexdigest as strings, so an uppercase digest would + // fail every install. + dep.sha256.to_ascii_lowercase(), + ) + } + + fn mutate( + lock: &mut DocumentMut, + plan: PoetryLockPlan, + dep: &PoetryLockDep, + ) -> Result<(), String> { + mutate_poetry_lock(lock, plan, dep.source_type, dep.filename) + } } -/// The rewrite's own refusals, settled before the lock is read: the -/// lowercase `sha256` to pin. -fn checked_sha256( +/// The rewrite's own refusals, settled before the lock is read. +fn check_poetry_artifact( name: &str, version: &str, source_type: &str, filename: &str, sha256: &str, -) -> Result { +) -> Result<(), String> { if !matches!(source_type, "file" | "url") || sha256.len() != 64 || !sha256.bytes().all(|byte| byte.is_ascii_hexdigit()) @@ -541,9 +582,7 @@ fn checked_sha256( if !crate::vendor::pypi_distribution::matches(filename, name, version) { return Err("Poetry patch wheel does not match the locked package".into()); } - // Poetry compares the lock's `sha256:` against `hashlib`'s lowercase - // hexdigest as strings, so an uppercase digest would fail every install. - Ok(sha256.to_ascii_lowercase()) + Ok(()) } /// Apply a planned rewrite to the parsed lock. @@ -552,13 +591,13 @@ fn mutate_poetry_lock( plan: PoetryLockPlan, source_type: &str, filename: &str, - sha256: &str, ) -> Result<(), String> { let PoetryLockPlan { format, effective_url, index, package_name, + sha256, } = plan; let package = lock .get_mut("package") @@ -606,7 +645,7 @@ fn mutate_poetry_lock( .ok_or_else(|| format!("[metadata.{field}] is not a table"))?; if format == "0" { let mut hashes = Array::new(); - hashes.push(sha256); + hashes.push(sha256.as_str()); table.insert(&package_name, value(hashes)); } else { let entry = legacy_files_entry(table, &package_name, files, rewritten); @@ -627,105 +666,24 @@ pub struct PoetryLockDep<'a> { pub sha256: &'a str, } -/// Every dep's [`rewrite_poetry_lock_in`] over `text`, each against the -/// previous one's output: one parse and one render of the lock for all of -/// them when the batch can vouch for the step-by-step result, else step by -/// step. +/// Every dep's rewrite over `text`, each against the previous one's output +/// (see [`rewrite_all`]). pub fn rewrite_poetry_lock_all(text: &str, deps: &[PoetryLockDep]) -> LockBatch { rewrite_poetry_lock_all_with_version(text, deps).0 } -/// [`rewrite_poetry_lock_all`], plus the lock's `lock_version` when the -/// batch read it from its own parse (`None` when it went step by step, or -/// the lock has no supported version), so a caller needn't parse the lock -/// again for it. No package rewrite touches `[metadata] lock-version`, so -/// it is the rewritten lock's version too. +/// [`rewrite_poetry_lock_all`], plus the lock's `lock_version` when a plan +/// read it from a parse the rewrite took anyway (`None` when no dep got that +/// far, or the lock has no supported version), so a caller needn't parse the +/// lock again for it. No package rewrite touches `[metadata] lock-version`, +/// so it is the rewritten lock's version too. pub fn rewrite_poetry_lock_all_with_version( text: &str, deps: &[PoetryLockDep], ) -> (LockBatch, Option) { - let mut version = None; - match rewrite_poetry_lock_batch(text, deps, &mut version) { - Some(batch) => (batch, version), - None => (rewrite_poetry_lock_steps(text, deps), None), - } -} - -/// [`rewrite_poetry_lock_all`] over one parse and one render, or `None` (see -/// [`rewrite_batch`]); `version` gets the parse's `lock_version`. -fn rewrite_poetry_lock_batch( - text: &str, - deps: &[PoetryLockDep], - version: &mut Option, -) -> Option { - let names: Vec<&str> = deps.iter().map(|dep| dep.name).collect(); - rewrite_batch( - text, - &names, - |index, lock| { - if version.is_none() { - *version = lock_version(lock).ok().map(str::to_string); - } - let dep = &deps[index]; - let sha256 = checked_sha256( - dep.name, - dep.version, - dep.source_type, - dep.filename, - dep.sha256, - )?; - let plan = plan_poetry_rewrite( - lock, - dep.name, - dep.version, - dep.source_type, - dep.source_url, - &sha256, - )?; - Ok(plan.map(|plan| (plan, index, sha256))) - }, - |lock, (plan, index, sha256)| { - let dep = &deps[index]; - mutate_poetry_lock(lock, plan, dep.source_type, dep.filename, &sha256) - }, - poetry_lock_fragments_in::, - ) -} - -/// [`rewrite_poetry_lock_all`] one dep at a time. -fn rewrite_poetry_lock_steps(text: &str, deps: &[PoetryLockDep]) -> LockBatch { - let mut content = text.to_string(); - let mut parse = PoetryLockParse::default(); - let mut steps = Vec::with_capacity(deps.len()); - for dep in deps { - let step = match rewrite_poetry_lock_in( - &mut parse, - &content, - dep.name, - dep.version, - dep.source_type, - dep.source_url, - dep.filename, - dep.sha256, - ) { - Ok(Some(rewrite)) if rewrite.text != content => match rewrite.edits() { - Ok(edits) => { - let text = rewrite.text; - content = text; - LockStep::Rewritten(edits) - } - Err(detail) => LockStep::Refused(detail), - }, - Ok(Some(_)) => LockStep::Unchanged, - Ok(None) => LockStep::NotFound, - Err(detail) => LockStep::Refused(detail), - }; - steps.push(step); - } - LockBatch { - text: content, - steps, - } + let poetry = Poetry::default(); + let batch = rewrite_all(&poetry, text, deps); + (batch, poetry.lock_version.into_inner()) } /// Every refusal and not-applicable verdict of [`rewrite_poetry_lock_in`] @@ -736,7 +694,7 @@ fn plan_poetry_rewrite( version: &str, source_type: &str, source_url: &str, - sha256: &str, + sha256: String, ) -> Result, String> { let format = lock_version(lock)?.to_string(); if format == "0" && source_type == "url" { @@ -803,6 +761,7 @@ fn plan_poetry_rewrite( effective_url, index: indices[0], package_name, + sha256, })) } @@ -1171,24 +1130,6 @@ grouped = {version = "*", source = "pypi"} } } - /// #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] @@ -1460,7 +1401,7 @@ grouped = {version = "*", source = "pypi"} .unwrap(); let want = poetry_lock_edits(&text, &rewrite.text, "urllib3"); assert_eq!(rewrite.edits(), want, "{version} crlf={crlf}"); - let rederived = PoetryLockRewrite { + let rederived = FragmentRewrite { known_edits: None, ..rewrite }; @@ -1600,7 +1541,7 @@ mod parse_reuse_equivalence_tests { lock = lock.replace('\n', "\r\n"); } let mut got_text = lock; - let mut parse = PoetryLockParse::default(); + let mut parse = LockParse::default(); for (name, version, source_type, url) in steps(extra).iter() { let filename = url.rsplit('/').next().unwrap(); let got = rewrite_poetry_lock_in( @@ -1639,7 +1580,7 @@ mod parse_reuse_equivalence_tests { fn reused_parse_misses_on_changed_text() { let lock = grown("2.4.3", 2); let url = "https://patch.socket.dev/patch/pypi/pkg0/a/pkg0-1.26.18-py2.py3-none-any.whl"; - let mut parse = PoetryLockParse::default(); + let mut parse = LockParse::default(); let first = rewrite_poetry_lock_in( &mut parse, &lock, @@ -1675,6 +1616,7 @@ mod batch_equivalence_tests { //! step-by-step rewrite it replaces (#760): whenever the batch answers, //! its text and every dep's step are the step-by-step ones. use super::*; + use crate::utils::lock_fragments::{assert_batch_matches_steps, rewrite_steps}; const VERSIONS: &[&str] = &[ "0.12.17", "1.0.10", "1.1.15", "1.2.2", "1.3.2", "1.4.2", "1.5.1", "1.6.1", "1.7.1", @@ -1807,29 +1749,28 @@ mod batch_equivalence_tests { for (mix, (deps, batchable)) in mixes(extra).into_iter().enumerate() { let deps = lock_deps(&deps); let what = format!("{version} extra={extra} {style} mix={mix}"); - let steps = rewrite_poetry_lock_steps(&lock, &deps); + let steps = rewrite_steps(&Poetry::default(), &lock, &deps); // And again over the output: the idempotent re-scan. for (rerun, text) in [lock.clone(), steps.text.clone()].into_iter().enumerate() { cases += 1; - let steps = rewrite_poetry_lock_steps(&text, &deps); - let mut read_version = None; - let batch = rewrite_poetry_lock_batch(&text, &deps, &mut read_version); - if let Some(batch) = &batch { + let (_, batch) = + assert_batch_matches_steps::(&text, &deps, &what); + if batch.is_some() { batched += 1; - assert_eq!(batch, &steps, "{what}"); - // The version the batch read is the output's. - if read_version.is_some() { - let output = batch.text.parse::().unwrap(); - assert_eq!( - read_version.as_deref(), - lock_version(&output).ok(), - "{what}" - ); - } } - assert_eq!(rewrite_poetry_lock_all(&text, &deps), steps, "{what}"); + // The version the rewrite read is the output's. + let (all, read_version) = + rewrite_poetry_lock_all_with_version(&text, &deps); + if read_version.is_some() { + let output = all.text.parse::().unwrap(); + assert_eq!( + read_version.as_deref(), + lock_version(&output).ok(), + "{what}" + ); + } // Poetry 0.12 refuses every URL source: nothing to // render, so the batch answers. if style == "mixed" {