Explicitly propagate terminal size to gclient hooks.

gclient normally runs hooks in a pseudo terminal which has no
sense of the terminal size. If we want hooks to be able to change
what they output based on the terminal size, we have to explicitly
propagate the terminal size info down through the subprocess
environment.

Change-Id: I08f7c48ef78ea4eb9f5b791abb2a7e5ef8870050
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6161596
Reviewed-by: Scott Lee <ddoman@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@google.com>

gclient_utils.py

@@ -17,6 +17,7 @@ import platform
 import queue
 import re
 import shlex
+import shutil
 import stat
 import subprocess
 import sys
@@ -637,6 +638,22 @@ def CheckCallAndFilter(args,
         else:
             pipe_reader, pipe_writer = os.pipe()
 
+        # subprocess2 will use pseudoterminals (ptys) for the child process,
+        # and ptys do not support a terminal size directly, so if we want
+        # a hook to be able to customize what it does based on the terminal
+        # size, we need to explicitly pass the size information down to
+        # the subprocess. Setting COLUMNS and LINES explicitly in the
+        # environment will cause those values to be picked up by
+        # `shutil.get_terminal_size()` in the subprocess (and of course
+        # anything that checks for the env vars direcstly as well).
+        if 'env' not in kwargs:
+            kwargs['env'] = os.environ.copy()
+        if 'COLUMNS' not in kwargs['env'] or 'LINES' not in kwargs['env']:
+            size = shutil.get_terminal_size()
+            if size.columns and size.lines:
+                kwargs['env']['COLUMNS'] = str(size.columns)
+                kwargs['env']['LINES'] = str(size.lines)
+
         kid = subprocess2.Popen(args,
                                 bufsize=0,
                                 stdout=pipe_writer,

tests/gclient_utils_test.py

@@ -71,11 +71,15 @@ class CheckCallAndFilterTestCase(unittest.TestCase):
         ])
         self.assertEqual(self.stdout.getvalue(), b'')
 
-        mockPopen.assert_called_with(args,
-                                     cwd=cwd,
-                                     stdout=mock.ANY,
-                                     stderr=subprocess2.STDOUT,
-                                     bufsize=0)
+        kall = mockPopen.call_args
+        self.assertEqual(kall.args, (args, ))
+        self.assertEqual(kall.kwargs['cwd'], cwd)
+        self.assertEqual(kall.kwargs['stdout'], mock.ANY)
+        self.assertEqual(kall.kwargs['stderr'], subprocess2.STDOUT)
+        self.assertEqual(kall.kwargs['bufsize'], 0)
+        self.assertIn('env', kall.kwargs)
+        self.assertIn('COLUMNS', kall.kwargs['env'])
+        self.assertIn('LINES', kall.kwargs['env'])
 
     @mock.patch('time.sleep')
     @mock.patch('subprocess2.Popen')
@@ -117,18 +121,16 @@ class CheckCallAndFilterTestCase(unittest.TestCase):
 
         mockTime.assert_called_with(gclient_utils.RETRY_INITIAL_SLEEP)
 
-        self.assertEqual(mockPopen.mock_calls, [
-            mock.call(args,
-                      cwd=cwd,
-                      stdout=mock.ANY,
-                      stderr=subprocess2.STDOUT,
-                      bufsize=0),
-            mock.call(args,
-                      cwd=cwd,
-                      stdout=mock.ANY,
-                      stderr=subprocess2.STDOUT,
-                      bufsize=0),
-        ])
+        for i in range(2):
+            kall = mockPopen.mock_calls[i]
+            self.assertEqual(kall.args, (args, ))
+            self.assertEqual(kall.kwargs['cwd'], cwd)
+            self.assertEqual(kall.kwargs['stdout'], mock.ANY)
+            self.assertEqual(kall.kwargs['stderr'], subprocess2.STDOUT)
+            self.assertEqual(kall.kwargs['bufsize'], 0)
+            self.assertIn('env', kall.kwargs)
+            self.assertIn('COLUMNS', kall.kwargs['env'])
+            self.assertIn('LINES', kall.kwargs['env'])
 
         self.assertEqual(self.stdout.getvalue(), b'')
         self.assertEqual(