fix: make Version pickle-safe and backward-compatible with pre-26.1 pickles#1163
fix: make Version pickle-safe and backward-compatible with pre-26.1 pickles#1163
Conversation
Implement __reduce__ on Version so it pickles as (Version, ('1.2.3',)),
always reconstructing via __init__. This prevents internal types and
cached values from leaking into the pickle stream.
Add parametrized pickle round-trip tests covering simple, pre-release,
post-release, dev, epoch, and local version strings.
Add a minimal _structures.py shim that provides InfinityType and NegativeInfinityType the exact class names referenced in pickles created with packaging <= 25.x. Implement __setstate__ on Version to restore only core version fields and discard the stale _key cache, which may contain old _structures objects. Handles both the old dict format (packaging <= 25.x) and the new __slots__ tuple format (packaging 26.x). Add tests using real pickle bytes generated with packaging 25.0 that verify old pickles load correctly, produce correct comparisons, and re-pickle cleanly without _structures references.
|
Won't have time to review this until the evening, but for performance could we serialize as parts and reconstruct via from parts? |
|
We should also add a 26.2 pickle (current state) to protect for the future. |
|
Yes, if we care about performance, a getstate/setstate pair could serialize to/from parts. Maybe we could even mimic the 26.1 shape to reduce the number of different shapes to handle down to 2. But I'm not sure how the performance compares to pickling itself, it might not be that much compared to the pickle process itself? |
Done in ba06290 |
Please see #1163 (comment) |
|
Addressing now the missing unit-test coverage from previous build... |
Done |
…support new pickle format
henryiii
changed the title
henryiii
left a comment
henryiii
left a comment
There was a problem hiding this comment.
I like this, will wait for @notatallshaw to take a look too.
|
I'm happy for this to be merged, I won't be able to thoroughly review until at least late this night, and I don't want to be a blocker, at a skim this looks good. |
3 participants
Summary
Fixes #1162 —
Versionobjects pickled withpackaging< 26.1 fail to unpickle after upgrading, because the removed_structures.pymodule is referenced in the pickle stream.Changes
This PR is structured as two independent commits so that commit 1 can land on its own if maintaining backward compatibility for old pickles (commit 2) is not desired.
Commit 1: Forward-fix —
__reduce____reduce__toVersion, serializing as(Version, ("1.2.3",))— string-based reconstruction via__init__, never persisting internal caches or private types.test_pickle_roundtripcovering simple, pre-release, post-release, dev, epoch, and local versions.Commit 2: Backward-fix —
_structures.pyshim +__setstate___structures.pyshim definingInfinityTypeandNegativeInfinityType— just enough to be unpicklable, not functional for comparisons. No import-time warning (would conflict withfilterwarnings = ["error"]in pytest config).__setstate__toVersionthat handles both old dict-based state (25.x) and tuple-based__slots__state (26.0), discarding_key_cacheand_hash_cacheso they recompute with the current_cmpkey.packaging==25.0andpackaging==26.0(withhash(v)called to force_key_cachepopulation embeddingInfinityType/NegativeInfinityTypereferences).__reduce__-based pickle with no_structuresreferences.Testing
python -m pytest tests/test_version.py -x)