From cf8d673c821953ce03376d51682df5e96c1b61ac Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 20:26:29 +0000 Subject: [PATCH 1/5] Start fix for #331 Assisted-by: Claude Code:claude-opus-5-5 From f4843accce5f84a6af9d1438d6788e217487eee2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 20:33:35 +0000 Subject: [PATCH 2/5] Add tests for PDM rollback after pdm add re-scan A hosted re-scan after `pdm add` or `pdm lock --update-reuse` records the still-patched pdm.lock unit as the edit's original, so rollback reports success but leaves the Socket url and patched hash in the lock (#331). These tests cover both PDM shapes (url kept, and url dropped with the patched hash kept) on LF and CRLF checkouts, and fail on main. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/hosted_memory/ledger.rs | 64 +++++++++ .../tests/in_process_redirect_pdm.rs | 121 ++++++++++++++++++ 2 files changed, 185 insertions(+) diff --git a/crates/socket-patch-cli/src/hosted_memory/ledger.rs b/crates/socket-patch-cli/src/hosted_memory/ledger.rs index 2961d107d..7cd405f97 100644 --- a/crates/socket-patch-cli/src/hosted_memory/ledger.rs +++ b/crates/socket-patch-cli/src/hosted_memory/ledger.rs @@ -184,4 +184,68 @@ mod tests { let text = serialize(&ledger).unwrap(); assert!(text.ends_with("}\n")); } + + fn pdm(original: &str, new: &str) -> FileEdit { + FileEdit { + path: "pdm.lock".into(), + kind: "redirect_pdm_lock_package".into(), + action: "rewritten".into(), + key: Some("urllib3".into()), + original: Some(serde_json::json!(original)), + new: Some(serde_json::json!(new)), + } + } + + /// #331: a drifted PDM unit that still carries the Socket url or the + /// patched sha256 (`pdm add`, `pdm lock --update-reuse`) keeps the + /// recorded pristine `original`, re-laid in the relocked line endings; + /// a clean relock (`pdm lock`) still adopts the fresh one. + #[test] + fn pdm_rebase_keeps_the_pristine_original_while_the_patch_survives() { + let url = "url = \"https://patch.test/u.whl\"\r\n"; + let pristine = "files = [\"sha256:1111\"]\r\n"; + let wired = format!("files = [\"sha256:cccc\"]\r\n{url}"); + for (current, fresh_new, keeps) in [ + // url + patched hash kept, re-laid. + ( + "files = [ \"sha256:cccc\" ]\nurl = \"https://patch.test/u.whl\"\n", + "files = [\"sha256:cccc\"]\nurl = \"https://patch.test/u.whl\"\n", + true, + ), + // url dropped, patched hash kept. + ( + "files = [ \"sha256:cccc\" ]\n", + "files = [\"sha256:cccc\"]\nurl = \"https://patch.test/u.whl\"\n", + true, + ), + // `pdm lock`: back on the registry. + ( + "files = [ \"sha256:2222\" ]\n", + "files = [\"sha256:cccc\"]\nurl = \"https://patch.test/u.whl\"\n", + false, + ), + ] { + let mut ledger = RedirectState::new(); + ledger.edits.push(pdm(pristine, &wired)); + let files = BTreeMap::from([("pdm.lock".to_string(), current.to_string())]); + merge( + &mut ledger, + &[pdm(current, fresh_new)], + BTreeMap::new(), + &files, + ); + assert_eq!(ledger.edits.len(), 1, "rebased, not appended"); + let original = ledger.edits[0].original.as_ref().unwrap().as_str().unwrap(); + let want = if keeps { + "files = [\"sha256:1111\"]\n" + } else { + current + }; + assert_eq!(original, want, "current={current:?}"); + assert_eq!( + ledger.edits[0].new.as_ref().unwrap().as_str().unwrap(), + fresh_new + ); + } + } } diff --git a/crates/socket-patch-cli/tests/in_process_redirect_pdm.rs b/crates/socket-patch-cli/tests/in_process_redirect_pdm.rs index a42a8f6c9..4b01fc50c 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect_pdm.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect_pdm.rs @@ -556,3 +556,124 @@ async fn assert_relock_roundtrip(lock: &str, relocked: &str) { "rollback restores the relocked lock byte for byte" ); } + +/// #331: `pdm add ` / `pdm lock --update-reuse` re-render the +/// redirected unit but KEEP the patch data. PDM 2.26+ keeps the Socket `url` +/// and the patched sha256; PDM 2.12–2.20 drop the `url` but keep the patched +/// sha256. The re-scan then records that still-patched unit as its +/// `original`, so a rebase that adopted it would make `rollback` "revert" +/// patched → patched, report success and delete the ledger. The rebase must +/// keep the ledger's pristine `original` instead (with the relocked line +/// endings), so `rollback` lands on the upstream registry unit inside the +/// user's re-rendered lock. +#[tokio::test] +#[serial] +async fn rerender_keeping_the_patch_then_rescan_rolls_back_to_the_registry() { + for start in ["\r\n", "\n"] { + for keep_url in [true, false] { + let lock = LOCK.replace("\r\n", "\n").replace('\n', start); + assert_rerender_roundtrip(&lock, keep_url).await; + } + } +} + +/// Where the `urllib3` unit starts in `lock` (it is the last unit). +fn urllib3_unit(lock: &str) -> usize { + lock.find("[[package]]\nname = \"urllib3\"") + .expect("urllib3 unit") +} + +/// What `pdm add six==1.16.0` leaves: a new content hash, a `six` unit ahead +/// of `urllib3`, and the redirected `urllib3` unit re-laid by PDM — its +/// `files` back in PDM's one-entry-per-line layout, the Socket `url` kept +/// (2.26+) or dropped (2.12–2.20), and the patched sha256 kept either way. +/// PDM always writes LF. +fn pdm_add_rerender(redirected: &str, keep_url: bool) -> String { + const SIX: &str = "[[package]]\nname = \"six\"\nversion = \"1.16.0\"\nrequires_python = \">=2.7, !=3.0.*, !=3.1.*, !=3.2.*\"\nsummary = \"Python 2 and 3 compatibility utilities\"\ngroups = [\"default\"]\nfiles = [\n {file = \"six-1.16.0-py2.py3-none-any.whl\", hash = \"sha256:8abb2f1d86890a2dfb989f9a77cfcfd3e47c2a354b01111771326f8aa26e0254\"},\n {file = \"six-1.16.0.tar.gz\", hash = \"sha256:1e61c37477a1626458e36f7b1d82aa5c9b094fa4802892072e49de9c60c4c926\"},\n]\n\n"; + const WHEEL: &str = "urllib3-1.26.18-py2.py3-none-any.whl"; + let text = redirected.replace("\r\n", "\n").replace( + "68a0e962e677b7f765a49a6df99753b78d8a99dfe60e5694e6994dcc8efb44bc", + &"d".repeat(64), + ); + let at = urllib3_unit(&text); + let (head, unit) = text.split_at(at); + let socket_files = format!( + "files = [{{ file = \"{WHEEL}\", hash = \"sha256:{}\" }}]\n", + sha256() + ); + let pdm_files = format!( + "files = [\n {{file = \"{WHEEL}\", hash = \"sha256:{}\"}},\n]\n", + sha256() + ); + assert!(unit.contains(&socket_files), "{unit}"); + let mut unit = unit.replace(&socket_files, &pdm_files); + if !keep_url { + let url_line = format!("url = \"{HOSTED_URL}\"\n"); + assert!(unit.contains(&url_line), "{unit}"); + unit = unit.replace(&url_line, ""); + } + format!("{head}{SIX}{unit}") +} + +async fn assert_rerender_roundtrip(lock: &str, keep_url: bool) { + let server = MockServer::start().await; + mock_api(&server).await; + let tmp = tempfile::tempdir().unwrap(); + write_project(tmp.path(), lock); + let lock_path = tmp.path().join("pdm.lock"); + let ledger_path = tmp.path().join(".socket/vendor/redirect-state.json"); + let case = format!("keep_url={keep_url} crlf={}", lock.contains('\r')); + + assert_eq!( + run(hosted_args(tmp.path(), server.uri(), None)).await, + 0, + "{case}" + ); + let redirected = read(&lock_path); + assert!(redirected.contains(HOSTED_URL), "{case}"); + + let rerendered = pdm_add_rerender(&redirected, keep_url); + std::fs::write(&lock_path, &rerendered).unwrap(); + + // The re-scan converges the re-laid unit back onto the Socket wiring. + assert_eq!( + run(hosted_args(tmp.path(), server.uri(), None)).await, + 0, + "{case}" + ); + let rescanned = read(&lock_path); + assert!(rescanned.contains(HOSTED_URL), "{case}: {rescanned}"); + let ledger: serde_json::Value = serde_json::from_str(&read(&ledger_path)).unwrap(); + for edit in ledger["edits"].as_array().unwrap() { + let original = edit["original"].as_str().unwrap(); + assert!( + !original.contains(HOSTED_URL) && !original.contains(&sha256()), + "{case}: the ledger's original must stay the pristine unit: {ledger}" + ); + } + + let code = rollback::run(RollbackArgs { + targets: Vec::new(), + common: global(tmp.path(), server.uri()), + one_off: false, + preserve_state: false, + }) + .await; + assert_eq!( + code, 0, + "{case}: rollback after re-render + re-scan must succeed" + ); + let rolled_back = read(&lock_path); + assert!( + !rolled_back.contains(HOSTED_URL) && !rolled_back.contains(&sha256()), + "{case}: rollback must remove the Socket url and patched hash: {rolled_back}" + ); + // Exactly the user's re-rendered lock with the pristine (LF) unit back. + let pristine = LOCK.replace("\r\n", "\n"); + let expected = format!( + "{}{}", + &rerendered[..urllib3_unit(&rerendered)], + &pristine[urllib3_unit(&pristine)..] + ); + assert_eq!(rolled_back, expected, "{case}"); +} From 261637ebb54ac2f8a2e94cfbce2cbe882c1b1bf7 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 20:36:02 +0000 Subject: [PATCH 3/5] Keep PDM's pristine unit when a relock keeps the patch `pdm add` and `pdm lock --update-reuse` re-lay the redirected pdm.lock unit but keep Socket's url and patched hash, or just the hash on PDM 2.12-2.20. A hosted re-scan then adopted that still patched unit as the rollback target, so rollback reported success and deleted the ledger while the lock stayed patched (or, on 2.12-2.20, stopped installing). The rebase now adopts the re-scan's unit only once it no longer carries anything the redirect introduced; otherwise it keeps the recorded pristine unit in the relocked line endings. `pdm lock`, which re-resolves to the registry, behaves as before. Fixes #331 Assisted-by: Claude Code:claude-opus-5-5 --- CHANGELOG.md | 9 +++ .../src/commands/scan/hosted.rs | 64 ++++++++++++++++--- .../src/hosted_memory/ledger.rs | 5 +- docs/testing/pdm-compatibility.md | 8 +++ 4 files changed, 75 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b85d8bf44..1b3bacabf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -914,6 +914,15 @@ into the new version's section — see docs/releasing.md. ### Fixed +- **Hosted PDM `rollback` removes the patch after `pdm add` and a re-scan.** + `pdm add ` and `pdm lock --update-reuse` re-lay the redirected + `pdm.lock` unit but keep Socket's `url` and patched hash (PDM 2.26+), or + just the patched hash (2.12–2.20). A re-scan then recorded that + still-patched unit as the pristine one, so `rollback` reported success + and deleted the ledger while the lock stayed patched, or, on 2.12–2.20, + stopped installing. The re-scan now keeps the recorded pristine unit + whenever the re-laid one still carries the patch. + - **Hosted nuget redirects survive a `` in `nuget.config`.** The Socket source (and, in an existing ``, its mapping) was inserted ahead of the section's ``, which NuGet diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index c66d56fa5..0aa649eca 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -37,6 +37,55 @@ pub(crate) const REBASE_KINDS: &[&str] = &[ socket_patch_core::patch::redirect::vlt::KIND, ]; +/// The `original` a rebased PDM lock edit keeps: `recorded` is the ledger's +/// edit, `fresh` this run's edit for the same drifted unit. +/// +/// `pdm lock` re-resolves to the registry (and may reflow CRLF → LF), so the +/// fresh `original` IS the relocked-registry rollback target. But `pdm add +/// ` and `pdm lock --update-reuse` re-lay the unit while keeping the +/// patch: PDM 2.26+ keeps the Socket `url` and patched sha256, 2.12–2.20 keep +/// just the patched sha256. Adopting that still-patched fragment would make +/// rollback "revert" patched → patched and report success (#331). While the +/// fresh `original` still carries any string the recorded edit introduced +/// (a quoted value in its `new` that its `original` lacks: the url, the +/// patched hash), keep the recorded pristine `original`, re-laid in the +/// fresh fragment's line endings so it splices into the relocked text. +pub(crate) fn rebased_pdm_original( + recorded: &socket_patch_core::patch::redirect::FileEdit, + fresh: &socket_patch_core::patch::redirect::FileEdit, +) -> Option { + let as_str = + |v: &Option| v.as_ref().and_then(|v| v.as_str().map(str::to_owned)); + let (Some(pristine), Some(wired), Some(current)) = ( + as_str(&recorded.original), + as_str(&recorded.new), + as_str(&fresh.original), + ) else { + return fresh.original.clone(); + }; + let introduced = ['"', '\''].into_iter().flat_map(|quote| { + wired + .split(quote) + .skip(1) + .step_by(2) + .map(str::to_owned) + .collect::>() + }); + let still_patched = introduced + .filter(|token| !token.is_empty() && !pristine.contains(token.as_str())) + .any(|token| current.contains(token.as_str())); + if !still_patched { + return fresh.original.clone(); + } + let lf = pristine.replace("\r\n", "\n"); + let relaid = if current.contains("\r\n") { + lf.replace('\n', "\r\n") + } else { + lf + }; + Some(serde_json::Value::String(relaid)) +} + pub(crate) const REDIRECT_CANDIDATE_FILES: &[&str] = &[ "package-lock.json", "npm-shrinkwrap.json", @@ -2997,17 +3046,14 @@ pub(crate) async fn run_redirect_selected( .unwrap_or(0); if let Some(&target) = siblings.get(nth) { if !rebased.contains(&target) { - // `pdm lock` fully un-patches the lock (registry source - // restored) and may reflow line endings (CRLF → LF), so - // the fresh run's `original` IS the correct - // relocked-registry rollback target and the stale - // recorded one would restore a mismatched fragment. - // Poetry's relock instead KEEPS the Socket source (it - // only drops the inserted `files` line), so its oldest + // Poetry's relock KEEPS the Socket source (it only + // drops the inserted `files` line), so its oldest // `original` — the true pre-patch fragment — must - // survive; only its `new` is refreshed. + // survive; only its `new` is refreshed. PDM's depends + // on what the relock did: see `rebased_pdm_original`. if edit.kind == "redirect_pdm_lock_package" { - ledger.edits[target].original = edit.original.clone(); + ledger.edits[target].original = + rebased_pdm_original(&ledger.edits[target], edit); } ledger.edits[target].new = edit.new.clone(); ledger.edits[target].action = edit.action.clone(); diff --git a/crates/socket-patch-cli/src/hosted_memory/ledger.rs b/crates/socket-patch-cli/src/hosted_memory/ledger.rs index 7cd405f97..642171da8 100644 --- a/crates/socket-patch-cli/src/hosted_memory/ledger.rs +++ b/crates/socket-patch-cli/src/hosted_memory/ledger.rs @@ -12,7 +12,7 @@ use socket_patch_core::patch::redirect::{ }; use socket_patch_core::vendor::lock_inventory::{MemoryEntry, MemoryProject}; -use crate::commands::scan::hosted::{rebase_vlt_edits, REBASE_KINDS}; +use crate::commands::scan::hosted::{rebase_vlt_edits, rebased_pdm_original, REBASE_KINDS}; /// Load the project's ledger: `Ok(None)` when absent, `Err` (the disk /// message) when present but unreadable or malformed. @@ -92,7 +92,8 @@ pub(crate) fn merge( if let Some(&target) = siblings.get(nth) { if !rebased.contains(&target) { if edit.kind == "redirect_pdm_lock_package" { - ledger.edits[target].original = edit.original.clone(); + ledger.edits[target].original = + rebased_pdm_original(&ledger.edits[target], edit); } ledger.edits[target].new = edit.new.clone(); ledger.edits[target].action = edit.action.clone(); diff --git a/docs/testing/pdm-compatibility.md b/docs/testing/pdm-compatibility.md index 78a5a8c9d..a70425af8 100644 --- a/docs/testing/pdm-compatibility.md +++ b/docs/testing/pdm-compatibility.md @@ -66,6 +66,14 @@ Measured details: (pristine → current, never an appended chain), so `rollback` still lands on the pristine lock afterwards. This matters most for CRLF locks, which PDM re-renders to a different byte layout on relock. +- **`pdm add` and `pdm lock --update-reuse` keep the patch but re-lay its + unit.** PDM 2.26+ keeps the Socket `url` and patched hash; 2.12–2.20 drop + the `url` and keep the patched hash (a registry unit pinned to the patched + wheel, which `pdm sync` refuses). The re-scan rewires the unit and keeps + the ledger's recorded pristine unit, in the relocked line endings, as the + rollback target. It adopts the re-laid unit only once that unit no longer + carries anything the redirect introduced. So `rollback` restores the + upstream unit inside the user's re-rendered lock. - **Hosted mode verifies the lock's file hash** on install for every supported release (tamper the hash and `pdm sync` fails closed). Vendored mode's protection is the committed wheel bytes, verified by the same hash. From 1afe13d6de578f4ba909e520e815441d58416d78 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 20:43:10 +0000 Subject: [PATCH 4/5] Match real PDM 2.29.2 re-render in the #331 test Real `pdm add` moves the Socket url after `version` and re-lays `files` one entry per line; the regression test now reproduces that exact shape. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/in_process_redirect_pdm.rs | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/crates/socket-patch-cli/tests/in_process_redirect_pdm.rs b/crates/socket-patch-cli/tests/in_process_redirect_pdm.rs index 4b01fc50c..fbf03b421 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect_pdm.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect_pdm.rs @@ -585,9 +585,9 @@ fn urllib3_unit(lock: &str) -> usize { /// What `pdm add six==1.16.0` leaves: a new content hash, a `six` unit ahead /// of `urllib3`, and the redirected `urllib3` unit re-laid by PDM — its -/// `files` back in PDM's one-entry-per-line layout, the Socket `url` kept -/// (2.26+) or dropped (2.12–2.20), and the patched sha256 kept either way. -/// PDM always writes LF. +/// `files` back in PDM's one-entry-per-line layout, the Socket `url` moved +/// after `version` (2.26+) or dropped (2.12–2.20), and the patched sha256 +/// kept either way. PDM always writes LF. (Measured with real PDM 2.29.2.) fn pdm_add_rerender(redirected: &str, keep_url: bool) -> String { const SIX: &str = "[[package]]\nname = \"six\"\nversion = \"1.16.0\"\nrequires_python = \">=2.7, !=3.0.*, !=3.1.*, !=3.2.*\"\nsummary = \"Python 2 and 3 compatibility utilities\"\ngroups = [\"default\"]\nfiles = [\n {file = \"six-1.16.0-py2.py3-none-any.whl\", hash = \"sha256:8abb2f1d86890a2dfb989f9a77cfcfd3e47c2a354b01111771326f8aa26e0254\"},\n {file = \"six-1.16.0.tar.gz\", hash = \"sha256:1e61c37477a1626458e36f7b1d82aa5c9b094fa4802892072e49de9c60c4c926\"},\n]\n\n"; const WHEEL: &str = "urllib3-1.26.18-py2.py3-none-any.whl"; @@ -606,11 +606,15 @@ fn pdm_add_rerender(redirected: &str, keep_url: bool) -> String { sha256() ); assert!(unit.contains(&socket_files), "{unit}"); - let mut unit = unit.replace(&socket_files, &pdm_files); - if !keep_url { - let url_line = format!("url = \"{HOSTED_URL}\"\n"); - assert!(unit.contains(&url_line), "{unit}"); - unit = unit.replace(&url_line, ""); + let url_line = format!("url = \"{HOSTED_URL}\"\n"); + assert!(unit.contains(&url_line), "{unit}"); + let mut unit = unit + .replace(&socket_files, &pdm_files) + .replace(&url_line, ""); + if keep_url { + // PDM's own key order puts the url right after the version. + let version = "version = \"1.26.18\"\n"; + unit = unit.replacen(version, &format!("{version}{url_line}"), 1); } format!("{head}{SIX}{unit}") } From 162a56a47a0888a01d9b24fa6738cdffa50c53a0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 21:11:30 +0000 Subject: [PATCH 5/5] Keep the relocked boundary on a kept PDM unit A PDM lock fragment carries the unit's boundary: the blank lines and the next top-level header, or EOF. When `pdm add` appends a unit after the patched one, the kept pristine fragment still ended at EOF, so rollback replaced the live fragment with it and deleted the next unit's [[package]] header. The kept pristine body now takes the fresh fragment's boundary. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/commands/scan/hosted.rs | 39 +++++++++++++-- .../src/hosted_memory/ledger.rs | 24 ++++++++++ .../tests/in_process_redirect_pdm.rs | 48 +++++++++++++++---- 3 files changed, 97 insertions(+), 14 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index 0aa649eca..165ce8c19 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -48,8 +48,11 @@ pub(crate) const REBASE_KINDS: &[&str] = &[ /// rollback "revert" patched → patched and report success (#331). While the /// fresh `original` still carries any string the recorded edit introduced /// (a quoted value in its `new` that its `original` lacks: the url, the -/// patched hash), keep the recorded pristine `original`, re-laid in the -/// fresh fragment's line endings so it splices into the relocked text. +/// patched hash), keep the recorded pristine unit, re-laid in the fresh +/// fragment's line endings and ending in the fresh fragment's BOUNDARY (a +/// unit fragment carries its blank lines plus the next top-level header, or +/// EOF, and the relock may have added a unit after this one), so it splices +/// into the relocked text without eating the successor's header. pub(crate) fn rebased_pdm_original( recorded: &socket_patch_core::patch::redirect::FileEdit, fresh: &socket_patch_core::patch::redirect::FileEdit, @@ -77,13 +80,39 @@ pub(crate) fn rebased_pdm_original( if !still_patched { return fresh.original.clone(); } - let lf = pristine.replace("\r\n", "\n"); - let relaid = if current.contains("\r\n") { + let lf = split_pdm_boundary(&pristine).0.replace("\r\n", "\n"); + let body = if current.contains("\r\n") { lf.replace('\n', "\r\n") } else { lf }; - Some(serde_json::Value::String(relaid)) + let boundary = split_pdm_boundary(¤t).1; + Some(serde_json::Value::String(format!("{body}{boundary}"))) +} + +/// Split a PDM lock fragment into its body and its trailing boundary: the +/// line break ending the body's last line, then any blank or comment lines +/// and the next top-level header (`utils::pdm_lock`'s `next_header_end`), +/// or trailing blank lines up to EOF. A fragment with no boundary (a legacy +/// `[metadata.files]` entry) returns an empty one. +fn split_pdm_boundary(fragment: &str) -> (&str, &str) { + let lines: Vec<&str> = fragment.split_inclusive('\n').collect(); + let content = |line: &str| line.trim_end_matches(['\r', '\n']).trim().to_owned(); + let mut keep = lines.len(); + if keep > 1 && lines[keep - 1].starts_with('[') { + keep -= 1; + } + while keep > 1 && { + let line = content(lines[keep - 1]); + line.is_empty() || line.starts_with('#') + } { + keep -= 1; + } + let body: usize = lines[..keep].iter().map(|line| line.len()).sum(); + let body = fragment[..body] + .trim_end_matches('\n') + .trim_end_matches('\r'); + fragment.split_at(body.len()) } pub(crate) const REDIRECT_CANDIDATE_FILES: &[&str] = &[ diff --git a/crates/socket-patch-cli/src/hosted_memory/ledger.rs b/crates/socket-patch-cli/src/hosted_memory/ledger.rs index 642171da8..ea04218f6 100644 --- a/crates/socket-patch-cli/src/hosted_memory/ledger.rs +++ b/crates/socket-patch-cli/src/hosted_memory/ledger.rs @@ -249,4 +249,28 @@ mod tests { ); } } + + /// A relock that appends a unit after this one moves the fragment's + /// boundary from EOF to the successor's header: the kept pristine body + /// takes the fresh boundary, so rollback cannot eat that header. + #[test] + fn pdm_rebase_keeps_the_fresh_fragment_boundary() { + let pristine = "[[package]]\nfiles = [\"sha256:1111\"]\n"; + let wired = "[[package]]\nfiles = [\"sha256:cccc\"]\nurl = \"https://patch.test/u.whl\"\n"; + let current = "[[package]]\nfiles = [ \"sha256:cccc\" ]\n# note\n\n[[package]]"; + let fresh_new = "[[package]]\nfiles = [\"sha256:cccc\"]\nurl = \"https://patch.test/u.whl\"\n# note\n\n[[package]]"; + let mut ledger = RedirectState::new(); + ledger.edits.push(pdm(pristine, wired)); + let files = BTreeMap::from([("pdm.lock".to_string(), current.to_string())]); + merge( + &mut ledger, + &[pdm(current, fresh_new)], + BTreeMap::new(), + &files, + ); + assert_eq!( + ledger.edits[0].original.as_ref().unwrap().as_str().unwrap(), + "[[package]]\nfiles = [\"sha256:1111\"]\n# note\n\n[[package]]" + ); + } } diff --git a/crates/socket-patch-cli/tests/in_process_redirect_pdm.rs b/crates/socket-patch-cli/tests/in_process_redirect_pdm.rs index fbf03b421..999633706 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect_pdm.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect_pdm.rs @@ -571,12 +571,26 @@ async fn assert_relock_roundtrip(lock: &str, relocked: &str) { async fn rerender_keeping_the_patch_then_rescan_rolls_back_to_the_registry() { for start in ["\r\n", "\n"] { for keep_url in [true, false] { - let lock = LOCK.replace("\r\n", "\n").replace('\n', start); - assert_rerender_roundtrip(&lock, keep_url).await; + for later in [false, true] { + let lock = LOCK.replace("\r\n", "\n").replace('\n', start); + assert_rerender_roundtrip(&lock, keep_url, later).await; + } } } } +/// A `pdm add`ed package that sorts AFTER `urllib3`: PDM appends its unit, +/// which moves the `urllib3` fragment's boundary from EOF to a header. +const ZIPP: &str = "\n[[package]]\nname = \"zipp\"\nversion = \"3.20.2\"\nrequires_python = \">=3.8\"\nsummary = \"Backport of pathlib-compatible object wrapper for zip files\"\ngroups = [\"default\"]\nfiles = [\n {file = \"zipp-3.20.2-py3-none-any.whl\", hash = \"sha256:a817ac80d6cf4b23bf7f2828b7cabf326f15a001bea8b1f9b49631780ba28350\"},\n]\n"; + +/// The end of the `urllib3` unit starting at `at`: the next unit's header +/// (keeping the blank line before it with the successor) or EOF. +fn unit_end(lock: &str, at: usize) -> usize { + lock[at + 1..] + .find("\n\n[[package]]") + .map_or(lock.len(), |i| at + 1 + i + 1) +} + /// Where the `urllib3` unit starts in `lock` (it is the last unit). fn urllib3_unit(lock: &str) -> usize { lock.find("[[package]]\nname = \"urllib3\"") @@ -619,14 +633,27 @@ fn pdm_add_rerender(redirected: &str, keep_url: bool) -> String { format!("{head}{SIX}{unit}") } -async fn assert_rerender_roundtrip(lock: &str, keep_url: bool) { +/// [`pdm_add_rerender`], optionally also adding [`ZIPP`] after `urllib3`. +fn pdm_add_rerender_with(redirected: &str, keep_url: bool, later: bool) -> String { + let rerendered = pdm_add_rerender(redirected, keep_url); + if later { + format!("{rerendered}{ZIPP}") + } else { + rerendered + } +} + +async fn assert_rerender_roundtrip(lock: &str, keep_url: bool, later: bool) { let server = MockServer::start().await; mock_api(&server).await; let tmp = tempfile::tempdir().unwrap(); write_project(tmp.path(), lock); let lock_path = tmp.path().join("pdm.lock"); let ledger_path = tmp.path().join(".socket/vendor/redirect-state.json"); - let case = format!("keep_url={keep_url} crlf={}", lock.contains('\r')); + let case = format!( + "keep_url={keep_url} later={later} crlf={}", + lock.contains('\r') + ); assert_eq!( run(hosted_args(tmp.path(), server.uri(), None)).await, @@ -636,7 +663,7 @@ async fn assert_rerender_roundtrip(lock: &str, keep_url: bool) { let redirected = read(&lock_path); assert!(redirected.contains(HOSTED_URL), "{case}"); - let rerendered = pdm_add_rerender(&redirected, keep_url); + let rerendered = pdm_add_rerender_with(&redirected, keep_url, later); std::fs::write(&lock_path, &rerendered).unwrap(); // The re-scan converges the re-laid unit back onto the Socket wiring. @@ -672,12 +699,15 @@ async fn assert_rerender_roundtrip(lock: &str, keep_url: bool) { !rolled_back.contains(HOSTED_URL) && !rolled_back.contains(&sha256()), "{case}: rollback must remove the Socket url and patched hash: {rolled_back}" ); - // Exactly the user's re-rendered lock with the pristine (LF) unit back. + // Exactly the user's re-rendered lock with the pristine (LF) unit back, + // every other unit (a later one's header included) untouched. let pristine = LOCK.replace("\r\n", "\n"); + let at = urllib3_unit(&rerendered); let expected = format!( - "{}{}", - &rerendered[..urllib3_unit(&rerendered)], - &pristine[urllib3_unit(&pristine)..] + "{}{}{}", + &rerendered[..at], + &pristine[urllib3_unit(&pristine)..], + &rerendered[unit_end(&rerendered, at)..] ); assert_eq!(rolled_back, expected, "{case}"); }