Skip to content

fix: make Tags pickle-safe#1170

Merged
henryiii merged 2 commits into
pypa:mainfrom
henryiii:henryiii/fix/pickletags
Apr 24, 2026
Merged

fix: make Tags pickle-safe#1170
henryiii merged 2 commits into
pypa:mainfrom
henryiii:henryiii/fix/pickletags

Conversation

@henryiii
Copy link
Copy Markdown
Contributor

@henryiii henryiii commented Apr 20, 2026

Similar to #1168, but for Tags.

🤖 Using Copilot (auto model)

CC @eachimei.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds explicit pickle serialization support for packaging.tags.Tag, aligning it with the project’s recent “pickle-safe” work so that cached fields (like _hash) don’t break equality/hash behavior after unpickling and older pickles remain loadable.

Changes:

  • Implement Tag.__getstate__ to serialize only the core (interpreter, abi, platform) fields.
  • Extend Tag.__setstate__ to accept the new tuple format and the older (None, {slot: value}) slot-dict format.
  • Add tests covering round-trip behavior, invalid state rejection, and loading embedded “old-format” pickle bytes.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/packaging/tags.py Adds __getstate__ and expands __setstate__ to support new + legacy pickle state formats while recomputing _hash.
tests/test_tags.py Adds round-trip, invalid-state, and embedded-old-pickle compatibility tests for Tag.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/packaging/tags.py Outdated
Comment thread tests/test_tags.py


# Pickle bytes generated with packaging==25.0, Python 3.13.1, pickle protocol 2.
# Format: plain __dict__ (no __slots__).
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment says the packaging==25.0 pickle uses a plain __dict__ (no __slots__), but the embedded pickle bytes build a 2-tuple state (None, {...}) (e.g., N}...\x86), which corresponds to the __slots__-style state rather than a plain dict state. Please either regenerate the bytes from an actual 25.0 Tag pickle (and adjust Tag.__setstate__ accordingly if needed) or fix the comment/label so the test reflects the real format being exercised.

Suggested change
# Format: plain __dict__ (no __slots__).
# Format exercised by these bytes: __slots__-style state (None, {slot: value}).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@henryiii henryiii Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an amazing catch, by the way. It was copied from the wrong version, I regenerated it from 25.0. (I believe this was the one I added by hand)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really amazing!

@henryiii henryiii force-pushed the henryiii/fix/pickletags branch 2 times, most recently from 513d663 to 570567d Compare April 24, 2026 00:13
Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
Assisted-by: Copilot:claude-hauku-4.5
Assisted-by: Copilot:Kimi-K2.6
Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
@henryiii henryiii force-pushed the henryiii/fix/pickletags branch from 570567d to d099466 Compare April 24, 2026 01:26
@henryiii henryiii merged commit 823b44e into pypa:main Apr 24, 2026
57 checks passed
@henryiii henryiii deleted the henryiii/fix/pickletags branch April 24, 2026 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants