Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-125063: marshal: Add version 5, improve documentation #126829

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

encukou
Copy link
Member

@encukou encukou commented Nov 14, 2024

This builds on #126804, but I followed the rabbit hole much deeper.

In Python 3.14, slice objects can be marshalled.
This calls for a new version of the marshal format, so that backwards-incompatible
objects are rejected at serialization time.

  • Bump marshal version to 5
  • Reject slices in earlier versions
  • Reorganize the TYPE_ defines to make it easier to contribute
  • Adjust and improve the documentation.

📚 Documentation preview 📚: https://cpython-previews--126829.org.readthedocs.build/

Comment on lines +124 to 125
assert(Py_MARSHAL_VERSION >= 5);
PyObject *marshalled = PyMarshal_WriteObjectToString(code, Py_MARSHAL_VERSION);
Copy link
Member Author

@encukou encukou Nov 14, 2024

Choose a reason for hiding this comment

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

In the Makefile, _freeze_module doesn't depend on marshal.h, but it needs to be rebuilt when Py_MARSHAL_VERSION switches to 5.
Instead of properly changing the Makefile, which would be another rabbit hole, I decided to touch _freeze_module.c instead.

@encukou encukou added docs Documentation in the Doc dir and removed docs Documentation in the Doc dir labels Nov 14, 2024
Copy link
Contributor

@mdboom mdboom left a comment

Choose a reason for hiding this comment

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

Looks good to me. I had no idea changing marshal required all this, so I appreciate the backfill.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

2 participants