From: Daan De Meyer <daan.j.demeyer@gmail.com>
Date: Tue, 12 Apr 2022 20:52:10 +0000 (+0200)
Subject: machine: Rework run() output capture
X-Git-Tag: v13~50^2~7
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a7fb4efba7511be3f9e0d89b0b8481c627d41545;p=thirdparty%2Fmkosi.git

machine: Rework run() output capture

Let's not capture output by default. Instead, let's forward it
directly to stdout/stderr to simplify debugging. Similar to the
subprocess.run() function, let's add a capture_output argument
to allow configuring whether to capture the output.

We also remove the debug argument from Machine since logging to
stdout/stderr is now the default.
---

diff --git a/mkosi/machine.py b/mkosi/machine.py
index 336deb9c1..91c97bd15 100644
--- a/mkosi/machine.py
+++ b/mkosi/machine.py
@@ -9,7 +9,7 @@ import subprocess
 import sys
 import unittest
 from textwrap import dedent
-from typing import Any, Iterator, Optional, Sequence
+from typing import Any, Iterator, Optional, Sequence, TextIO, Union
 
 import pexpect  # type: ignore
 
@@ -35,11 +35,10 @@ from .backend import MkosiArgs, MkosiNotSupportedException, Verb, die
 
 
 class Machine:
-    def __init__(self, args: Sequence[str] = [], debug: bool = False) -> None:
+    def __init__(self, args: Sequence[str] = []) -> None:
         # Remains None until image is built and booted, then receives pexpect process.
         self._serial: Optional[pexpect.spawn] = None
         self.exit_code: int = -1
-        self.debug = debug
         self.stack = contextlib.ExitStack()
         self.args: MkosiArgs
 
@@ -139,15 +138,19 @@ class Machine:
             self._serial.expect("#")
             self.stack = stack.pop_all()
 
-    def run(self, commands: Sequence[str], timeout: int = 900, check: bool = True) -> CompletedProcess:
+    def run(
+        self,
+        commands: Sequence[str],
+        timeout: int = 900,
+        check: bool = True,
+        capture_output: bool = False,
+    ) -> CompletedProcess:
         self._ensure_booted()
 
-        process = run_command_image(self.args, commands, timeout, check, subprocess.PIPE, subprocess.PIPE)
-        if self.debug:
-            print(f"Stdout:\n {process.stdout}")
-            print(f"Stderr:\n {process.stderr}")
+        stdout: Union[int, TextIO] = subprocess.PIPE if capture_output else sys.stdout
+        stderr: Union[int, TextIO] = subprocess.PIPE if capture_output else sys.stderr
 
-        return process
+        return run_command_image(self.args, commands, timeout, check, stdout, stderr)
 
     def kill(self) -> None:
         self.__exit__(None, None, None)
diff --git a/tests/test_machine.py b/tests/test_machine.py
index 05132ec13..6555bc9bd 100644
--- a/tests/test_machine.py
+++ b/tests/test_machine.py
@@ -22,7 +22,7 @@ class MkosiMachineTestCase(MkosiMachineTest):
             pytest.xfail("QEMU's CPU does not support the CentOS EPEL image arch when running without KVM")
 
     def test_simple_run(self) -> None:
-        process = self.machine.run(["echo", "This is a test."])
+        process = self.machine.run(["echo", "This is a test."], capture_output=True)
         assert process.stdout.strip("\n") == "This is a test."
 
     def test_wrong_command(self) -> None:
@@ -36,7 +36,7 @@ class MkosiMachineTestCase(MkosiMachineTest):
         result = self.machine.run(["NonExisting", "Command"], check=False)
         assert result.returncode in (1, 127, 203)
 
-        result = self.machine.run(["ls", "-"], check=False)
+        result = self.machine.run(["ls", "-"], check=False, capture_output=True)
         assert result.returncode == 2
         assert "No such file or directory" in result.stderr