From ee29119915c621f86a1bbf6bc3bd7ede5ea48973 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 08:24:30 +0000 Subject: [PATCH 1/7] Start fix for #1623 Assisted-by: Claude Code:claude-opus-5-5 From 719905919451d08d00559a3c4cc498b12674c078 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 08:39:22 +0000 Subject: [PATCH 2/7] Keep pip lock wheel hashes on revert Reverting one of several vendored packages in a `pip lock` pylock.toml put that package's [packages.wheels.hashes] table after the next [[packages]] header. pip then refused the lock and the other package's revert reported drift, while the CLI exited 0. The restore copied tables from the recorded original text together with their positions in that text, and toml_edit prints tables in position order. Copied tables now drop those positions, so they print inside the table they are restored into, and a replaced key keeps its comments. Fixes #1623 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/pypi_lock.rs | 107 +++++++++++++++++- 1 file changed, 105 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/pypi_lock.rs b/crates/socket-patch-core/src/vendor/pypi_lock.rs index a017943a5..63493fb1e 100644 --- a/crates/socket-patch-core/src/vendor/pypi_lock.rs +++ b/crates/socket-patch-core/src/vendor/pypi_lock.rs @@ -641,6 +641,42 @@ fn pair_changed( (!(already_original && unaccounted)).then_some(pairs) } +/// A copy of `item`, a part of the recorded text, to put into the live +/// document. Parsed standard tables keep their position in the text they +/// came from, and toml_edit prints tables in position order, so a recorded +/// `[[packages.wheels]]` / `[packages.wheels.hashes]` table could print +/// under a later `[[packages]]` header of the live document and re-parse +/// as part of that package (#1623). Without a position, each table prints +/// right after the one before it, i.e. inside the table it now sits in. +fn detached(item: &Item) -> Item { + fn clear(table: &mut Table) { + table.set_position(None); + for (_, child) in table.iter_mut() { + clear_positions(child); + } + } + fn clear_positions(item: &mut Item) { + match item { + Item::Table(table) => clear(table), + Item::ArrayOfTables(array) => array.iter_mut().for_each(clear), + _ => {} + } + } + let mut item = item.clone(); + clear_positions(&mut item); + item +} + +/// `original` in place of `live`, keeping the live table's own position +/// (see [`detached`]) and, by replacing in place, the live key's comments. +fn replace_item(live: &mut Item, original: &Item) { + let position = live.as_table().and_then(Table::position); + *live = detached(original); + if let Some(table) = live.as_table_mut() { + table.set_position(position); + } +} + fn restore_table(live: &mut dyn TableLike, original: &dyn TableLike, new: &dyn TableLike) -> bool { let keys: BTreeSet = original .iter() @@ -656,7 +692,12 @@ fn restore_table(live: &mut dyn TableLike, original: &dyn TableLike, new: &dyn T } if equal_item(live.get(&key), after) { if let Some(before) = before { - live.insert(&key, before.clone()); + match live.get_mut(&key) { + Some(current) => replace_item(current, before), + None => { + live.insert(&key, detached(before)); + } + } } else { live.remove(&key); } @@ -780,7 +821,7 @@ fn restore_item(live: &mut Item, original: &Item, new: &Item) -> bool { return false; } if item_text(live) == item_text(new) { - *live = original.clone(); + replace_item(live, original); return false; } if let (Some(live), Some(original), Some(new)) = ( @@ -1604,6 +1645,68 @@ mod tests { assert_eq!(restored, LOCK); } + /// #1623: `pip lock` writes each wheel as a `[[packages.wheels]]` table + /// with a `[packages.wheels.hashes]` sub-table. Restoring one package's + /// wheels while a later package is still vendored must print them under + /// their own package, not after the next `[[packages]]` header. + #[test] + fn pip_lock_wheel_tables_revert_under_their_own_package() { + const PIP_LOCK: &str = "lock-version = \"1.0\"\ncreated-by = \"pip\"\n\n[[packages]]\nname = \"idna\"\nversion = \"3.7\"\n\n[[packages.wheels]]\nname = \"idna-3.7-py3-none-any.whl\"\nurl = \"https://example.test/idna-3.7-py3-none-any.whl\"\n\n[packages.wheels.hashes]\nsha256 = \"idna\"\n\n[[packages]]\nname = \"six\"\nversion = \"1.16.0\"\n\n[[packages.wheels]]\nname = \"six-1.16.0-py2.py3-none-any.whl\"\nurl = \"https://example.test/six-1.16.0-py2.py3-none-any.whl\"\n\n[packages.wheels.hashes]\nsha256 = \"six\"\n"; + let first = rewrite_python_lock( + PIP_LOCK, + "idna", + "3.7", + ArtifactSource::Path(".socket/vendor/pypi/a/idna-3.7-py3-none-any.whl"), + "first", + ) + .unwrap() + .unwrap(); + let both = rewrite_python_lock( + &first, + "six", + "1.16.0", + ArtifactSource::Path(".socket/vendor/pypi/b/six-1.16.0-py2.py3-none-any.whl"), + "second", + ) + .unwrap() + .unwrap(); + let six_only = rewrite_python_lock( + PIP_LOCK, + "six", + "1.16.0", + ArtifactSource::Path(".socket/vendor/pypi/b/six-1.16.0-py2.py3-none-any.whl"), + "second", + ) + .unwrap() + .unwrap(); + + // Revert the first-vendored package alone: idna's wheel tables come + // back exactly where `pip lock` wrote them. + let (second_only, drifted) = restore_document(&both, PIP_LOCK, &first).unwrap(); + assert!(!drifted, "{second_only}"); + assert_eq!(second_only, six_only); + let parsed: DocumentMut = second_only.parse().unwrap(); + let packages = parsed["packages"].as_array_of_tables().unwrap(); + assert_eq!( + packages.get(0).unwrap()["wheels"][0]["hashes"]["sha256"].as_str(), + Some("idna") + ); + assert!(packages.get(1).unwrap().get("wheels").is_none()); + + // Then the second one: the original bytes. + let (restored, drifted) = restore_document(&second_only, &first, &both).unwrap(); + assert!(!drifted, "{restored}"); + assert_eq!(restored, PIP_LOCK); + + // The other order (`vendor --revert` walks the ledger) ends the same. + let (first_only, drifted) = restore_document(&both, &first, &both).unwrap(); + assert!(!drifted); + assert_eq!(first_only, first); + let (restored, drifted) = restore_document(&first_only, PIP_LOCK, &first).unwrap(); + assert!(!drifted); + assert_eq!(restored, PIP_LOCK); + } + #[test] fn script_sources_revert_out_of_order_without_empty_tables() { for suffix in ["", "\n[tool.uv.sources]\n"] { From 1082b0f8846903e7874639741d9c526a82b782c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 09:16:47 +0000 Subject: [PATCH 3/7] Keep original table order on pylock revert The first fix for #1623 dropped the recorded positions of restored tables entirely. That put them at the end of their parent, so a revert could print a package's wheels after its other sub-tables, or move a pyproject [tool.uv.sources] above a [tool.ruff] it originally followed. The file still parsed the same but was no longer the original bytes. Restored tables now keep their recorded position as a mark, and a final pass places each one right after the live copy of the table it followed in the original. That lookup stays inside the array-of-tables element the restore paired, so a package the user added since vendoring can't pull a restored table to the wrong package. Refs #1623 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/pypi_lock.rs | 284 ++++++++++++++++-- 1 file changed, 266 insertions(+), 18 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/pypi_lock.rs b/crates/socket-patch-core/src/vendor/pypi_lock.rs index 63493fb1e..6a2882b07 100644 --- a/crates/socket-patch-core/src/vendor/pypi_lock.rs +++ b/crates/socket-patch-core/src/vendor/pypi_lock.rs @@ -644,26 +644,18 @@ fn pair_changed( /// A copy of `item`, a part of the recorded text, to put into the live /// document. Parsed standard tables keep their position in the text they /// came from, and toml_edit prints tables in position order, so a recorded -/// `[[packages.wheels]]` / `[packages.wheels.hashes]` table could print -/// under a later `[[packages]]` header of the live document and re-parse -/// as part of that package (#1623). Without a position, each table prints -/// right after the one before it, i.e. inside the table it now sits in. +/// `[[packages.wheels]]` / `[packages.wheels.hashes]` table would print +/// among the live document's tables by its recorded number: under a later +/// `[[packages]]` header, re-parsed as part of that package (#1623). The +/// copy's tables carry their recorded position as a negative mark instead, +/// and [`place_restored`] puts them back after the table they followed. fn detached(item: &Item) -> Item { - fn clear(table: &mut Table) { - table.set_position(None); - for (_, child) in table.iter_mut() { - clear_positions(child); - } - } - fn clear_positions(item: &mut Item) { - match item { - Item::Table(table) => clear(table), - Item::ArrayOfTables(array) => array.iter_mut().for_each(clear), - _ => {} - } - } let mut item = item.clone(); - clear_positions(&mut item); + for_each_table(&mut item, &mut |table| { + if let Some(position) = table.position() { + table.set_position(Some(-position - 1)); + } + }); item } @@ -677,6 +669,188 @@ fn replace_item(live: &mut Item, original: &Item) { } } +/// Every header table under `item`, depth first in toml_edit's print walk. +fn for_each_table(item: &mut Item, visit: &mut dyn FnMut(&mut Table)) { + match item { + Item::Table(table) => { + if !table.is_dotted() { + visit(table); + } + for (_, child) in table.iter_mut() { + for_each_table(child, visit); + } + } + Item::ArrayOfTables(array) => { + for table in array.iter_mut() { + visit(table); + for (_, child) in table.iter_mut() { + for_each_table(child, visit); + } + } + } + _ => {} + } +} + +#[derive(Clone, PartialEq)] +enum Step { + Key(String), + Index(usize), +} + +/// The path and position of every header table in `table`, in the order +/// of [`for_each_table`]. +fn table_paths(table: &Table, path: &mut Vec, out: &mut Vec<(Vec, Option)>) { + if !table.is_dotted() { + out.push((path.clone(), table.position())); + } + for (key, item) in table.iter() { + match item { + Item::Table(child) => { + path.push(Step::Key(key.to_string())); + table_paths(child, path, out); + path.pop(); + } + Item::ArrayOfTables(array) => { + for (index, child) in array.iter().enumerate() { + path.push(Step::Key(key.to_string())); + path.push(Step::Index(index)); + table_paths(child, path, out); + path.truncate(path.len() - 2); + } + } + _ => {} + } + } +} + +/// Renumbers `live`'s tables so each table restored from `original` (a +/// negative mark, see [`detached`]) prints right after the live copy of +/// the table it followed in `original`, keeping the original order among +/// restored tables and the live order everywhere else. +/// +/// The table a restored one followed is looked up by path, but only below +/// the deepest array-of-tables element that holds the restored table and +/// was not itself restored: element indices above it may differ between +/// the two documents (a package the user added since vendoring), while +/// the restore paired that element with its original. +fn place_restored(live: &mut DocumentMut, original: &DocumentMut) { + let mut tables = Vec::new(); + table_paths(live.as_table(), &mut Vec::new(), &mut tables); + if !tables + .iter() + .any(|(_, position)| position.is_some_and(|position| position < 0)) + { + return; + } + let mut originals = Vec::new(); + table_paths(original.as_table(), &mut Vec::new(), &mut originals); + let kept = |position: Option| position.is_none_or(|position| position >= 0); + // Where each kept table prints: its own position, or the one it + // inherits from the kept table before it. + let mut last = -1; + let effective: Vec> = tables + .iter() + .map(|(_, position)| match position { + Some(position) if *position >= 0 => { + last = *position; + Some(last) + } + Some(_) => None, + None => Some(last), + }) + .collect(); + let printed_at = |prefix: &[Step], exact: bool| { + tables + .iter() + .zip(&effective) + .filter(|((path, position), _)| { + kept(*position) + && if exact { + path == prefix + } else { + path.starts_with(prefix) + } + }) + .filter_map(|(_, at)| *at) + .max() + }; + let anchor = |live_path: &[Step], mark: isize| -> isize { + let recorded = -mark - 1; + let Some(original_path) = originals + .iter() + .find(|(_, position)| *position == Some(recorded)) + .map(|(path, _)| path) + else { + return -1; + }; + let scope = (1..=live_path.len()) + .rev() + .filter(|&len| matches!(live_path[len - 1], Step::Index(_))) + .find(|&len| { + tables + .iter() + .any(|(path, position)| path == &live_path[..len] && kept(*position)) + }) + .unwrap_or(0); + let below = live_path.len() - scope; + if original_path.len() < below { + return -1; + } + let original_scope = &original_path[..original_path.len() - below]; + let live_scope = &live_path[..scope]; + let mut before: Vec<_> = originals + .iter() + .filter(|(path, position)| { + path.starts_with(original_scope) + && position.is_some_and(|position| position < recorded) + }) + .collect(); + before.sort_by_key(|(_, position)| std::cmp::Reverse(*position)); + for (path, _) in before { + let relative = &path[original_scope.len()..]; + let ancestor = original_path.starts_with(path); + let unit = match relative + .iter() + .position(|step| matches!(step, Step::Index(_))) + { + Some(index) if !ancestor => &relative[..index], + _ => relative, + }; + let target: Vec = live_scope.iter().chain(unit).cloned().collect(); + if let Some(at) = printed_at(&target, ancestor) { + return at; + } + } + printed_at(live_scope, true).unwrap_or(-1) + }; + let keys: Vec> = tables + .iter() + .map(|(path, position)| { + position.map(|position| { + if position >= 0 { + (position, false, 0) + } else { + (anchor(path, position), true, -position - 1) + } + }) + }) + .collect(); + let mut order: Vec = (0..tables.len()).filter(|&i| keys[i].is_some()).collect(); + order.sort_by_key(|&i| keys[i]); + let mut renumbered = vec![None; tables.len()]; + for (rank, index) in order.into_iter().enumerate() { + renumbered[index] = Some(rank as isize); + } + let mut index = 0; + for_each_table(live.as_item_mut(), &mut |table| { + if let Some(position) = renumbered[index] { + table.set_position(Some(position)); + } + index += 1; + }); +} + fn restore_table(live: &mut dyn TableLike, original: &dyn TableLike, new: &dyn TableLike) -> bool { let keys: BTreeSet = original .iter() @@ -901,6 +1075,9 @@ pub(crate) fn restore_document( .parse() .map_err(|error| format!("invalid vendored TOML: {error}"))?; let drifted = restore_item(live.as_item_mut(), original.as_item(), new.as_item()); + if !drifted { + place_restored(&mut live, &original); + } Ok(( if drifted { current_text @@ -1707,6 +1884,77 @@ mod tests { assert_eq!(restored, PIP_LOCK); } + /// #1623: restored tables go back between the live tables they sat + /// between, not after every other sub-table of their package. + #[test] + fn pip_lock_wheel_tables_revert_before_later_sub_tables() { + const PIP_LOCK: &str = "lock-version = \"1.0\"\n\n[[packages]]\nname = \"idna\"\nversion = \"3.7\"\n\n[[packages.wheels]]\nname = \"idna-3.7-py3-none-any.whl\"\nurl = \"https://example.test/idna.whl\"\n\n[packages.wheels.hashes]\nsha256 = \"idna\"\n\n[[packages.attestation-identities]]\nkind = \"GitHub\"\n\n[[packages]]\nname = \"six\"\nversion = \"1.16.0\"\n\n[[packages.wheels]]\nname = \"six-1.16.0-py2.py3-none-any.whl\"\nurl = \"https://example.test/six.whl\"\n\n[packages.wheels.hashes]\nsha256 = \"six\"\n\n[packages.tool.pdm]\ngroups = [\"default\"]\n"; + let first = rewrite_python_lock( + PIP_LOCK, + "idna", + "3.7", + ArtifactSource::Path(".socket/vendor/pypi/a/idna-3.7-py3-none-any.whl"), + "first", + ) + .unwrap() + .unwrap(); + let both = rewrite_python_lock( + &first, + "six", + "1.16.0", + ArtifactSource::Path(".socket/vendor/pypi/b/six-1.16.0-py2.py3-none-any.whl"), + "second", + ) + .unwrap() + .unwrap(); + let (six_only, drifted) = restore_document(&both, PIP_LOCK, &first).unwrap(); + assert!(!drifted, "{six_only}"); + let (restored, drifted) = restore_document(&six_only, &first, &both).unwrap(); + assert!(!drifted, "{restored}"); + assert_eq!(restored, PIP_LOCK); + // A package the user added in front since vendoring shifts every + // element index; the wheels still go back under their own package. + let added = "[[packages]]\nname = \"certifi\"\nversion = \"1\"\n\n[[packages.wheels]]\nname = \"certifi-1-py3-none-any.whl\"\n\n"; + let at = |text: &str| text.find("[[packages]]").unwrap(); + let mut live = both.clone(); + live.insert_str(at(&both), added); + let (restored, drifted) = restore_document(&live, PIP_LOCK, &first).unwrap(); + assert!(!drifted, "{restored}"); + let mut expected = six_only.clone(); + expected.insert_str(at(&six_only), added); + assert_eq!(restored, expected); + } + + /// A table restored whole keeps its sub-tables where the original + /// printed them, even after a sibling table the user edited. + #[test] + fn whole_table_restore_keeps_original_sub_table_order() { + let original = "[project]\nname = \"app\"\n\n[tool.uv]\npackage = false\n\n[tool.ruff]\nline-length = 88\n\n[tool.uv.sources]\nsix = { index = \"internal\" }\n"; + let new = original.replace( + "six = { index = \"internal\" }", + "six = { path = \".socket/vendor/pypi/u/six.whl\" }", + ); + let live = new.replace("line-length = 88", "line-length = 100"); + let (restored, drifted) = restore_document(&live, original, &new).unwrap(); + assert!(!drifted, "{restored}"); + assert_eq!( + restored, + original.replace("line-length = 88", "line-length = 100") + ); + } + + /// A live file that differs from the vendored text only in line endings + /// restores to the original layout, in the live line endings. + #[test] + fn crlf_checkout_restores_out_of_order_tables_in_place() { + let original = "[a]\nx = 1\n\n[b]\ny = 1\n\n[a.c]\nz = 1\n"; + let new = original.replace("z = 1", "z = 2"); + let live = new.replace('\n', "\r\n"); + let (restored, drifted) = restore_document(&live, original, &new).unwrap(); + assert!(!drifted, "{restored}"); + assert_eq!(restored, original.replace('\n', "\r\n")); + } + #[test] fn script_sources_revert_out_of_order_without_empty_tables() { for suffix in ["", "\n[tool.uv.sources]\n"] { From 769c5c344dbe80721f699fa433d227662c163f90 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 10:05:09 +0000 Subject: [PATCH 4/7] Place restored pylock tables by key order Replace the cross-document placement pass with a local rule. Only keys a revert puts back (vendoring removed them) drop their recorded table positions; they go back at their original key slot and print right after the sibling before them. Whole-table replacements keep their recorded positions as before, so unrelated pyproject sections keep their original order, and large locks don't pay a quadratic lookup. Refs #1623 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/pypi_lock.rs | 287 +++++------------- 1 file changed, 70 insertions(+), 217 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/pypi_lock.rs b/crates/socket-patch-core/src/vendor/pypi_lock.rs index 6a2882b07..5d4a25389 100644 --- a/crates/socket-patch-core/src/vendor/pypi_lock.rs +++ b/crates/socket-patch-core/src/vendor/pypi_lock.rs @@ -641,216 +641,6 @@ fn pair_changed( (!(already_original && unaccounted)).then_some(pairs) } -/// A copy of `item`, a part of the recorded text, to put into the live -/// document. Parsed standard tables keep their position in the text they -/// came from, and toml_edit prints tables in position order, so a recorded -/// `[[packages.wheels]]` / `[packages.wheels.hashes]` table would print -/// among the live document's tables by its recorded number: under a later -/// `[[packages]]` header, re-parsed as part of that package (#1623). The -/// copy's tables carry their recorded position as a negative mark instead, -/// and [`place_restored`] puts them back after the table they followed. -fn detached(item: &Item) -> Item { - let mut item = item.clone(); - for_each_table(&mut item, &mut |table| { - if let Some(position) = table.position() { - table.set_position(Some(-position - 1)); - } - }); - item -} - -/// `original` in place of `live`, keeping the live table's own position -/// (see [`detached`]) and, by replacing in place, the live key's comments. -fn replace_item(live: &mut Item, original: &Item) { - let position = live.as_table().and_then(Table::position); - *live = detached(original); - if let Some(table) = live.as_table_mut() { - table.set_position(position); - } -} - -/// Every header table under `item`, depth first in toml_edit's print walk. -fn for_each_table(item: &mut Item, visit: &mut dyn FnMut(&mut Table)) { - match item { - Item::Table(table) => { - if !table.is_dotted() { - visit(table); - } - for (_, child) in table.iter_mut() { - for_each_table(child, visit); - } - } - Item::ArrayOfTables(array) => { - for table in array.iter_mut() { - visit(table); - for (_, child) in table.iter_mut() { - for_each_table(child, visit); - } - } - } - _ => {} - } -} - -#[derive(Clone, PartialEq)] -enum Step { - Key(String), - Index(usize), -} - -/// The path and position of every header table in `table`, in the order -/// of [`for_each_table`]. -fn table_paths(table: &Table, path: &mut Vec, out: &mut Vec<(Vec, Option)>) { - if !table.is_dotted() { - out.push((path.clone(), table.position())); - } - for (key, item) in table.iter() { - match item { - Item::Table(child) => { - path.push(Step::Key(key.to_string())); - table_paths(child, path, out); - path.pop(); - } - Item::ArrayOfTables(array) => { - for (index, child) in array.iter().enumerate() { - path.push(Step::Key(key.to_string())); - path.push(Step::Index(index)); - table_paths(child, path, out); - path.truncate(path.len() - 2); - } - } - _ => {} - } - } -} - -/// Renumbers `live`'s tables so each table restored from `original` (a -/// negative mark, see [`detached`]) prints right after the live copy of -/// the table it followed in `original`, keeping the original order among -/// restored tables and the live order everywhere else. -/// -/// The table a restored one followed is looked up by path, but only below -/// the deepest array-of-tables element that holds the restored table and -/// was not itself restored: element indices above it may differ between -/// the two documents (a package the user added since vendoring), while -/// the restore paired that element with its original. -fn place_restored(live: &mut DocumentMut, original: &DocumentMut) { - let mut tables = Vec::new(); - table_paths(live.as_table(), &mut Vec::new(), &mut tables); - if !tables - .iter() - .any(|(_, position)| position.is_some_and(|position| position < 0)) - { - return; - } - let mut originals = Vec::new(); - table_paths(original.as_table(), &mut Vec::new(), &mut originals); - let kept = |position: Option| position.is_none_or(|position| position >= 0); - // Where each kept table prints: its own position, or the one it - // inherits from the kept table before it. - let mut last = -1; - let effective: Vec> = tables - .iter() - .map(|(_, position)| match position { - Some(position) if *position >= 0 => { - last = *position; - Some(last) - } - Some(_) => None, - None => Some(last), - }) - .collect(); - let printed_at = |prefix: &[Step], exact: bool| { - tables - .iter() - .zip(&effective) - .filter(|((path, position), _)| { - kept(*position) - && if exact { - path == prefix - } else { - path.starts_with(prefix) - } - }) - .filter_map(|(_, at)| *at) - .max() - }; - let anchor = |live_path: &[Step], mark: isize| -> isize { - let recorded = -mark - 1; - let Some(original_path) = originals - .iter() - .find(|(_, position)| *position == Some(recorded)) - .map(|(path, _)| path) - else { - return -1; - }; - let scope = (1..=live_path.len()) - .rev() - .filter(|&len| matches!(live_path[len - 1], Step::Index(_))) - .find(|&len| { - tables - .iter() - .any(|(path, position)| path == &live_path[..len] && kept(*position)) - }) - .unwrap_or(0); - let below = live_path.len() - scope; - if original_path.len() < below { - return -1; - } - let original_scope = &original_path[..original_path.len() - below]; - let live_scope = &live_path[..scope]; - let mut before: Vec<_> = originals - .iter() - .filter(|(path, position)| { - path.starts_with(original_scope) - && position.is_some_and(|position| position < recorded) - }) - .collect(); - before.sort_by_key(|(_, position)| std::cmp::Reverse(*position)); - for (path, _) in before { - let relative = &path[original_scope.len()..]; - let ancestor = original_path.starts_with(path); - let unit = match relative - .iter() - .position(|step| matches!(step, Step::Index(_))) - { - Some(index) if !ancestor => &relative[..index], - _ => relative, - }; - let target: Vec = live_scope.iter().chain(unit).cloned().collect(); - if let Some(at) = printed_at(&target, ancestor) { - return at; - } - } - printed_at(live_scope, true).unwrap_or(-1) - }; - let keys: Vec> = tables - .iter() - .map(|(path, position)| { - position.map(|position| { - if position >= 0 { - (position, false, 0) - } else { - (anchor(path, position), true, -position - 1) - } - }) - }) - .collect(); - let mut order: Vec = (0..tables.len()).filter(|&i| keys[i].is_some()).collect(); - order.sort_by_key(|&i| keys[i]); - let mut renumbered = vec![None; tables.len()]; - for (rank, index) in order.into_iter().enumerate() { - renumbered[index] = Some(rank as isize); - } - let mut index = 0; - for_each_table(live.as_item_mut(), &mut |table| { - if let Some(position) = renumbered[index] { - table.set_position(Some(position)); - } - index += 1; - }); -} - fn restore_table(live: &mut dyn TableLike, original: &dyn TableLike, new: &dyn TableLike) -> bool { let keys: BTreeSet = original .iter() @@ -866,10 +656,11 @@ fn restore_table(live: &mut dyn TableLike, original: &dyn TableLike, new: &dyn T } if equal_item(live.get(&key), after) { if let Some(before) = before { + // In place, so the live key keeps its comments. match live.get_mut(&key) { - Some(current) => replace_item(current, before), + Some(current) => *current = before.clone(), None => { - live.insert(&key, detached(before)); + live.insert(&key, before.clone()); } } } else { @@ -899,6 +690,64 @@ fn restore_table(live: &mut dyn TableLike, original: &dyn TableLike, new: &dyn T drifted } +/// [`restore_table`] on a header table, with the sub-tables of each key it +/// put back printed where `original` had them: right after the key it +/// followed. The copies carry their positions in the recorded text, which +/// mean nothing among the live table's positions (#1623), so they drop +/// them and print after the table before them in key order. +fn restore_header_table(live: &mut Table, original: &dyn TableLike, new: &dyn TableLike) -> bool { + fn unplace(item: &mut Item) { + let tables: Vec<&mut Table> = match item { + Item::Table(table) => vec![table], + Item::ArrayOfTables(array) => array.iter_mut().collect(), + _ => return, + }; + for table in tables { + table.set_position(None); + table.iter_mut().for_each(|(_, child)| unplace(child)); + } + } + let present: BTreeSet = live.iter().map(|(key, _)| key.to_string()).collect(); + let drifted = restore_table(live, original, new); + let original_keys: Vec = original.iter().map(|(key, _)| key.to_string()).collect(); + let added: BTreeSet<&String> = original_keys + .iter() + .filter(|key| { + !present.contains(*key) + && live + .get(key) + .is_some_and(|item| item.is_table() || item.is_array_of_tables()) + }) + .collect(); + if added.is_empty() { + return drifted; + } + for key in &added { + if let Some(item) = live.get_mut(key) { + unplace(item); + } + } + let mut order: Vec = live + .iter() + .map(|(key, _)| key.to_string()) + .filter(|key| !added.contains(key)) + .collect(); + for (index, key) in original_keys.iter().enumerate() { + if !added.contains(key) { + continue; + } + let at = original_keys[..index] + .iter() + .rev() + .find_map(|previous| order.iter().position(|key| key == previous)) + .map_or(0, |found| found + 1); + order.insert(at, key.clone()); + } + let rank = |key: &toml_edit::Key| order.iter().position(|known| known == key.get()); + live.sort_values_by(|left, _, right, _| rank(left).cmp(&rank(right))); + drifted +} + /// Whether `live` still holds at least one field exactly as the vendoring /// wrote it. A table paired only by its name (a requirement's identity /// ignores `specifier`) may be a namesake the user added after removing @@ -995,9 +844,16 @@ fn restore_item(live: &mut Item, original: &Item, new: &Item) -> bool { return false; } if item_text(live) == item_text(new) { - replace_item(live, original); + *live = original.clone(); return false; } + if let (Some(live), Some(original), Some(new)) = ( + live.as_table_mut(), + original.as_table_like(), + new.as_table_like(), + ) { + return restore_header_table(live, original, new); + } if let (Some(live), Some(original), Some(new)) = ( live.as_table_like_mut(), original.as_table_like(), @@ -1044,7 +900,7 @@ fn restore_item(live: &mut Item, original: &Item, new: &Item) -> bool { drifted = true; continue; } - drifted |= restore_table(current, original[index], new[index]); + drifted |= restore_header_table(current, original[index], new[index]); } return drifted; } @@ -1075,9 +931,6 @@ pub(crate) fn restore_document( .parse() .map_err(|error| format!("invalid vendored TOML: {error}"))?; let drifted = restore_item(live.as_item_mut(), original.as_item(), new.as_item()); - if !drifted { - place_restored(&mut live, &original); - } Ok(( if drifted { current_text From a230b16a40bbd3696d8be672f306274bc2749941 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 10:24:56 +0000 Subject: [PATCH 5/7] Keep restored pylock tables under their package Restoring a vendored table that was changed rather than removed (a legacy uv [[distribution.wheel]]) still copied its recorded positions. A package the user added in front shifts the live numbering, so the wheel printed under that package instead. A final pass now keeps every table under an array-of-tables element between that element's header and the next one. That is the only order TOML reads meaning from, and the pass leaves tables the live text placed alone. Re-added keys are put back by re-inserting the entries instead of sorting, which left a dotted table's keys in the parent's order, and an in-place restore no longer puts a table into an inline table. Refs #1623 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/pypi_lock.rs | 147 +++++++++++++++--- 1 file changed, 122 insertions(+), 25 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/pypi_lock.rs b/crates/socket-patch-core/src/vendor/pypi_lock.rs index 5d4a25389..dbe19dc91 100644 --- a/crates/socket-patch-core/src/vendor/pypi_lock.rs +++ b/crates/socket-patch-core/src/vendor/pypi_lock.rs @@ -642,6 +642,17 @@ fn pair_changed( } fn restore_table(live: &mut dyn TableLike, original: &dyn TableLike, new: &dyn TableLike) -> bool { + restore_table_keys(live, original, new, &mut Vec::new()) +} + +/// [`restore_table`], noting in `inserted` each key it put back that the +/// live table no longer had. +fn restore_table_keys( + live: &mut dyn TableLike, + original: &dyn TableLike, + new: &dyn TableLike, + inserted: &mut Vec, +) -> bool { let keys: BTreeSet = original .iter() .chain(new.iter()) @@ -658,9 +669,15 @@ fn restore_table(live: &mut dyn TableLike, original: &dyn TableLike, new: &dyn T if let Some(before) = before { // In place, so the live key keeps its comments. match live.get_mut(&key) { - Some(current) => *current = before.clone(), + Some(current) if current.is_value() == before.is_value() => { + *current = before.clone(); + } + Some(_) => { + live.insert(&key, before.clone()); + } None => { live.insert(&key, before.clone()); + inserted.push(key.clone()); } } } else { @@ -690,11 +707,11 @@ fn restore_table(live: &mut dyn TableLike, original: &dyn TableLike, new: &dyn T drifted } -/// [`restore_table`] on a header table, with the sub-tables of each key it -/// put back printed where `original` had them: right after the key it -/// followed. The copies carry their positions in the recorded text, which -/// mean nothing among the live table's positions (#1623), so they drop -/// them and print after the table before them in key order. +/// [`restore_table`] on a header table, with each table-valued key it put +/// back (vendoring had removed it) returned to its slot in `original`'s +/// key order instead of the end. The copied sub-tables drop the positions +/// they had in the recorded text, which mean nothing among the live +/// table's (#1623), so they print right after the table before them. fn restore_header_table(live: &mut Table, original: &dyn TableLike, new: &dyn TableLike) -> bool { fn unplace(item: &mut Item) { let tables: Vec<&mut Table> = match item { @@ -707,22 +724,16 @@ fn restore_header_table(live: &mut Table, original: &dyn TableLike, new: &dyn Ta table.iter_mut().for_each(|(_, child)| unplace(child)); } } - let present: BTreeSet = live.iter().map(|(key, _)| key.to_string()).collect(); - let drifted = restore_table(live, original, new); - let original_keys: Vec = original.iter().map(|(key, _)| key.to_string()).collect(); - let added: BTreeSet<&String> = original_keys - .iter() - .filter(|key| { - !present.contains(*key) - && live - .get(key) - .is_some_and(|item| item.is_table() || item.is_array_of_tables()) - }) - .collect(); - if added.is_empty() { + let mut inserted = Vec::new(); + let drifted = restore_table_keys(live, original, new, &mut inserted); + inserted.retain(|key| { + live.get(key) + .is_some_and(|item| item.is_table() || item.is_array_of_tables()) + }); + if inserted.is_empty() { return drifted; } - for key in &added { + for key in &inserted { if let Some(item) = live.get_mut(key) { unplace(item); } @@ -730,24 +741,71 @@ fn restore_header_table(live: &mut Table, original: &dyn TableLike, new: &dyn Ta let mut order: Vec = live .iter() .map(|(key, _)| key.to_string()) - .filter(|key| !added.contains(key)) + .filter(|key| !inserted.contains(key)) .collect(); + let original_keys: Vec = original.iter().map(|(key, _)| key.to_string()).collect(); for (index, key) in original_keys.iter().enumerate() { - if !added.contains(key) { + if !inserted.contains(key) { continue; } let at = original_keys[..index] .iter() .rev() - .find_map(|previous| order.iter().position(|key| key == previous)) + .find_map(|previous| order.iter().position(|known| known == previous)) .map_or(0, |found| found + 1); order.insert(at, key.clone()); } - let rank = |key: &toml_edit::Key| order.iter().position(|known| known == key.get()); - live.sort_values_by(|left, _, right, _| rank(left).cmp(&rank(right))); + // Re-adding every entry in order keeps each key's own formatting. + let entries: Vec<_> = order + .iter() + .filter_map(|key| live.remove_entry(key)) + .collect(); + for (key, item) in entries { + live.insert_formatted(&key, item); + } drifted } +/// Keeps every table under an array-of-tables element printed between +/// that element's header and the next element's. A restored item is a +/// copy from the recorded text, and its standard tables keep their +/// positions there, while toml_edit prints tables in position order. Once +/// the live document numbers its tables differently (another package was +/// vendored, or the user added one), a restored sub-table could print +/// after the next element's header and re-parse as part of that element +/// (#1623). Tables the live text placed already sit in their range. +fn keep_under_owner(table: &mut Table, low: isize, high: isize) { + let clamp = |position: isize| position.max(low).min(high); + for (_, item) in table.iter_mut() { + match item { + Item::Table(child) => { + if let Some(position) = child.position() { + child.set_position(Some(clamp(position))); + } + keep_under_owner(child, low, high); + } + Item::ArrayOfTables(array) => { + let mut starts = Vec::with_capacity(array.len()); + let mut last = low; + for element in array.iter() { + last = element + .position() + .map_or(last, |position| clamp(position).max(last)); + starts.push(last); + } + for (index, element) in array.iter_mut().enumerate() { + if element.position().is_some() { + element.set_position(Some(starts[index])); + } + let end = starts.get(index + 1).copied().unwrap_or(high); + keep_under_owner(element, starts[index], end); + } + } + _ => {} + } + } +} + /// Whether `live` still holds at least one field exactly as the vendoring /// wrote it. A table paired only by its name (a requirement's identity /// ignores `specifier`) may be a namesake the user added after removing @@ -931,6 +989,9 @@ pub(crate) fn restore_document( .parse() .map_err(|error| format!("invalid vendored TOML: {error}"))?; let drifted = restore_item(live.as_item_mut(), original.as_item(), new.as_item()); + if !drifted { + keep_under_owner(live.as_table_mut(), isize::MIN, isize::MAX); + } Ok(( if drifted { current_text @@ -1778,6 +1839,42 @@ mod tests { assert_eq!(restored, expected); } + /// #1623, replaced rather than re-added: a vendored `[[distribution.wheel]]` + /// array goes back with its recorded positions, which a package the user + /// added in front has shifted. It must still print under its own + /// `[[distribution]]`, not under the user's package. + #[test] + fn replaced_wheel_tables_stay_under_their_own_element() { + let original = "version = 1\n\n[[distribution]]\nname = \"idna\"\nversion = \"3.7\"\n\n[[distribution.wheel]]\nurl = \"https://example.test/idna.whl\"\nhash = \"sha256:i\"\n\n[[distribution]]\nname = \"six\"\nversion = \"1.16.0\"\n\n[[distribution.wheel]]\nurl = \"https://example.test/six.whl\"\nhash = \"sha256:s\"\n"; + let new = original + .replace( + "https://example.test/idna.whl", + ".socket/vendor/pypi/a/idna.whl", + ) + .replace("sha256:i", "sha256:vendored"); + let added = "[[distribution]]\nname = \"certifi\"\nversion = \"1\"\n\n[[distribution.wheel]]\nurl = \"https://example.test/certifi-a.whl\"\n\n[[distribution.wheel]]\nurl = \"https://example.test/certifi-b.whl\"\n\n"; + let at = |text: &str| text.find("[[distribution]]").unwrap(); + let mut live = new.clone(); + live.insert_str(at(&new), added); + let (restored, drifted) = restore_document(&live, original, &new).unwrap(); + assert!(!drifted, "{restored}"); + let mut expected = original.to_string(); + expected.insert_str(at(original), added); + assert_eq!(restored, expected); + } + + /// Putting a removed sub-table back at its key slot leaves the keys of a + /// dotted table in the same header alone. + #[test] + fn re_added_sub_table_keeps_dotted_key_order() { + let original = "[p]\nname = 1\nversion = 1\nd.version = 1\nd.name = 2\n\n[p.c]\nz = 1\n"; + let new = "[p]\nname = 1\nversion = 1\nd.version = 1\nd.name = 2\n"; + let live = new.replace("name = 1", "name = 3"); + let (restored, drifted) = restore_document(&live, original, new).unwrap(); + assert!(!drifted, "{restored}"); + assert_eq!(restored, original.replace("name = 1", "name = 3")); + } + /// A table restored whole keeps its sub-tables where the original /// printed them, even after a sibling table the user edited. #[test] From ea1498ec7c31b97d74065ece9d895c2f74292e8b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 10:34:22 +0000 Subject: [PATCH 6/7] Merge duplicate insert arms in pylock restore Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/vendor/pypi_lock.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/pypi_lock.rs b/crates/socket-patch-core/src/vendor/pypi_lock.rs index dbe19dc91..35654a4d5 100644 --- a/crates/socket-patch-core/src/vendor/pypi_lock.rs +++ b/crates/socket-patch-core/src/vendor/pypi_lock.rs @@ -672,12 +672,11 @@ fn restore_table_keys( Some(current) if current.is_value() == before.is_value() => { *current = before.clone(); } - Some(_) => { - live.insert(&key, before.clone()); - } - None => { + current => { + if current.is_none() { + inserted.push(key.clone()); + } live.insert(&key, before.clone()); - inserted.push(key.clone()); } } } else { From 963ed9df6c70378cde6a13ece40c0158b695c1a7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 17:30:47 +0000 Subject: [PATCH 7/7] CI: pin uv for the Pipenv pylock legs Ported from #1639. uv 0.13.2 and later refuse the pylock.toml Pipenv 2026.8.0 writes (environments = []) with "The target environment is not supported by the lockfile". The unpinned setup-uv step picked that release up, and the Pipenv e2e leg now fails for every PR in the merge queue. Pinning uv 0.13.1 restores the leg. The change is identical to #1639, so it is a no-op once that lands. Assisted-by: Claude Code:claude-opus-5-5 --- .github/workflows/ci.yml | 6 ++++++ .github/workflows/pipenv-compatibility.yml | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eef9a64dd..764e0bbbf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1465,6 +1465,12 @@ jobs: - name: Setup uv (Pipenv / pip legs) if: matrix.pipenv != '' || matrix.pip != '' uses: astral-sh/setup-uv@eb1897b8dc4b5d5bfe39a428a8f2304605e0983c # v7.0.0 + with: + # Pin uv: it is the PEP 751 oracle the pylock tests install with, + # and uv 0.13.2 started refusing Pipenv's `environments = []` + # pylock ("target environment is not supported by the lockfile"). + # Bump deliberately, after checking the Pipenv pylock legs pass. + version: "0.13.1" - name: Install the Pythons the Pipenv / pip legs run on if: matrix.pipenv != '' || matrix.pip != '' diff --git a/.github/workflows/pipenv-compatibility.yml b/.github/workflows/pipenv-compatibility.yml index cea9c0e5e..a786e3276 100644 --- a/.github/workflows/pipenv-compatibility.yml +++ b/.github/workflows/pipenv-compatibility.yml @@ -101,6 +101,12 @@ jobs: save-if: ${{ github.ref == 'refs/heads/main' }} - name: Install uv uses: astral-sh/setup-uv@d0cc045d04ccac9d8b7881df0226f9e82c39688e # v6.8.0 + with: + # Pin uv: it is the PEP 751 oracle the pylock tests install with, + # and uv 0.13.2 started refusing Pipenv's `environments = []` + # pylock ("target environment is not supported by the lockfile"). + # Bump deliberately, after checking the Pipenv pylock legs pass. + version: "0.13.1" - name: Install the Pythons the Pipenv releases run on run: uv python install 3.8 3.12 - name: Build the CLI