gclient: Preserve ANSI color codes when calling hooks.

If a hook prints error/warning output that's color-coded, gclient
will cause the coloring to be disabled since those hooks aren't
called directly from a terminal.

By emulating a terminal when launching subprocs from gclient, we can
convince them to keep the color escape codes.

LED builds with both //third_party/depot_tools rolled to this CL, as
well as depot_tools in the recipe bundle rolled to this CL:
linux: https://ci.chromium.org/swarming/task/4e40237985888310
mac: https://ci.chromium.org/swarming/task/4e4023ea0c829710
win: https://ci.chromium.org/swarming/task/4e4024612e03dc10

Bug: 1034063
Change-Id: I4150f66ef215ece06f4c32482dcd4ded14eb1bfe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2368435
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Ben Pastene <bpastene@chromium.org>

gclient_utils.py

@@ -10,6 +10,7 @@ import codecs
 import collections
 import contextlib
 import datetime
+import errno
 import functools
 import io
 import logging
@@ -585,9 +586,19 @@ def CheckCallAndFilter(args, print_stdout=False, filter_fn=None,
   sleep_interval = RETRY_INITIAL_SLEEP
   run_cwd = kwargs.get('cwd', os.getcwd())
   for attempt in range(RETRY_MAX + 1):
+    # If our stdout is a terminal, then pass in a psuedo-tty pipe to our
+    # subprocess when filtering its output. This makes the subproc believe
+    # it was launched from a terminal, which will preserve ANSI color codes.
+    if sys.stdout.isatty():
+      pipe_reader, pipe_writer = os.openpty()
+    else:
+      pipe_reader, pipe_writer = os.pipe()
+
     kid = subprocess2.Popen(
-        args, bufsize=0, stdout=subprocess2.PIPE, stderr=subprocess2.STDOUT,
+        args, bufsize=0, stdout=pipe_writer, stderr=subprocess2.STDOUT,
         **kwargs)
+    # Close the write end of the pipe once we hand it off to the child proc.
+    os.close(pipe_writer)
 
     GClientChildren.add(kid)
 
@@ -608,7 +619,14 @@ def CheckCallAndFilter(args, print_stdout=False, filter_fn=None,
     try:
       line_start = None
       while True:
-        in_byte = kid.stdout.read(1)
+        try:
+          in_byte = os.read(pipe_reader, 1)
+        except (IOError, OSError) as e:
+          if e.errno == errno.EIO:
+            # An errno.EIO means EOF?
+            in_byte = None
+          else:
+            raise e
         is_newline = in_byte in (b'\n', b'\r')
         if not in_byte:
           break
@@ -629,8 +647,8 @@ def CheckCallAndFilter(args, print_stdout=False, filter_fn=None,
       if line_start is not None:
         filter_line(command_output, line_start)
 
+      os.close(pipe_reader)
       rv = kid.wait()
-      kid.stdout.close()
 
       # Don't put this in a 'finally,' since the child may still run if we get
       # an exception.

tests/gclient_scm_test.py

@@ -1009,7 +1009,7 @@ class BranchHeadsTest(fake_repos.FakeReposTestBase):
     self.options = BaseGitWrapperTestCase.OptionsObject()
     self.url = self.git_base + 'repo_1'
     self.mirror = None
-    mock.patch('sys.stdout').start()
+    mock.patch('sys.stdout', StringIO()).start()
     self.addCleanup(mock.patch.stopall)
 
   def setUpMirror(self):
@@ -1122,7 +1122,7 @@ class GerritChangesTest(fake_repos.FakeReposTestBase):
     self.options = BaseGitWrapperTestCase.OptionsObject()
     self.url = self.git_base + 'repo_1'
     self.mirror = None
-    mock.patch('sys.stdout').start()
+    mock.patch('sys.stdout', StringIO()).start()
     self.addCleanup(mock.patch.stopall)
 
   def setUpMirror(self):

tests/gclient_utils_test.py

@@ -32,23 +32,31 @@ import subprocess2
 class CheckCallAndFilterTestCase(unittest.TestCase):
   class ProcessIdMock(object):
     def __init__(self, test_string, return_code=0):
-      self.stdout = io.BytesIO(test_string.encode('utf-8'))
+      self.stdout = test_string.encode('utf-8')
       self.pid = 9284
       self.return_code = return_code
 
     def wait(self):
       return self.return_code
 
+  def PopenMock(self, *args, **kwargs):
+    kid = self.kids.pop(0)
+    stdout = kwargs.get('stdout')
+    os.write(stdout, kid.stdout)
+    return kid
+
   def setUp(self):
     super(CheckCallAndFilterTestCase, self).setUp()
     self.printfn = io.StringIO()
     self.stdout = io.BytesIO()
+    self.kids = []
     if sys.version_info.major == 2:
       mock.patch('sys.stdout', self.stdout).start()
       mock.patch('__builtin__.print', self.printfn.write).start()
     else:
       mock.patch('sys.stdout', mock.Mock()).start()
       mock.patch('sys.stdout.buffer', self.stdout).start()
+      mock.patch('sys.stdout.isatty', return_value=False).start()
       mock.patch('builtins.print', self.printfn.write).start()
     mock.patch('sys.stdout.flush', lambda: None).start()
     self.addCleanup(mock.patch.stopall)
@@ -59,7 +67,8 @@ class CheckCallAndFilterTestCase(unittest.TestCase):
     args = ['boo', 'foo', 'bar']
     test_string = 'ahah\naccb\nallo\naddb\n✔'
 
-    mockPopen.return_value = self.ProcessIdMock(test_string)
+    self.kids = [self.ProcessIdMock(test_string)]
+    mockPopen.side_effect = self.PopenMock
 
     line_list = []
     result = gclient_utils.CheckCallAndFilter(
@@ -77,7 +86,7 @@ class CheckCallAndFilterTestCase(unittest.TestCase):
     self.assertEqual(self.stdout.getvalue(), b'')
 
     mockPopen.assert_called_with(
-        args, cwd=cwd, stdout=subprocess2.PIPE, stderr=subprocess2.STDOUT,
+        args, cwd=cwd, stdout=mock.ANY, stderr=subprocess2.STDOUT,
         bufsize=0)
 
   @mock.patch('time.sleep')
@@ -87,10 +96,11 @@ class CheckCallAndFilterTestCase(unittest.TestCase):
     args = ['boo', 'foo', 'bar']
     test_string = 'ahah\naccb\nallo\naddb\n✔'
 
-    mockPopen.side_effect = [
+    self.kids = [
         self.ProcessIdMock(test_string, 1),
-        self.ProcessIdMock(test_string, 0),
+        self.ProcessIdMock(test_string, 0)
     ]
+    mockPopen.side_effect = self.PopenMock
 
     line_list = []
     result = gclient_utils.CheckCallAndFilter(
@@ -120,10 +130,10 @@ class CheckCallAndFilterTestCase(unittest.TestCase):
         mockPopen.mock_calls,
         [
             mock.call(
-                args, cwd=cwd, stdout=subprocess2.PIPE,
+                args, cwd=cwd, stdout=mock.ANY,
                 stderr=subprocess2.STDOUT, bufsize=0),
             mock.call(
-                args, cwd=cwd, stdout=subprocess2.PIPE,
+                args, cwd=cwd, stdout=mock.ANY,
                 stderr=subprocess2.STDOUT, bufsize=0),
         ])
 
@@ -139,7 +149,8 @@ class CheckCallAndFilterTestCase(unittest.TestCase):
     args = ['boo', 'foo', 'bar']
     test_string = 'ahah\naccb\nallo\naddb\n✔'
 
-    mockPopen.return_value = self.ProcessIdMock(test_string)
+    self.kids = [self.ProcessIdMock(test_string)]
+    mockPopen.side_effect = self.PopenMock
 
     result = gclient_utils.CheckCallAndFilter(
         args, cwd=cwd, show_header=True, always_show_header=True,