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

Fix Python 3.10+ issues from PY_SSIZE_T_CLEAN not being set #1671

Merged
merged 2 commits into from
Apr 20, 2022

Conversation

chrisburr
Copy link
Contributor

Writing files with XRootD and Python 3.10 currently fails with:

In [4]: with XRootD.client.File() as file:
   ...:     status, _ = file.open(f"{eos.url}{dest}", OpenFlags.NEW)
   ...:     assert status.ok, status
   ...:     status, _ = file.write(new_data)
   ...:     assert status.ok, status
   ...:
---------------------------------------------------------------------------
SystemError                               Traceback (most recent call last)
Input In [4], in <cell line: 1>()
      2 status, _ = file.open(f"{eos.url}{dest}", OpenFlags.NEW)
      3 assert status.ok, status
----> 4 status, _ = file.write(new_data)
      5 assert status.ok, status

File ~/mambaforge/envs/logse-sqlite/lib/python3.10/site-packages/XRootD/client/file.py:190, in File.write(self, buffer, offset, size, timeout, callback)
    187   callback = CallbackWrapper(callback, None)
    188   return XRootDStatus(self.__file.write(buffer, offset, size, timeout, callback))
--> 190 status, response = self.__file.write(buffer, offset, size, timeout)
    191 return XRootDStatus(status), None

SystemError: PY_SSIZE_T_CLEAN macro must be defined for '#' formats

This PR fixes the issue by defining PY_SSIZE_T_CLEAN as suggested. Though I'm leaving it as a draft at the moment as I still need to change int to Py_ssize_t in the various places to be fully correct.

There is no issue with backwards compatibilty as support for PY_SSIZE_T_CLEAN was added in Python 2.5

See the release notes and PEP-353 for details.

@simonmichal
Copy link
Contributor

@chrisburr : thanks a lot for the PR, just let me know once it's ready for review!

@chrisburr chrisburr marked this pull request as ready for review April 13, 2022 16:17
@chrisburr
Copy link
Contributor Author

I think this is ready.

The test failure seems to be unrelated but if it's not expected to fail I'll look into it further.

@simonmichal simonmichal merged commit cbe1b62 into xrootd:master Apr 20, 2022
@chrisburr chrisburr deleted the patch-5 branch April 20, 2022 07:44
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.

None yet

2 participants