python: Use universal_newlines to get str from subprocess.

We can replace ‘universal_newlines’ with ‘text’ when we bump our
minimum Python version to 3.7.

Signed-off-by: Anders Kaseorg <anders@zulip.com>

scripts/lib/node_cache.py

@@ -2,9 +2,10 @@ import hashlib
 import json
 import os
 import shutil
+import subprocess
 from typing import List, Optional
 
-from scripts.lib.zulip_tools import run, subprocess_text_output
+from scripts.lib.zulip_tools import run
 
 ZULIP_PATH = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 ZULIP_SRV_PATH = "/srv"
@@ -30,14 +31,16 @@ def generate_sha1sum_node_modules(
     PACKAGE_JSON_FILE_PATH = os.path.join(setup_dir, 'package.json')
     YARN_LOCK_FILE_PATH = os.path.join(setup_dir, 'yarn.lock')
     sha1sum = hashlib.sha1()
-    sha1sum.update(subprocess_text_output(['cat', PACKAGE_JSON_FILE_PATH]).encode('utf8'))
+    with open(PACKAGE_JSON_FILE_PATH, "rb") as fb:
+        sha1sum.update(fb.read().strip())
     if os.path.exists(YARN_LOCK_FILE_PATH):
         # For backwards compatibility, we can't assume yarn.lock exists
-        sha1sum.update(subprocess_text_output(['cat', YARN_LOCK_FILE_PATH]).encode('utf8'))
+        with open(YARN_LOCK_FILE_PATH, "rb") as fb:
+            sha1sum.update(fb.read().strip())
     with open(YARN_PACKAGE_JSON) as f:
         yarn_version = json.load(f)['version']
         sha1sum.update(yarn_version.encode("utf8"))
-    sha1sum.update(subprocess_text_output(['node', '--version']).encode('utf8'))
+    sha1sum.update(subprocess.check_output(['node', '--version']).strip())
     yarn_args = get_yarn_args(production=production)
     sha1sum.update(''.join(sorted(yarn_args)).encode('utf8'))
     return sha1sum.hexdigest()

scripts/lib/upgrade-zulip-stage-2

@@ -232,7 +232,7 @@ migrations_needed = False
 if not args.skip_migrations:
     logging.info("Checking for needed migrations")
     migrations_output = subprocess.check_output(["./manage.py", "showmigrations"],
-                                                preexec_fn=su_to_zulip).decode("utf-8")
+                                                preexec_fn=su_to_zulip, universal_newlines=True)
     need_create_large_indexes = False
     for ln in migrations_output.split("\n"):
         line_str = ln.strip()

scripts/lib/zulip_tools.py

@@ -106,9 +106,6 @@ def is_invalid_upgrade(current_version: str, new_version: str) -> bool:
         return True
     return False
 
-def subprocess_text_output(args: Sequence[str]) -> str:
-    return subprocess.check_output(args, universal_newlines=True).strip()
-
 def get_zulip_pwent() -> pwd.struct_passwd:
     deploy_root_uid = os.stat(get_deploy_root()).st_uid
     if deploy_root_uid != 0:

scripts/setup/restore-backup

@@ -123,7 +123,7 @@ def restore_backup(tarball_file: IO[bytes]) -> None:
             rabbitmq_host = subprocess.check_output(
                 [os.path.join(settings.DEPLOY_ROOT,
                               "scripts", "get-django-setting"),
-                 "RABBITMQ_HOST"]).strip().decode("utf-8")
+                 "RABBITMQ_HOST"], universal_newlines=True).strip()
             if rabbitmq_host in ["127.0.0.1", "::1", "localhost", "localhost6"]:
                 run([os.path.join(settings.DEPLOY_ROOT,
                                   "scripts", "setup", "configure-rabbitmq")])

tools/i18n/process-mobile-i18n

@@ -10,8 +10,8 @@ def get_json_filename(locale: str) -> str:
     return f"locale/{locale}/mobile.json"
 
 def get_locales() -> List[str]:
-    output = check_output(['git', 'ls-files', 'locale'])
+    output = check_output(['git', 'ls-files', 'locale'], universal_newlines=True)
-    tracked_files = output.decode().split()
+    tracked_files = output.split()
     regex = re.compile(r'locale/(\w+)/LC_MESSAGES/django.po')
     locales = ['en']
     for tracked_file in tracked_files:

tools/review

@@ -15,7 +15,7 @@ def run(command: List[str]) -> None:
     subprocess.check_call(command)
 
 def check_output(command: List[str]) -> str:
-    return subprocess.check_output(command).decode('ascii')
+    return subprocess.check_output(command, universal_newlines=True)
 
 def get_git_branch() -> str:
     command = ['git', 'rev-parse', '--abbrev-ref', 'HEAD']

tools/update-zuliprc-api-field

@@ -57,7 +57,7 @@ for zuliprc_path in zuliprc_paths_list:
             arguments = ['./manage.py', 'print_initial_password', email]
         # We redirect 'stderr' to 'stdout' to avoid 'Connection to 127.0.0.1 closed'
         # appearing after this script finishes.
-        output = subprocess.check_output(arguments, stderr=subprocess.STDOUT).decode('UTF-8')
+        output = subprocess.check_output(arguments, stderr=subprocess.STDOUT, universal_newlines=True)
         new_key = output.split()[6]
 
         if new_key != key:

zerver/lib/mdiff.py

@@ -15,5 +15,5 @@ def diff_strings(output: str, expected_output: str) -> str:
         raise DiffException(msg)
 
     command = ['node', mdiff_path, output, expected_output]
-    diff = subprocess.check_output(command).decode('utf-8')
+    diff = subprocess.check_output(command, universal_newlines=True)
     return diff

zerver/logging_handlers.py

@@ -25,7 +25,8 @@ def try_git_describe() -> Optional[str]:
             ['git', 'describe', '--tags', '--match=[0-9]*', '--always', '--dirty', '--long'],
             stderr=subprocess.PIPE,
             cwd=os.path.join(os.path.dirname(__file__), '..'),
-        ).strip().decode('utf-8')
+            universal_newlines=True,
+        ).strip()
     except (FileNotFoundError, subprocess.CalledProcessError):  # nocoverage
         return None
 

zerver/management/commands/compilemessages.py

@@ -72,8 +72,8 @@ class Command(compilemessages.Command):
             raise Exception(f"Unknown language {locale}")
 
     def get_locales(self) -> List[str]:
-        output = check_output(['git', 'ls-files', 'locale'])
+        output = check_output(['git', 'ls-files', 'locale'], universal_newlines=True)
-        tracked_files = output.decode().split()
+        tracked_files = output.split()
         regex = re.compile(r'locale/(\w+)/LC_MESSAGES/django.po')
         locales = ['en']
         for tracked_file in tracked_files:

zerver/openapi/test_curl_examples.py

@@ -63,7 +63,7 @@ def test_generated_curl_examples_for_success(client: Client) -> None:
                 try:
                     # We split this across two lines so if curl fails and
                     # returns non-JSON output, we'll still print it.
-                    response_json = subprocess.check_output(generated_curl_command).decode('utf-8')
+                    response_json = subprocess.check_output(generated_curl_command, universal_newlines=True)
                     response = json.loads(response_json)
                     assert(response["result"] == "success")
                 except (AssertionError, Exception):

zerver/tests/test_email_mirror.py

@@ -1126,12 +1126,12 @@ class TestScriptMTA(ZulipTestCase):
 
         mail_template = self.fixture_data('simple.txt', type='email')
         mail = mail_template.format(stream_to_address=stream_to_address, sender=sender)
-        read_pipe, write_pipe = os.pipe()
+        subprocess.run(
-        os.write(write_pipe, mail.encode())
-        os.close(write_pipe)
-        subprocess.check_call(
             [script, '-r', stream_to_address, '-s', settings.SHARED_SECRET, '-t'],
-            stdin=read_pipe)
+            input=mail,
+            check=True,
+            universal_newlines=True,
+        )
 
     def test_error_no_recipient(self) -> None:
         script = os.path.join(os.path.dirname(__file__),
@@ -1142,21 +1142,17 @@ class TestScriptMTA(ZulipTestCase):
         stream_to_address = encode_email_address(stream)
         mail_template = self.fixture_data('simple.txt', type='email')
         mail = mail_template.format(stream_to_address=stream_to_address, sender=sender)
-        read_pipe, write_pipe = os.pipe()
+        p = subprocess.run(
-        os.write(write_pipe, mail.encode())
+            [script, '-s', settings.SHARED_SECRET, '-t'],
-        os.close(write_pipe)
+            input=mail,
-        success_call = True
+            stdout=subprocess.PIPE,
-        try:
+            universal_newlines=True,
-            subprocess.check_output([script, '-s', settings.SHARED_SECRET, '-t'],
+        )
-                                    stdin=read_pipe)
+        self.assertEqual(
-        except subprocess.CalledProcessError as e:
+            p.stdout,
-            self.assertEqual(
+            '5.1.1 Bad destination mailbox address: No missed message email address.\n',
-                e.output,
+        )
-                b'5.1.1 Bad destination mailbox address: No missed message email address.\n',
+        self.assertEqual(p.returncode, 67)
-            )
-            self.assertEqual(e.returncode, 67)
-            success_call = False
-        self.assertFalse(success_call)
 
 
 class TestEmailMirrorTornadoView(ZulipTestCase):