[damned-lies] Replace Popen by higher level run in run_shell_command



commit 5c3cf951928fc6ae2000eb3d44199dbfb43cad3e
Author: Claude Paroz <claude 2xlibre net>
Date:   Fri Sep 17 09:16:23 2021 +0200

    Replace Popen by higher level run in run_shell_command

 common/utils.py         | 28 +++++++++-------------------
 stats/utils.py          | 26 ++++++++++++++------------
 vertimus/tests/tests.py |  4 +++-
 3 files changed, 26 insertions(+), 32 deletions(-)
---
diff --git a/common/utils.py b/common/utils.py
index d1c4bf3c..a8f75719 100644
--- a/common/utils.py
+++ b/common/utils.py
@@ -1,8 +1,7 @@
-import errno
 import logging
 import os
 import socket
-from subprocess import Popen, PIPE
+from subprocess import PIPE, run
 
 from django.conf import settings
 from django.core.mail import EmailMessage
@@ -22,27 +21,18 @@ STATUS_OK = 0
 
 
 def run_shell_command(cmd, input_data=None, raise_on_error=False, env=None,
-                      cwd=None, **popen_kwargs):
+                      cwd=None, **extra_kwargs):
     logging.debug(cmd)
 
-    stdin = None
-    if input_data:
-        stdin = PIPE
     if env is not None:
         env = dict(os.environ, **env)
-    if cwd is not None:
-        cwd = str(cwd)  # Popen cwd doesn't support Path on Python 3.5
     shell = not isinstance(cmd, list)
-    pipe = Popen(cmd, shell=shell, stdin=stdin, stdout=PIPE, stderr=PIPE, env=env, cwd=cwd, **popen_kwargs)
-    if input_data:
-        try:
-            pipe.stdin.write(input_data)
-        except IOError as e:
-            if e.errno != errno.EPIPE:
-                raise
-    output, errout = pipe.communicate()
-    status = pipe.returncode
-    logging.debug(output + errout)
+    result = run(
+        cmd, shell=shell, input=input_data, encoding='utf-8',
+        stdout=PIPE, stderr=PIPE, env=env, cwd=cwd, **extra_kwargs,
+    )
+    status = result.returncode
+    logging.debug(result.stdout + result.stderr)
     if raise_on_error and status != STATUS_OK:
         is_git_command = cmd.startswith('git') if isinstance(cmd, str) else cmd[0] == 'git'
         if is_git_command and status == -25:
@@ -55,7 +45,7 @@ def run_shell_command(cmd, input_data=None, raise_on_error=False, env=None,
             'Command: "%s", Error: %s' % (cmd, errout.decode('utf-8') if errout else output)
         )
 
-    return (status, output.decode('utf-8'), errout.decode('utf-8'))
+    return (status, result.stdout, result.stderr)
 
 
 def lc_sorted(*args, **kwargs):
diff --git a/stats/utils.py b/stats/utils.py
index 3689637e..2b2eb2f8 100644
--- a/stats/utils.py
+++ b/stats/utils.py
@@ -501,6 +501,14 @@ def check_po_conformity(pofile):
     # Allow pofile provided as open file (e.g. to validate a temp uploaded file)
     if isinstance(pofile, File):
         input_data = pofile.read()
+        if isinstance(input_data, bytes):
+            try:
+                input_data = input_data.decode('utf-8')
+            except UnicodeDecodeError:
+                return [(
+                    "warn",
+                    gettext_noop("PO file “%s” is not UTF-8 encoded.") % pofile.name
+                )]
         input_file = "-"
     else:
         input_data = None
@@ -518,21 +526,15 @@ def check_po_conformity(pofile):
         errors.append(("warn", gettext_noop("This PO file has an executable bit set.")))
 
     # Check if PO file is in UTF-8
-    if input_file == "-":
-        try:
-            input_data.decode('UTF-8')
-            status = STATUS_OK
-        except UnicodeDecodeError:
-            status = STATUS_OK + 1
-    else:
+    if input_file != "-":
         command = "msgconv -t UTF-8 \"%s\" | diff -i -I '^#~' -u \"%s\" - >/dev/null" % (
             pofile, pofile)
         status, _, _ = run_shell_command(command, env=C_ENV)
-    if status != STATUS_OK:
-        errors.append((
-            "warn",
-            gettext_noop("PO file “%s” is not UTF-8 encoded.") % pofile.name
-        ))
+        if status != STATUS_OK:
+            errors.append((
+                "warn",
+                gettext_noop("PO file “%s” is not UTF-8 encoded.") % pofile.name
+            ))
     return errors
 
 
diff --git a/vertimus/tests/tests.py b/vertimus/tests/tests.py
index 6b1dd599..9916f7b9 100644
--- a/vertimus/tests/tests.py
+++ b/vertimus/tests/tests.py
@@ -685,7 +685,9 @@ class VertimusTest(TeamsAndRolesMixin, TestCase):
         post_content = QueryDict('action=WC&comment=Test1')
         post_file = MultiValueDict({'file': [SimpleUploadedFile('filename.po', b'Not valid po file 
content')]})
         form = ActionForm(self.pt, None, [ActionWC()], True, post_content, post_file)
-
+        self.assertTrue('file' in form.errors)
+        post_file = MultiValueDict({'file': [SimpleUploadedFile('filename.po', 'Niña'.encode('latin-1'))]})
+        form = ActionForm(self.pt, None, [ActionWC()], True, post_content, post_file)
         self.assertTrue('file' in form.errors)
 
         # Test a valid po file


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]