Merge branch 'bschubert/fix-binary-output-capture' into 'master'

Don't close stdout when running 'checkout' to stdout

Closes #486

See merge request BuildStream/buildstream!905

3 files changed, 18 insertions, 11 deletions

diff --git a/buildstream/_stream.py b/buildstream/_stream.py
index e7a71978b..6e2e8b25b 100644
--- a/buildstream/_stream.py
+++ b/buildstream/_stream.py
@@ -423,9 +423,16 @@ class Stream():
                 else:
                     if location == '-':
                         with target.timed_activity("Creating tarball"):
-                            with os.fdopen(sys.stdout.fileno(), 'wb') as fo:
-                                with tarfile.open(fileobj=fo, mode="w|") as tf:
-                                    sandbox_vroot.export_to_tar(tf, '.')
+                            # Save the stdout FD to restore later
+                            saved_fd = os.dup(sys.stdout.fileno())
+                            try:
+                                with os.fdopen(sys.stdout.fileno(), 'wb') as fo:
+                                    with tarfile.open(fileobj=fo, mode="w|") as tf:
+                                        sandbox_vroot.export_to_tar(tf, '.')
+                            finally:
+                                # No matter what, restore stdout for further use
+                                os.dup2(saved_fd, sys.stdout.fileno())
+                                os.close(saved_fd)
                     else:
                         with target.timed_activity("Creating tarball '{}'"
                                                    .format(location)):
diff --git a/tests/frontend/buildcheckout.py b/tests/frontend/buildcheckout.py
index 4d409cdfe..a0b461762 100644
--- a/tests/frontend/buildcheckout.py
+++ b/tests/frontend/buildcheckout.py
@@ -128,7 +128,6 @@ def test_build_checkout_tarball(datafiles, cli):
     assert os.path.join('.', 'usr', 'include', 'pony.h') in tar.getnames()
 
 
-@pytest.mark.skip(reason="Capturing the binary output is causing a stacktrace")
 @pytest.mark.datafiles(DATA_DIR)
 def test_build_checkout_tarball_stdout(datafiles, cli):
     project = os.path.join(datafiles.dirname, datafiles.basename)
@@ -143,7 +142,7 @@ def test_build_checkout_tarball_stdout(datafiles, cli):
 
     checkout_args = ['checkout', '--tar', 'target.bst', '-']
 
-    result = cli.run(project=project, args=checkout_args)
+    result = cli.run(project=project, args=checkout_args, binary_capture=True)
     result.assert_success()
 
     with open(tarball, 'wb') as f:
diff --git a/tests/testutils/runcli.py b/tests/testutils/runcli.py
index 3535e94ea..ce5864bdf 100644
--- a/tests/testutils/runcli.py
+++ b/tests/testutils/runcli.py
@@ -17,7 +17,7 @@ import pytest
 # CliRunner convenience API (click.testing module) does not support
 # separation of stdout/stderr.
 #
-from _pytest.capture import MultiCapture, FDCapture
+from _pytest.capture import MultiCapture, FDCapture, FDCaptureBinary
 
 # Import the main cli entrypoint
 from buildstream._frontend import cli as bst_cli
@@ -234,9 +234,10 @@ class Cli():
     #    silent (bool): Whether to pass --no-verbose
     #    env (dict): Environment variables to temporarily set during the test
     #    args (list): A list of arguments to pass buildstream
+    #    binary_capture (bool): Whether to capture the stdout/stderr as binary
     #
     def run(self, configure=True, project=None, silent=False, env=None,
-            cwd=None, options=None, args=None):
+            cwd=None, options=None, args=None, binary_capture=False):
         if args is None:
             args = []
         if options is None:
@@ -278,7 +279,7 @@ class Cli():
             except ValueError:
                 sys.__stdout__ = open('/dev/stdout', 'w')
 
-            result = self.invoke(bst_cli, bst_args)
+            result = self.invoke(bst_cli, bst_args, binary_capture=binary_capture)
 
         # Some informative stdout we can observe when anything fails
         if self.verbose:
@@ -295,7 +296,7 @@ class Cli():
 
         return result
 
-    def invoke(self, cli, args=None, color=False, **extra):
+    def invoke(self, cli, args=None, color=False, binary_capture=False, **extra):
         exc_info = None
         exception = None
         exit_code = 0
@@ -305,8 +306,8 @@ class Cli():
         old_stdin = sys.stdin
         with open(os.devnull) as devnull:
             sys.stdin = devnull
-
-            capture = MultiCapture(out=True, err=True, in_=False, Capture=FDCapture)
+            capture_kind = FDCaptureBinary if binary_capture else FDCapture
+            capture = MultiCapture(out=True, err=True, in_=False, Capture=capture_kind)
             capture.start_capturing()
 
             try: