Python 3 fixes - fix issues with binaries, option, pantsd, java, and build graph

src/python/pants/java/distribution/distribution.py

@@ -14,6 +14,7 @@
 from collections import namedtuple
 from contextlib import contextmanager
 
+from future.utils import PY3
 from six import string_types
 
 from pants.base.revision import Revision
@@ -182,7 +183,7 @@ def binary(self, name):
     If this distribution has no valid command of the given name raises Distribution.Error.
     If this distribution is a JDK checks both `bin` and `jre/bin` for the binary.
     """
-    if not isinstance(name, string_types):
+    if not isinstance(name, str):
       raise ValueError('name must be a binary name, given {} of type {}'.format(name, type(name)))
     self.validate()
     return self._validated_executable(name)
@@ -234,17 +235,17 @@ def _get_version(self, java):
   def _get_system_properties(self, java):
     if not self._system_properties:
       with temporary_dir() as classpath:
-        with open(os.path.join(classpath, 'SystemProperties.class'), 'w+') as fp:
+        with open(os.path.join(classpath, 'SystemProperties.class'), 'w+b') as fp:
           fp.write(pkgutil.get_data(__name__, 'SystemProperties.class'))
         cmd = [java, '-cp', classpath, 'SystemProperties']
         process = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         stdout, stderr = process.communicate()
         if process.returncode != 0:
           raise self.Error('Failed to determine java system properties for {} with {} - exit code'
-                           ' {}: {}'.format(java, ' '.join(cmd), process.returncode, stderr))
+                           ' {}: {}'.format(java, ' '.join(cmd), process.returncode, stderr.decode('utf-8')))
 
       props = {}
-      for line in stdout.split(os.linesep):
+      for line in stdout.decode('utf-8').split(os.linesep):
         key, _, val = line.partition('=')
         props[key] = val
       self._system_properties = props
@@ -369,7 +370,8 @@ def jvm_locations(self):
     if os.path.exists(self._osx_java_home_exe):
       try:
         plist = subprocess.check_output([self._osx_java_home_exe, '--failfast', '--xml'])
-        for distribution in plistlib.readPlistFromString(plist):
+        plist_results = plistlib.loads(plist) if PY3 else plistlib.readPlistFromString(plist)
+        for distribution in plist_results:
           home = distribution['JVMHomePath']
           yield self.Location.from_home(home)
       except subprocess.CalledProcessError:

src/python/pants/java/jar/manifest.py

@@ -6,8 +6,7 @@
 
 from builtins import object
 from contextlib import closing
-
-from six import StringIO
+from io import BytesIO
 
 
 class Manifest(object):
@@ -20,7 +19,7 @@ class Manifest(object):
   @staticmethod
   def _wrap(text):
     text = text.encode('ascii')
-    with closing(StringIO(text)) as fp:
+    with closing(BytesIO(text)) as fp:
       yield fp.read(70)
       while True:
         chunk = fp.read(69)
@@ -42,12 +41,11 @@ def addentry(self, header, value):
     if len(header) > 68:
       raise ValueError('Header name must be 68 characters or less, given {}'.format(header))
     if self._contents:
-      self._contents += '\n'
-    self._contents += '\n'.join(self._wrap('{header}: {value}'.format(header=header, value=value)))
+      self._contents += b'\n'
+    self._contents += b'\n'.join(self._wrap('{header}: {value}'.format(header=header, value=value)))
 
   def contents(self):
-    padded = self._contents + '\n'
-    return padded.encode('ascii')
+    return self._contents + b'\n'
 
   def is_empty(self):
     if self._contents.strip():

src/python/pants/java/nailgun_client.py

@@ -201,7 +201,8 @@ def execute(self, main_class, cwd=None, *args, **environment):
     :param dict environment: an env mapping made available to native nails via the nail context
     :returns: the exit code of the main_class.
     """
-    environment = dict(self.ENV_DEFAULTS.items() + environment.items())
+    environment = dict(**environment)
+    environment.update(self.ENV_DEFAULTS)
     cwd = cwd or self._workdir
 
     # N.B. This can throw NailgunConnectionError (catchable via NailgunError).

src/python/pants/java/nailgun_executor.py

@@ -76,7 +76,7 @@ class NailgunExecutor(Executor, FingerprintedProcessManager):
 
   _NAILGUN_SPAWN_LOCK = threading.Lock()
   _SELECT_WAIT = 1
-  _PROCESS_NAME = b'java'
+  _PROCESS_NAME = 'java'
 
   def __init__(self, identity, workdir, nailgun_classpath, distribution,
                connect_timeout=10, connect_attempts=5, metadata_base_dir=None):
@@ -103,10 +103,10 @@ def __str__(self):
 
   def _create_owner_arg(self, workdir):
     # Currently the owner is identified via the full path to the workdir.
-    return '='.join((self._PANTS_OWNER_ARG_PREFIX, workdir))
+    return b'='.join((self._PANTS_OWNER_ARG_PREFIX, workdir))
 
   def _create_fingerprint_arg(self, fingerprint):
-    return '='.join((self.FINGERPRINT_CMD_KEY, fingerprint))
+    return b'='.join((self.FINGERPRINT_CMD_KEY, fingerprint))
 
   @staticmethod
   def _fingerprint(jvm_options, classpath, java_version):
@@ -119,9 +119,9 @@ def _fingerprint(jvm_options, classpath, java_version):
     """
     digest = hashlib.sha1()
     # TODO(John Sirois): hash classpath contents?
-    [digest.update(item) for item in (''.join(sorted(jvm_options)),
-                                      ''.join(sorted(classpath)),
-                                      repr(java_version))]
+    [digest.update(item) for item in (b''.join(sorted(jvm_options)),
+                                      b''.join(sorted(classpath)),
+                                      repr(java_version).encode('utf-8'))]
     return digest.hexdigest()
 
   def _runner(self, classpath, main, jvm_options, args, cwd=None):
@@ -226,8 +226,8 @@ def ensure_connectable(self, nailgun):
   def _spawn_nailgun_server(self, fingerprint, jvm_options, classpath, stdout, stderr, stdin):
     """Synchronously spawn a new nailgun server."""
     # Truncate the nailguns stdout & stderr.
-    safe_file_dump(self._ng_stdout, '')
-    safe_file_dump(self._ng_stderr, '')
+    safe_file_dump(self._ng_stdout, b'')
+    safe_file_dump(self._ng_stderr, b'')
 
     jvm_options = jvm_options + [self._PANTS_NG_BUILDROOT_ARG,
                                  self._create_owner_arg(self._workdir),

src/python/pants/java/nailgun_protocol.py

@@ -8,9 +8,6 @@
 import struct
 from builtins import bytes, object, str, zip
 
-import six
-from future.utils import binary_type
-
 
 STDIO_DESCRIPTORS = (0, 1, 2)
 
@@ -147,9 +144,9 @@ def write_chunk(cls, sock, chunk_type, payload=b''):
   @classmethod
   def construct_chunk(cls, chunk_type, payload, encoding='utf-8'):
     """Construct and return a single chunk."""
-    if isinstance(payload, six.text_type):
+    if isinstance(payload, str):
       payload = payload.encode(encoding)
-    elif not isinstance(payload, six.binary_type):
+    elif not isinstance(payload, bytes):
       raise TypeError('cannot encode type: {}'.format(type(payload)))
 
     header = struct.pack(cls.HEADER_FMT, len(payload), chunk_type)
@@ -228,7 +225,7 @@ def send_stderr(cls, sock, payload):
     cls.write_chunk(sock, ChunkType.STDERR, payload)
 
   @classmethod
-  def send_exit(cls, sock, payload=''):
+  def send_exit(cls, sock, payload=b''):
     """Send the Exit chunk over the specified socket."""
     cls.write_chunk(sock, ChunkType.EXIT, payload)
 
@@ -249,9 +246,9 @@ def isatty_to_env(cls, stdin, stdout, stderr):
     def gen_env_vars():
       for fd_id, fd in zip(STDIO_DESCRIPTORS, (stdin, stdout, stderr)):
         is_atty = fd.isatty()
-        yield (cls.TTY_ENV_TMPL.format(fd_id), binary_type(str(int(is_atty))))
+        yield (cls.TTY_ENV_TMPL.format(fd_id), str(int(is_atty)).encode('utf-8'))
         if is_atty:
-          yield (cls.TTY_PATH_ENV.format(fd_id), os.ttyname(fd.fileno()) or '')
+          yield (cls.TTY_PATH_ENV.format(fd_id), os.ttyname(fd.fileno()) or b'')
     return dict(gen_env_vars())
 
   @classmethod

src/python/pants/option/BUILD

@@ -3,6 +3,7 @@
 
 python_library(
   dependencies=[
+    '3rdparty/python:configparser',
     '3rdparty/python:future',
     '3rdparty/python:ansicolors',
     '3rdparty/python:setuptools',

src/python/pants/option/config.py

@@ -4,14 +4,14 @@
 
 from __future__ import absolute_import, division, print_function, unicode_literals
 
+import configparser
 import getpass
 import io
 import itertools
 import os
 from contextlib import contextmanager
 
 import six
-from six.moves import configparser
 from twitter.common.collections import OrderedSet
 
 from pants.base.build_environment import get_buildroot, get_pants_cachedir, get_pants_configdir
@@ -85,7 +85,7 @@ def _meta_load(cls, open_ctx, config_items, seed_values=None):
     for config_item in config_items:
       parser = cls._create_parser(seed_values)
       with open_ctx(config_item) as ini:
-        parser.readfp(ini)
+        parser.read_file(ini)
       config_path = config_item.path if hasattr(config_item, 'path') else config_item
       single_file_configs.append(_SingleFileConfig(config_path, parser))
 
@@ -120,7 +120,7 @@ def update_dir_from_seed_values(key, default):
     update_dir_from_seed_values('pants_supportdir', 'build-support')
     update_dir_from_seed_values('pants_distdir', 'dist')
 
-    return configparser.SafeConfigParser(all_seed_values)
+    return configparser.ConfigParser(all_seed_values)
 
   def get(self, section, option, type_=six.string_types, default=None):
     """Retrieves option from the specified section (or 'DEFAULT') and attempts to parse it as type.

src/python/pants/pantsd/pants_daemon.py

@@ -326,9 +326,9 @@ def _run_services(self, services):
       self._logger.info('starting service {}'.format(service))
       try:
         service_thread.start()
-      except (RuntimeError, service.ServiceError):
+      except (RuntimeError, FSEventService.ServiceError):
         self.shutdown(service_thread_map)
-        raise self.StartupFailure('service {} failed to start, shutting down!'.format(service))
+        raise PantsDaemon.StartupFailure('service {} failed to start, shutting down!'.format(service))
 
     # Once all services are started, write our pid.
     self.write_pid()
@@ -339,7 +339,7 @@ def _run_services(self, services):
       for service, service_thread in service_thread_map.items():
         if not service_thread.is_alive():
           self.shutdown(service_thread_map)
-          raise self.RuntimeFailure('service failure for {}, shutting down!'.format(service))
+          raise PantsDaemon.RuntimeFailure('service failure for {}, shutting down!'.format(service))
         else:
           # Avoid excessive CPU utilization.
           service_thread.join(self.JOIN_TIMEOUT_SECONDS)

src/python/pants/pantsd/process_manager.py

@@ -176,7 +176,8 @@ def read_metadata_by_name(self, name, metadata_key, caster=None):
     """
     file_path = self._metadata_file_path(name, metadata_key)
     try:
-      return self._maybe_cast(read_file(file_path).strip(), caster)
+      metadata = read_file(file_path, binary_mode=False).strip()
+      return self._maybe_cast(metadata, caster)
     except (IOError, OSError):
       return None
 
@@ -189,7 +190,7 @@ def write_metadata_by_name(self, name, metadata_key, metadata_value):
     """
     self._maybe_init_metadata_dir_by_name(name)
     file_path = self._metadata_file_path(name, metadata_key)
-    safe_file_dump(file_path, metadata_value)
+    safe_file_dump(file_path, metadata_value, binary_mode=False)
 
   def await_metadata_by_name(self, name, metadata_key, timeout, caster=None):
     """Block up to a timeout for process metadata to arrive on disk.
@@ -215,7 +216,7 @@ def purge_metadata_by_name(self, name):
     try:
       rm_rf(meta_dir)
     except OSError as e:
-      raise self.MetadataError('failed to purge metadata directory {}: {!r}'.format(meta_dir, e))
+      raise ProcessMetadataManager.MetadataError('failed to purge metadata directory {}: {!r}'.format(meta_dir, e))
 
 
 class ProcessManager(ProcessMetadataManager):
@@ -319,7 +320,7 @@ def get_subprocess_output(cls, command, ignore_stderr=True, **kwargs):
       kwargs.setdefault('stderr', subprocess.STDOUT)
 
     try:
-      return subprocess.check_output(command, **kwargs)
+      return subprocess.check_output(command, **kwargs).decode('utf-8').strip()
     except (OSError, subprocess.CalledProcessError) as e:
       subprocess_output = getattr(e, 'output', '').strip()
       raise cls.ExecutionError(str(e), subprocess_output)
@@ -400,7 +401,7 @@ def purge_metadata(self, force=False):
     :raises: `ProcessManager.MetadataError` when OSError is encountered on metadata dir removal.
     """
     if not force and self.is_alive():
-      raise self.MetadataError('cannot purge metadata for a running process!')
+      raise ProcessMetadataManager.MetadataError('cannot purge metadata for a running process!')
 
     super(ProcessManager, self).purge_metadata_by_name(self._name)
 
@@ -434,7 +435,7 @@ def terminate(self, signal_chain=KILL_CHAIN, kill_wait=KILL_WAIT_SEC, purge=True
           pass
 
     if alive:
-      raise self.NonResponsiveProcess('failed to kill pid {pid} with signals {chain}'
+      raise ProcessManager.NonResponsiveProcess('failed to kill pid {pid} with signals {chain}'
                                       .format(pid=self.pid, chain=signal_chain))
 
     if purge:

src/python/pants/pantsd/service/fs_event_service.py

@@ -123,7 +123,7 @@ def run(self):
     """Main service entrypoint. Called via Thread.start() via PantsDaemon.run()."""
 
     if not (self._watchman and self._watchman.is_alive()):
-      raise self.ServiceError('watchman is not running, bailing!')
+      raise PantsService.ServiceError('watchman is not running, bailing!')
 
     # Enable watchman for the build root.
     self._watchman.watch_project(self._build_root)

src/python/pants/pantsd/watchman.py

@@ -76,7 +76,7 @@ def _is_valid_executable(self, binary_path):
 
   def _normalize_watchman_path(self, watchman_path):
     if not self._is_valid_executable(watchman_path):
-      raise self.ExecutionError('invalid watchman binary at {}!'.format(watchman_path))
+      raise ProcessManager.ExecutionError('invalid watchman binary at {}!'.format(watchman_path))
     return os.path.abspath(watchman_path)
 
   def _maybe_init_metadata(self):
@@ -100,11 +100,11 @@ def _parse_pid_from_output(self, output):
     except ValueError:
       # JSON parse failure.
       self._logger.critical('invalid output from watchman!\n{output!s}'.format(output=output))
-      raise self.InvalidCommandOutput(output)
+      raise ProcessManager.InvalidCommandOutput(output)
     except KeyError:
       # Key access error on 'pid' - bad output from watchman.
       self._logger.critical('no pid from watchman!')
-      raise self.InvalidCommandOutput(output)
+      raise ProcessManager.InvalidCommandOutput(output)
 
   def launch(self):
     """Launch and synchronously write metadata.
@@ -134,7 +134,7 @@ def launch(self):
     # to give the server-side socket setup a few chances to quiesce before potentially orphaning it.
 
     get_output = functools.partial(self.get_subprocess_output, cmd)
-    output = retry_on_exception(get_output, 3, (self.ExecutionError,), lambda n: n * .5)
+    output = retry_on_exception(get_output, 3, (ProcessManager.ExecutionError,), lambda n: n * .5)
 
     # Parse the watchman PID from the cli output.
     pid = self._parse_pid_from_output(output)

src/python/pants/pantsd/watchman_launcher.py

@@ -90,7 +90,7 @@ def maybe_launch(self):
       self._logger.debug('launching watchman')
       try:
         self.watchman.launch()
-      except (self.watchman.ExecutionError, self.watchman.InvalidCommandOutput) as e:
+      except (Watchman.ExecutionError, Watchman.InvalidCommandOutput) as e:
         self._logger.fatal('failed to launch watchman: {!r})'.format(e))
         raise
 

tests/python/pants_test/binaries/BUILD

@@ -11,5 +11,6 @@ python_tests(
     'src/python/pants/util:contextutil',
     'src/python/pants/util:dirutil',
     'tests/python/pants_test:test_base',
+    'tests/python/pants_test:base_test',
   ]
 )