Skip to content

Fixed #299 -- work on very old python2.7s :-( #300

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

Merged
merged 1 commit into from
Apr 16, 2018

Conversation

alex
Copy link
Contributor

@alex alex commented Apr 15, 2018

Tested on debian wheezy (which is a royal PITA). There's some other failing tests (all related to memoryview(array.array(...))), but this fixes all of the exceptions seen in #299

@alex
Copy link
Contributor Author

alex commented Apr 15, 2018

FWIW, I structured the code like this so we only have the overhead of creating the buffer object on Pythons where it's relevant, and there's no additional branching in every single msgpack opcode.

@jfolz
Copy link
Contributor

jfolz commented Apr 15, 2018

Looks like a really simple patch that's easy to remove when 2.7 support is dropped.

@methane methane merged commit b10cf78 into msgpack:master Apr 16, 2018
@alex alex deleted the fix-old-python-bytearray branch April 16, 2018 03:19
@alex
Copy link
Contributor Author

alex commented Apr 16, 2018

Thanks!

@@ -234,6 +234,12 @@ def __init__(self, file_like=None, read_size=0, use_list=True, raw=True,

#: array of bytes fed.
self._buffer = bytearray()
# Some very old pythons don't support `struct.unpack_from()` with a
# `bytearray`. So we wrap it in a `buffer()` there.
if sys.version_info < (2, 7, 6):
Copy link

Choose a reason for hiding this comment

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

Note that the issue occurs on OSX with python 2.7.10... apparently this fix will not apply to OSX's /usr/bin/python

~: /usr/bin/python -c 'import sys; print(sys.version_info)'
sys.version_info(major=2, minor=7, micro=10, releaselevel='final', serial=0)

@alex
Copy link
Contributor Author

alex commented Apr 16, 2018 via email

@zsimic
Copy link

zsimic commented Apr 16, 2018 via email

@methane
Copy link
Member

methane commented Apr 17, 2018

On 10.13.4 (High Sierra),

11:07 $ /usr/bin/python
Python 2.7.10 (default, Oct  6 2017, 22:29:07)
[GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.31)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import struct
>>> struct.unpack_from("i", bytearray("1234"))
(875770417,)

unpack_from supports bytearray.
I have Sierra MacBook at my office. I'll try on it too.

@methane
Copy link
Member

methane commented Apr 17, 2018

On 10.12.6 (Sierra):

$ /usr/bin/python2.7
Python 2.7.10 (default, Feb  7 2017, 00:08:15)
[GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.34)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import struct
>>> struct.unpack_from("i", bytearray("1234"))
(875770417,)

@zsimic Is your python really 2.7.10?
virtualenv copies python. It means you may use old python if you created the virtualenv with old Python.
What .venv/bin/python -V prints?

@zsimic
Copy link

zsimic commented Apr 17, 2018

You're right @methane it does work with stock python 2.7.10

I have a pyenv-installed 2.7.0 (it's a recent pyenv install, don't know why it picked 2.7.0), and 2.7.0 was selected when I observed the issue, not my system python.

Sorry about the confusion

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