Python 3 - fixes to get most of src unit tests green

build-support/known_py3_failures.txt

@@ -1,47 +1,7 @@
-tests/python/pants_test/backend/codegen/thrift/java:java
 tests/python/pants_test/backend/codegen/thrift/python:python
-tests/python/pants_test/backend/jvm/tasks:coursier_resolve
 tests/python/pants_test/backend/jvm/tasks:jar_publish
 tests/python/pants_test/backend/jvm/tasks:junit_run
-tests/python/pants_test/backend/jvm/tasks:resources_task
 tests/python/pants_test/backend/python/tasks:tasks
-tests/python/pants_test/binaries:binaries
-tests/python/pants_test/build_graph:build_file_parser
 tests/python/pants_test/build_graph:build_graph
-tests/python/pants_test/build_graph:target
-tests/python/pants_test/cache:artifact_cache
-tests/python/pants_test/cache:cache_setup
-tests/python/pants_test/cache:caching
-tests/python/pants_test/cache:caching_dereference_symlinks
-tests/python/pants_test/core_tasks:bash_completion
-tests/python/pants_test/core_tasks:list_goals
-tests/python/pants_test/core_tasks:roots
-tests/python/pants_test/core_tasks:run_prep_command
-tests/python/pants_test/engine/legacy:structs
-tests/python/pants_test/engine:build_files
-tests/python/pants_test/engine:engine
-tests/python/pants_test/engine:isolated_process
-tests/python/pants_test/engine:mapper
-tests/python/pants_test/engine:parsers
-tests/python/pants_test/engine:scheduler
-tests/python/pants_test/engine:selectors
-tests/python/pants_test/engine:test_round_engine
-tests/python/pants_test/goal:artifact_cache_stats
 tests/python/pants_test/goal:other
-tests/python/pants_test/help:build_dictionary_info_extracter
-tests/python/pants_test/init:init
-tests/python/pants_test/invalidation:build_invalidator
-tests/python/pants_test/invalidation:cache_manager
-tests/python/pants_test/jvm/subsystems:jvm
-tests/python/pants_test/jvm:test_safeargs
-tests/python/pants_test/option:testing
-tests/python/pants_test/pantsd:pants_daemon
 tests/python/pants_test/pantsd:process_manager
-tests/python/pants_test/pantsd:watchman
-tests/python/pants_test/source:source_root
-tests/python/pants_test/task:console_task
-tests/python/pants_test/task:simple_codegen_task
-tests/python/pants_test/task:task
-tests/python/pants_test/task:testrunner_task_mixin
-tests/python/pants_test/util:contextutil
-tests/python/pants_test/util:dirutil

src/python/pants/binaries/binary_util.py

@@ -311,8 +311,8 @@ def __init__(self, baseurls, binary_tool_fetcher, path_by_id=None,
     self._uname_func = uname_func or os.uname
 
   _ID_BY_OS = {
-    'linux': lambda release, machine: ('linux', machine),
     'darwin': lambda release, machine: ('darwin', release.split('.')[0]),
+    'linux': lambda release, machine: ('linux', machine),
   }
 
   # FIXME(cosmicexplorer): we create a HostPlatform in this class instead of in the constructor
@@ -330,8 +330,8 @@ def _host_platform(self):
       # TODO: test this!
       raise self.MissingMachineInfo(
         "Pants could not resolve binaries for the current host: platform '{}' was not recognized. "
-        "Recognized platforms are: {}."
-        .format(os_id_key, self._ID_BY_OS.keys()))
+        "Recognized platforms are: [{}]."
+        .format(os_id_key, ', '.join(sorted(self._ID_BY_OS.keys()))))
     try:
       os_name, arch_or_version = self._path_by_id[os_id_tuple]
       host_platform = HostPlatform(os_name, arch_or_version)

src/python/pants/core_tasks/bash_completion.py

@@ -89,7 +89,7 @@ def bash_scope_key(sc):
 
     generator = Generator(
       resource_string(__name__,
-                      os.path.join('templates', 'bash_completion', 'autocomplete.sh.mustache')),
+                      os.path.join('templates', 'bash_completion', 'autocomplete.sh.mustache')).decode('utf-8'),
       scopes_text=' '.join(sorted(list(cmd_line_scopes))),
       options_text=options_text
     )

src/python/pants/core_tasks/run_prep_command.py

@@ -83,7 +83,7 @@ def has_prep(target):
 
           if environ:
             if not process.returncode:
-              environment_vars = stdout.split('\0')
+              environment_vars = stdout.decode('utf-8').split('\0')
               for kvpair in environment_vars:
                 var, value = kvpair.split('=', 1)
                 os.environ[var] = value

src/python/pants/engine/isolated_process.py

@@ -6,7 +6,7 @@
 
 import logging
 
-from future.utils import text_type
+from future.utils import binary_type, text_type
 
 from pants.engine.fs import DirectoryDigest
 from pants.engine.rules import RootRule, rule
@@ -69,13 +69,20 @@ def __new__(
     )
 
 
-class ExecuteProcessResult(datatype(['stdout', 'stderr', 'output_directory_digest'])):
+class ExecuteProcessResult(datatype([('stdout', binary_type),
+                                     ('stderr', binary_type),
+                                     ('output_directory_digest', DirectoryDigest)
+                                     ])):
   """Result of successfully executing a process.
 
   Requesting one of these will raise an exception if the exit code is non-zero."""
 
 
-class FallibleExecuteProcessResult(datatype(['stdout', 'stderr', 'exit_code', 'output_directory_digest'])):
+class FallibleExecuteProcessResult(datatype([('stdout', binary_type),
+                                             ('stderr', binary_type),
+                                             ('exit_code', int),
+                                             ('output_directory_digest', DirectoryDigest)
+                                             ])):
   """Result of executing a process.
 
   Requesting one of these will not raise an exception if the exit code is non-zero."""

src/python/pants/engine/mapper.py

@@ -98,7 +98,7 @@ def create(cls, spec_path, address_maps):
     :rtype: :class:`AddressFamily`
     :raises: :class:`MappingError` if the given address maps do not form a family.
     """
-    if spec_path == b'.':
+    if spec_path == '.':
       spec_path = ''
     for address_map in address_maps:
       if not address_map.path.startswith(spec_path):

src/python/pants/engine/round_engine.py

@@ -36,7 +36,7 @@ def attempt(self, explain):
     goal_workdir = os.path.join(self._context.options.for_global_scope().pants_workdir,
                                 self._goal.name)
     with self._context.new_workunit(name=self._goal.name, labels=[WorkUnitLabel.GOAL]):
-      for name, task_type in reversed(self._tasktypes_by_name.items()):
+      for name, task_type in reversed(list(self._tasktypes_by_name.items())):
         task_workdir = os.path.join(goal_workdir, name)
         task = task_type(self._context, task_workdir)
         log_config = WorkUnit.LogConfig(level=task.get_options().level, colors=task.get_options().colors)
@@ -49,7 +49,7 @@ def attempt(self, explain):
             task.execute()
 
       if explain:
-        reversed_tasktypes_by_name = reversed(self._tasktypes_by_name.items())
+        reversed_tasktypes_by_name = reversed(list(self._tasktypes_by_name.items()))
         goal_to_task = ', '.join(
             '{}->{}'.format(name, task_type.__name__) for name, task_type in reversed_tasktypes_by_name)
         print('{goal} [{goal_to_task}]'.format(goal=self._goal.name, goal_to_task=goal_to_task))

src/python/pants/fs/fs.py

@@ -41,7 +41,7 @@ def safe_filename(name, extension=None, digest=None, max_length=_MAX_FILENAME_LE
     return filename
   else:
     digest = digest or hashlib.sha1()
-    digest.update(filename)
+    digest.update(filename.encode('utf-8'))
     hexdigest = digest.hexdigest()[:16]
 
     # Prefix and suffix length: max length less 2 periods, the extension length, and the digest length.

src/python/pants/help/build_dictionary_info_extracter.py

@@ -47,6 +47,15 @@ class BuildDictionaryInfoExtracter(object):
     FunctionArg('tags', '', True, None),
   ]
 
+  @classmethod
+  def _is_custom_init(cls, init_func):
+    """Check if init derives from object or from a custom implementation.
+
+    In Py2, we could check if __init__ was overridden with inspect.ismethod(obj_type). This no longer works in Py3, so
+    we have to use an alternative check.
+    """
+    return 'slot wrapper' not in str(init_func)
+
   @classmethod
   def get_description_from_docstring(cls, obj):
     """Returns a pair (description, details) from the obj's docstring.
@@ -126,13 +135,13 @@ def get_args_for_target_type(cls, target_type):
 
   @classmethod
   def _get_args_for_target_type(cls, target_type):
-    args = {} # name: info.
+    args = {}  # name: info.
 
     # Target.__init__ has several args that are passed to it by TargetAddressable and not by
     # the BUILD file author, so we can't naively inspect it.  Instead we special-case its
     # true BUILD-file-facing arguments here.
     for arg in cls.basic_target_args:
-      args[arg.name] = arg # Don't yield yet; subclass might supply a better description.
+      args[arg.name] = arg  # Don't yield yet; subclass might supply a better description.
 
     # Non-BUILD-file-facing Target.__init__ args that some Target subclasses capture in their
     # own __init__ for various reasons.
@@ -143,15 +152,15 @@ def _get_args_for_target_type(cls, target_type):
     # so clobber its entry in the args dict.
     methods_seen = set()  # Ensure we only look at each __init__ method once.
     for _type in reversed([t for t in target_type.mro() if issubclass(t, Target)]):
-      if (inspect.ismethod(_type.__init__) and
+      if (cls._is_custom_init(_type.__init__) and
           _type.__init__ not in methods_seen and
           _type.__init__ != Target.__init__):
         for arg in cls._get_function_args(_type.__init__):
           args[arg.name] = arg
         methods_seen.add(_type.__init__)
 
     for arg_name in sorted(args.keys()):
-      if not arg_name in ignore_args:
+      if arg_name not in ignore_args:
         yield args[arg_name]
 
   @classmethod
@@ -167,10 +176,7 @@ def _get_function_args(cls, func):
     arg_descriptions = cls.get_arg_descriptions_from_docstring(func)
     argspec = inspect.getargspec(func)
     arg_names = argspec.args
-    # Python2 treats __init__ as a method. Python3 doesn't.
-    # Neither treat __new__ as a method.
-    # Always treat __init__ and __new__ as methods, to drop self from their args.
-    if inspect.ismethod(func) or func.__name__ == '__new__' or func.__name__ == '__init__':
+    if arg_names and arg_names[0] in {'self', 'cls'}:
       arg_names = arg_names[1:]
     num_defaulted_args = len(argspec.defaults) if argspec.defaults is not None else 0
     first_defaulted_arg = len(arg_names) - num_defaulted_args
@@ -208,7 +214,7 @@ def get_object_args(self, alias):
       raise TaskError('No such object type: {}'.format(alias))
     if inspect.isfunction(obj_type) or inspect.ismethod(obj_type):
       return self.get_function_args(obj_type)
-    elif inspect.isclass(obj_type) and inspect.ismethod(obj_type.__init__):
+    elif inspect.isclass(obj_type) and self._is_custom_init(obj_type.__init__):
       return self.get_function_args(obj_type.__init__)
     elif inspect.isclass(obj_type):
       return self.get_function_args(obj_type.__new__)

src/python/pants/invalidation/build_invalidator.py

@@ -214,16 +214,16 @@ def _sha_file_by_id(self, id):
     return os.path.join(self._root, safe_filename(id, extension='.hash'))
 
   def _write_sha(self, cache_key):
-    with open(self._sha_file(cache_key), 'wb') as fd:
-      fd.write(cache_key.hash.encode('utf-8'))
+    with open(self._sha_file(cache_key), 'w') as fd:
+      fd.write(cache_key.hash)
 
   def _read_sha(self, cache_key):
     return self._read_sha_by_id(cache_key.id)
 
   def _read_sha_by_id(self, id):
     try:
-      with open(self._sha_file_by_id(id), 'rb') as fd:
-        return fd.read().strip().decode('utf-8')
+      with open(self._sha_file_by_id(id), 'r') as fd:
+        return fd.read().strip()
     except IOError as e:
       if e.errno != errno.ENOENT:
         raise

src/python/pants/pantsd/pants_daemon.py

@@ -32,6 +32,7 @@
 from pants.util.collections import combined_dict
 from pants.util.contextutil import stdio_as
 from pants.util.memo import memoized_property
+from pants.util.strutil import ensure_text
 
 
 class _LoggerStream(object):
@@ -328,7 +329,7 @@ def _run_services(self, services):
 
     # Once all services are started, write our pid.
     self.write_pid()
-    self.write_metadata_by_name('pantsd', self.FINGERPRINT_KEY, self.options_fingerprint.decode('utf-8'))
+    self.write_metadata_by_name('pantsd', self.FINGERPRINT_KEY, ensure_text(self.options_fingerprint))
 
     # Monitor services.
     while not self.is_killed:

src/python/pants/util/contextutil.py

@@ -74,7 +74,9 @@ def _purge_env():
   # invokes or other things that may access the environment at the C level may not see the
   # correct env vars (i.e. we can't just replace os.environ with an empty dict).
   # See https://docs.python.org/2/library/os.html#os.unsetenv for more info.
-  for k in os.environ.keys():
+  #
+  # Wraps iterable in list() to make a copy and avoid issues with deleting while iterating.
+  for k in list(os.environ.keys()):
     del os.environ[k]
 
 

tests/python/pants_test/binaries/test_binary_util.py

@@ -9,7 +9,7 @@
 from builtins import object, open, str
 
 import mock
-from future.utils import PY2, PY3
+from future.utils import PY2
 
 from pants.binaries.binary_util import (BinaryRequest, BinaryToolFetcher, BinaryToolUrlGenerator,
                                         BinaryUtil)
@@ -228,14 +228,11 @@ def uname_func():
     the_raised_exception_message = str(cm.exception)
 
     self.assertIn(BinaryUtil.MissingMachineInfo.__name__, the_raised_exception_message)
-    expected_msg_prefix = (
+    expected_msg = (
       "Error resolving binary request BinaryRequest(supportdir=supportdir, version=version, "
       "name=name, platform_dependent=True, external_url_generator=None, archiver=None): "
       "Pants could not resolve binaries for the current host: platform 'vms' was not recognized. "
-      "Recognized platforms are: ")
-    expected_msg = expected_msg_prefix + ("dict_keys([\\'linux\', \\'darwin\'])."
-                                          if PY3 else
-                                          "[u'darwin', u'linux'].")
+      "Recognized platforms are: [darwin, linux].")
     self.assertIn(expected_msg, the_raised_exception_message)
 
   def test_select_binary_base_path_missing_version(self):

tests/python/pants_test/build_graph/test_build_file_parser.py

@@ -71,10 +71,12 @@ def test_name_injection(self):
   def test_addressable_exceptions(self):
     self.add_to_build_file('b/BUILD', 'java_library(name="foo", "bad_arg")')
     build_file_b = self.create_buildfile('b/BUILD')
-    expected_msg = ('positional argument follows keyword argument'
-                    if PY3 else
-                    'non-keyword arg after keyword arg')
-    self.assert_parser_error(build_file_b, expected_msg)
+    try:
+      # Python 3.5+ message
+      self.assert_parser_error(build_file_b, 'positional argument follows keyword argument')
+    except AssertionError:
+      # Python 2.7-3.4 message
+      self.assert_parser_error(build_file_b, 'non-keyword arg after keyword arg')
 
     self.add_to_build_file('d/BUILD', dedent(
       """

tests/python/pants_test/build_graph/test_subproject_integration.py

@@ -76,7 +76,7 @@
 def harness():
   try:
     for name, content in BUILD_FILES.items():
-      safe_file_dump(name, dedent(content))
+      safe_file_dump(name, dedent(content), binary_mode=False)
     yield
   finally:
     safe_rmtree(SUBPROJ_SPEC)

tests/python/pants_test/build_graph/test_target.py

@@ -9,6 +9,7 @@
 from hashlib import sha1
 
 import mock
+from future.utils import PY3
 
 from pants.base.exceptions import TargetDefinitionException
 from pants.base.fingerprint_strategy import DefaultFingerprintStrategy
@@ -156,13 +157,15 @@ def test_create_sources_field_with_string_fails(self):
     # No key_arg.
     with self.assertRaises(TargetDefinitionException) as cm:
       target.create_sources_field(sources='a-string', sources_rel_path='')
-    self.assertIn("Expected a glob, an address or a list, but was <type \'unicode\'>",
+    self.assertIn("Expected a glob, an address or a list, but was {}"
+                  .format('<class \'str\'>' if PY3 else '<type \'unicode\'>'),
                   str(cm.exception))
 
     # With key_arg.
     with self.assertRaises(TargetDefinitionException) as cm:
       target.create_sources_field(sources='a-string', sources_rel_path='', key_arg='my_cool_field')
-    self.assertIn("Expected 'my_cool_field' to be a glob, an address or a list, but was <type \'unicode\'>",
+    self.assertIn("Expected 'my_cool_field' to be a glob, an address or a list, but was {}"
+                  .format('<class \'str\'>' if PY3 else '<type \'unicode\'>'),
                   str(cm.exception))
     #could also test address case, but looks like nothing really uses it.
 
@@ -186,14 +189,14 @@ def test_transitive_invalidation_hash(self):
     self.assertEqual(hash_value, target_a.transitive_invalidation_hash())
 
     hasher = sha1()
-    hasher.update(hash_value)
+    hasher.update(hash_value.encode('utf-8'))
     dep_hash = hasher.hexdigest()[:12]
     target_hash = target_b.invalidation_hash()
     hash_value = '{}.{}'.format(target_hash, dep_hash)
     self.assertEqual(hash_value, target_b.transitive_invalidation_hash())
 
     hasher = sha1()
-    hasher.update(hash_value)
+    hasher.update(hash_value.encode('utf-8'))
     dep_hash = hasher.hexdigest()[:12]
     target_hash = target_c.invalidation_hash()
     hash_value = '{}.{}'.format(target_hash, dep_hash)
@@ -206,7 +209,7 @@ def direct(self, target):
 
     fingerprint_strategy = TestFingerprintStrategy()
     hasher = sha1()
-    hasher.update(target_b.invalidation_hash(fingerprint_strategy=fingerprint_strategy))
+    hasher.update(target_b.invalidation_hash(fingerprint_strategy=fingerprint_strategy).encode('utf-8'))
     dep_hash = hasher.hexdigest()[:12]
     target_hash = target_c.invalidation_hash(fingerprint_strategy=fingerprint_strategy)
     hash_value = '{}.{}'.format(target_hash, dep_hash)