Version 2.5.0-5ubuntu1 breaks python code using the cpp module

[dobey]

The 2.5.0-5ubuntu1 upload in proposed includes the following change:

  * python-protobuf: switch back to the pure Python implementation, as
    upstream appears to no longer be maintaining the current C++ based Python
    binding. See the following upstream issues for details:
    - https://code.google.com/p/protobuf/issues/detail?id=434
    - https://code.google.com/p/protobuf/issues/detail?id=503

However, the cpp_message.py is still included, and tries to import the C++ based module which no longer exists, which results in the following failure in the ubuntuone-storage-protocol tests:

Traceback (most recent call last):
  File "/usr/bin/u1trial", line 40, in <module>
    main()
  File "/usr/lib/python2.7/dist-packages/ubuntuone-dev-tools/ubuntuone/devtools/runners/__init__.py", line 277, in main
    suite = test_runner.get_suite(options)
  File "/usr/lib/python2.7/dist-packages/ubuntuone-dev-tools/ubuntuone/devtools/runners/__init__.py", line 173, in get_suite
    config['ignore-paths']))
  File "/usr/lib/python2.7/dist-packages/ubuntuone-dev-tools/ubuntuone/devtools/runners/__init__.py", line 157, in _collect_tests
    module_suite = self._load_unittest(filepath)
  File "/usr/lib/python2.7/dist-packages/ubuntuone-dev-tools/ubuntuone/devtools/runners/__init__.py", line 98, in _load_unittest
    module = __import__(modpath, None, None, [""])
  File "/tmp/adt-run.i9ormp/dsc0-build/ubuntuone-storage-protocol-13.10/tests/test_bytesproducer.py", line 42, in <module>
    from ubuntuone.storageprotocol import client, protocol_pb2
  File "/tmp/adt-run.i9ormp/dsc0-build/ubuntuone-storage-protocol-13.10/ubuntuone/storageprotocol/client.py", line 51, in <module>
    from ubuntuone.storageprotocol import (
  File "/tmp/adt-run.i9ormp/dsc0-build/ubuntuone-storage-protocol-13.10/ubuntuone/storageprotocol/delta.py", line 35, in <module>
    from ubuntuone.storageprotocol import protocol_pb2
  File "/tmp/adt-run.i9ormp/dsc0-build/ubuntuone-storage-protocol-13.10/ubuntuone/storageprotocol/protocol_pb2.py", line 4, in <module>
    from google.protobuf import descriptor as _descriptor
  File "/usr/lib/python2.7/dist-packages/google/protobuf/descriptor.py", line 45, in <module>
    from google.protobuf.internal import cpp_message
  File "/usr/lib/python2.7/dist-packages/google/protobuf/internal/cpp_message.py", line 39, in <module>
    from google.protobuf.internal import _net_proto2___python
ImportError: cannot import name _net_proto2___python
make[1]: *** [override_dh_auto_test] Error 1

It seems like this python module should instead issue a DeprecationWarning and fall back to using the pure python implementation, whilst code which uses the cpp module in some way, transitions over to the pure python implementation.

[Launchpad Janitor (janitor)]

This bug was fixed in the package protobuf - 2.5.0-5ubuntu2

---------------
protobuf (2.5.0-5ubuntu2) trusty; urgency=medium

  * Remove cpp_message.py from the python bindings package, since it requires
    the C++ module implementation that we're no longer shipping.
    LP: #1275826.
 -- Steve Langasek <email address hidden> Mon, 03 Feb 2014 20:17:09 +0000

[Martin Pitt (pitti)]

Seems this was not quite sufficient? It now fails differently:

Traceback (most recent call last):
  File "/usr/bin/u1trial", line 40, in <module>
    main()
  File "/usr/lib/python2.7/dist-packages/ubuntuone-dev-tools/ubuntuone/devtools/runners/__init__.py", line 277, in main
    suite = test_runner.get_suite(options)
  File "/usr/lib/python2.7/dist-packages/ubuntuone-dev-tools/ubuntuone/devtools/runners/__init__.py", line 173, in get_suite
    config['ignore-paths']))
  File "/usr/lib/python2.7/dist-packages/ubuntuone-dev-tools/ubuntuone/devtools/runners/__init__.py", line 157, in _collect_tests
    module_suite = self._load_unittest(filepath)
  File "/usr/lib/python2.7/dist-packages/ubuntuone-dev-tools/ubuntuone/devtools/runners/__init__.py", line 98, in _load_unittest
    module = __import__(modpath, None, None, [""])
  File "/tmp/adt-run.aeAq8m/dsc0-build/ubuntuone-storage-protocol-13.10/tests/test_delta_info.py", line 35, in <module>
    from ubuntuone.storageprotocol import (
  File "/tmp/adt-run.aeAq8m/dsc0-build/ubuntuone-storage-protocol-13.10/ubuntuone/storageprotocol/protocol_pb2.py", line 4, in <module>
    from google.protobuf import descriptor as _descriptor
  File "/usr/lib/python2.7/dist-packages/google/protobuf/descriptor.py", line 45, in <module>
    from google.protobuf.internal import cpp_message
ImportError: cannot import name cpp_message

https://jenkins.qa.ubuntu.com/job/trusty-adt-ubuntuone-storage-protocol/14/ARCH=amd64,label=adt/console

[dobey (dobey)]

@Martin, that's interesting, since that code is generated by protobuf-compiler. Looks like it also needs to be updated to not generate code that imports that module directly. But I think that's a different bug than this one specifically.

[Steve Langasek (vorlon)]

Yes, this is a bug in protobuf for allowing the behavior of the library to be permuted in an unsupported way by an environment variable.

But equally, it is a bug in ubuntuone-storage-protocol for testing paths that are irrelevant to its operation.

u1-storage-protocol should be fixed to drop the gratuitous tests.

[dobey (dobey)]

@Steve, the path is relevant to the operation of ubuntuone-storage-protocol as it is being used on the servers (which are obviously not running on trusty yet). The same ubuntuone-storage-protocol code is run on both clients and servers, and so we need to ensure it does not break on either place (which is why the tests are being run with the cpp module at all).

[Martin Pitt (pitti)]

> by an environment variable

Oh, that's the PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=cpp thing in run-tests? I just looked for an env variable and found this, that looks very plausible. So u1-s-p needs to drop that then, or perhaps better conditionalize it on the version of protobuf that is installed?. Thanks for pointing out!

[Matthias Klose (doko)]

ftbfs, see
https://launchpadlibrarian.net/167353879/buildlog_ubuntu-trusty-i386.ubuntuone-storage-protocol_13.10-0ubuntu2_FAILEDTOBUILD.txt.gz

[Steve Langasek (vorlon)]

Hi Rodney,

> the path is relevant to the operation of ubuntuone-storage-protocol as it is being
> used on the servers (which are obviously not running on trusty yet). The same
> ubuntuone-storage-protocol code is run on both clients and servers, and so we need to
> ensure it does not break on either place (which is why the tests are being run with the
> cpp module at all).

Just so I understand: does this mean that somewhere (where - server?), you are overriding the default behavior of the protobuf module by using this environment variable?

[dobey (dobey)]

On Mon, 2014-02-24 at 19:12 +0000, Steve Langasek wrote:
> Just so I understand: does this mean that somewhere (where - server?),
> you are overriding the default behavior of the protobuf module by using
> this environment variable?

Yes. The cpp module makes things significantly faster on the server, so
it is being used there. The servers are all still running on 12.04
though, and probably will be for some time.

[Launchpad Janitor (janitor)]

This bug was fixed in the package ubuntuone-storage-protocol - 13.10-0ubuntu3

---------------
ubuntuone-storage-protocol (13.10-0ubuntu3) trusty; urgency=medium

  * debian/patches/00_cpp-tests-check.patch:
    - Avoid running the tests with the protobuf cpp module on newer
      versions of protobuf where the cpp module was removed. (LP: #1275826)
  * debian/control:
    - Bump Standards-Version to 3.9.5.
 -- Rodney Dawes <email address hidden> Mon, 24 Feb 2014 14:24:11 -0500

[Colin Watson (cjwatson)]

This appears to have been fixed properly with the new C++-based module in 2.6.0. I've just synced that (actually 2.6.1-1) into vivid after checking that all our other remaining patches are obsolete.

[Steve Langasek (vorlon)]

We never got around to this while 14.04 was the current LTS. It's fixed in 16.04 LTS. Marking wontfix as it seems unlikely to be a priority now.