Repository navigation
Fix pylock revert misplacing wheel hashes (#1623) - #1625
Open
Mikola Lysenko (mikolalysenko) wants to merge 6 commits into
Open
Mikola Lysenko (mikolalysenko) wants to merge 6 commits into
Mikola Lysenko (mikolalysenko) wants to merge 6 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
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
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
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
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
Assisted-by: Claude Code:claude-opus-5-5
Mikola Lysenko (mikolalysenko)
marked this pull request as ready for review
October 17, 2026 10:34
Collaborator
Author
|
BugBot review Generated by Claude Code |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ea1498e. Configure here.
Collaborator
Author
|
Ready for review (burn-down agent).
Slack announcement could not be sent this run (no Slack send tool available); the next run will retry. Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
LLM Description written by Claude Code:claude-opus-5-5
Fixes #1623
Summary
vendor --revert/removeof the first-vendored of two packages in apip lockpylock.toml wrote an invalid lock with exit 0. idna's[packages.wheels.hashes]table was printed after six's[[packages]]header, so pip refused the file and six's revert reported drift. After
this change the revert gives back the original bytes, in either order.
Root cause
restore_document(crates/socket-patch-core/src/vendor/pypi_lock.rs)merges the recorded original back into the live document by cloning
items out of the parsed original. Parsed standard tables keep their
position in the text they came from, and toml_edit prints tables in
position order. Once the live document numbers its tables differently
(another package's
[[packages.wheels]]was replaced by an inlinearchive, or the user added a package in front), a restored sub-tablecould print after the next array element's header and re-parse as part
of that element. A single vendored package never hit this because
restore_documentshort-circuits when the live text equals thevendored text.
Fix
restore_header_table: a table-valued key the revert re-adds (vendoringremoved it, as
pip lock'swheelsis replaced byarchive) goes backat its original key slot. Its copied sub-tables drop their recorded
positions, so they print right after the table before them.
keep_under_owner: a final O(n) pass keeps every table under anarray-of-tables element between that element's header and the next
one. TOML reads meaning only from that order. The pass covers the
replace paths too (for example a legacy uv
[[distribution.wheel]]that vendoring changed rather than removed), and it leaves alone any
table the live text already placed.
written into an inline table.
Tests (red → green)
pip_lock_wheel_tables_revert_under_their_own_packageattestation-identitiessub-table, and with a package the user added in frontpip_lock_wheel_tables_revert_before_later_sub_tables[[distribution.wheel]]with a user package in frontreplaced_wheel_tables_stay_under_their_own_elementwhole_table_restore_keeps_original_sub_table_order,crlf_checkout_restores_out_of_order_tables_in_place,re_added_sub_table_keeps_dotted_key_orderRan locally:
cargo fmt --all -- --check: cleancargo clippy --workspace --all-features -- -D warnings: cleancargo test -p socket-patch-core --lib: 6540 passed. 4 failed, all because the sandbox runs as root, so chmod-based write-failure tests can't fail a write. None touch this code:copy_tree::relax_loop_must_not_traverse_symlinked_root,vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry,pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouched,pypi_requirements::wire_failure_rolls_back_already_written_files. CI runs as non-root.cargo test -p socket-patch-cli --all-features --test e2e_vendor_pypi_build(with--ignoredtoo, real uv 0.11.32),mode_migration_pypi,in_process_vendor_pypi_takeover,vendor_ledger_schema_e2e: all pass.cargo test --workspace --all-features: linking every test binary used up the sandbox's disk allowance, so I ran the crates and suites above instead.npm/,pypi/,gem/) are untouched; this is core TOML merging only./code-review high (4 rounds)
I fixed the confirmed findings, including two earlier designs that the
review showed reordered unrelated pyproject sections or were cubic on
large locks. Not fixed, all layout-only (the lock parses to the same
data, never under the wrong parent):
table can move when the user added tables in front. Main does the
same (pre-existing).
index) is still appendedat the end of its table. Main does the same (pre-existing; an
existing test pins the append order).
out of order later in the file, and a clamped table inside an element
the user both edited and shifted, can print in a different spot than
the original. Neither shape comes from
pip lock,uvorpdmwriters.
Not addressed: the issue's side note about comment decor (Poetry
routine, ledger #309) beyond keeping the live key's own comment on an
in-place restore.
https://claude.ai/code/session_01QaZG4zyZ3anK6ChQi3KwCF
Generated by Claude Code