Fix jvm compile unicode issues when using Python 3

[Eric-Arellano]

When running ./pants3 test tests/python/pants_test/goal::, it was discovered that there were several unicode issues affecting the backend/jvm set of modules that the tests had not picked up.

[illicitonion]

Looks great :) Thanks!

[illicitonion]

It would be good to fix this, if you happen to have the context in your head for the root cause...

[Eric-Arellano]

So I discovered this is a much bigger issue than just zinc_compile! I think we are registering default values for list option types as bytes. I've narrowed it down to this diff so far, which intentionally throws an error to help debug when simply running ./pants.

diff --git a/src/python/pants/option/options.py b/src/python/pants/option/opti
ons.py
index e974cd096..7c2c022d6 100644
--- a/src/python/pants/option/options.py
+++ b/src/python/pants/option/options.py
@@ -379,6 +379,9 @@ class Options(object):
     if inherit_from_enclosing_scope:
       self._check_and_apply_deprecations(scope, values)

+    ba = [a for a in values if isinstance(a, bytes)]
+    if len(ba) != 0:
+      raise ValueError(ba)
     return values

   def get_fingerprintable_for_scope(self, bottom_scope, include_passthru=False,

I'll keep investigating! Will wait to merge this PR.

[Eric-Arellano]

Okay I investigated this for a long time today, and @OniOni checked it out on Friday.

I think I found why ./pants is using both unicode and bytes for options: Python 2 stores attributes as a map between byte strings to the corresponding values.

# in Py2
>>> [type(x) for x in dir("test")]

[<type 'str'>, ...]

I resolved the issue down to running ./pants with the following diff:

diff --git a/src/python/pants/option/option_value_container.py b/src/python/pants/option/option_value_container.py
index 3e6d824ca..c5fb50c12 100644
--- a/src/python/pants/option/option_value_container.py
+++ b/src/python/pants/option/option_value_container.py
@@ -120,6 +122,8 @@ class OptionValueContainer(object):
 
   # Support attribute setting, e.g., opts.foo = 42.
   def __setattr__(self, key, value):
+    if isinstance(key, bytes):
+     raise ValueError(key)
     if key == '_value_map':
       return super(OptionValueContainer, self).__setattr__(key, value)
     self._set(key, value)
@@ -128,6 +132,8 @@ class OptionValueContainer(object):
   # Note: Called only if regular attribute lookup fails,
   # so method and member access will be handled the normal way.
   def __getattr__(self, key):
+    if isinstance(key, bytes):
+      raise ValueError(key)
     if key == '_value_map':
       # In case we get called in copy/deepcopy, which don't invoke the ctor.
       raise AttributeError(key)

I do not think it makes sense to force the OptionValueContainer to use unicode always, because that's not how Py2 works. Instead, we keep Py2 as is, and this ends up not even being an issue in Py3 because Py3 represents attributes as unicode strings.

[illicitonion]

Looks great :) Thanks!