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

Use the _pb2 suffix in the file name for protocol files generated with protoc #14

Closed
ao2 opened this issue Aug 25, 2016 · 7 comments
Closed

Comments

@ao2
Copy link
Contributor

ao2 commented Aug 25, 2016

Hi,

since python-protobuf 3.0.0 landed in Debian, programs using python-axolotl fail with a message like this:

$ gajim
Traceback (most recent call last):
  File "gajim.py", line 533, in <module>
    interface.run()
  File "/usr/share/gajim/src/gui_interface.py", line 2733, in run
    gajim.plugin_manager = plugins.PluginManager()
  File "/usr/share/gajim/src/plugins/helpers.py", line 129, in __call__
    cls.instance=super(Singleton, cls).__call__(*args,**kw)
  File "/usr/share/gajim/src/plugins/pluginmanager.py", line 104, in __init__
    pc = PluginManager.scan_dir_for_plugins(path)
  File "/usr/share/gajim/src/plugins/helpers.py", line 114, in wrapper
    result = f(*args, **kwargs)
  File "/usr/share/gajim/src/plugins/pluginmanager.py", line 454, in scan_dir_for_plugins
    module = __import__(module_name)
  File "/usr/share/gajim/src/common/demandimport.py", line 95, in _demandimport
    return _import(name, globals, locals, fromlist, level)
  File "/home/ao2/.local/share/gajim/plugins/omemo/__init__.py", line 50, in <module>
    from omemo.state import OmemoState
  File "/usr/share/gajim/src/common/demandimport.py", line 114, in _demandimport
    mod = _origimport(name, globals, locals)
  File "/home/ao2/.local/share/gajim/plugins/omemo/omemo/state.py", line 29, in <module>
    from axolotl.protocol.prekeywhispermessage import PreKeyWhisperMessage
  File "/usr/share/gajim/src/common/demandimport.py", line 114, in _demandimport
    mod = _origimport(name, globals, locals)
  File "/usr/lib/python2.7/dist-packages/axolotl/protocol/prekeywhispermessage.py", line 5, in <module>
    from .whispermessage import WhisperMessage
  File "/usr/share/gajim/src/common/demandimport.py", line 112, in _demandimport
    return _origimport(name, globals, locals, fromlist, level)
  File "/usr/lib/python2.7/dist-packages/axolotl/protocol/whispermessage.py", line 4, in <module>
    from . import whisperprotos
  File "/usr/share/gajim/src/common/demandimport.py", line 112, in _demandimport
    return _origimport(name, globals, locals, fromlist, level)
  File "/usr/lib/python2.7/dist-packages/axolotl/protocol/whisperprotos.py", line 41, in <module>
    options=None),
  File "/usr/lib/python2.7/dist-packages/google/protobuf/descriptor.py", line 501, in __new__
    _message.Message._CheckCalledFromGeneratedFile()
TypeError: Descriptors should not be created directly, but only retrieved from their parent.

The same happens when running the tests with python2.7 -m unittest discover -v.

As analyzed in Debian bug #835271 (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=835271) it looks like this is an issue in python-axolotl rather than in python-protobuf itself, basically the newer protobuf code checks that the generated protocol files have the _pb2.py suffix in their names, and this does not happen in python-axolotl.

The issue happens only when PyPy is not used (https://github.com/google/protobuf/blob/2fe0556c7abbe31c02147b9171397737a35bfe3b/python/google/protobuf/pyext/descriptor.cc#L94) which apparently is the case for the python-protobuf Debian package.

This behavior also looks consistent with the protbuf documentation: https://developers.google.com/protocol-buffers/docs/pythontutorial#compiling-your-protocol-buffers even if the doc does not state clearly that having the _pb2 suffix is a requirement.

Anyway, renaming the generated protbuf files fixes the issue, see this patch:
https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=835271;filename=0001-Keep-the-_pb2-suffix-in-the-name-for-protobuf-genera.patch;msg=22

Should I submit a pull request? Against the develop branch?

Another question: the protobuf documentation suggests to use the same base name used for the .proto files also in the generated files, in python-axolotl that would mean having WhisperTextProtocol_pb2.py instead of whisperprotos.py and LocalStorageProtocol_pb2.py instead of storageprotos.py; or alternatively have the protocol files named whisperprotos.proto and storageprotos.proto.

Do you want to follow this scheme that makes it clear what the generated files are, or do you want to keep the current names?

Thanks,
Antonio

@fuelflo
Copy link

fuelflo commented Oct 6, 2016

+1

@lovetox
Copy link
Contributor

lovetox commented Oct 18, 2016

@tgalal are you alive?

@dllud
Copy link

dllud commented Oct 31, 2016

Ubuntu 16.10 brings python-protobuf 3.0.0. Now even more python-axolotl users are facing this issue.

Antonio's patch works great, but one cannot ask non tech-savvy users to patch python-axolotl themselves.

@tgalal could you please merge these changes? It seems to be a quick job.

@ao2
Copy link
Contributor Author

ao2 commented Oct 31, 2016

@dllud the python-axolotl package in Debian unstable (0.1.35-1) provides the patch already, ask the Ubuntu maintainers to update the package.

@tgalal the referred patch might not be the final one, see the questions I asked at the end of my first message.

Thanks,
Antonio

@dllud
Copy link

dllud commented Oct 31, 2016

@ao2 Thanks. I've opened a bug report in Ubuntu's bugtracker: Bug #1637958 “Use _pb2 suffix in protobuf's generated files” : Bugs : python-axolotl package : Ubuntu

@rafaelsilverioit
Copy link

I have sent a pull request to master branch, since another people may face the same issue. #17

While it is not merged here, you can clone my master branch.

@tgalal
Copy link
Owner

tgalal commented Mar 22, 2017

@ao2 sorry it took me too long. I merged your patch, I think the names and everything is fine now as it is in your patch. Thanks a lot!

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

No branches or pull requests

6 participants