From 8cfa1c37d01d8c74afbdf54d83b8f2c2e3145fe9 Mon Sep 17 00:00:00 2001
From: Darko Poljak <darko.poljak@gmail.com>
Date: Tue, 12 Sep 2017 10:16:27 +0200
Subject: [PATCH] Fix copy-paste bug. Refactor and simplify code.

---
 cdist/exec/local.py  | 40 +++++++++++++++++++---------------------
 cdist/exec/remote.py | 22 +++++++++-------------
 cdist/exec/util.py   | 16 +++++++++-------
 3 files changed, 37 insertions(+), 41 deletions(-)

diff --git a/cdist/exec/local.py b/cdist/exec/local.py
index b46e8a52..575950c1 100644
--- a/cdist/exec/local.py
+++ b/cdist/exec/local.py
@@ -203,12 +203,6 @@ class Local(object):
         self.log.trace("Local mkdir: %s", path)
         os.makedirs(path, exist_ok=True)
 
-    def _log_std_fd(self, stdfd, which, quiet, save_output):
-        if not quiet and save_output and stdfd is not None:
-            stdfd.seek(0, 0)
-            self.log.trace("Local {}:\n{}\n".format(
-                which, stdfd.read().decode()))
-
     def run(self, command, env=None, return_output=False, message_prefix=None,
             stdout=None, stderr=None, save_output=True, quiet_mode=False):
         """Run the given command with the given environment.
@@ -219,15 +213,20 @@ class Local(object):
                 "list or tuple argument expected, got: %s" % command)
 
         quiet = self.quiet_mode or quiet_mode
+        do_save_output = save_output and not quiet
 
         close_stdout = False
         close_stderr = False
-        if not quiet and save_output and not return_output and stdout is None:
-            stdout = util._get_std_fd(self, 'stdout')
-            close_stdout = True
-        if not quiet and save_output and stderr is None:
-            stderr = util._get_std_fd(self, 'stderr')
-            close_stderr = True
+        if quiet:
+            stderr = subprocess.DEVNULL
+            stdout = subprocess.DEVNULL
+        elif do_save_output:
+            if not return_output and stdout is None:
+                stdout = util.get_std_fd(self.stdout_base_path, 'local')
+                close_stdout = True
+            if stderr is None:
+                stderr = util.get_std_fd(self.stderr_base_path, 'local')
+                close_stderr = True
 
         if env is None:
             env = os.environ.copy()
@@ -246,20 +245,19 @@ class Local(object):
 
         self.log.trace("Local run: %s", command)
         try:
-            if quiet:
-                stderr = subprocess.DEVNULL
             if return_output:
                 output = subprocess.check_output(
-                    command, env=env, stderr=stderr)
-                self._log_std_fd(stderr, 'stderr', quiet, save_output)
-                return output.decode()
+                    command, env=env, stderr=stderr).decode()
             else:
-                if quiet:
-                    stdout = subprocess.DEVNULL
                 subprocess.check_call(command, env=env, stderr=stderr,
                                       stdout=stdout)
-                self._log_std_fd(stderr, 'stderr', quiet, save_output)
-                self._log_std_fd(stdout, 'stdout', quiet, save_output)
+                output = None
+
+            if do_save_output:
+                util.log_std_fd(self.log, stderr, 'Local stderr')
+                util.log_std_fd(self.log, stdout, 'Local stdout')
+
+            return output
         except subprocess.CalledProcessError as e:
             util.handle_called_process_error(e, command)
         except OSError as error:
diff --git a/cdist/exec/remote.py b/cdist/exec/remote.py
index e6047af8..2331d3b2 100644
--- a/cdist/exec/remote.py
+++ b/cdist/exec/remote.py
@@ -295,12 +295,6 @@ class Remote(object):
         return self._run_command(cmd, env=env, return_output=return_output,
                                  stdout=stdout, stderr=stderr)
 
-    def _log_std_fd(self, stdfd, which):
-        if stdfd is not None and stdfd != subprocess.DEVNULL:
-            stdfd.seek(0, 0)
-            self.log.trace("Remote {}: {}".format(
-                which, stdfd.read().decode()))
-
     def _run_command(self, command, env=None, return_output=False, stdout=None,
                      stderr=None):
         """Run the given command with the given environment.
@@ -316,10 +310,10 @@ class Remote(object):
         close_stdout = False
         close_stderr = False
         if not return_output and stdout is None:
-            stdout = util._get_std_fd(self, 'stdout')
+            stdout = util.get_std_fd(self.stdout_base_path, 'remote')
             close_stdout = True
         if stderr is None:
-            stderr = util._get_std_fd(self, 'stderr')
+            stderr = util.get_std_fd(self.stderr_base_path, 'remote')
             close_stderr = True
 
         # export target_host, target_hostname, target_fqdn
@@ -335,14 +329,16 @@ class Remote(object):
                 stderr = subprocess.DEVNULL
             if return_output:
                 output = subprocess.check_output(command, env=os_environ,
-                                                 stderr=stderr)
-                self._log_std_fd(stderr, 'stderr')
-                return output.decode()
+                                                 stderr=stderr).decode()
             else:
                 subprocess.check_call(command, env=os_environ, stdout=stdout,
                                       stderr=stderr)
-                self._log_std_fd(stderr, 'stderr')
-                self._log_std_fd(stdout, 'stdout')
+                output = None
+
+            util.log_std_fd(self.log, stderr, 'Remote stderr')
+            util.log_std_fd(self.log, stdout, 'Remote stdout')
+
+            return output
         except subprocess.CalledProcessError as e:
             util.handle_called_process_error(e, command)
         except OSError as error:
diff --git a/cdist/exec/util.py b/cdist/exec/util.py
index 561b13aa..3d484013 100644
--- a/cdist/exec/util.py
+++ b/cdist/exec/util.py
@@ -163,12 +163,14 @@ def _call_get_stdout(command, env=None, stderr=None):
     return output
 
 
-def _get_std_fd(obj, which):
-    if which == 'stdout':
-        base = obj.stdout_base_path
-    else:
-        base = obj.stderr_base_path
-
-    path = os.path.join(base, 'remote')
+def get_std_fd(base_path, name):
+    path = os.path.join(base_path, name)
     stdfd = open(path, 'ba+')
     return stdfd
+
+
+def log_std_fd(log, stdfd, prefix):
+    if stdfd is not None and stdfd != subprocess.DEVNULL:
+        stdfd.seek(0, 0)
+        log.trace("{}: {}".format(
+            prefix, stdfd.read().decode()))