I've seen it before. It is probably due to the protobuf version mismatch -- the proto bindings are generated with new protobuf-3, while the scripts uses system protobuf-2 that happens to be available on the machine. I thought I fixed it with  https://codereview.chromium.org/1887423003/, but it seems it didn't work quite well.

Unfortunately I think we need to revert the smaller CL:

https://codereview.chromium.org/1878383003

I am testing that now.

Update: there are apparently other issues with the protobuf change, so if that gets reverted no need to revert https://codereview.chromium.org/1878383003.

Update: the protobuf update is actually not getting reverted -- it turns out that that new crash that was thought to be caused update actually wasn't (the update changed some names, so it no longer matched the old crash).

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90ade35dd2736cc1fa3eb9fc67e83640345b519b

commit 90ade35dd2736cc1fa3eb9fc67e83640345b519b
Author: stevenjb <stevenjb@chromium.org>
Date: Fri Apr 22 00:00:28 2016

Revert "Re-land: Add download_file_types.proto with ascii->binary conversion, as a resource."

This reverts commit d6e8e6aaa8d07a0d576adbd2dafd0789b124660e.

Note: This revert deletes some files / code that were modified later,
but were fixes to this change and should be easily recoverable.

BUG= 605592 
TBR=nparker@chromium.org, xyzzyz@chromium.org,

Review URL: https://codereview.chromium.org/1911883002

Cr-Commit-Position: refs/heads/master@{#388960}

[modify] https://crrev.com/90ade35dd2736cc1fa3eb9fc67e83640345b519b/chrome/browser/BUILD.gn
[modify] https://crrev.com/90ade35dd2736cc1fa3eb9fc67e83640345b519b/chrome/browser/browser_resources.grd
[delete] https://crrev.com/0f50d795b3a201afb0dfe3607aa11c671a27260a/chrome/browser/resources/safe_browsing/BUILD.gn
[delete] https://crrev.com/0f50d795b3a201afb0dfe3607aa11c671a27260a/chrome/browser/resources/safe_browsing/README.md
[delete] https://crrev.com/0f50d795b3a201afb0dfe3607aa11c671a27260a/chrome/browser/resources/safe_browsing/download_file_types.asciipb
[delete] https://crrev.com/0f50d795b3a201afb0dfe3607aa11c671a27260a/chrome/browser/resources/safe_browsing/gen_file_type_proto.py
[modify] https://crrev.com/90ade35dd2736cc1fa3eb9fc67e83640345b519b/chrome/chrome_common.gypi
[modify] https://crrev.com/90ade35dd2736cc1fa3eb9fc67e83640345b519b/chrome/chrome_resources.gyp
[modify] https://crrev.com/90ade35dd2736cc1fa3eb9fc67e83640345b519b/chrome/common/safe_browsing/BUILD.gn
[delete] https://crrev.com/0f50d795b3a201afb0dfe3607aa11c671a27260a/chrome/common/safe_browsing/download_file_types.proto

assigning to Adam since he's working on repro'ing the issue.

I almost have the ChromeOS checkout compiling. It will probably take an hour or so to figure out what's the source of the problem here.

Okay, I reproduced the issue in ChromeOS chroot. Investigating the cause.

This is pretty crazy:

(cr) xyzzyz@xyzzyz ~/chrome_root/src/c/Release/pyproto $ python
Python 2.7.10 (default, Apr 20 2016, 21:12:55) 
[GCC 4.9.x 20150123 (prerelease)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import six
>>> six.__file__
'/usr/lib64/python2.7/site-packages/six.pyc'
>>> from google.protobuf import text_format
>>> text_format.__file__
'/usr/lib64/python2.7/site-packages/google/protobuf/text_format.pyc'
>>> 
(cr) xyzzyz@xyzzyz ~/chrome_root/src/c/Release/pyproto $ python
Python 2.7.10 (default, Apr 20 2016, 21:12:55) 
[GCC 4.9.x 20150123 (prerelease)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.path = ['']
>>> import six
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: No module named six
>>> from google.protobuf import text_format
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python2.7/site-packages/google/protobuf/text_format.py", line 39, in <module>
    import cStringIO
ImportError: No module named cStringIO
>>> 

After clearing sys.path, why it still knows where to look for protobuf, while it doesn't know where to look for six?

There is also sys.path_importer_cache, and sys.meta_path.  Python module-finding looks pretty complex. https://docs.python.org/3/reference/import.html

(cr) xyzzyz@xyzzyz ~/chrome_root/src/c/Release $ python
Python 2.7.10 (default, Apr 20 2016, 21:12:55) 
[GCC 4.9.x 20150123 (prerelease)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.modules
{'google': <module 'google' (built-in)>, ...}

Oh.

I tracked down the core reason: there are PTH files in /usr/lib64/python2.7/site-packages that are supposed to setup paths for site libraries, see https://docs.python.org/2/library/site.html

In particular, there are two files referring to the google module, google_apputils-0.4.0-py2.7-nspkg.pth and protobuf-2.6.1-py2.7-nspkg.pth which execute this lovely snippet:

import sys, types, os;p = os.path.join(sys._getframe(1).f_locals['sitedir'], *('google',));ie = os.path.exists(os.path.join(p,'__init__.py'));m = not ie and sys.modules.setdefault('google', types.ModuleType('google'));mp = (m or []) and m.__dict__.setdefault('__path__',[]);(p not in mp) and mp.append(p)

which basically adds the google module to sys.modules with the site-packages/google path. After renaming these files, importing Chromium protobuf from pyproto works just fine.

The question now remains: how to properly ensure that the script uses our own protobuf instead of system one? As nparker said in #12, there's more to module importing than just sys.path... We need some Python expert help here -- I'll start by asking on Stack Overflow :)

Whoa.  So, does it work to run python with '-s' to suppress automatic import?  That may break other things.

(cr) xyzzyz@xyzzyz ~/chrome_root/src/c/Release/pyproto $ python -s -S
Python 2.7.10 (default, Apr 20 2016, 21:12:55) 
[GCC 4.9.x 20150123 (prerelease)] on linux2
>>> from google.protobuf import text_format
>>> text_format.__file__
'google/protobuf/text_format.pyc'

(cr) xyzzyz@xyzzyz ~/chrome_root/src/c/Release/pyproto $ python
Python 2.7.10 (default, Apr 20 2016, 21:12:55) 
[GCC 4.9.x 20150123 (prerelease)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from google.protobuf import text_format
>>> text_format.__file__
'/usr/lib64/python2.7/site-packages/google/protobuf/text_format.pyc'

Disabling global site with -S works (-s only disables user site).

I asked the wise elder on Stack Overflow, let's see what's their recommendation:
http://stackoverflow.com/questions/36804860/shadowing-a-site-module-by-my-own-module-in-python

In fact, I believe that disabling site-packages for Python targets in Chromium would be beneficial -- our scripts shouldn't depend on whatever might be available on the system, instead they should ship their own copies in third_party/ and use that.

Yup, that may fix other things.

I created a thread on chromium-dev to discuss that.

I'm trying this on my original CL: https://codereview.chromium.org/1917653002

Since the try bots didn't catch this in the first place, can you try that CL on your chromeos build?  Thanks.

Your patch is not going to work, because it will pass -S argument to the script, and not to Python -- that is, we need `python -S script.py`, while your patch gives only `python script.py -S`.

We'd have to fix the ninja action() rule.

As a workaround, you might want to try to add a shell script that would run Python with "-s -S", as opposed to running Python script directly.

I've fixed the GYP build by adding -S before the script name (http://crrev.com/1917653002). But for GN, the docs say that action() supports only Python, and it appears to execute it python with its own args rather than using the "#!/usr/bin/python <args>" I provide at the top of the script.

I don't see any support for environment variables, so using PYTHONPATH is not an option.

I see some arbitrary executables being run from GN as a tool(...) -- could we use that instead of an action?  Then we could control the python args.

PYTHONPATH wouldn't help anyway, as it just modifies sys.path.

I don't know anything about tool(...), but if action() can only run Python scripts, then you can create a Python script that runs another Python script in a subprocess. This is pretty ugly, and should be only temporary solution until we figure out the proper way to do it.

I've added such a wrapping, within the script itself.  

From my side this is fixed.

If there's a general fix for Python <-> Proto interactions, we should still do that.  I think we either need to standardize what's in every site install (hard, or impossible), or add -Ss in action() and potentially exclude some useful modules.  

Looks like a more general soln would be to call //build/gn_run_binary.py with some args to run python with custom args.

Closing this after filing http://crbug.com/614082 for the cleanup.

With this change building with virtualenv is broken:

$ ninja -v -j1 -C out/Defalt chrome                                
ninja: Entering directory `out/Defalt'
[1/17787] touch obj/chrome/browser/resources/safe_browsing/make_file_types_protobuf.inputdeps.stamp
[2/17787] python ../../chrome/browser/resources/safe_browsing/gen_file_type_proto.py -w -t linux -i ../../chrome
/browser/resources/safe_browsing/download_file_types.asciipb -d gen/chrome/browser/resources/safe_browsing -o do
wnload_file_types.pb -p pyproto -p pyproto/chrome/common/safe_browsing
FAILED: gen/chrome/browser/resources/safe_browsing/download_file_types.pb 
python ../../chrome/browser/resources/safe_browsing/gen_file_type_proto.py -w -t linux -i ../../chrome/browser/r
esources/safe_browsing/download_file_types.asciipb -d gen/chrome/browser/resources/safe_browsing -o download_fil
e_types.pb -p pyproto -p pyproto/chrome/common/safe_browsing
Traceback (most recent call last):
  File "../../chrome/browser/resources/safe_browsing/gen_file_type_proto.py", line 15, in <module>
    import optparse
ImportError: No module named optparse
ninja: build stopped: subcommand failed.

virtualenv relies on site.py. '-S -s' definitely breaks it. I have to remove '-w' from chrome/browser/resources/safe_browsing/BUILD.gn for correct building.

My machine has Arch Linux z86_64. Have no external protobuf libraries installed.

What is virtualenv?  I'm not sure of the solution to this, if your environment requires the system libraries, and on other environments we have to exclude them because they have old proto libs.  An idea: We could have a common scheme in GN for including required python modules, so all python scripts would be launched in a consistent environment.

virtualenv is a handful Python tool to create isolated Python environments. [1] In this environment, system Python packages are ignored. It can also use an alternative Python version as the default in such environments.

On Arch Linux, /usr/bin/python is symlinked to /usr/bin/python3. In Chromium, there are too many Python2-specific scripts that use simple `python` but not `python2`. As a workaround, I have to create a virtual environment to force Python 2 if `python` is invoked.

A unified entry point for Python sounds great. Things like subproces.call(['python2', ...]) can alleviate porting issues between Python 2 and Python 3.

[1] https://virtualenv.pypa.io/en/stable/
[2] https://github.com/servo/servo

Sorry for the second reference in the previous comment. I was to mention servo, which also uses virtualenv for building. Later I decided not to discuss non-Chromium projects here but forgot to remove the reference.

This hermitic environment could be something like //build/gn_run_binary.py (related: http://crbug.com/614082)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a0c75183f6dab0b8ab2953b34595d6fd5651b9bc

commit a0c75183f6dab0b8ab2953b34595d6fd5651b9bc
Author: sanfin <sanfin@google.com>
Date: Mon Oct 31 20:03:30 2016

Allow gen_file_type_proto.py to work in virtualenv.

There were complaints in  crbug.com/605592  that virtualenv builds
are broken with the strategy of passing -w to gen_file_type_proto.py.
Systems can use virtualenv to isolate the Python environment from
dist-packages, making the nested call to this script redundant (and
broken, since -S breaks some assumptions from virtualenv).

BUG= 605592 
BUG=614082
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2341173004
Cr-Commit-Position: refs/heads/master@{#428777}

[modify] https://crrev.com/a0c75183f6dab0b8ab2953b34595d6fd5651b9bc/chrome/browser/resources/safe_browsing/gen_file_type_proto.py