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

Python 3.12 patch #1158

Merged
merged 11 commits into from
Dec 5, 2023
Merged

Python 3.12 patch #1158

merged 11 commits into from
Dec 5, 2023

Conversation

Thrameos
Copy link
Contributor

@Thrameos Thrameos commented Dec 5, 2023

This should fix Python 3.12 though not without serious wrinkles. The new PyLong mutates a field in such a way that it will break anything that wants to add memory to the tail of the object to save a few bytes. I though that given that we were following the exact mechanism that Python requires for its own structures that we would be safe. The mutation is entirely in private parts of Python so I can't just use the private symbols so we track with any changes. It doesn't just break our code but also the dictoffset and weakoffset fields in the Python base class, so I can't see that it will remain in the current state. Thus this is at best a version that will work until the repair to the memory model is done, and then we likely have to rework another time.

Changes include:

  • switch to mallinfo2 to avoid deprecation warning.
  • change to PyLong model
  • A shift in PyBuffer models that came in 3.9 but didn't burn us until now.
  • Fix for wstr which may not be well covered
  • Disable part of the doc system until I can figure out what Java 17 did
  • Update the tests to avoid deprecation warnings

@Thrameos Thrameos added the bug Unable to deliver desired behavior (crash, fail, untested) label Dec 5, 2023
@Thrameos Thrameos self-assigned this Dec 5, 2023
Copy link

codecov bot commented Dec 5, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (c11c3a2) 87.84% compared to head (b5c6083) 87.83%.

Files Patch % Lines
native/python/pyjp_module.cpp 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1158      +/-   ##
==========================================
- Coverage   87.84%   87.83%   -0.02%     
==========================================
  Files         112      112              
  Lines       10276    10283       +7     
  Branches     4032     4034       +2     
==========================================
+ Hits         9027     9032       +5     
- Misses        698      699       +1     
- Partials      551      552       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@marscher marscher left a comment

Choose a reason for hiding this comment

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

Thank you so much @Thrameos. Could you please merge latest master to enable testing for Python 3.12?
I think we should add a nightly version of Python to our matrix (if that'd be installable easily) in order to see such upcoming problems earlier. However this could also introduce lots of noise during the development of a new Python minor (which is apprently a major in semantic versioning related to the amount of breaking changes).

@marscher marscher merged commit 33597f8 into master Dec 5, 2023
24 of 26 checks passed
@marscher marscher deleted the py12 branch December 5, 2023 17:40
@marscher
Copy link
Member

marscher commented Dec 5, 2023

Thanks again! This will make a lot of people happy I'd guess 👍

@pelson
Copy link
Contributor

pelson commented Dec 8, 2023

Looks like it may be this change which started a Python 3.7 nightly test to fail. Example in https://gitlab.cern.ch/scripting-tools/pyjapc/-/jobs/34505253.

I'm pinned back right now, but can investigate later next week if needed.

@Thrameos
Copy link
Contributor Author

Thrameos commented Dec 8, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unable to deliver desired behavior (crash, fail, untested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants