Skip to content

Commit

Permalink
Fix jvm compile unicode issues when using Python 3 (#6987)
Browse files Browse the repository at this point in the history
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.

We also stop using `six.string_types`, as we should always be explicit about unicode vs bytes.
  • Loading branch information
Eric-Arellano authored Jan 9, 2019
1 parent 5d2235c commit 0bc0178
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def compute_fingerprint(self, target):
classpath_entries = self._classpath_products.get_artifact_classpath_entries_for_targets(
[target])
for _, entry in classpath_entries:
hasher.update(str(entry.coordinate))
hasher.update(str(entry.coordinate).encode('utf-8'))
return hasher.hexdigest() if PY3 else hasher.hexdigest().decode('utf-8')

def direct(self, target):
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/jvm/subsystems/zinc.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ def _compiler_bridge_cache_dir(self):
"""
hasher = sha1()
for cp_entry in [self.zinc, self.compiler_interface, self.compiler_bridge]:
hasher.update(os.path.relpath(cp_entry, self._workdir()))
hasher.update(os.path.relpath(cp_entry, self._workdir()).encode('utf-8'))
key = hasher.hexdigest()[:12]

return os.path.join(self._workdir(), 'zinc', 'compiler-bridge', key)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ def _compile_vts(self, vts, ctx, upstream_analysis, dependency_classpath, progre
self.context.log.warn('Skipping {} compile for targets with no sources:\n {}'
.format(self.name(), vts.targets))
else:
counter_val = str(counter()).rjust(counter.format_length(), b' ')
counter_val = str(counter()).rjust(counter.format_length(), ' ')
counter_str = '[{}/{}] '.format(counter_val, counter.size)
# Do some reporting.
self.context.log.info(
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/jvm/tasks/jvm_compile/zinc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ python_library(
'src/python/pants/util:contextutil',
'src/python/pants/util:dirutil',
'src/python/pants/util:memo',
'src/python/pants/util:strutil',
],
)
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from contextlib import closing
from xml.etree import ElementTree

from future.utils import PY3, text_type
from future.utils import PY2, PY3, text_type

from pants.backend.jvm.subsystems.java import Java
from pants.backend.jvm.subsystems.jvm_platform import JvmPlatform
Expand All @@ -37,6 +37,7 @@
from pants.util.dirutil import fast_relpath, safe_open
from pants.util.memo import memoized_method, memoized_property
from pants.util.meta import classproperty
from pants.util.strutil import ensure_text


# Well known metadata file required to register scalac plugins with nsc.
Expand Down Expand Up @@ -382,10 +383,15 @@ def relative_to_exec_root(path):
zinc_args.extend(ctx.sources)

self.log_zinc_file(ctx.analysis_file)
with open(ctx.zinc_args_file, 'wb') as fp:
with open(ctx.zinc_args_file, 'w') as fp:
for arg in zinc_args:
# NB: in Python 2, options are stored sometimes as bytes and sometimes as unicode in the OptionValueContainer.
# This is due to how Python 2 natively stores attributes as a map of `str` (aka `bytes`) to their value. So,
# the setattr() and getattr() functions sometimes use bytes.
if PY2:
arg = ensure_text(arg)
fp.write(arg)
fp.write(b'\n')
fp.write('\n')

if self.execution_strategy == self.HERMETIC:
zinc_relpath = fast_relpath(self._zinc.zinc, get_buildroot())
Expand Down
1 change: 0 additions & 1 deletion src/python/pants/option/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ python_library(
'3rdparty/python:future',
'3rdparty/python:ansicolors',
'3rdparty/python:setuptools',
'3rdparty/python:six',
'3rdparty/python/twitter/commons:twitter.common.collections',
'src/python/pants/base:build_environment',
'src/python/pants/base:deprecated',
Expand Down
17 changes: 8 additions & 9 deletions src/python/pants/option/custom_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,12 @@
from __future__ import absolute_import, division, print_function, unicode_literals

import re
from builtins import object

import six
from builtins import bytes, object, str

from pants.option.errors import ParseError
from pants.util.eval import parse_expression
from pants.util.memo import memoized_method
from pants.util.objects import enum
from pants.util.strutil import ensure_text


class UnsetBool(object):
Expand Down Expand Up @@ -207,8 +204,10 @@ def create(cls, value):
May also be a comma-separated sequence of modifications.
:rtype: `ListValueComponent`
"""
if isinstance(value, six.string_types):
value = ensure_text(value)
if isinstance(value, bytes):
value = value.decode('utf-8')

if isinstance(value, str):
comma_separated_exprs = cls._split_modifier_expr(value)
if len(comma_separated_exprs) > 1:
return cls.merge([cls.create(x) for x in comma_separated_exprs])
Expand All @@ -230,7 +229,7 @@ def create(cls, value):
appends = _convert(value[1:], (list, tuple))
elif value.startswith('-[') or value.startswith('-('):
filters = _convert(value[1:], (list, tuple))
elif isinstance(value, six.string_types):
elif isinstance(value, str):
appends = [value]
else:
appends = _convert('[{}]'.format(value), list)
Expand Down Expand Up @@ -287,8 +286,8 @@ def create(cls, value):
or a string representation (possibly prefixed by +) of a dict.
:rtype: `DictValueComponent`
"""
if isinstance(value, six.string_types):
value = ensure_text(value)
if isinstance(value, bytes):
value = value.decode('utf-8')
if isinstance(value, cls): # Ensure idempotency.
action = value.action
val = value.val
Expand Down
4 changes: 1 addition & 3 deletions src/python/pants/option/optionable.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
from abc import abstractproperty
from builtins import str

from six import string_types

from pants.engine.selectors import Get
from pants.option.errors import OptionsError
from pants.option.scope import Scope, ScopedOptions, ScopeInfo
Expand Down Expand Up @@ -149,5 +147,5 @@ def __init__(self):
# its __init__, as we do here. We usually only create a single instance of an Optionable
# subclass anyway.
cls = type(self)
if not isinstance(cls.options_scope, string_types):
if not isinstance(cls.options_scope, str):
raise NotImplementedError('{} must set an options_scope class-level property.'.format(cls))

0 comments on commit 0bc0178

Please sign in to comment.