Properly handle unicode and byte streams with pantsd for Python 3 (#7130

)

## Problem
Any test with `@ensure_daemon` fails when ran with `./pants3`. This is due to unicode issues.

For example, `./pants3 test tests/python/pants_test/base:exiter_integration` will fail with:

```python
Exception caught: (builtins.TypeError)
  File "/home/eric/pants/src/python/pants/bin/pants_loader.py", line 89, in <module>
    main()
  File "/home/eric/pants/src/python/pants/bin/pants_loader.py", line 85, in main
    PantsLoader.run()
  File "/home/eric/pants/src/python/pants/bin/pants_loader.py", line 81, in run
    cls.load_and_execute(entrypoint)
  File "/home/eric/pants/src/python/pants/bin/pants_loader.py", line 74, in load_and_execute
    entrypoint_main()
  File "/home/eric/pants/src/python/pants/bin/pants_exe.py", line 39, in main
    PantsRunner(exiter, start_time=start_time).run()
  File "/home/eric/pants/src/python/pants/bin/pants_runner.py", line 48, in run
    return RemotePantsRunner(self._exiter, self._args, self._env, options_bootstrapper).run()
  File "/home/eric/pants/src/python/pants/bin/remote_pants_runner.py", line 190, in run
    self._run_pants_with_retry(pantsd_handle)
  File "/home/eric/pants/src/python/pants/bin/remote_pants_runner.py", line 114, in _run_pants_with_retry
    return self._connect_and_execute(pantsd_handle.port)
  File "/home/eric/pants/src/python/pants/bin/remote_pants_runner.py", line 155, in _connect_and_execute
    result = client.execute(self.PANTS_COMMAND, *self._args, **modified_env)
  File "/home/eric/pants/src/python/pants/java/nailgun_client.py", line 269, in execute
    return self._session.execute(cwd, main_class, *args, **environment)
  File "/home/eric/pants/src/python/pants/java/nailgun_client.py", line 109, in execute
    return self._process_session()
  File "/home/eric/pants/src/python/pants/java/nailgun_client.py", line 80, in _process_session
    self._write_flush(self._stdout, payload)
  File "/home/eric/pants/src/python/pants/java/nailgun_client.py", line 63, in _write_flush
    fd.write(payload)

Exception message: write() argument must be str, not bytes
```

## Solution
Use `sys.std{out,err}.buffer` with Py3. 

We reaffirmed in #7073 a prior decision that the `Exiter` related code should be using a bytes interface. However, we did not fix the pantsd related code because it was causing regressions. We now fix these usages.

Note that in `pants_daemon.py`, we override `sys.stdout` to our own custom `_LoggerStream` object. To ensure Python 3 support, we add a `buffer` property.

## Caveats
### Exiter integration still fails on macOS
`tests/python/pants_test/base:exiter_integration` will fail on macOS still, due to an upstream non fork safe osx lib (see https://bugs.python.org/issue28342). But this bug also affects Python 2.

### Python 3.7 daemon never executes
See #7160.

.travis.yml

@@ -642,169 +642,165 @@ matrix:
         - *py3_linux_test_config_env
         - CACHE_NAME=integrationshard0
       script:
-        - ./build-support/bin/travis-ci.sh -c -i 0/15
+        - ./build-support/bin/travis-ci.sh -c -i 0/17
 
     - <<: *py3_linux_test_config
       name: "Integration tests for pants - shard 1 (Py3 PEX)"
       env:
         - *py3_linux_test_config_env
         - CACHE_NAME=integrationshard1
       script:
-        - ./build-support/bin/travis-ci.sh -c -i 1/15
+        - ./build-support/bin/travis-ci.sh -c -i 1/17
 
     - <<: *py3_linux_test_config
       name: "Integration tests for pants - shard 2 (Py3 PEX)"
       env:
         - *py3_linux_test_config_env
         - CACHE_NAME=integrationshard2
       script:
-        - ./build-support/bin/travis-ci.sh -c -i 2/15
+        - ./build-support/bin/travis-ci.sh -c -i 2/17
 
     - <<: *py3_linux_test_config
       name: "Integration tests for pants - shard 3 (Py3 PEX)"
       env:
         - *py3_linux_test_config_env
         - CACHE_NAME=integrationshard3
       script:
-        - ./build-support/bin/travis-ci.sh -c -i 3/15
+        - ./build-support/bin/travis-ci.sh -c -i 3/17
 
     - <<: *py3_linux_test_config
       name: "Integration tests for pants - shard 4 (Py3 PEX)"
       env:
         - *py3_linux_test_config_env
         - CACHE_NAME=integrationshard4
       script:
-        - ./build-support/bin/travis-ci.sh -c -i 4/15
+        - ./build-support/bin/travis-ci.sh -c -i 4/17
 
     - <<: *py3_linux_test_config
       name: "Integration tests for pants - shard 5 (Py3 PEX)"
       env:
         - *py3_linux_test_config_env
         - CACHE_NAME=integrationshard5
       script:
-        - ./build-support/bin/travis-ci.sh -c -i 5/15
+        - ./build-support/bin/travis-ci.sh -c -i 5/17
 
     - <<: *py3_linux_test_config
       name: "Integration tests for pants - shard 6 (Py3 PEX)"
       env:
         - *py3_linux_test_config_env
         - CACHE_NAME=integrationshard6
       script:
-        - ./build-support/bin/travis-ci.sh -c -i 6/15
+        - ./build-support/bin/travis-ci.sh -c -i 6/17
 
     - <<: *py3_linux_test_config
       name: "Integration tests for pants - shard 7 (Py3 PEX)"
       env:
         - *py3_linux_test_config_env
         - CACHE_NAME=integrationshard7
       script:
-        - ./build-support/bin/travis-ci.sh -c -i 7/15
+        - ./build-support/bin/travis-ci.sh -c -i 7/17
 
     - <<: *py3_linux_test_config
       name: "Integration tests for pants - shard 8 (Py3 PEX)"
       env:
         - *py3_linux_test_config_env
         - CACHE_NAME=integrationshard8
       script:
-        - ./build-support/bin/travis-ci.sh -c -i 8/15
+        - ./build-support/bin/travis-ci.sh -c -i 8/17
 
     - <<: *py3_linux_test_config
       name: "Integration tests for pants - shard 9 (Py3 PEX)"
       env:
         - *py3_linux_test_config_env
         - CACHE_NAME=integrationshard9
       script:
-        - ./build-support/bin/travis-ci.sh -c -i 9/15
+        - ./build-support/bin/travis-ci.sh -c -i 9/17
 
     - <<: *py3_linux_test_config
       name: "Integration tests for pants - shard 10 (Py3 PEX)"
       env:
         - *py3_linux_test_config_env
         - CACHE_NAME=integrationshard10
       script:
-        - ./build-support/bin/travis-ci.sh -c -i 10/15
+        - ./build-support/bin/travis-ci.sh -c -i 10/17
 
     - <<: *py3_linux_test_config
       name: "Integration tests for pants - shard 11 (Py3 PEX)"
       env:
         - *py3_linux_test_config_env
         - CACHE_NAME=integrationshard11
       script:
-        - ./build-support/bin/travis-ci.sh -c -i 11/15
+        - ./build-support/bin/travis-ci.sh -c -i 11/17
 
     - <<: *py3_linux_test_config
       name: "Integration tests for pants - shard 12 (Py3 PEX)"
       env:
         - *py3_linux_test_config_env
         - CACHE_NAME=integrationshard12
       script:
-        - ./build-support/bin/travis-ci.sh -c -i 12/15
+        - ./build-support/bin/travis-ci.sh -c -i 12/17
 
     - <<: *py3_linux_test_config
       name: "Integration tests for pants - shard 13 (Py3 PEX)"
       env:
         - *py3_linux_test_config_env
         - CACHE_NAME=integrationshard13
       script:
-        - ./build-support/bin/travis-ci.sh -c -i 13/15
+        - ./build-support/bin/travis-ci.sh -c -i 13/17
 
     - <<: *py3_linux_test_config
       name: "Integration tests for pants - shard 14 (Py3 PEX)"
       env:
         - *py3_linux_test_config_env
         - CACHE_NAME=integrationshard14
       script:
-        - ./build-support/bin/travis-ci.sh -c -i 14/15
+        - ./build-support/bin/travis-ci.sh -c -i 14/17
 
-    - <<: *py2_linux_test_config
-      name: "Blacklisted integration tests for pants - shard 0 (Py2 PEX w/ Py3 constraints)"
-      stage: *test
+    - <<: *py3_linux_test_config
+      name: "Integration tests for pants - shard 15 (Py3 PEX)"
       env:
-        - *py2_linux_test_config_env
-        - PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS="['CPython>=3.6']"
-        - CACHE_NAME=integrationshard0.py2blacklist
+        - *py3_linux_test_config_env
+        - CACHE_NAME=integrationshard15
       script:
-        - ./build-support/bin/travis-ci.sh -c2w -i 0/5
+        - ./build-support/bin/travis-ci.sh -c -i 15/17
 
-    - <<: *py2_linux_test_config
-      name: "Blacklisted integration tests for pants - shard 1 (Py2 PEX w/ Py3 constraints)"
-      stage: *test
+    - <<: *py3_linux_test_config
+      name: "Integration tests for pants - shard 16 (Py3 PEX)"
       env:
-        - *py2_linux_test_config_env
-        - PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS="['CPython>=3.6']"
-        - CACHE_NAME=integrationshard1.py2blacklist
+        - *py3_linux_test_config_env
+        - CACHE_NAME=integrationshard16
       script:
-        - ./build-support/bin/travis-ci.sh -c2w -i 1/5
+        - ./build-support/bin/travis-ci.sh -c -i 16/17
 
     - <<: *py2_linux_test_config
-      name: "Blacklisted integration tests for pants - shard 2 (Py2 PEX w/ Py3 constraints)"
+      name: "Blacklisted integration tests for pants - shard 0 (Py2 PEX w/ Py3 constraints)"
       stage: *test
       env:
         - *py2_linux_test_config_env
         - PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS="['CPython>=3.6']"
-        - CACHE_NAME=integrationshard2.py2blacklist
+        - CACHE_NAME=integrationshard0.py2blacklist
       script:
-        - ./build-support/bin/travis-ci.sh -c2w -i 2/5
+        - ./build-support/bin/travis-ci.sh -c2w -i 0/3
 
     - <<: *py2_linux_test_config
-      name: "Blacklisted integration tests for pants - shard 3 (Py2 PEX w/ Py3 constraints)"
+      name: "Blacklisted integration tests for pants - shard 1 (Py2 PEX w/ Py3 constraints)"
       stage: *test
       env:
         - *py2_linux_test_config_env
         - PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS="['CPython>=3.6']"
-        - CACHE_NAME=integrationshard3.py2blacklist
+        - CACHE_NAME=integrationshard1.py2blacklist
       script:
-        - ./build-support/bin/travis-ci.sh -c2w -i 3/5
+        - ./build-support/bin/travis-ci.sh -c2w -i 1/3
 
     - <<: *py2_linux_test_config
-      name: "Blacklisted integration tests for pants - shard 4 (Py2 PEX w/ Py3 constraints)"
+      name: "Blacklisted integration tests for pants - shard 2 (Py2 PEX w/ Py3 constraints)"
       stage: *test
       env:
         - *py2_linux_test_config_env
         - PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS="['CPython>=3.6']"
-        - CACHE_NAME=integrationshard4.py2blacklist
+        - CACHE_NAME=integrationshard2.py2blacklist
       script:
-        - ./build-support/bin/travis-ci.sh -c2w -i 4/5
+        - ./build-support/bin/travis-ci.sh -c2w -i 2/3
 
     - <<: *py2_linux_test_config
       name: "Integration tests for pants - shard 0 (Py2 PEX)"

build-support/known_py3_pex_failures.txt

@@ -1,11 +1,6 @@
 tests/python/pants_test/backend/jvm/tasks:scala_repl_integration
 tests/python/pants_test/backend/project_info/tasks:idea_plugin_integration
 tests/python/pants_test/backend/python/tasks:integration
-tests/python/pants_test/backend/python:integration
-tests/python/pants_test/base:exiter_integration
-tests/python/pants_test/engine/legacy:changed_integration
 tests/python/pants_test/engine/legacy:console_rule_integration
-tests/python/pants_test/engine/legacy:owners_integration
-tests/python/pants_test/engine:scheduler_integration
 tests/python/pants_test/pantsd:pantsd_integration
 tests/python/pants_test/projects:testprojects_integration

build-support/travis/generate_travis_yml.py

@@ -8,8 +8,8 @@
 import pystache
 
 
-num_py3_integration_shards = 15
-num_py2_blacklist_integration_shards = 5
+num_py3_integration_shards = 17
+num_py2_blacklist_integration_shards = 3
 num_cron_integration_shards = 20
 
 

src/python/pants/base/exception_sink.py

@@ -143,9 +143,6 @@ def reset_exiter(cls, exiter):
     # NB: mutate process-global state!
     sys.excepthook = cls._log_unhandled_exception_and_exit
 
-  # TODO(7080): audit all usages of this function and add Py3 support, e.g. use `sys.stderr.buffer` instead
-  # of sys.stdout. However, we shouldn't need to proactively enforce that the stream is BytesIO, because Py3
-  # should already check when we try to use this.
   @classmethod
   def reset_interactive_output_stream(cls, interactive_output_stream):
     """

src/python/pants/bin/daemon_pants_runner.py

@@ -12,7 +12,7 @@
 from builtins import open, zip
 from contextlib import contextmanager
 
-from future.utils import raise_with_traceback
+from future.utils import PY3, raise_with_traceback
 from setproctitle import setproctitle as set_process_title
 
 from pants.base.build_environment import get_buildroot
@@ -284,7 +284,7 @@ def post_fork_child(self):
     # TODO: test the above!
     ExceptionSink.reset_exiter(self._exiter)
 
-    ExceptionSink.reset_interactive_output_stream(sys.stderr)
+    ExceptionSink.reset_interactive_output_stream(sys.stderr.buffer if PY3 else sys.stderr)
 
     # Ensure anything referencing sys.argv inherits the Pailgun'd args.
     sys.argv = self._args

src/python/pants/bin/remote_pants_runner.py

@@ -11,7 +11,7 @@
 from builtins import object, str
 from contextlib import contextmanager
 
-from future.utils import raise_with_traceback
+from future.utils import PY3, raise_with_traceback
 
 from pants.base.exception_sink import ExceptionSink
 from pants.console.stty_utils import STTYSettings
@@ -57,8 +57,8 @@ def __init__(self, exiter, args, env, options_bootstrapper, stdin=None, stdout=N
     self._options_bootstrapper = options_bootstrapper
     self._bootstrap_options = options_bootstrapper.bootstrap_options
     self._stdin = stdin or sys.stdin
-    self._stdout = stdout or sys.stdout
-    self._stderr = stderr or sys.stderr
+    self._stdout = stdout or (sys.stdout.buffer if PY3 else sys.stdout)
+    self._stderr = stderr or (sys.stderr.buffer if PY3 else sys.stderr)
 
   @contextmanager
   def _trapped_signals(self, client):

src/python/pants/java/nailgun_client.py

@@ -11,6 +11,8 @@
 import sys
 from builtins import object, str
 
+from future.utils import PY3
+
 from pants.java.nailgun_io import NailgunStreamWriter
 from pants.java.nailgun_protocol import ChunkType, NailgunProtocol
 from pants.util.osutil import safe_kill
@@ -179,8 +181,8 @@ def __init__(self, host=DEFAULT_NG_HOST, port=DEFAULT_NG_PORT, ins=sys.stdin, ou
     self._address = (host, port)
     self._address_string = ':'.join(str(i) for i in self._address)
     self._stdin = ins
-    self._stdout = out or sys.stdout
-    self._stderr = err or sys.stderr
+    self._stdout = out or (sys.stdout.buffer if PY3 else sys.stdout)
+    self._stderr = err or (sys.stderr.buffer if PY3 else sys.stderr)
     self._workdir = workdir or os.path.abspath(os.path.curdir)
     self._exit_on_broken_pipe = exit_on_broken_pipe
     self._expects_pid = expects_pid

src/python/pants/pantsd/pants_daemon.py

@@ -39,7 +39,14 @@
 
 
 class _LoggerStream(object):
-  """A sys.{stdout,stderr} replacement that pipes output to a logger."""
+  """A sys.std{out,err} replacement that pipes output to a logger.
+
+  N.B. `logging.Logger` expects unicode. However, most of our outstream logic, such as in `Exiter.py`,
+  will use `sys.std{out,err}.buffer` and thus a bytes interface when running with Python 3. So, we must provide
+  a `buffer` property, and change the semantics of the buffer to always convert the message to unicode. This
+  is an unfortunate code smell, as `logging` does not expose a bytes interface so this is
+  the best solution we could think of.
+  """
 
   def __init__(self, logger, log_level, handler):
     """
@@ -53,6 +60,7 @@ def __init__(self, logger, log_level, handler):
     self._handler = handler
 
   def write(self, msg):
+    msg = ensure_text(msg)
     for line in msg.rstrip().splitlines():
       self._logger.log(self._log_level, line.rstrip())
 
@@ -65,6 +73,10 @@ def isatty(self):
   def fileno(self):
     return self._handler.stream.fileno()
 
+  @property
+  def buffer(self):
+    return self
+
 
 class PantsDaemon(FingerprintedProcessManager):
   """A daemon that manages PantsService instances."""

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

@@ -11,6 +11,7 @@
 import threading
 from builtins import open
 
+from future.utils import PY3
 from twitter.common.dirutil import Fileset
 
 from pants.init.target_roots_calculator import TargetRootsCalculator
@@ -135,7 +136,7 @@ def _process_event_queue(self):
     try:
       subscription, is_initial_event, files = (event['subscription'],
                                                event['is_fresh_instance'],
-                                               [f.decode('utf-8') for f in event['files']])
+                                               event['files'] if PY3 else [f.decode('utf-8') for f in event['files']])
     except (KeyError, UnicodeDecodeError) as e:
       self._logger.warn('%r raised by invalid watchman event: %s', e, event)
       return