Skip to content

Commit 79c2597

Browse files
Fix per-patch Poetry/PDM lock re-parse (#760, #762) (#877)
* Start fix for #760, #762 Assisted-by: Claude Code:claude-opus-5-5 * Test that a Poetry/PDM lock renders once per scan Hosted scans rewrite poetry.lock and pdm.lock once per patched package, rendering and re-parsing the whole lock each time. Count the engine's whole-lock renders and require one per lock for a dozen patched packages. Both tests fail today with 12 renders. Refs #760, #762 Assisted-by: Claude Code:claude-opus-5-5 * Rewrite each Poetry/PDM lock in one pass A hosted scan rewrote poetry.lock and pdm.lock once per patched package, and every rewrite rendered and re-parsed the whole lock. A project with a dozen patches paid for a dozen full parses, which made Poetry and PDM scans 3.5-4.5x slower per package than other managers. The shared lock-splice engine now plans every package against one parsed lock, applies all the changes, renders and re-parses once, and splices each package's changed fragments into the original text. The result is checked against the rendering byte for byte. When a lock mixes line endings, a package is rewritten twice, or any check fails, the rewrite falls back to the old package-by-package path, so output and recorded edits never change. Differential tests run both paths over every Poetry and PDM lock generation, LF, CRLF and mixed, with refusals, missing packages and re-runs mixed in, and require identical text and per-package results. Fixes #760, #762 Assisted-by: Claude Code:claude-opus-5-5 * Read PDM lock_version from the parsed original The rewrite never changes [metadata] lock_version, so read it from the parse the presence probe already took instead of parsing the output. Assisted-by: Claude Code:claude-opus-5-5 * Route Gradle digests through utils::digest main has failed socket-patch-core's lib tests since Gradle support (#646) and the digest helpers (#865) both landed. The guard test production_digests_go_through_the_helpers flags three files #646 added that still hash inline: crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs. That breaks test, test-release and coverage on every open PR. Each inline sha1/sha256 call now goes through sha1_hex_of or sha256_hex_of, which compute the same lowercase hex. Behaviour is unchanged. Assisted-by: Claude Code:claude-opus-5-5 * Drop the unrelated formatting sweep Running cargo fmt over the whole workspace reformatted 117 files this PR does not otherwise touch, because main is not rustfmt-clean and CI does not check formatting. Restore those files to main and keep the diff to the Poetry/PDM rewrite and the ported digest fix. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d201fce commit 79c2597

5 files changed

Lines changed: 962 additions & 140 deletions

File tree

‎crates/socket-patch-core/src/patch/redirect/pdm.rs‎

Lines changed: 140 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use serde_json::json;
44

55
use super::{DepOverride, FileEdit, RewriteResult, RewriteWarning};
66
use crate::crawlers::python_crawler::canonicalize_pypi_name;
7-
use crate::utils::pdm_lock::{rewrite_pdm_lock_in, PdmLockParse};
7+
use crate::utils::pdm_lock::{rewrite_pdm_lock_all, LockBatch, LockStep, PdmLockDep, PdmLockParse};
88

99
pub(super) fn rewrite(
1010
files: &BTreeMap<String, String>,
@@ -14,28 +14,44 @@ pub(super) fn rewrite(
1414
let Some(original) = files.get("pdm.lock") else {
1515
return;
1616
};
17-
let mut text = original.clone();
18-
let mut stale_warned = false;
19-
// Each lock state is parsed once: the presence probe, the rewrite, the
20-
// format probe and the next dep all share it.
17+
// Each dep's intake, in dep order: skipped (the lock lacks it), refused
18+
// before the lock is read, or one dep of the batch rewrite. A package
19+
// `pdm.lock` simply does not contain is not installed by pdm — a sibling
20+
// `requirements.txt`/pylock may legitimately carry it — so we neither
21+
// redirect it here nor veto the other pypi rewriters. Only a package the
22+
// lock DOES contain but the plan refuses (source conflict, unsupported
23+
// format, forked variants, bad hashes) withholds siblings. No rewrite
24+
// adds or drops a package, so the probe reads the original lock.
2125
let mut parse = PdmLockParse::default();
22-
for dep in overrides.iter().filter(|dep| dep.ecosystem == "pypi") {
23-
// A package `pdm.lock` simply does not contain is not installed by pdm —
24-
// a sibling `requirements.txt`/pylock may legitimately carry it — so we
25-
// neither redirect it here nor veto the other pypi rewriters. Only a
26-
// package the lock DOES contain but the plan refuses (source conflict,
27-
// unsupported format, forked variants, bad hashes) withholds siblings.
28-
if !lock_contains(&mut parse, &text, &dep.name) {
29-
continue;
30-
}
31-
match plan_in(&mut parse, &text, dep) {
32-
Ok((rewritten, edits)) => {
26+
let intake: Vec<(&DepOverride, Result<Artifact, String>)> = overrides
27+
.iter()
28+
.filter(|dep| dep.ecosystem == "pypi")
29+
.filter(|dep| lock_contains(&mut parse, original, &dep.name))
30+
.map(|dep| (dep, artifact_of(dep)))
31+
.collect();
32+
let lock_deps: Vec<PdmLockDep> = intake
33+
.iter()
34+
.filter_map(|(dep, artifact)| Some(lock_dep(dep, artifact.as_ref().ok()?)))
35+
.collect();
36+
// Every dep is rewritten over one parse and one render of the lock.
37+
let LockBatch { text, steps } = rewrite_pdm_lock_all(original, &lock_deps);
38+
let mut steps = steps.into_iter();
39+
// No rewrite touches `[metadata] lock_version`: read once, from the
40+
// parse the presence probe already took.
41+
let lock_ver: Option<String> = parse.parsed(original).ok().and_then(|lock| {
42+
crate::utils::pdm_lock::lock_version(lock)
43+
.ok()
44+
.map(str::to_string)
45+
});
46+
let mut stale_warned = false;
47+
for (dep, intake) in intake {
48+
let step = match intake {
49+
Ok(_) => steps.next().expect("one step per batched dep"),
50+
Err(detail) => LockStep::Refused(detail),
51+
};
52+
match step {
53+
LockStep::Rewritten(_) | LockStep::Unchanged => {
3354
result.confirmed_pdm_uuids.insert(dep.patch_uuid.clone());
34-
let lock_ver: Option<String> = parse.parsed(&rewritten).ok().and_then(|lock| {
35-
crate::utils::pdm_lock::lock_version(lock)
36-
.ok()
37-
.map(str::to_string)
38-
});
3955
if lock_ver.as_deref() == Some("2") {
4056
result.warnings.push(RewriteWarning { code: "redirect_pdm_legacy_sync_required".into(), detail: "PDM 0.x may regenerate freshly generated locks during install; use `pdm sync` to preserve this patch, or upgrade PDM".into() });
4157
}
@@ -65,10 +81,14 @@ pub(super) fn rewrite(
6581
),
6682
});
6783
}
68-
text = rewritten;
69-
result.edits.extend(edits);
84+
if let LockStep::Rewritten(edits) = step {
85+
result
86+
.edits
87+
.extend(edits.into_iter().map(|(old, new)| file_edit(dep, old, new)));
88+
}
7089
}
71-
Err(detail) => {
90+
LockStep::NotFound => unreachable!("PDM refuses a package its lock lacks"),
91+
LockStep::Refused(detail) => {
7292
result.refused_pdm_uuids.insert(dep.patch_uuid.clone());
7393
result.warnings.push(RewriteWarning {
7494
code: "redirect_pdm_refused".into(),
@@ -106,17 +126,11 @@ fn lock_contains(parse: &mut PdmLockParse, text: &str, name: &str) -> bool {
106126
}
107127
}
108128

109-
#[cfg(test)]
110-
fn plan(text: &str, dep: &DepOverride) -> Result<(String, Vec<FileEdit>), String> {
111-
plan_in(&mut PdmLockParse::default(), text, dep)
112-
}
129+
/// A dep's wheel filename and SHA-256.
130+
type Artifact<'a> = (String, &'a str);
113131

114-
/// Plan `dep`'s rewrite of `text`, reusing (and refreshing) `parse`.
115-
fn plan_in(
116-
parse: &mut PdmLockParse,
117-
text: &str,
118-
dep: &DepOverride,
119-
) -> Result<(String, Vec<FileEdit>), String> {
132+
/// `dep`'s [`Artifact`], or its refusal before the lock is read.
133+
fn artifact_of(dep: &DepOverride) -> Result<Artifact<'_>, String> {
120134
let sha256 = dep
121135
.integrity
122136
.sha256
@@ -133,28 +147,48 @@ fn plan_in(
133147
.path_segments()
134148
.and_then(|mut segments| segments.next_back())
135149
.ok_or("missing PDM wheel filename")?;
136-
let rewrite = rewrite_pdm_lock_in(
137-
parse,
138-
text,
139-
&dep.name,
140-
&dep.version,
141-
("url", &dep.artifact_url),
150+
Ok((filename.to_string(), sha256))
151+
}
152+
153+
/// `dep`'s arguments to the lock rewrite, given its [`artifact_of`].
154+
fn lock_dep<'a>(dep: &'a DepOverride, (filename, sha256): &'a Artifact<'a>) -> PdmLockDep<'a> {
155+
PdmLockDep {
156+
name: &dep.name,
157+
version: &dep.version,
158+
source: ("url", &dep.artifact_url),
142159
filename,
143160
sha256,
144-
)?;
145-
let edits = rewrite
146-
.edits()?
147-
.into_iter()
148-
.map(|(old, new)| FileEdit {
149-
path: "pdm.lock".into(),
150-
kind: "redirect_pdm_lock_package".into(),
151-
action: "rewritten".into(),
152-
key: Some(dep.name.clone()),
153-
original: Some(json!(old)),
154-
new: Some(json!(new)),
155-
})
156-
.collect();
157-
Ok((rewrite.text, edits))
161+
}
162+
}
163+
164+
fn file_edit(dep: &DepOverride, old: String, new: String) -> FileEdit {
165+
FileEdit {
166+
path: "pdm.lock".into(),
167+
kind: "redirect_pdm_lock_package".into(),
168+
action: "rewritten".into(),
169+
key: Some(dep.name.clone()),
170+
original: Some(json!(old)),
171+
new: Some(json!(new)),
172+
}
173+
}
174+
175+
/// `dep`'s rewrite of `text` alone: the rewritten text and its edits.
176+
#[cfg(test)]
177+
fn plan(text: &str, dep: &DepOverride) -> Result<(String, Vec<FileEdit>), String> {
178+
let artifact = artifact_of(dep)?;
179+
let LockBatch { text, mut steps } = rewrite_pdm_lock_all(text, &[lock_dep(dep, &artifact)]);
180+
match steps.remove(0) {
181+
LockStep::Rewritten(edits) => Ok((
182+
text,
183+
edits
184+
.into_iter()
185+
.map(|(old, new)| file_edit(dep, old, new))
186+
.collect(),
187+
)),
188+
LockStep::Unchanged => Ok((text, Vec::new())),
189+
LockStep::NotFound => unreachable!("PDM refuses a package its lock lacks"),
190+
LockStep::Refused(detail) => Err(detail),
191+
}
158192
}
159193

160194
#[cfg(test)]
@@ -428,4 +462,56 @@ mod parse_reuse_equivalence_tests {
428462
assert!(confirmed > 100, "only {confirmed} confirmed");
429463
g.finish();
430464
}
465+
466+
/// A dozen patched packages in one lock cost one whole-lock render and
467+
/// re-parse, not one per package (#762).
468+
#[test]
469+
fn many_patches_render_the_lock_once() {
470+
use crate::utils::lock_fragments::RENDERS;
471+
let url = |name: &str| {
472+
format!("https://patch.socket.dev/patch/pypi/{name}/a/{name}-1.26.18-py3-none-any.whl")
473+
};
474+
let mut checked = 0;
475+
for (fixture, lock) in fixtures() {
476+
for crlf in [false, true] {
477+
let mut lock = grown(&lock.replace("\r\n", "\n"), 11);
478+
if crlf {
479+
lock = lock.replace('\n', "\r\n");
480+
}
481+
let names = std::iter::once("urllib3".to_string())
482+
.chain((0..11).map(|n| format!("pkg{n}")));
483+
let deps: Vec<DepOverride> = names
484+
.enumerate()
485+
.map(|(n, name)| DepOverride {
486+
ecosystem: "pypi".into(),
487+
artifact_url: url(&name),
488+
name,
489+
namespace: None,
490+
version: "1.26.18".into(),
491+
token: String::new(),
492+
patch_uuid: format!("00000000-0000-4000-8000-{n:012}"),
493+
registry_override: None,
494+
integrity: Integrity {
495+
sha256: Some("a".repeat(64)),
496+
..Default::default()
497+
},
498+
})
499+
.collect();
500+
let files = BTreeMap::from([("pdm.lock".to_string(), lock)]);
501+
RENDERS.with(|renders| renders.set(0));
502+
let mut got = RewriteResult::default();
503+
rewrite(&files, &deps, &mut got);
504+
if got.confirmed_pdm_uuids.len() != 12 {
505+
continue; // a generation this dep shape doesn't land on
506+
}
507+
checked += 1;
508+
assert_eq!(
509+
RENDERS.with(|renders| renders.get()),
510+
1,
511+
"{fixture} crlf={crlf}"
512+
);
513+
}
514+
}
515+
assert!(checked >= 16, "only {checked} locks landed every dep");
516+
}
431517
}

‎crates/socket-patch-core/src/patch/redirect/poetry.rs‎

Lines changed: 69 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use toml_edit::DocumentMut;
99

1010
use super::{DepOverride, FileEdit, RewriteResult, RewriteWarning};
1111
use crate::utils::poetry_lock::{
12-
generated_by_version, lock_version, rewrite_poetry_lock_in, PoetryLockParse,
12+
generated_by_version, lock_version, rewrite_poetry_lock_all, LockBatch, LockStep, PoetryLockDep,
1313
};
1414

1515
/// Whether the lock was written by a Poetry release older than 1.4. Those
@@ -56,52 +56,40 @@ pub(super) fn rewrite_poetry(
5656
}
5757
}
5858
for (path, original) in locks {
59-
let mut content = original.clone();
59+
let deps: Vec<PoetryLockDep> = usable
60+
.iter()
61+
.map(|&(dep, sha256)| PoetryLockDep {
62+
name: &dep.name,
63+
version: &dep.version,
64+
source_type: "url",
65+
source_url: &dep.artifact_url,
66+
filename: dep.artifact_url.rsplit('/').next().unwrap_or(""),
67+
sha256,
68+
})
69+
.collect();
70+
// Every dep is rewritten over one parse and one render of the lock.
71+
let LockBatch {
72+
text: content,
73+
steps,
74+
} = rewrite_poetry_lock_all(original, &deps);
6075
let mut stale_warned = false;
6176
// `pre_1_4_writer` reads only the first line and `[metadata]
62-
// lock-version`, which no package rewrite touches: judged once, on the
63-
// lock as of its first rewrite (where it was always first judged).
77+
// lock-version`, which no package rewrite touches: judged once.
6478
let mut writer_format: Option<Option<&'static str>> = None;
65-
// Each lock state is parsed once: a rewrite hands its parsed output
66-
// to the next dep.
67-
let mut parse = PoetryLockParse::default();
68-
for &(dep, sha256) in &usable {
69-
let filename = dep.artifact_url.rsplit('/').next().unwrap_or("");
70-
match rewrite_poetry_lock_in(
71-
&mut parse,
72-
&content,
73-
&dep.name,
74-
&dep.version,
75-
"url",
76-
&dep.artifact_url,
77-
filename,
78-
sha256,
79-
) {
80-
Ok(Some(rewrite)) if rewrite.text != content => {
81-
match rewrite.edits() {
82-
Ok(edits) => {
83-
for (original, new) in edits {
84-
result.edits.push(FileEdit {
85-
path: path.clone(),
86-
kind: "redirect_poetry_lock_package".into(),
87-
action: "rewritten".into(),
88-
key: Some(format!("{}@{}", dep.name, dep.version)),
89-
original: Some(Value::String(original)),
90-
new: Some(Value::String(new)),
91-
});
92-
}
93-
}
94-
Err(detail) => {
95-
result.refused_python_lock_uuids.insert(dep.patch_uuid.clone());
96-
result.warnings.push(RewriteWarning {
97-
code: "redirect_poetry_lock_unsupported".into(),
98-
detail: format!("{path}: {detail}"),
99-
});
100-
continue;
101-
}
79+
for (&(dep, _), step) in usable.iter().zip(steps) {
80+
match step {
81+
LockStep::Rewritten(edits) => {
82+
for (original, new) in edits {
83+
result.edits.push(FileEdit {
84+
path: path.clone(),
85+
kind: "redirect_poetry_lock_package".into(),
86+
action: "rewritten".into(),
87+
key: Some(format!("{}@{}", dep.name, dep.version)),
88+
original: Some(Value::String(original)),
89+
new: Some(Value::String(new)),
90+
});
10291
}
10392
result.confirmed_python_lock_uuids.insert(dep.patch_uuid.clone());
104-
content = rewrite.text;
10593
if !stale_warned {
10694
if let Some(format) =
10795
*writer_format.get_or_insert_with(|| pre_1_4_writer(&content))
@@ -135,14 +123,14 @@ pub(super) fn rewrite_poetry(
135123
}
136124
}
137125
// Already redirected to this artifact (idempotent re-scan).
138-
Ok(Some(_)) => {
126+
LockStep::Unchanged => {
139127
result.confirmed_python_lock_uuids.insert(dep.patch_uuid.clone());
140128
}
141-
Ok(None) => result.warnings.push(RewriteWarning {
129+
LockStep::NotFound => result.warnings.push(RewriteWarning {
142130
code: "redirect_poetry_entry_not_found".into(),
143131
detail: format!("no {path} entry for {}@{}", dep.name, dep.version),
144132
}),
145-
Err(detail) => {
133+
LockStep::Refused(detail) => {
146134
result.refused_python_lock_uuids.insert(dep.patch_uuid.clone());
147135
result.warnings.push(RewriteWarning {
148136
code: "redirect_poetry_lock_unsupported".into(),
@@ -274,4 +262,40 @@ mod equivalence_tests {
274262
}
275263
g.finish();
276264
}
265+
266+
/// A dozen patched packages in one lock cost one whole-lock render and
267+
/// re-parse, not one per package (#760).
268+
#[test]
269+
fn many_patches_render_the_lock_once() {
270+
use crate::utils::lock_fragments::RENDERS;
271+
for version in VERSIONS.iter().filter(|version| !version.starts_with("0.")) {
272+
for crlf in [false, true] {
273+
let mut lock = grown(version, 11);
274+
// Give every clone its own populated legacy integrity entry,
275+
// as Poetry writes them.
276+
if let Some(start) = lock.find("\nurllib3 = [\n") {
277+
let end = start + lock[start..].find("\n]").unwrap() + 2;
278+
let entry = lock[start..end].to_string();
279+
let clones: String = (0..11)
280+
.map(|i| entry.replacen("urllib3 =", &format!("pkg{i} ="), 1))
281+
.collect();
282+
lock.insert_str(end, &clones);
283+
}
284+
if crlf {
285+
lock = lock.replace('\n', "\r\n");
286+
}
287+
let files = BTreeMap::from([("poetry.lock".to_string(), lock)]);
288+
let mut deps = vec![dep("urllib3", "1.26.18", Some(SHA), 0)];
289+
deps.extend((0..11).map(|i| dep(&format!("pkg{i}"), "1.26.18", Some(SHA), i + 1)));
290+
RENDERS.with(|renders| renders.set(0));
291+
let got = run(rewrite_poetry, &files, &deps);
292+
assert_eq!(got.confirmed_python_lock_uuids.len(), 12, "{version}");
293+
assert_eq!(
294+
RENDERS.with(|renders| renders.get()),
295+
1,
296+
"{version} crlf={crlf}"
297+
);
298+
}
299+
}
300+
}
277301
}

0 commit comments

Comments
 (0)