From 61af95492831053254174106c2f535194d7cacd5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 03:59:45 +0000 Subject: [PATCH 1/3] Start refactor for #780 Assisted-by: Claude Code:claude-opus-5-5 From ed1db60781eb6651ec108862d534ebee68846caa Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 04:20:16 +0000 Subject: [PATCH 2/3] Locate hosted gem lock edits via formats::gem The hosted planner's Bundler-lock leg (converge_gem_lock_source) walked Gemfile.lock with its own section model (GemLockSection) and its own DEPENDENCIES name rule (gem_lock_dependency_name), beside the shared formats::gem reader that the inventory, VEX and upstream restore use. Two models of one format drift: a fix to how sections or DEPENDENCIES entries are read had to land twice. The shared reader now records each source section's line span and remote line numbers, and the DEPENDENCIES section's span and entries (name and source pin under one rule). The hosted leg locates the spec, its section, the remote and the DEPENDENCIES entry from that model and keeps only the line splice. GemLockSection, its header walk and gem_lock_dependency_name are deleted. Output bytes don't change. The new hosted tests (second GEM section, CRLF, each DEPENDENCIES spelling, a refreshed and re-sorted owned section, the refused shapes, an unterminated last line) pass against both the old and the new implementation. The only inputs read differently are malformed ones: a whitespace-only line is now a blank separator rather than a header, and a header or remote with trailing whitespace is trimmed, as the shared reader always did. Refs #780 (hosted slice; the vendored slice remains). Assisted-by: Claude Code:claude-opus-5-5 --- .../src/formats/gem/hosted.rs | 406 +++++++++++++----- .../socket-patch-core/src/formats/gem/mod.rs | 166 ++++++- 2 files changed, 468 insertions(+), 104 deletions(-) diff --git a/crates/socket-patch-core/src/formats/gem/hosted.rs b/crates/socket-patch-core/src/formats/gem/hosted.rs index db978a79a..f9f4b1cfc 100644 --- a/crates/socket-patch-core/src/formats/gem/hosted.rs +++ b/crates/socket-patch-core/src/formats/gem/hosted.rs @@ -1,12 +1,15 @@ //! The hosted planner's Bundler-lock leg: converge a redirected gem's source //! attribution (its spec block moved into a patch-registry `GEM` section, //! DEPENDENCIES source-pinned) as a line splice that keeps every line's own -//! `\r\n` / `\n` ending. The read model in the parent keeps no spans or -//! endings, which is why this walker is separate from it. +//! `\r\n` / `\n` ending. Sections, remotes and DEPENDENCIES entries are +//! located with the parent's read model ([`super::parse`]); the splice +//! itself works on the lock's `split_inclusive('\n')` lines, which the +//! model's line numbers index. use regex::Regex; use serde_json::Value; +use super::Section; use crate::patch::redirect::{gem_index_url_pattern, DepOverride, FileEdit, RewriteResult}; /// A lock line without its `\r?\n` ending (never more than one of each). @@ -15,35 +18,6 @@ fn gem_lock_line_content(line: &str) -> &str { line.strip_suffix('\r').unwrap_or(line) } -/// The gem name of a 2-space DEPENDENCIES entry (` rails`, ` rails!`, -/// ` rails (= 7.0.0)!`) — the text before any constraint, sans source pin. -fn gem_lock_dependency_name(entry: &str) -> &str { - let entry = entry.trim_start(); - let entry = entry.split(" (").next().unwrap_or(entry); - entry.trim_end_matches('!') -} - -/// One parsed `GEM` section of a Gemfile.lock: its header line index, its -/// `remote:` lines (index + URL) and the exclusive end index — the start of -/// the next column-0 header (trailing blank separator included) or EOF. -struct GemLockSection { - start: usize, - remotes: Vec<(usize, String)>, - end: usize, -} - -impl GemLockSection { - /// Bundler's source identifier for the section (its remote URLs) — the - /// key `SourceList#lock_rubygems_sources` sorts `GEM` sections by. - fn identifier(&self) -> String { - self.remotes - .iter() - .map(|(_, url)| url.as_str()) - .collect::>() - .join(", ") - } -} - /// Move `GEM` section `sec_idx` (whose remote now reads `index_url`) to /// where bundler writes it: before the first other section whose /// identifier sorts after `index_url`, else after the last one. The section @@ -51,7 +25,7 @@ impl GemLockSection { /// touched) when it already sits there. fn place_gem_section_sorted( lines: &mut Vec, - sections: &[GemLockSection], + sections: &[&Section<'_>], sec_idx: usize, index_url: &str, eol: &str, @@ -61,7 +35,8 @@ fn place_gem_section_sorted( if before == others().find(|&k| k > sec_idx) { return false; } - let (start, end) = (sections[sec_idx].start, sections[sec_idx].end); + let range = sections[sec_idx].lines(); + let (start, end) = (range.start, range.end); let mut block: Vec = lines.drain(start..end).collect(); let n = block.len(); // The moved section needs its own blank separator (and a final newline @@ -76,7 +51,7 @@ fn place_gem_section_sorted( } let shifted = |at: usize| if at > start { at - n } else { at }; let at = match before { - Some(k) => shifted(sections[k].start), + Some(k) => shifted(sections[k].lines().start), None => { let last = others() .next_back() @@ -122,56 +97,26 @@ pub(crate) fn converge_gem_lock_source( ) -> bool { let eol = crate::utils::line_endings::terminator(lk); let mut lines: Vec = lk.split_inclusive('\n').map(str::to_string).collect(); - let is_header = |c: &str| !c.is_empty() && !c.starts_with(' '); - - // Parse: GEM sections, the dep's 4-space spec entry, DEPENDENCIES range. - let spec_content = format!(" {} ({})", dep.name, dep.version); - let mut sections: Vec = Vec::new(); - let mut spec_at: Vec<(usize, usize)> = Vec::new(); // (section idx, line idx) - let mut deps_range: Option<(usize, usize)> = None; // exclusive of header - let mut i = 0; - while i < lines.len() { - let c = gem_lock_line_content(&lines[i]); - if !is_header(c) { - i += 1; - continue; - } - let header_is_gem = c == "GEM"; - let start = i; - let mut remotes = Vec::new(); - let mut j = i + 1; - while j < lines.len() && !is_header(gem_lock_line_content(&lines[j])) { - let cj = gem_lock_line_content(&lines[j]); - if header_is_gem { - if let Some(url) = cj.strip_prefix(" remote: ") { - remotes.push((j, url.to_string())); - } - if cj == spec_content { - spec_at.push((sections.len(), j)); - } - } - j += 1; - } - if header_is_gem { - sections.push(GemLockSection { - start, - remotes, - end: j, - }); - } else if c == "DEPENDENCIES" { - deps_range = Some((start + 1, j)); - } - i = j; - } - - let spec_pos = if spec_at.len() == 1 { - Some(spec_at[0]) - } else { - None - }; - let (Some((sec_idx, spec_idx)), Some((deps_start, deps_end))) = (spec_pos, deps_range) else { + // Locate: GEM sections, the dep's 4-space spec entry, DEPENDENCIES. + let model = super::parse(lk); + let sections: Vec<&Section<'_>> = model.gem_sections().collect(); + let spec_raw = format!("{} ({})", dep.name, dep.version); + let spec_at: Vec<(usize, usize)> = sections // (section idx, line idx) + .iter() + .enumerate() + .flat_map(|(k, section)| { + section + .specs + .iter() + .filter(|spec| spec.raw == spec_raw) + .map(move |spec| (k, spec.line_no - 1)) + }) + .collect(); + let (&[(sec_idx, spec_idx)], Some(deps)) = (spec_at.as_slice(), &model.dependencies) else { return false; }; + // Exclusive of the header. + let (deps_start, deps_end) = (deps.line_no, deps.end); if sections[sec_idx].remotes.len() != 1 { return false; } @@ -184,7 +129,8 @@ pub(crate) fn converge_gem_lock_source( if deps_start < sections[sec_idx].end { return false; } - let (remote_idx, remote_url) = sections[sec_idx].remotes[0].clone(); + let remote_idx = sections[sec_idx].remote_line_nos[0] - 1; + let remote_url = sections[sec_idx].remotes[0].to_string(); let socket_remote_re = Regex::new(&format!("^{}$", gem_index_url_pattern(dep, index_url))) .expect("anchored index-url pattern from the escaped URL is valid"); let mut changed = false; @@ -192,11 +138,11 @@ pub(crate) fn converge_gem_lock_source( // DEPENDENCIES pin first — its lines sit AFTER the GEM sections, so the // spec move below never invalidates these indices (and vice versa would). let target = format!(" {} (= {})!", dep.name, dep.version); - let is_entry = |c: &str| c.starts_with(" ") && !c.starts_with(" "); - let entry_idx = (deps_start..deps_end).find(|&k| { - let ck = gem_lock_line_content(&lines[k]); - is_entry(ck) && gem_lock_dependency_name(ck) == dep.name - }); + let entry_idx = deps + .entries + .iter() + .find(|entry| entry.name == dep.name) + .map(|entry| entry.line_no - 1); match entry_idx { Some(k) if gem_lock_line_content(&lines[k]) == target => {} Some(k) => { @@ -214,17 +160,21 @@ pub(crate) fn converge_gem_lock_source( changed = true; } None => { - // Transitive dep: bundler keeps DEPENDENCIES sorted by name. - let mut at = deps_end; - for (k, line) in lines.iter().enumerate().take(deps_end).skip(deps_start) { - let ck = gem_lock_line_content(line); - if ck.is_empty() - || (is_entry(ck) && gem_lock_dependency_name(ck) > dep.name.as_str()) - { - at = k; - break; - } - } + // Transitive dep: bundler keeps DEPENDENCIES sorted by name, so + // it goes before the first entry that sorts after it, and never + // past the section's blank separator. + let next_entry = deps + .entries + .iter() + .find(|entry| entry.name > dep.name.as_str()) + .map(|entry| entry.line_no - 1); + let separator = + (deps_start..deps_end).find(|&k| gem_lock_line_content(&lines[k]).is_empty()); + let at = next_entry + .into_iter() + .chain(separator) + .min() + .unwrap_or(deps_end); lines.insert(at, format!("{target}{eol}")); result.edits.push(FileEdit { path: lock_name.into(), @@ -297,7 +247,7 @@ pub(crate) fn converge_gem_lock_source( // Section bounds after the drain (every drained line sat inside // section `sec_idx`, which keeps its start). let bounds = |k: usize| -> (usize, usize) { - let s = §ions[k]; + let s = sections[k].lines(); match k.cmp(&sec_idx) { std::cmp::Ordering::Less => (s.start, s.end), std::cmp::Ordering::Equal => (s.start, s.end - n), @@ -371,3 +321,259 @@ pub(crate) fn checksum_entry_span(lock: &str, name: &str, version: &str) -> Opti } None } + +#[cfg(test)] +mod tests { + use super::*; + use crate::patch::redirect::{Integrity, RegistryOverride, RegistryOverrideIdentifiers}; + + const INDEX_URL: &str = "https://patch.test/gem/tok/uuid/"; + + fn dep(name: &str, version: &str) -> DepOverride { + DepOverride { + ecosystem: "gem".into(), + name: name.into(), + namespace: None, + version: version.into(), + token: "tok".into(), + patch_uuid: "uuid".into(), + artifact_url: format!("https://patch.test/{name}-{version}.gem"), + registry_override: Some(RegistryOverride { + kind: "rubygems-compact-index".into(), + index_url: INDEX_URL.into(), + identifiers: RegistryOverrideIdentifiers { + name: name.into(), + version: version.into(), + ..Default::default() + }, + }), + integrity: Integrity::default(), + } + } + + /// `(converged, lock after, edit kinds/actions)` for one dep. + fn converge(lock: &str, dep: &DepOverride) -> (bool, String, Vec) { + let mut lk = lock.to_string(); + let mut changed = false; + let mut result = RewriteResult::default(); + let ok = converge_gem_lock_source( + &mut lk, + dep, + INDEX_URL, + "Gemfile.lock", + &mut changed, + &mut result, + ); + assert_eq!(changed, lk != lock, "lock_changed must report a change"); + let edits = result + .edits + .iter() + .map(|e| format!("{}:{}", e.kind, e.action)) + .collect(); + (ok, lk, edits) + } + + const TWO_SOURCES: &str = "GEM + remote: https://gems.mirror.example/ + specs: + nokogiri (1.16.0) + racc (~> 1.4) + +GEM + remote: https://rubygems.org/ + specs: + racc (1.7.3) + rails (7.0.0) + racc + +PLATFORMS + ruby + +DEPENDENCIES + nokogiri! + rails (~> 7.0) + +BUNDLED WITH + 2.6.2 +"; + + /// A transitive gem in the SECOND `GEM` section: located through the + /// shared reader's section spans, moved into a patch-registry section + /// sorted between the two remotes, and pinned in DEPENDENCIES before the + /// first entry that sorts after it. CRLF endings are kept line by line. + #[test] + fn transitive_gem_in_the_second_gem_section_converges() { + let expected = "GEM + remote: https://gems.mirror.example/ + specs: + nokogiri (1.16.0) + racc (~> 1.4) + +GEM + remote: https://patch.test/gem/tok/uuid/ + specs: + racc (1.7.3) + +GEM + remote: https://rubygems.org/ + specs: + rails (7.0.0) + racc + +PLATFORMS + ruby + +DEPENDENCIES + nokogiri! + racc (= 1.7.3)! + rails (~> 7.0) + +BUNDLED WITH + 2.6.2 +"; + for crlf in [false, true] { + let spell = |s: &str| { + if crlf { + s.replace('\n', "\r\n") + } else { + s.to_string() + } + }; + let (ok, out, edits) = converge(&spell(TWO_SOURCES), &dep("racc", "1.7.3")); + assert!(ok); + assert_eq!(out, spell(expected), "crlf={crlf}"); + assert_eq!( + edits, + [ + "redirect_gemfile_lock_dependency_pin:added", + "redirect_gemfile_lock_gem_source:rewritten" + ] + ); + // Converged is a fixed point. + let (ok, again, edits) = converge(&out, &dep("racc", "1.7.3")); + assert!(ok && edits.is_empty(), "{edits:?}"); + assert_eq!(again, out); + } + } + + /// A direct gem's DEPENDENCIES entry is found by the shared name rule + /// whatever its spelling and rewritten to the exact source pin; the spec + /// moves with its dependency sublines. + #[test] + fn direct_gem_entry_spellings_are_rewritten_to_the_source_pin() { + for entry in ["rails", "rails!", "rails (~> 7.0)", "rails (= 7.0.0)!"] { + let lock = TWO_SOURCES.replace(" rails (~> 7.0)\n", &format!(" {entry}\n")); + let (ok, out, edits) = converge(&lock, &dep("rails", "7.0.0")); + assert!(ok, "{entry}"); + assert!( + out.contains("DEPENDENCIES\n nokogiri!\n rails (= 7.0.0)!\n\n"), + "{entry}: {out}" + ); + assert!( + out.contains( + "GEM\n remote: https://patch.test/gem/tok/uuid/\n specs:\n rails (7.0.0)\n racc\n\nGEM\n remote: https://rubygems.org/\n specs:\n racc (1.7.3)\n\n" + ), + "{entry}: {out}" + ); + let pin_edits = edits + .iter() + .filter(|e| e.starts_with("redirect_gemfile_lock_dependency_pin")) + .count(); + assert_eq!( + pin_edits, + usize::from(entry != "rails (= 7.0.0)!"), + "{entry}" + ); + } + } + + /// An already-converged section under a superseding patch: the remote is + /// refreshed on its own line and the section re-placed where bundler + /// sorts it. + #[test] + fn owned_section_is_refreshed_and_re_sorted() { + let lock = "GEM + remote: https://gems.mirror.example/ + specs: + nokogiri (1.16.0) + +GEM + remote: https://rubygems.org/ + specs: + rails (7.0.0) + +GEM + remote: https://patch.test/gem/tok/old/ + specs: + racc (1.7.3) + +DEPENDENCIES + racc (= 1.7.3)! + rails +"; + let (ok, out, edits) = converge(lock, &dep("racc", "1.7.3")); + assert!(ok); + assert_eq!( + out, + "GEM + remote: https://gems.mirror.example/ + specs: + nokogiri (1.16.0) + +GEM + remote: https://patch.test/gem/tok/uuid/ + specs: + racc (1.7.3) + +GEM + remote: https://rubygems.org/ + specs: + rails (7.0.0) + +DEPENDENCIES + racc (= 1.7.3)! + rails +" + ); + assert_eq!( + edits, + [ + "redirect_gemfile_lock_source_url:rewritten", + "redirect_gemfile_lock_section_order:moved" + ] + ); + } + + /// DEPENDENCIES before the dep's `GEM` section, a duplicated spec, a + /// multi-remote section and a missing DEPENDENCIES section are refused + /// untouched. + #[test] + fn unsafe_shapes_are_refused_untouched() { + let locks = [ + "DEPENDENCIES\n racc\n\nGEM\n remote: https://rubygems.org/\n specs:\n racc (1.7.3)\n", + "GEM\n remote: https://a.example/\n specs:\n racc (1.7.3)\n\nGEM\n remote: https://b.example/\n specs:\n racc (1.7.3)\n\nDEPENDENCIES\n racc\n", + "GEM\n remote: https://a.example/\n remote: https://b.example/\n specs:\n racc (1.7.3)\n\nDEPENDENCIES\n racc\n", + "GEM\n remote: https://rubygems.org/\n specs:\n racc (1.7.3)\n", + "GEM\n remote: https://rubygems.org/\n specs:\n racc (1.7.4)\n\nDEPENDENCIES\n racc\n", + ]; + for lock in locks { + let (ok, out, edits) = converge(lock, &dep("racc", "1.7.3")); + assert!(!ok, "{lock}"); + assert_eq!(out, lock); + assert!(edits.is_empty()); + } + } + + /// A lock without a final newline: the unterminated DEPENDENCIES entry + /// is rewritten in place and stays unterminated. + #[test] + fn unterminated_last_dependency_is_rewritten_in_place() { + let lock = "GEM\n remote: https://rubygems.org/\n specs:\n racc (1.7.3)\n\nDEPENDENCIES\n racc"; + let (ok, out, _) = converge(lock, &dep("racc", "1.7.3")); + assert!(ok); + assert_eq!( + out, + "GEM\n remote: https://patch.test/gem/tok/uuid/\n specs:\n racc (1.7.3)\n\nGEM\n remote: https://rubygems.org/\n specs:\n\nDEPENDENCIES\n racc (= 1.7.3)!" + ); + } +} diff --git a/crates/socket-patch-core/src/formats/gem/mod.rs b/crates/socket-patch-core/src/formats/gem/mod.rs index 11c936529..d4a835422 100644 --- a/crates/socket-patch-core/src/formats/gem/mod.rs +++ b/crates/socket-patch-core/src/formats/gem/mod.rs @@ -90,12 +90,32 @@ pub(crate) struct SpecLine<'t> { pub(crate) struct Section<'t> { pub(crate) header: &'t str, pub(crate) line_no: usize, + /// The number of the section's last line: the line before the next + /// column-0 header, or the file's last line (blank separators belong to + /// the section they follow). As a 0-based index it is the exclusive end + /// of [`Self::lines`]. + pub(crate) end: usize, /// The section's `remote:` values, trimmed, as written. pub(crate) remotes: Vec<&'t str>, + /// The line number of each of [`Self::remotes`], in the same order. + pub(crate) remote_line_nos: Vec, pub(crate) specs: Vec>, } impl<'t> Section<'t> { + /// The section's lines as 0-based indices into the lock's + /// `split_inclusive('\n')` lines: its header through its last line. + pub(crate) fn lines(&self) -> std::ops::Range { + self.line_no - 1..self.end + } + + /// Bundler's source identifier for the section: its remotes joined by + /// `", "`, the key `SourceList#lock_rubygems_sources` sorts `GEM` + /// sections by. + pub(crate) fn identifier(&self) -> String { + self.remotes.join(", ") + } + /// The section's remotes as download bases: trailing `/` trimmed, empty /// ones dropped, in lock order (duplicates kept). pub(crate) fn remote_bases(&self) -> impl Iterator + '_ { @@ -106,9 +126,33 @@ impl<'t> Section<'t> { } } +/// One 2-space `DEPENDENCIES` entry (`rack`, `rack!`, `rack (~> 3.1)`, +/// `rack (= 3.2.6)!`). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct Dependency<'t> { + pub(crate) line_no: usize, + /// The gem name: the text before any space, `(` or `!`. + pub(crate) name: &'t str, + /// Whether bundler marks the entry source-pinned (a trailing `!`). + pub(crate) pinned: bool, +} + +/// The `DEPENDENCIES` section. +#[derive(Debug)] +pub(crate) struct Dependencies<'t> { + /// The header's line number. + pub(crate) line_no: usize, + /// The section's last line number, as [`Section::end`]. + pub(crate) end: usize, + /// Its entries, in lock order. + pub(crate) entries: Vec>, +} + #[derive(Debug)] pub(crate) struct GemfileLock<'t> { pub(crate) sections: Vec>, + /// The (last) `DEPENDENCIES` section; `None` when the lock has none. + pub(crate) dependencies: Option>, /// `None` when the lock has no `CHECKSUMS` section (bundler < 2.6). /// Keyed by `(name, parenthesized token)`; the value is the entry's /// lowercase sha256, or `None` for a bare / malformed / conflicting one. @@ -241,8 +285,21 @@ pub(crate) fn parse(text: &str) -> GemfileLock<'_> { let mut in_dependencies = false; let mut pinned: BTreeSet<&str> = BTreeSet::new(); let mut direct: BTreeSet<&str> = BTreeSet::new(); + let mut dependencies: Option> = None; let mut seen_header = false; let mut bundler_shaped = false; + // The section or DEPENDENCIES block a column-0 header closes. + let close = |sections: &mut Vec>, + dependencies: &mut Option>, + current: Option, + in_dependencies: bool, + end: usize| { + if let Some(i) = current { + sections[i].end = end; + } else if let Some(deps) = dependencies.as_mut().filter(|_| in_dependencies) { + deps.end = end; + } + }; for (idx, raw) in text.split('\n').enumerate() { let line_no = idx + 1; @@ -256,6 +313,13 @@ pub(crate) fn parse(text: &str) -> GemfileLock<'_> { problems.push(format!("line {line_no} is a merge-conflict marker")); } seen_header = true; + close( + &mut sections, + &mut dependencies, + current, + in_dependencies, + idx, + ); bundler_shaped |= BUNDLER_HEADERS.contains(&header); in_specs = false; in_checksums = header == "CHECKSUMS"; @@ -263,11 +327,20 @@ pub(crate) fn parse(text: &str) -> GemfileLock<'_> { if in_checksums && checksums.is_none() { checksums = Some(HashMap::new()); } + if in_dependencies { + dependencies = Some(Dependencies { + line_no, + end: line_no, + entries: Vec::new(), + }); + } current = if SOURCE_HEADERS.contains(&header) { sections.push(Section { header, line_no, + end: line_no, remotes: Vec::new(), + remote_line_nos: Vec::new(), specs: Vec::new(), }); Some(sections.len() - 1) @@ -289,6 +362,7 @@ pub(crate) fn parse(text: &str) -> GemfileLock<'_> { 2 => { if let Some(remote) = trimmed.strip_prefix("remote:") { sec.remotes.push(remote.trim()); + sec.remote_line_nos.push(line_no); } in_specs = trimmed == "specs:"; } @@ -302,13 +376,18 @@ pub(crate) fn parse(text: &str) -> GemfileLock<'_> { } else if in_dependencies && indent == 2 { let name = trimmed.split([' ', '(', '!']).next().unwrap_or_default(); if !name.is_empty() { + let entry = Dependency { + line_no, + name, + pinned: trimmed.ends_with('!'), + }; direct.insert(name); - } - if let Some(entry) = trimmed.strip_suffix('!') { - let name = entry.split([' ', '(']).next().unwrap_or_default(); - if !name.is_empty() { + if entry.pinned { pinned.insert(name); } + if let Some(deps) = dependencies.as_mut() { + deps.entries.push(entry); + } } } else if in_checksums && indent == 2 { if let (Some(map), Some((key, sha))) = (checksums.as_mut(), parse_checksum(trimmed)) { @@ -322,11 +401,20 @@ pub(crate) fn parse(text: &str) -> GemfileLock<'_> { } } } + let last_line = text.split_inclusive('\n').count(); + close( + &mut sections, + &mut dependencies, + current, + in_dependencies, + last_line, + ); if !bundler_shaped { problems.push("it has no Bundler lockfile sections".to_string()); } GemfileLock { sections, + dependencies, checksums, pinned, direct, @@ -427,6 +515,76 @@ fn parse_checksum(entry: &str) -> Option<((&str, &str), Option)> { mod tests { use super::*; + /// Section spans, remote line numbers and DEPENDENCIES entries: what the + /// hosted lock writer splices by. Blank (and whitespace-only) separator + /// lines belong to the section they follow; a CRLF lock gives the same + /// numbers as its LF spelling, and the last section runs to the last + /// line whether or not the file ends with a newline. + #[test] + fn section_spans_remote_lines_and_dependency_entries() { + let lf = "GEM\n remote: https://a.example/\n specs:\n rack (3.1.0)\n\nGEM\n remote: https://b.example/\n specs:\n rails (7.0.0)\n rack\n \n\nPATH\n remote: .\n specs:\n app (0.1.0)\n\nDEPENDENCIES\n app!\n rack\n rack (~> 3.1)\n rails (= 7.0.0)!\n nested (1.0)\n\nBUNDLED WITH\n 2.6.2\n\n"; + for text in [ + lf.to_string(), + lf.replace('\n', "\r\n"), + lf.trim_end().to_string(), + ] { + let lock = parse(&text); + let spans: Vec<_> = lock + .sections + .iter() + .map(|s| { + ( + s.header, + s.lines(), + s.remote_line_nos.clone(), + s.identifier(), + ) + }) + .collect(); + assert_eq!( + spans, + [ + ("GEM", 0..5, vec![2], "https://a.example/".to_string()), + ("GEM", 5..12, vec![7], "https://b.example/".to_string()), + ("PATH", 12..17, vec![14], ".".to_string()), + ] + ); + let deps = lock.dependencies.as_ref().expect("DEPENDENCIES"); + assert_eq!((deps.line_no, deps.end), (18, 24)); + let entries: Vec<_> = deps + .entries + .iter() + .map(|e| (e.line_no, e.name, e.pinned)) + .collect(); + assert_eq!( + entries, + [ + (19, "app", true), + (20, "rack", false), + (21, "rack", false), + (22, "rails", true) + ] + ); + assert_eq!( + lock.direct.iter().copied().collect::>(), + ["app", "rack", "rails"] + ); + assert_eq!( + lock.pinned.iter().copied().collect::>(), + ["app", "rails"] + ); + let lines = text.split_inclusive('\n').count(); + assert_eq!(lines, if text.ends_with('\n') { 27 } else { 26 }); + } + // The last section runs to the last line. + let lock = parse("GEM\n remote: https://a.example/\n specs:\n rack (3.1.0)\n\n"); + assert_eq!(lock.sections[0].lines(), 0..5); + assert!(lock.dependencies.is_none()); + let lock = parse("DEPENDENCIES\n rack"); + let deps = lock.dependencies.expect("DEPENDENCIES"); + assert_eq!((deps.line_no, deps.end, deps.entries.len()), (1, 2, 1)); + } + #[test] fn spec_and_checksum_grammar() { let sha = "deadbeef".repeat(8); From 2db3823795d7e29353c1f7b296f761aa5b1111b3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 05:27:57 +0000 Subject: [PATCH 3/3] Keep hosted gem convergence off the digest path The scan benchmark flagged bundler hosted and rescan as 14% slower after the hosted lock leg moved onto formats::gem::parse: it parses the lock once per converged gem, and every parse validated each CHECKSUMS digest into a map, about 80% of its cost on an 800-gem lock. The hosted leg never reads a digest. The CHECKSUMS entries are now recorded as lines and their digests read on the first checksum() call (owned keys, so the lock type stays covariant). The hosted splice also holds its lines as Cow and copies only the lines it edits, not every line of the lock per gem. Release timing of 20 already-converged gems on an 800-gem lock (the rescan pass): 13.44 ms on main, 13.77 ms on this branch, against about 26 ms before this commit. Output bytes are unchanged. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/formats/gem/hosted.rs | 41 ++++++------ .../socket-patch-core/src/formats/gem/mod.rs | 63 ++++++++++++++----- .../socket-patch-core/src/vex/discover/gem.rs | 2 +- 3 files changed, 70 insertions(+), 36 deletions(-) diff --git a/crates/socket-patch-core/src/formats/gem/hosted.rs b/crates/socket-patch-core/src/formats/gem/hosted.rs index f9f4b1cfc..6a12e2517 100644 --- a/crates/socket-patch-core/src/formats/gem/hosted.rs +++ b/crates/socket-patch-core/src/formats/gem/hosted.rs @@ -6,6 +6,8 @@ //! itself works on the lock's `split_inclusive('\n')` lines, which the //! model's line numbers index. +use std::borrow::Cow; + use regex::Regex; use serde_json::Value; @@ -23,12 +25,12 @@ fn gem_lock_line_content(line: &str) -> &str { /// identifier sorts after `index_url`, else after the last one. The section /// moves whole, its lines keeping their own endings. Returns false (nothing /// touched) when it already sits there. -fn place_gem_section_sorted( - lines: &mut Vec, +fn place_gem_section_sorted<'a>( + lines: &mut Vec>, sections: &[&Section<'_>], sec_idx: usize, index_url: &str, - eol: &str, + eol: &'a str, ) -> bool { let others = || (0..sections.len()).filter(|&k| k != sec_idx); let before = others().find(|&k| sections[k].identifier().as_str() > index_url); @@ -37,16 +39,16 @@ fn place_gem_section_sorted( } let range = sections[sec_idx].lines(); let (start, end) = (range.start, range.end); - let mut block: Vec = lines.drain(start..end).collect(); + let mut block: Vec> = lines.drain(start..end).collect(); let n = block.len(); // The moved section needs its own blank separator (and a final newline // if it was the file's last line). if let Some(last) = block.last_mut() { if !last.ends_with('\n') { - last.push_str(eol); + last.to_mut().push_str(eol); } if !gem_lock_line_content(last).is_empty() { - block.push(eol.to_string()); + block.push(Cow::Borrowed(eol)); } } let shifted = |at: usize| if at > start { at - n } else { at }; @@ -96,7 +98,8 @@ pub(crate) fn converge_gem_lock_source( result: &mut RewriteResult, ) -> bool { let eol = crate::utils::line_endings::terminator(lk); - let mut lines: Vec = lk.split_inclusive('\n').map(str::to_string).collect(); + // Borrowed until edited: most of a large lock's lines are only moved. + let mut lines: Vec> = lk.split_inclusive('\n').map(Cow::Borrowed).collect(); // Locate: GEM sections, the dep's 4-space spec entry, DEPENDENCIES. let model = super::parse(lk); let sections: Vec<&Section<'_>> = model.gem_sections().collect(); @@ -148,7 +151,7 @@ pub(crate) fn converge_gem_lock_source( Some(k) => { let old = gem_lock_line_content(&lines[k]).trim_start().to_string(); let ending = lines[k][gem_lock_line_content(&lines[k]).len()..].to_string(); - lines[k] = format!("{target}{ending}"); + lines[k] = Cow::Owned(format!("{target}{ending}")); result.edits.push(FileEdit { path: lock_name.into(), kind: "redirect_gemfile_lock_dependency_pin".into(), @@ -175,7 +178,7 @@ pub(crate) fn converge_gem_lock_source( .chain(separator) .min() .unwrap_or(deps_end); - lines.insert(at, format!("{target}{eol}")); + lines.insert(at, Cow::Owned(format!("{target}{eol}"))); result.edits.push(FileEdit { path: lock_name.into(), kind: "redirect_gemfile_lock_dependency_pin".into(), @@ -194,7 +197,7 @@ pub(crate) fn converge_gem_lock_source( if remote_url != index_url { let ending = lines[remote_idx][gem_lock_line_content(&lines[remote_idx]).len()..].to_string(); - lines[remote_idx] = format!(" remote: {index_url}{ending}"); + lines[remote_idx] = Cow::Owned(format!(" remote: {index_url}{ending}")); result.edits.push(FileEdit { path: lock_name.into(), kind: "redirect_gemfile_lock_source_url".into(), @@ -242,7 +245,7 @@ pub(crate) fn converge_gem_lock_source( { last += 1; } - let moved: Vec = lines.drain(spec_idx..=last).collect(); + let moved: Vec> = lines.drain(spec_idx..=last).collect(); let n = moved.len(); // Section bounds after the drain (every drained line sat inside // section `sec_idx`, which keeps its start). @@ -258,20 +261,20 @@ pub(crate) fn converge_gem_lock_source( .find(|&k| sections[k].identifier().as_str() > index_url) .map(|k| bounds(k).0) .unwrap_or_else(|| bounds(sections.len() - 1).1); - let mut block: Vec = Vec::with_capacity(moved.len() + 4); - block.push(format!("GEM{eol}")); - block.push(format!(" remote: {index_url}{eol}")); - block.push(format!(" specs:{eol}")); + let mut block: Vec> = Vec::with_capacity(moved.len() + 4); + block.push(Cow::Owned(format!("GEM{eol}"))); + block.push(Cow::Owned(format!(" remote: {index_url}{eol}"))); + block.push(Cow::Owned(format!(" specs:{eol}"))); for line in moved { // Moved lines keep their own bytes; only a final line that lacked // a newline (EOF) gains the file's ending. if line.ends_with('\n') { block.push(line); } else { - block.push(format!("{line}{eol}")); + block.push(Cow::Owned(format!("{line}{eol}"))); } } - block.push(eol.to_string()); + block.push(Cow::Borrowed(eol)); lines.splice(insert_at..insert_at, block); result.edits.push(FileEdit { path: lock_name.into(), @@ -285,7 +288,9 @@ pub(crate) fn converge_gem_lock_source( } if changed { - *lk = lines.concat(); + let text = lines.concat(); + drop(lines); + *lk = text; *lock_changed = true; } true diff --git a/crates/socket-patch-core/src/formats/gem/mod.rs b/crates/socket-patch-core/src/formats/gem/mod.rs index d4a835422..122c02b18 100644 --- a/crates/socket-patch-core/src/formats/gem/mod.rs +++ b/crates/socket-patch-core/src/formats/gem/mod.rs @@ -24,6 +24,7 @@ pub(crate) mod manifest; pub(crate) mod mirror; use std::collections::{BTreeSet, HashMap, HashSet}; +use std::sync::OnceLock; use crate::utils::digest::sha256_hex; use crate::utils::purl::simple_purl; @@ -68,6 +69,9 @@ const BUNDLER_HEADERS: [&str; 9] = [ /// Git merge-conflict markers (bundler refuses a lock carrying them). const CONFLICT_MARKERS: [&str; 4] = ["<<<<<<<", "=======", ">>>>>>>", "|||||||"]; +/// `CHECKSUMS` digests by gem name, then by parenthesized token. +type ChecksumMap = HashMap)>>; + /// A parsed `name (version[-platform])` spec entry. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) struct Spec<'t> { @@ -153,10 +157,17 @@ pub(crate) struct GemfileLock<'t> { pub(crate) sections: Vec>, /// The (last) `DEPENDENCIES` section; `None` when the lock has none. pub(crate) dependencies: Option>, - /// `None` when the lock has no `CHECKSUMS` section (bundler < 2.6). - /// Keyed by `(name, parenthesized token)`; the value is the entry's - /// lowercase sha256, or `None` for a bare / malformed / conflicting one. - pub(crate) checksums: Option>>, + /// The `CHECKSUMS` section's 2-space entries, trimmed; `None` when the + /// lock has no `CHECKSUMS` section (bundler < 2.6). Their digests are + /// read only when a reader asks for one ([`Self::checksum`]): the + /// hosted lock writer parses the lock once per converged gem and never + /// does, and validating every digest dominates the parse. + checksum_entries: Option>, + /// [`Self::checksum_entries`] by name, then parenthesized token; the + /// value is the entry's lowercase sha256, or `None` for a bare / + /// malformed / conflicting one. Built on first use, with owned keys so + /// the lock stays covariant in `'t`. + checksums: OnceLock, /// `DEPENDENCIES` entries bundler marks source-pinned (`name …!`). pub(crate) pinned: BTreeSet<&'t str>, /// Every `DEPENDENCIES` entry: the gems bundler treats as DIRECT @@ -202,7 +213,30 @@ impl<'t> GemfileLock<'t> { /// The sha256 `CHECKSUMS` pins `name (token)`, if the lock records a /// valid, unambiguous one. pub(crate) fn checksum(&self, name: &'t str, token: &'t str) -> Option<&str> { - self.checksums.as_ref()?.get(&(name, token))?.as_deref() + let entries = self.checksum_entries.as_ref()?; + let checksums = self.checksums.get_or_init(|| { + let mut map = ChecksumMap::new(); + for ((name, token), sha) in entries.iter().filter_map(|entry| parse_checksum(entry)) { + let tokens = map.entry(name.to_string()).or_default(); + match tokens.iter_mut().find(|(t, _)| t == token) { + Some((_, prev)) if *prev != sha => *prev = None, + Some(_) => {} + None => tokens.push((token.to_string(), sha)), + } + } + map + }); + checksums + .get(name)? + .iter() + .find(|(t, _)| t == token)? + .1 + .as_deref() + } + + /// Whether the lock has a `CHECKSUMS` section (bundler >= 2.6). + pub(crate) fn has_checksums(&self) -> bool { + self.checksum_entries.is_some() } /// [`Self::checksum`] as the lock pin readers record: the valid sha256 @@ -277,7 +311,7 @@ pub(crate) fn gem_download_url(base: &str, name: &str, version: &str) -> Option< /// Parse a Bundler lock (see the module docs). pub(crate) fn parse(text: &str) -> GemfileLock<'_> { let mut sections: Vec> = Vec::new(); - let mut checksums: Option>> = None; + let mut checksum_entries: Option> = None; let mut problems: Vec = Vec::new(); let mut current: Option = None; let mut in_specs = false; @@ -324,8 +358,8 @@ pub(crate) fn parse(text: &str) -> GemfileLock<'_> { in_specs = false; in_checksums = header == "CHECKSUMS"; in_dependencies = header == "DEPENDENCIES"; - if in_checksums && checksums.is_none() { - checksums = Some(HashMap::new()); + if in_checksums && checksum_entries.is_none() { + checksum_entries = Some(Vec::new()); } if in_dependencies { dependencies = Some(Dependencies { @@ -390,14 +424,8 @@ pub(crate) fn parse(text: &str) -> GemfileLock<'_> { } } } else if in_checksums && indent == 2 { - if let (Some(map), Some((key, sha))) = (checksums.as_mut(), parse_checksum(trimmed)) { - map.entry(key) - .and_modify(|prev| { - if *prev != sha { - *prev = None; - } - }) - .or_insert(sha); + if let Some(entries) = checksum_entries.as_mut() { + entries.push(trimmed); } } } @@ -415,7 +443,8 @@ pub(crate) fn parse(text: &str) -> GemfileLock<'_> { GemfileLock { sections, dependencies, - checksums, + checksum_entries, + checksums: OnceLock::new(), pinned, direct, problems, diff --git a/crates/socket-patch-core/src/vex/discover/gem.rs b/crates/socket-patch-core/src/vex/discover/gem.rs index 6d79cbf7e..2d05686bf 100644 --- a/crates/socket-patch-core/src/vex/discover/gem.rs +++ b/crates/socket-patch-core/src/vex/discover/gem.rs @@ -577,7 +577,7 @@ fn spec_ref( file, Some(*remote), lock.integrity(spec.name, spec.version), - lock.checksums.is_some(), + lock.has_checksums(), )); } Wiring::Vendored(vref) => {