From 1501590f8847e6c51190f089c15f26a03a0ad427 Mon Sep 17 00:00:00 2001
From: Darko Poljak <darko.poljak@gmail.com>
Date: Wed, 13 Jul 2016 12:21:41 +0200
Subject: [PATCH] Fix missing stderr in case of script error in python >= 3.5.

---
 cdist/exec/util.py | 127 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 98 insertions(+), 29 deletions(-)

diff --git a/cdist/exec/util.py b/cdist/exec/util.py
index 6e04523b..9c8ff48d 100644
--- a/cdist/exec/util.py
+++ b/cdist/exec/util.py
@@ -25,7 +25,98 @@ from tempfile import TemporaryFile
 
 import cdist
 
-STDERR_UNSUPPORTED = '<Not supported in this python version>'
+
+STDERR_UNSUPPORTED = '<Not yet supported>'
+
+
+# IMPORTANT:
+# with the code below in python 3.5 when command is executed and error
+# occurs then stderr is not captured.
+# As it seems from documentation, it is only captured when using
+# subprocess.run method with stderr=subprocess.PIPE and is captured
+# into CompletedProcess resulting object or into CalledProcessError
+# in case of error (only if specified capturing).
+#
+# If using PIPE then the run is slow. run method uses communicate method
+# and internally it uses buffering.
+#
+# For now we will use capturing only stdout. stderr is written directly to
+# stderr from child process.
+#
+# STDERR_UNSUPPORTED = '<Not supported in this python version>'
+#
+#
+# def call_get_output(command, env=None):
+#     """Run the given command with the given environment.
+#     Return the tuple of stdout and stderr output as a byte strings.
+#     """
+#
+#     assert isinstance(command, (list, tuple)), (
+#             "list or tuple argument expected, got: {}".format(command))
+#
+#     if sys.version_info >= (3, 5):
+#         return call_get_out_err(command, env)
+#     else:
+#         return (call_get_stdout(command, env), STDERR_UNSUPPORTED)
+#
+#
+# def handle_called_process_error(err, command):
+#     if sys.version_info >= (3, 5):
+#         errout = err.stderr
+#     else:
+#         errout = STDERR_UNSUPPORTED
+#     raise cdist.Error("Command failed: " + " ".join(command) +
+#              " with returncode: {} and stdout: {}, stderr: {}".format(
+#                           err.returncode, err.output, errout))
+#
+#
+# def call_get_stdout(command, env=None):
+#     """Run the given command with the given environment.
+#     Return the stdout output as a byte string, stderr is ignored.
+#     """
+#     assert isinstance(command, (list, tuple)), (
+#         "list or tuple argument expected, got: {}".format(command))
+#
+#     with TemporaryFile() as fout:
+#         subprocess.check_call(command, env=env, stdout=fout)
+#         fout.seek(0)
+#         output = fout.read()
+#
+#     return output
+#
+#
+# def call_get_out_err(command, env=None):
+#     """Run the given command with the given environment.
+#     Return the tuple of stdout and stderr output as a byte strings.
+#     """
+#     assert isinstance(command, (list, tuple)), (
+#         "list or tuple argument expected, got: {}".format(command))
+#
+#     with TemporaryFile() as fout, TemporaryFile() as ferr:
+#         subprocess.check_call(command, env=env,
+#                               stdout=fout, stderr=ferr)
+#         fout.seek(0)
+#         ferr.seek(0)
+#         output = (fout.read(), ferr.read())
+#
+#     return output
+
+#
+# The code below with bufsize=0 does not work either, communicate
+# internally uses buffering. It works in case of error, but if everything
+# is ok and there is no output in stderr then execution is very very slow.
+#
+# def _call_get_out_err(command, env=None):
+#     """Run the given command with the given environment.
+#     Return the tuple of stdout and stderr output as a byte strings.
+#     """
+#     assert isinstance(command, (list, tuple)), (
+#         "list or tuple argument expected, got: {}".format(command))
+#
+#     result = subprocess.run(command, env=env, bufsize=0,
+#             stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True)
+#
+#     return (result.stdout, result.stderr)
 
 
 def call_get_output(command, env=None):
@@ -35,24 +126,19 @@ def call_get_output(command, env=None):
 
     assert isinstance(command, (list, tuple)), (
             "list or tuple argument expected, got: {}".format(command))
-
-    if sys.version_info >= (3, 5):
-        return call_get_out_err(command, env)
-    else:
-        return (call_get_stdout(command, env), STDERR_UNSUPPORTED)
+    return (_call_get_stdout(command, env), STDERR_UNSUPPORTED)
 
 
 def handle_called_process_error(err, command):
-    if sys.version_info >= (3, 5):
-        errout = err.stderr
-    else:
-        errout = STDERR_UNSUPPORTED
+    errout = STDERR_UNSUPPORTED
     raise cdist.Error("Command failed: " + " ".join(command) +
-                      " with returncode: {} and stdout: {}, stderr: {}".format(
+                      (" with returncode: {}\n"
+                       "stdout: {}\n"
+                       "stderr: {}").format(
                           err.returncode, err.output, errout))
 
 
-def call_get_stdout(command, env=None):
+def _call_get_stdout(command, env=None):
     """Run the given command with the given environment.
     Return the stdout output as a byte string, stderr is ignored.
     """
@@ -65,20 +151,3 @@ def call_get_stdout(command, env=None):
         output = fout.read()
 
     return output
-
-
-def call_get_out_err(command, env=None):
-    """Run the given command with the given environment.
-    Return the tuple of stdout and stderr output as a byte strings.
-    """
-    assert isinstance(command, (list, tuple)), (
-        "list or tuple argument expected, got: {}".format(command))
-
-    with TemporaryFile() as fout, TemporaryFile() as ferr:
-        subprocess.check_call(command, env=env,
-                              stdout=fout, stderr=ferr)
-        fout.seek(0)
-        ferr.seek(0)
-        output = (fout.read(), ferr.read())
-
-    return output