From 0bc0178a54ae4610c92ee8bcd45c77abdab48dfe Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 9 Jan 2019 04:31:09 -0700 Subject: [PATCH] Fix jvm compile unicode issues when using Python 3 (#6987) 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. --- .../jvm/subsystems/dependency_context.py | 2 +- src/python/pants/backend/jvm/subsystems/zinc.py | 2 +- .../jvm/tasks/jvm_compile/jvm_compile.py | 2 +- .../backend/jvm/tasks/jvm_compile/zinc/BUILD | 1 + .../jvm/tasks/jvm_compile/zinc/zinc_compile.py | 12 +++++++++--- src/python/pants/option/BUILD | 1 - src/python/pants/option/custom_types.py | 17 ++++++++--------- src/python/pants/option/optionable.py | 4 +--- 8 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/python/pants/backend/jvm/subsystems/dependency_context.py b/src/python/pants/backend/jvm/subsystems/dependency_context.py index 5607df08d27..5ee8c221589 100644 --- a/src/python/pants/backend/jvm/subsystems/dependency_context.py +++ b/src/python/pants/backend/jvm/subsystems/dependency_context.py @@ -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): diff --git a/src/python/pants/backend/jvm/subsystems/zinc.py b/src/python/pants/backend/jvm/subsystems/zinc.py index 23f7d0ae7cf..3e1d8aa0ada 100644 --- a/src/python/pants/backend/jvm/subsystems/zinc.py +++ b/src/python/pants/backend/jvm/subsystems/zinc.py @@ -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) diff --git a/src/python/pants/backend/jvm/tasks/jvm_compile/jvm_compile.py b/src/python/pants/backend/jvm/tasks/jvm_compile/jvm_compile.py index 28c29c488e1..14baca31993 100644 --- a/src/python/pants/backend/jvm/tasks/jvm_compile/jvm_compile.py +++ b/src/python/pants/backend/jvm/tasks/jvm_compile/jvm_compile.py @@ -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( diff --git a/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/BUILD b/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/BUILD index 6990668c3ca..0a44e99c2ef 100644 --- a/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/BUILD +++ b/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/BUILD @@ -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', ], ) diff --git a/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py b/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py index cbc45ac7b03..e7ad7f5bf4d 100644 --- a/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py +++ b/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py @@ -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 @@ -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. @@ -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()) diff --git a/src/python/pants/option/BUILD b/src/python/pants/option/BUILD index 5c2296c49e9..598451df065 100644 --- a/src/python/pants/option/BUILD +++ b/src/python/pants/option/BUILD @@ -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', diff --git a/src/python/pants/option/custom_types.py b/src/python/pants/option/custom_types.py index e7302ccd285..7b5ee91a0df 100644 --- a/src/python/pants/option/custom_types.py +++ b/src/python/pants/option/custom_types.py @@ -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): @@ -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]) @@ -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) @@ -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 diff --git a/src/python/pants/option/optionable.py b/src/python/pants/option/optionable.py index 744e10387d5..b71c9ef685f 100644 --- a/src/python/pants/option/optionable.py +++ b/src/python/pants/option/optionable.py @@ -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 @@ -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))