[master 16/30] Handle subprocess returning bytes (#1014220)

M4rtinK installerbot-noreply at redhat.com
Mon Jun 1 14:04:33 UTC 2015


From: Martin Kolman <mkolman at redhat.com>

The subprocess module returns bytes in Python 3,
so we need to (try) decoding the output (as utf-8)
for functions that expect text string output and
there is now a execWithCaptureBinary for forwarding
binary output when running commands.

We also need to make sure that the output is "printable"
when binary output logging to the program.log is enabled.
---
 pyanaconda/iutil.py | 42 ++++++++++++++++++++++++++++++++++++++++--
 scripts/makeupdates | 24 ++++++++++++++----------
 2 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py
index 1092397..d34bfcf 100644
--- a/pyanaconda/iutil.py
+++ b/pyanaconda/iutil.py
@@ -245,6 +245,10 @@ def sigusr1_preexec():
 def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None, log_output=True,
         binary_output=False, filter_stderr=False):
     """ Run an external program, log the output and return it to the caller
+
+        NOTE/WARNING: UnicodeDecodeError will be raised if the output of the of the
+                      external command can't be decoded as UTF-8.
+
         :param argv: The command to run and argument
         :param root: The directory to chroot to before running command.
         :param stdin: The file object to read stdin from.
@@ -275,7 +279,15 @@ def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None, log_ou
 
             if log_output:
                 with program_log_lock:
-                    for line in output_lines:
+                    if binary_output:
+                        # try to decode as utf-8 and replace all undecodable data by
+                        # "safe" printable representations when logging binary output
+                        decoded_output_lines = output_lines.decode("utf-8", "replace")
+                    else:
+                        # output_lines should already be a Unicode string
+                        decoded_output_lines = output_lines
+
+                    for line in decoded_output_lines:
                         program_log.info(line.strip())
 
             if stdout:
@@ -283,7 +295,10 @@ def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None, log_ou
 
         # If stderr was filtered, log it separately
         if filter_stderr and err_string and log_output:
-            err_lines = err_string.splitlines(True)
+            # try to decode as utf-8 and replace all undecodable data by
+            # "safe" printable representations when logging binary output
+            decoded_err_string = err_string.decode("utf-8", "replace")
+            err_lines = decoded_err_string.splitlines(True)
 
             with program_log_lock:
                 for line in err_lines:
@@ -350,6 +365,26 @@ def execWithCapture(command, argv, stdin=None, root='/', log_output=True, filter
     return _run_program(argv, stdin=stdin, root=root, log_output=log_output,
             filter_stderr=filter_stderr)[1]
 
+def execWithCaptureBinary(command, argv, stdin=None, root='/', log_output=False, filter_stderr=False):
+    """ Run an external program and capture standard out and err as binary data.
+        The binary data output is not logged by default but logging can be enabled.
+        :param command: The command to run
+        :param argv: The argument list
+        :param stdin: The file object to read stdin from.
+        :param root: The directory to chroot to before running command.
+        :param log_output: Whether to log the binary output of the command
+        :param filter_stderr: Whether stderr should be excluded from the returned output
+        :return: The output of the command
+    """
+    if flags.testing:
+        log.info("not running command because we're testing: %s %s",
+                 command, " ".join(argv))
+        return ""
+
+    argv = [command] + argv
+    return _run_program(argv, stdin=stdin, root=root, log_output=log_output,
+                        filter_stderr=filter_stderr, binary_output=True)[1]
+
 def execReadlines(command, argv, stdin=None, root='/', env_prune=None):
     """ Execute an external command and return the line output of the command
         in real-time.
@@ -358,6 +393,9 @@ def execReadlines(command, argv, stdin=None, root='/', env_prune=None):
         end of output and the process exiting. If the child process closes
         stdout and then keeps on truckin' there will be problems.
 
+        NOTE/WARNING: UnicodeDecodeError will be raised if the output of the
+                      external command can't be decoded as UTF-8.
+
         :param command: The command to run
         :param argv: The argument list
         :param stdin: The file object to read stdin from.
diff --git a/scripts/makeupdates b/scripts/makeupdates
index 8605c76..d324874 100755
--- a/scripts/makeupdates
+++ b/scripts/makeupdates
@@ -180,12 +180,13 @@ def doGitDiff(tag, args=None):
     proc = subprocess.Popen(cmd,
                             stdout=subprocess.PIPE,
                             stderr=subprocess.PIPE)
-    output = proc.communicate()
+    output, rc = proc.communicate()
+    output = output.decode("utf-8")
 
     if proc.returncode:
-        raise RuntimeError("Error running %s: %s" % (" ".join(cmd), output[1]))
+        raise RuntimeError("Error running %s: %s" % (" ".join(cmd), rc))
 
-    lines = output[0].split('\n')
+    lines = output.split('\n')
     return lines
 
 def doGitContentDiff(tag, args=None):
@@ -195,11 +196,13 @@ def doGitContentDiff(tag, args=None):
     proc = subprocess.Popen(cmd,
                             stdout=subprocess.PIPE,
                             stderr=subprocess.PIPE)
-    output = proc.communicate()
-    if proc.returncode:
-        raise RuntimeError("Error running %s: %s" % (" ".join(cmd), output[1]))
+    output, rc = proc.communicate()
+    output = output.decode("utf-8")
+
+    if rc:
+        raise RuntimeError("Error running %s: %s" % (" ".join(cmd), rc))
 
-    lines = output[0].split('\n')
+    lines = output.split('\n')
     return lines
 
 def download_to_file(url, path):
@@ -749,10 +752,11 @@ def remove_local_packages(packages, arch, release_id, added_rpms):
                                 '%{NAME} %{VERSION} %{RELEASE}', rpm_path],
                                 stdout=subprocess.PIPE,
                                 stderr=None)
-        proc_output = proc.communicate()
-        if proc.returncode != 0:
+        output, rc = proc.communicate()
+        output = output.decode("utf-8")
+        if rc != 0:
             continue
-        name, version, release = proc_output[0].split()
+        name, version, release = output.split()
         # get the build number and release id
         build_id, package_release_id = release.rsplit(".", 1)
 


-- 
To view this commit on github, visit https://github.com/rhinstaller/anaconda/commit/f0fdaf06d7dce2a88bb005651d760b3c4c52fd57


More information about the anaconda-patches mailing list