New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 674760 link

Starred by 3 users

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 0
Type: Bug

Blocked on:
issue 703256

Blocking:
issue 669669
issue 682318



Sign in to add a comment

chromite uses system protobuf instead of the bundled one (only partially)

Project Member Reported by pprabhu@chromium.org, Dec 15 2016

Issue description

protobuf module is bundled with chromite under third_party/google/protobuf.
But, we actually end up getting protobuf from dist-packages.

Worst, this is an older version (on our builders) than the one bundled with chromite.

This resulted in a weird bug that is blocking ts_mon roll (https://chromium-review.googlesource.com/#/c/418469/):
- ts_mon tries:
 from google.protobuf import symbol_database
- system protobuf doesn't have this module, so it gets picked up from chromite's third_party/google/protobuf.
- This module tries to grab another protobuf module protobuf.descriptor_pool.Default()
- Since this module exists in the system installation, that one is picked. But the older version (pre 3.0.01b) didn't have Default(), so we end up with AttributeException.

Can repro with: crosreview.com/420491
Example builder run: http://shortn/_j87OQy3dIb
 
The reason is that google is a namespace module installed via debian packages. Just fixing up paths as we do for chromite/autotest is not enough to correctly register a module in the namespace (and override the one from the system).

For more context see:
https://github.com/google/protobuf/issues/1484
http://setuptools.readthedocs.io/en/latest/pkg_resources.html#namespace-package-support

The workaround mentioned in the bug hasn't worked for me yet.
Of course, the right solution is to go venv.
Cc: ayatane@chromium.org
Blocking: 669669
Labels: -current-issue
Cc: akes...@chromium.org
Blocking: 682318
Owner: ayatane@chromium.org

Comment 8 by pho...@chromium.org, Mar 15 2017

Labels: -Pri-2 Pri-0
We need to upgrade ts-mon due to https://bugs.chromium.org/p/chromium/issues/detail?id=652509. The old endpoint will be turned off in a couple of weeks.

Comment 9 by pho...@chromium.org, Mar 16 2017

Owner: pho...@chromium.org
Blockedon: 703256
Project Member

Comment 11 by bugdroid1@chromium.org, Mar 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/57b6254b22644db28ede27de18a066afb91c5c35

commit 57b6254b22644db28ede27de18a066afb91c5c35
Author: Paul Hobbs <phobbs@google.com>
Date: Tue Mar 21 12:02:07 2017

Fix dependency hell for google.protobuf

Use pkg_resources to create a namespace module for google and
google.protobuf, and then fix up the .__path__ attributes of the
modules. If we import the modules normally, the .__path__ attributes of
the modules have the wrong ordering; namely, the global packages are
ahead of the local third_party package in the __path__ list despite
being after them in sys.path. By creating the namespace modules ahead of
time, the ordering matches that of sys.path and prevents out-of-date
global packages from shadowing the third_party packages.

BUG= chromium:674760 
TEST="import google.protobuf.symbol_database" succeeds now.

Change-Id: I135b1ef7af18fc504b84e3f390d6c1758cb1d834
Reviewed-on: https://chromium-review.googlesource.com/456757
Commit-Ready: Paul Hobbs <phobbs@google.com>
Tested-by: Paul Hobbs <phobbs@google.com>
Reviewed-by: Paul Hobbs <phobbs@google.com>

[modify] https://crrev.com/57b6254b22644db28ede27de18a066afb91c5c35/__init__.py

Project Member

Comment 12 by bugdroid1@chromium.org, Mar 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/8d91aa7ee3820f707629b53bb881e983f09e7c5b

commit 8d91aa7ee3820f707629b53bb881e983f09e7c5b
Author: Paul Hobbs <phobbs@google.com>
Date: Tue Mar 21 22:37:20 2017

Fix google.protobuf's __path__

Previously, the __path__ was set to the directory containing the __file__ of the
__init__ module for google.protobuf; this logic assumed that it would import the
third-party google.protobuf module. This is not always correct, so instead we
should filter out the entries of the path that aren't in the third_party
directories list.

BUG= chromium:674760 
TEST="import chromite; import google.protobuf; print(google.protobuf.__path__)"

Change-Id: Ic90ce0e7f0e6c1699e8f3ca8f8491315794dad08
Reviewed-on: https://chromium-review.googlesource.com/457498
Tested-by: Paul Hobbs <phobbs@google.com>
Reviewed-by: Aviv Keshet <akeshet@chromium.org>
Reviewed-by: Allen Li <ayatane@chromium.org>
Trybot-Ready: Paul Hobbs <phobbs@google.com>

[modify] https://crrev.com/8d91aa7ee3820f707629b53bb881e983f09e7c5b/__init__.py

Status: Fixed (was: Started)

Comment 14 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 16 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment