Protobuf 2.6.0 incompatibility issues with Python 3.x #7

Closed
KJaunay opened this Issue Sep 2, 2014 · 17 comments

Comments

Projects
None yet

KJaunay commented Sep 2, 2014

I've run into a problem when using Python 3.4 and protobuf 2.6.0.
I get several errors including:
protobuf/python/google/protobuf/internal/decoder.py, line 167
_DecodeVarint = _VarintDecoder((1 << 64) - 1, long)
NameError: name 'long' is not defined

@xfxyjwf xfxyjwf added this to the Python 3 support milestone Sep 3, 2014

Member

gpshead commented Sep 3, 2014

Blanket Python 3 support was unintentionally listed in these release notes. We need to fix that up for a 2.6.x release. The code itself is ready with some 2to3 use, the setup.py and deps are not done yet. Sorry about that!

Here's my summary:

  • Support for Python 3 is almost in place. Our setup.py does not yet do the right thing for 2to3 conversions with Python 3 but for the adventurous, manually run the following files through 2to3 and it should work in Python 3.3+:

protobuf/descriptor.py
protobuf/internal/cpp_message.py
protobuf/internal/decoder.py
protobuf/internal/python_message.py
protobuf/internal/type_checkers.py
protobuf/internal/message_cpp2_test.py
protobuf/internal/message_factory_test.py
protobuf/internal/message_factory_cpp2_test.py
protobuf/internal/message_factory_python_test.py
protobuf/internal/message_python_test.py
protobuf/internal/message_test.py
protobuf/internal/reflection_cpp2_generated_test.py
protobuf/internal/reflection_python_test.py
protobuf/internal/reflection_test.py
protobuf/internal/test_util.py
protobuf/internal/text_format_test.py
protobuf/message_factory.py
protobuf/text_encoding.py
protobuf/text_format.py

setup.py also specifies an external dep on google-apputils needed in order to run the unittests. We need to push out an updated version of that with Python 3 support.

Any specific reason for wanting to use 2to3 instead of something like https://github.com/python-modernize/python-modernize to do the transition once?

@borntyping borntyping referenced this issue in borntyping/python-riemann-client Sep 5, 2014

Closed

python3 support #15

mrovner commented Sep 6, 2014

It says
"Modernizes Python code for eventual Python 3 migration. Build on top of
2to3"

and it's not apparent why it's better ;)

Also 2to3 support is in setuptools already.

On Thu, Sep 4, 2014 at 6:59 AM, Brett Cannon notifications@github.com
wrote:

Any specific reason for wanting to use 2to3 instead of something like
https://github.com/python-modernize/python-modernize to do the transition
once?


Reply to this email directly or view it on GitHub
#7 (comment).

Thanks,
--Mike

By unifying the code base to work on both Python 2 and 3 it makes development easier and installation faster (unless you release separate Python 2 and 3 packages). By not having to wait for 2to3 you eliminate a compile step during development. As for installation, you can forgo that step entirely. Plus the coding practices used in converted source is more in line with modern best practices and syntax.

Kentzo commented Sep 26, 2014

I wonder if we can reuse code from my https://github.com/GreatFruitOmsk/protobuf-py3

Contributor

tseaver commented Nov 18, 2014

I'm +1 on @Kentzo's approach, and would volunteer to help review #66. As @brettcannon says, avoiding 2to3 is pretty much the "widely accepted" best practice for libraries which need to support both Python2 and Python3: e.g., see Lennart Regebro's excellent outline of strategies.

I would love to see a python 3 compatible official protobuf available by Christmas.

FWIW, using the current trunk of protobuf, the only change I had to make to the package after installation was change a number of str("", "utf-8") references in descriptor_pb2.py. Oddly enough, the protobuf I have installed from homebrew (2.6.1) generates _pb2.py files with a function that looks like this:

_b=sys.version_info[0]<3 and (lambda x:x) or (lambda x:x.encode('latin1'))

And it uses that to encode strings instead like this:

default_value=_b("").decode('utf-8')

So.. not sure why the descriptor_pb2.py file is out of date. If someone could push those changes into that file, then I believe trunk would at least work on python3 -- but obviously #66 would be even better.

Contributor

tseaver commented Nov 30, 2014

That function should not be encoding literals as latin under Py3k, especially since they are going to be immediately decoded as utf-8. Use of latin1 in anything but edgiest cases (decoding wonky / unknown data, or encoding for antique wire protocols) is a sign of papered-over trouble: "mojibake ahead", it could read.

So, I stand corrected. When I install protobuf properly, it installs/runs from trunk without any noticable problems now. Python 3.4.2 on OSX. Now someone just needs to push an updated package to pypi... :)

It would be nice to get better python3 support in trunk to support the edge cases properly, as @tseaver mentions.

Contributor

Ivoz commented Dec 2, 2014

@gpshead would you be fine with only supporting python 2.6+? Current readme.txt mentions 2.4.

Single-source cross python support is, FTR, like 10x easier if you only support python 2.6+ and 3.3+.

Member

gpshead commented Dec 14, 2014

Absolutely, 2.6 is a good minimum. Anything older is obsolete (I consider 2.6 obsolete as well, but it is easy enough to support).

One of the reasons we started with some files needing to be run through 2to3 is that we have a few internal things still running on the old App Engine 2.5 legacy runtime that needed proto2 code and using 2to3 made maintaining 2.5 compatibility easier. I believe we have dealt with those internally now but I need to double check. We should absolutely be able to make a single source 2.6, 2.7, >= 3.3+ compatible version in the future.

Contributor

tseaver commented Jan 30, 2015

FWIW, #164 modernizes the python code to drop all the PY25 stuff.

@xfxyjwf xfxyjwf added the obsolete label Dec 22, 2015

@xfxyjwf xfxyjwf closed this Dec 22, 2015

thanatos commented Jul 13, 2016

Why is this bug marked as Closed & obsolete? I'm currently trying to get a third-party package (heka_py) that uses protobuf to work, and near as I can tell, the latest version is not Python 3 compatible.

There's a package python3_protobuf on PyPI (I have no idea if this is official or not) but it is on Protobuf 2.5, and unfortunately Heka assumes 2.6; so that doesn't work. protobuf itself doesn't support 3, so that also doesn't work. Near as I can tell, protocol buffers and Python 3 don't mix.

@thanatos https://pypi.python.org/pypi/protobuf pip install protobuf==3.0.0b3 is python3-compatible.

thanatos commented Jul 15, 2016

@thanatos https://pypi.python.org/pypi/protobuf pip install protobuf==3.0.0b3 is python3-compatible.

While technically true, my understanding is that the major number here is bumped because it isn't backwards-compatible with compiled protocol buffers from protobuf v2; empirically, this seems to be the case; e.g., <a Message derived class>.INTEGER doesn't exist in protobuf==3.0.0b3.

(Did it change to VALUE_INTEGER_FIELD_NUMBER? (Nope.) Perhaps if this is the only change that heka_py requries, I might be able to make a small patch to heka_py to make it protobuf==3.0.0b3 compatible? But the problem seems to still stand — if I pull in a library that relies on protobuf, that restricts me to legacy Python.)

Member

xfxyjwf commented Jul 15, 2016

On Fri, Jul 15, 2016 at 2:41 PM, Roy Wellington Ⅳ notifications@github.com
wrote:

@thanatos https://github.com/thanatos
https://pypi.python.org/pypi/protobuf pip install protobuf==3.0.0b3 is
python3-compatible.

While technically true, my understanding is that the major number here is
bumped because it isn't backwards-compatible with compiled protocol buffers
from protobuf v2; empirically, this seems to be the case; e.g., .INTEGER doesn't exist in protobuf==3.0.0b3.

We bumped the major version number because of the new proto3 syntax
support, not because it's incompatible. If you find incompatible issues
with old generated code, please help file a bug for it.


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#7 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AIJ6iiG4RJcNqEfQ0dyXn0benZXh40lIks5qV_6bgaJpZM4Cdsma
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment